This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING][PR gcov-profile/43341] Make layout of gcov structures fixed
On Wed, 5 May 2010, Jan Hubicka wrote:
> 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.
>
> So the problem is that you compile libgcov with default setting but the object
> files you link with with something like -fpack-all-structures-you-can-see
> (and ABI breaking flag I believe we can have)?
No, the problem is that one of source files can have unmatched
#pragma pack(n)
that causes non-default packing, and that packing is not reset when we
generate gcov structures at runtime. There's a testcase at the end of the
patch.
> >
> > 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.
>
> This will break bootstrap with non-GCC compiler since gcov-io.h is included to
> GCC sources. So at last we need to add some macros to avoid this being used
> when buiding GCC.
Indeed, I totally overlooked that issue. Then I probably can drop the
attributes changes, and reset the alignment in coverage.c as well.
Updated patch with that change below. Bootstrapping and regtesting on amd64-linux.
2010-05-05 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.
(build_gcov_info): Ditto.
(build_ctr_info_value): Ditto.
(create_coverage): Reset field alignment before generating structures.
* gcov-io.h (struct gcov_ctr_info): Shuffle fields for better packing.
(struct gcov_info): Ditto.
* gcc.dg/tree-prof/pr43341.c: New testcase.
diff --git a/gcc/coverage.c b/gcc/coverage.c
index e04d22b..71e36cc 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,12 @@ 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;
+
finish_builtin_struct (type, "__gcov_ctr_info", fields, NULL_TREE);
return type;
@@ -756,13 +756,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 +792,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 +833,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 +856,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 +885,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 ());
@@ -956,12 +956,16 @@ create_coverage (void)
{
tree gcov_info, gcov_init, body, t;
char name_buf[32];
+ unsigned saved_alignment = maximum_field_alignment;
no_coverage = 1; /* Disable any further coverage. */
if (!prg_ctr_mask)
return;
+ /* Reset the alignment, if it was changed with #pragma pack. */
+ maximum_field_alignment = initial_max_fld_align;
+
t = build_gcov_info ();
gcov_info = build_decl (BUILTINS_LOCATION,
@@ -992,6 +996,8 @@ create_coverage (void)
/* Generate a constructor to run it. */
cgraph_build_static_cdtor ('I', body, DEFAULT_INIT_PRIORITY);
+
+ maximum_field_alignment = saved_alignment;
}
/* Perform file-level initialization. Read in data file, generate name
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index ffc62ca..570c8c2 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -425,22 +425,21 @@ 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. */
};
/* 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
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)