This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING][PR gcov-profile/43341] Make layout of gcov structures fixed


(Cc'ing gcov maintainers)

Hi,

Programs with object files with non-default structure packing at the end of
the TU segfault on 64-bit architectures with -fprofile-generate because the
layout of runtime-generated gcov structures does not match the layout of the
same structures in libgcov.

The simpler fix would be to reset max_field_alignment in create_coverage, but
I have not tested that, since a slightly better approach would be to remove
unnecessary holes from those structs.

This patch shuffles the fields of the affected structures so that they can be
packed without holes and sets 'packed' and 'aligned(sizeof(void*))' attributes
on them.

Bootstrapped and regtested on x86_64-linux.  Ok for trunk?

2010-04-22  Alexander Monakov  <amonakov@ispras.ru>
    
    	PR gcov-profile/43341
    	* coverage.c (build_ctr_info_type): Change order of fields to match
    	declaration in gcov-io.h.  Indicate that the structure is packed and
    	aligned to sizeof (void*).
    	(build_gcov_info): Ditto.
    	(build_ctr_info_value): Change order of fields to match declaration.
    	* gcov-io.h (struct gcov_ctr_info): Shuffle fields for better packing.
    	Make structure packed and aligned to sizeof (void*).
    	(struct gcov_info): Ditto.
    
    	* gcc.dg/tree-prof/pr43341.c: New testcase.

diff --git a/gcc/coverage.c b/gcc/coverage.c
index e04d22b..0bfb06a 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -717,12 +717,6 @@ build_ctr_info_type (void)
   tree gcov_ptr_type = build_pointer_type (get_gcov_type ());
   tree gcov_merge_fn_type;
 
-  /* counters */
-  field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
-  TREE_CHAIN (field) = fields;
-  fields = field;
-
   /* values */
   field = build_decl (BUILTINS_LOCATION,
 		      FIELD_DECL, NULL_TREE, gcov_ptr_type);
@@ -740,6 +734,14 @@ build_ctr_info_type (void)
   TREE_CHAIN (field) = fields;
   fields = field;
 
+  /* counters */
+  field = build_decl (BUILTINS_LOCATION,
+		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
+  TREE_CHAIN (field) = fields;
+  fields = field;
+
+  TYPE_PACKED (type) = 1;
+  TYPE_ALIGN (type) = TYPE_ALIGN (ptr_type_node);
   finish_builtin_struct (type, "__gcov_ctr_info", fields, NULL_TREE);
 
   return type;
@@ -756,13 +758,6 @@ build_ctr_info_value (unsigned int counter, tree type)
   tree fields = TYPE_FIELDS (type);
   tree fn;
 
-  /* counters */
-  value = tree_cons (fields,
-		     build_int_cstu (get_gcov_unsigned_t (),
-				     prg_n_ctrs[counter]),
-		     value);
-  fields = TREE_CHAIN (fields);
-
   if (prg_n_ctrs[counter])
     {
       tree array_type;
@@ -799,6 +794,13 @@ build_ctr_info_value (unsigned int counter, tree type)
   value = tree_cons (fields,
 		     build1 (ADDR_EXPR, TREE_TYPE (fields), fn),
 		     value);
+  fields = TREE_CHAIN (fields);
+
+  /* counters */
+  value = tree_cons (fields,
+		     build_int_cstu (get_gcov_unsigned_t (),
+				     prg_n_ctrs[counter]),
+		     value);
 
   /* FIXME: use build_constructor directly.  */
   value = build_constructor_from_list (type, nreverse (value));
@@ -833,44 +835,6 @@ build_gcov_info (void)
   type = lang_hooks.types.make_type (RECORD_TYPE);
   const_type = build_qualified_type (type, TYPE_QUAL_CONST);
 
-  /* Version ident */
-  field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
-  TREE_CHAIN (field) = fields;
-  fields = field;
-  value = tree_cons (field, build_int_cstu (TREE_TYPE (field), GCOV_VERSION),
-		     value);
-
-  /* next -- NULL */
-  field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, build_pointer_type (const_type));
-  TREE_CHAIN (field) = fields;
-  fields = field;
-  value = tree_cons (field, null_pointer_node, value);
-
-  /* stamp */
-  field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
-  TREE_CHAIN (field) = fields;
-  fields = field;
-  value = tree_cons (field, build_int_cstu (TREE_TYPE (field), local_tick),
-		     value);
-
-  /* Filename */
-  string_type = build_pointer_type (build_qualified_type (char_type_node,
-						    TYPE_QUAL_CONST));
-  field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, string_type);
-  TREE_CHAIN (field) = fields;
-  fields = field;
-  da_file_name_len = strlen (da_file_name);
-  filename_string = build_string (da_file_name_len + 1, da_file_name);
-  TREE_TYPE (filename_string) = build_array_type
-    (char_type_node, build_index_type
-     (build_int_cst (NULL_TREE, da_file_name_len)));
-  value = tree_cons (field, build1 (ADDR_EXPR, string_type, filename_string),
-		     value);
-
   /* Build the fn_info type and initializer.  */
   fn_info_type = build_fn_info_type (n_ctr_types);
   fn_info_ptr_type = build_pointer_type (build_qualified_type
@@ -894,13 +858,26 @@ build_gcov_info (void)
   else
     fn_info_value = null_pointer_node;
 
-  /* number of functions */
+  /* next -- NULL */
   field = build_decl (BUILTINS_LOCATION,
-		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
+		      FIELD_DECL, NULL_TREE, build_pointer_type (const_type));
   TREE_CHAIN (field) = fields;
   fields = field;
-  value = tree_cons (field,
-		     build_int_cstu (get_gcov_unsigned_t (), n_fns),
+  value = tree_cons (field, null_pointer_node, value);
+
+  /* Filename */
+  string_type = build_pointer_type (build_qualified_type (char_type_node,
+						    TYPE_QUAL_CONST));
+  field = build_decl (BUILTINS_LOCATION,
+		      FIELD_DECL, NULL_TREE, string_type);
+  TREE_CHAIN (field) = fields;
+  fields = field;
+  da_file_name_len = strlen (da_file_name);
+  filename_string = build_string (da_file_name_len + 1, da_file_name);
+  TREE_TYPE (filename_string) = build_array_type
+    (char_type_node, build_index_type
+     (build_int_cst (NULL_TREE, da_file_name_len)));
+  value = tree_cons (field, build1 (ADDR_EXPR, string_type, filename_string),
 		     value);
 
   /* fn_info table */
@@ -910,6 +887,31 @@ build_gcov_info (void)
   fields = field;
   value = tree_cons (field, fn_info_value, value);
 
+  /* Version ident */
+  field = build_decl (BUILTINS_LOCATION,
+		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
+  TREE_CHAIN (field) = fields;
+  fields = field;
+  value = tree_cons (field, build_int_cstu (TREE_TYPE (field), GCOV_VERSION),
+		     value);
+
+  /* stamp */
+  field = build_decl (BUILTINS_LOCATION,
+		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
+  TREE_CHAIN (field) = fields;
+  fields = field;
+  value = tree_cons (field, build_int_cstu (TREE_TYPE (field), local_tick),
+		     value);
+
+  /* number of functions */
+  field = build_decl (BUILTINS_LOCATION,
+		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
+  TREE_CHAIN (field) = fields;
+  fields = field;
+  value = tree_cons (field,
+		     build_int_cstu (get_gcov_unsigned_t (), n_fns),
+		     value);
+
   /* counter_mask */
   field = build_decl (BUILTINS_LOCATION,
 		      FIELD_DECL, NULL_TREE, get_gcov_unsigned_t ());
@@ -939,6 +941,8 @@ build_gcov_info (void)
   fields = field;
   value = tree_cons (field, ctr_info_value, value);
 
+  TYPE_PACKED (type) = 1;
+  TYPE_ALIGN (type) = TYPE_ALIGN (ptr_type_node);
   finish_builtin_struct (type, "__gcov_info", fields, NULL_TREE);
 
   /* FIXME: use build_constructor directly.  */
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index ffc62ca..66cfd1e 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -425,29 +425,28 @@ typedef void (*gcov_merge_fn) (gcov_type *, gcov_unsigned_t);
 /* Information about counters.  */
 struct gcov_ctr_info
 {
+  gcov_type *values;		/* values of counters.  */
+  gcov_merge_fn merge;		/* The function used to merge them.  */
   gcov_unsigned_t num;		/* number of counters.  */
-  gcov_type *values;		/* their values.  */
-  gcov_merge_fn merge;  	/* The function used to merge them.  */
-};
+} __attribute__ ((packed, aligned (__alignof__ (void *))));
 
 /* Information about a single object file.  */
 struct gcov_info
 {
-  gcov_unsigned_t version;	/* expected version number */
   struct gcov_info *next;	/* link to next, used by libgcov */
-
-  gcov_unsigned_t stamp;	/* uniquifying time stamp */
   const char *filename;		/* output file name */
+  const struct gcov_fn_info *functions; /* table of functions */
 
+  gcov_unsigned_t version;	/* expected version number */
+  gcov_unsigned_t stamp;	/* uniquifying time stamp */
   unsigned n_functions;		/* number of functions */
-  const struct gcov_fn_info *functions; /* table of functions */
 
   unsigned ctr_mask;		/* mask of counters instrumented.  */
   struct gcov_ctr_info counts[0]; /* count data. The number of bits
 				     set in the ctr_mask field
 				     determines how big this array
 				     is.  */
-};
+} __attribute__ ((packed, aligned (__alignof__ (void *))));
 
 /* Register a new object file module.  */
 extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN;
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr43341.c b/gcc/testsuite/gcc.dg/tree-prof/pr43341.c
new file mode 100644
index 0000000..edf0879
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr43341.c
@@ -0,0 +1,7 @@
+/* { dg-options "-O2" } */
+
+int main(int argc, char **argv)
+{
+  return 0;
+}
+#pragma pack(1)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]