After the refactoring has been checked in, the bug fixing part is simply a moving a function call.
Tested by running ./buildit with both x86-64 and power64 targets. The last time regression of tls-tests.c disappeared. So it is really flaky in our testing environment. thanks Carrot 2013-05-02 Guozhi Wei <car...@google.com> * coverage.c (coverage_obj_init): Move the call of build_init_ctor to (coverage_obj_finish): here. Index: coverage.c =================================================================== --- coverage.c (revision 198654) +++ coverage.c (working copy) @@ -2504,8 +2504,7 @@ cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); } -/* Create the gcov_info types and object. Generate the constructor - function to call __gcov_init. Does not generate the initializer +/* Create the gcov_info types and object. Does not generate the initializer for the object. Returns TRUE if coverage data is being emitted. */ static bool @@ -2557,8 +2556,6 @@ ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); DECL_NAME (gcov_info_var) = get_identifier (name_buf); - build_init_ctor (gcov_info_type); - return true; } @@ -2581,7 +2578,8 @@ } /* Finalize the coverage data. Generates the array of pointers to - function objects from CTOR. Generate the gcov_info initializer. */ + function objects from CTOR. Generate the gcov_info initializer. + Generate the constructor function to call __gcov_init. */ static void coverage_obj_finish (VEC(constructor_elt,gc) *ctor) @@ -2599,9 +2597,12 @@ DECL_NAME (fn_info_ary) = get_identifier (name_buf); DECL_INITIAL (fn_info_ary) = build_constructor (fn_info_ary_type, ctor); varpool_finalize_decl (fn_info_ary); - + DECL_INITIAL (gcov_info_var) = build_info (TREE_TYPE (gcov_info_var), fn_info_ary); + + build_init_ctor (TREE_TYPE (gcov_info_var)); + varpool_finalize_decl (gcov_info_var); } On Thu, May 2, 2013 at 11:15 AM, Xinliang David Li <davi...@google.com> wrote: > I suggest submitting the refactoring part of the changes to GCC trunk first. > > thanks, > > David > > On Thu, May 2, 2013 at 11:06 AM, Carrot Wei <car...@google.com> wrote: >> This patch fixes google bug 8397853 and targets google 4.7 branch. >> >> In LIPO mode, when coverage_obj_init is called, cgraph_state is >> CGRAPH_STATE_FINISHED. The variable gcov_info_var is created but not >> initialized. When cgraph_build_static_cdtor is called, the new function and >> variables are expanded immediately since cgraph_state is >> CGRAPH_STATE_FINISHED. >> It causes gcov_info_var into .bss section. But later in function >> coverage_obj_finish we initialize gcov_info_var with non zero contents, so it >> should not be put into .bss section. >> >> In FDO mode we don't have this problem because when coverage_obj_init is >> called, >> cgraph_state is CGRAPH_STATE_IPA_SSA. When cgraph_build_static_cdtor is >> called, >> the new function is not immediately expanded. The variable will have been >> properly initialized when it is expanded. >> >> It can be fixed by moving the construction of gcov constructor after >> initialization of gcov_info_var. >> >> Tested with following testing: >> x86-64 bootstrap. >> x86-64 regression test. >> power64 regression test on qemu. >> >> The only regression for power64 is >> FAIL: gcc.dg/torture/tls/tls-test.c -O2 -flto -fno-use-linker-plugin >> -flto-partition=none execution test >> It is a flaky test case in our testing environment since all other executions >> with different compiler options failed. All testing of tls-test.c pass native >> power64 testing. >> >> thanks >> Carrot >> >> 2013-05-02 Guozhi Wei <car...@google.com> >> >> * coverage.c (gcov_info_type): New global variable. >> (coverage_obj_init): Move the construction of gcov constructor to >> (build_init_ctor): here. >> (coverage_obj_finish): Call build_init_ctor after initialization of >> gcov_info_var. >> >> >> Index: coverage.c >> =================================================================== >> --- coverage.c (revision 198425) >> +++ coverage.c (working copy) >> @@ -123,6 +123,7 @@ >> >> /* Coverage info VAR_DECL and function info type nodes. */ >> static GTY(()) tree gcov_info_var; >> +static GTY(()) tree gcov_info_type; >> static GTY(()) tree gcov_fn_info_type; >> static GTY(()) tree gcov_fn_info_ptr_type; >> >> @@ -2478,14 +2479,12 @@ >> return build_constructor (info_type, v1); >> } >> >> -/* Create the gcov_info types and object. Generate the constructor >> - function to call __gcov_init. Does not generate the initializer >> +/* Create the gcov_info types and object. Does not generate the initializer >> for the object. Returns TRUE if coverage data is being emitted. */ >> >> static bool >> coverage_obj_init (void) >> { >> - tree gcov_info_type, ctor, stmt, init_fn; >> unsigned n_counters = 0; >> unsigned ix; >> struct coverage_data *fn; >> @@ -2531,24 +2530,6 @@ >> ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); >> DECL_NAME (gcov_info_var) = get_identifier (name_buf); >> >> - /* Build a decl for __gcov_init. */ >> - init_fn = build_pointer_type (gcov_info_type); >> - init_fn = build_function_type_list (void_type_node, init_fn, NULL); >> - init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >> - get_identifier ("__gcov_init"), init_fn); >> - TREE_PUBLIC (init_fn) = 1; >> - DECL_EXTERNAL (init_fn) = 1; >> - DECL_ASSEMBLER_NAME (init_fn); >> - >> - /* Generate a call to __gcov_init(&gcov_info). */ >> - ctor = NULL; >> - stmt = build_fold_addr_expr (gcov_info_var); >> - stmt = build_call_expr (init_fn, 1, stmt); >> - append_to_statement_list (stmt, &ctor); >> - >> - /* Generate a constructor to run it. */ >> - cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >> - >> return true; >> } >> >> @@ -2570,6 +2551,32 @@ >> return ctor; >> } >> >> +/* Generate the constructor function to call __gcov_init. */ >> + >> +static void >> +build_init_ctor () >> +{ >> + tree ctor, stmt, init_fn; >> + >> + /* Build a decl for __gcov_init. */ >> + init_fn = build_pointer_type (gcov_info_type); >> + init_fn = build_function_type_list (void_type_node, init_fn, NULL); >> + init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >> + get_identifier ("__gcov_init"), init_fn); >> + TREE_PUBLIC (init_fn) = 1; >> + DECL_EXTERNAL (init_fn) = 1; >> + DECL_ASSEMBLER_NAME (init_fn); >> + >> + /* Generate a call to __gcov_init(&gcov_info). */ >> + ctor = NULL; >> + stmt = build_fold_addr_expr (gcov_info_var); >> + stmt = build_call_expr (init_fn, 1, stmt); >> + append_to_statement_list (stmt, &ctor); >> + >> + /* Generate a constructor to run it. */ >> + cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >> +} >> + >> /* Finalize the coverage data. Generates the array of pointers to >> function objects from CTOR. Generate the gcov_info initializer. */ >> >> @@ -2589,9 +2596,12 @@ >> DECL_NAME (fn_info_ary) = get_identifier (name_buf); >> DECL_INITIAL (fn_info_ary) = build_constructor (fn_info_ary_type, ctor); >> varpool_finalize_decl (fn_info_ary); >> - >> + >> DECL_INITIAL (gcov_info_var) >> = build_info (TREE_TYPE (gcov_info_var), fn_info_ary); >> + >> + build_init_ctor (); >> + >> varpool_finalize_decl (gcov_info_var); >> }