PR 28034: -fsection-anchors and coverage counters

Richard Sandiford richard@codesourcery.com
Tue Jun 20 16:59:00 GMT 2006


PR 28034 reports that -fprofile-generate and -fprofile-use don't work
with -fsection-anchors.  This is due to the way that coverage counters
are handled.  coverage_counter_alloc() initially creates an array of
1000 counters, and only supplies the real type at the end of compilation:

      /* Generate and save a copy of this so it can be shared.  */
      /* We don't know the size yet; make it big enough that nobody
         will make any clever transformation on it.  */
      char buf[20];
      tree gcov_type_node = get_gcov_type ();
      tree domain_tree
        = build_index_type (build_int_cst (NULL_TREE, 1000)); /* replaced later */
      tree gcov_type_array_type
        = build_array_type (gcov_type_node, domain_tree);

This interacts badly with the object_block code, which places the decl
using it's original DECL_SIZE.  If the final array has fewer than 1000
counters (as in the PR's reduced testcase), we get some silly padding,
but correct code.  If the array has more than 1000 counters (as in the
original testcase), the padding calculation wraps.

I'm a little nervous about changing types like this.  Couldn't the
optimisers take liberties if we access the 1001th element of
1000-element array?  (In the future, if not now.)

However, I'm taking the attitude that this code was deemed acceptable,
and that it's therefore an -fsection-anchors bug.  In that case, the fix
is to avoid using section anchors for tree_ctr_tables[].  It's possible
that we'll have other situations where a decl's type and size changes,
so I thought it would be better to add a general interface for it.

Boostrapped & regression-tested on x86_64-linux-gnu.  Of course, that
target wouldn't show up the original problem, because -fsection-anchors
hasn't been ported to x86_64 yet.  It does test the !use_object_blocks_p()
cases in the new code though.

Janis has also done a C-only bootstrap on powerpc64-linux-gnu (thanks)
and has verified that the patch fixes the original problem.  OK to install?

Richard


gcc/
	PR middle-end/28034
	* coverage.c (coverage_counter_alloc): Pass counter decls to
	postpone_final_type.
	(build_ctr_info_value): Use set_final_type.
	* varasm.c (final_type_postponed): New variable.
	(use_blocks_for_decl_p): Return false for decls whose final type
	has been postponed.
	(postpone_final_type): New function.
	(set_final_type): Likewise.
	* output.h (postpone_final_size): Declare.
	(set_final_type): Likewise.

Index: gcc/coverage.c
===================================================================
--- gcc/coverage.c	(revision 114768)
+++ gcc/coverage.c	(working copy)
@@ -403,6 +403,7 @@ coverage_counter_alloc (unsigned counter
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPBX", counter + 1);
       DECL_NAME (tree_ctr_tables[counter]) = get_identifier (buf);
       DECL_ALIGN (tree_ctr_tables[counter]) = TYPE_ALIGN (gcov_type_node);
+      postpone_final_type (tree_ctr_tables[counter]);
     }
   fn_b_ctrs[counter] = fn_n_ctrs[counter];
   fn_n_ctrs[counter] += num;
@@ -727,9 +728,7 @@ build_ctr_info_value (unsigned int count
       array_type = build_array_type (TREE_TYPE (TREE_TYPE (fields)),
 				     array_type);
 
-      TREE_TYPE (tree_ctr_tables[counter]) = array_type;
-      DECL_SIZE (tree_ctr_tables[counter]) = TYPE_SIZE (array_type);
-      DECL_SIZE_UNIT (tree_ctr_tables[counter]) = TYPE_SIZE_UNIT (array_type);
+      set_final_type (tree_ctr_tables[counter], array_type);
       assemble_variable (tree_ctr_tables[counter], 0, 0, 0);
 
       value = tree_cons (fields,
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 114768)
+++ gcc/varasm.c	(working copy)
@@ -188,6 +188,10 @@ #define IN_NAMED_SECTION(DECL) \
    && DECL_SECTION_NAME (DECL) != NULL_TREE)
 #endif
 
+/* The set of decls whose final type has not yet been determined.
+   The decls are indexed by DECL_UID.  */
+static GTY(()) bitmap final_type_postponed;
+
 /* Hash table of named sections.  */
 static GTY((param_is (section))) htab_t section_htab;
 
@@ -978,6 +982,11 @@ use_blocks_for_decl_p (tree decl)
   if (DECL_INITIAL (decl) == decl)
     return false;
 
+  /* Don't use blocks for decls whose final types are not yet known.  */
+  if (final_type_postponed
+      && bitmap_bit_p (final_type_postponed, DECL_UID (decl)))
+    return false;
+
   return true;
 }
 
@@ -4715,6 +4724,31 @@ weak_finish (void)
     }
 }
 
+/* Assert that DECL's current type might not be its final type.  */
+
+void
+postpone_final_type (tree decl)
+{
+  if (use_object_blocks_p ())
+    {
+      if (final_type_postponed == NULL)
+	final_type_postponed = bitmap_gc_alloc ();
+      bitmap_set_bit (final_type_postponed, DECL_UID (decl));
+    }
+}
+
+/* Set DECL's final type to TYPE.  */
+
+void
+set_final_type (tree decl, tree type)
+{
+  if (use_object_blocks_p ())
+    bitmap_clear_bit (final_type_postponed, DECL_UID (decl));
+  TREE_TYPE (decl) = type;
+  DECL_SIZE (decl) = TYPE_SIZE (type);
+  DECL_SIZE_UNIT (decl) = TYPE_SIZE_UNIT (type);
+}
+
 /* Emit the assembly bits to indicate that DECL is globally visible.  */
 
 static void
Index: gcc/output.h
===================================================================
--- gcc/output.h	(revision 114768)
+++ gcc/output.h	(working copy)
@@ -158,6 +158,9 @@ extern void merge_weak (tree, tree);
 /* Emit any pending weak declarations.  */
 extern void weak_finish (void);
 
+extern void postpone_final_type (tree);
+extern void set_final_type (tree, tree);
+
 /* Decode an `asm' spec for a declaration as a register name.
    Return the register number, or -1 if nothing specified,
    or -2 if the ASMSPEC is not `cc' or `memory' and is not recognized,



More information about the Gcc-patches mailing list