On Wed, Sep 10, 2014 at 3:31 PM, Xinliang David Li <davi...@google.com> wrote: > Can you share the buildinfo reader code with the merger by defininig > some hooks for different callbacks?
Do you mean the two blobs of code guarded by 'if (tag == GCOV_TAG_BUILD_INFO)' that I added here and the existing one in gcov_exit_merge_gcda further down in the same file? Sure, I could outline that and pass in the gi_ptr for the merger case. Let me know if you meant something else. Teresa > > David > > On Wed, Sep 10, 2014 at 10:24 AM, Teresa Johnson <tejohn...@google.com> wrote: >> While porting recent support for a build info section in the gcda from >> google/4_8 to 4_9 and doing manual testing, I discovered that it does >> not interact well with the COMDAT fixup handling. This patch fixes the >> issue, and adds a test case that exposes the problem without the fix. >> >> Here is the google/4_8 patch - I plan to commit there first then port >> it along with the original build info patch to 4_9. >> >> Passes regression tests - ok for google branches? >> >> Thanks, >> Teresa >> >> 2014-09-10 Teresa Johnson <tejohn...@google.com> >> >> libgcc: >> * libgcov-driver.c (gcov_scan_to_function_data): Rename from >> gcov_scan_summary_end, scan past BUILD_INFO section. >> (gcov_dump_module_info): Rename gcov_scan_summary_end to >> gcov_scan_to_function_data. >> >> gcc/testsuite: >> * g++.dg/tree-prof/lipo/buildinfo.txt: Input for >> -fprofile-generate-buildinfo option. >> * g++.dg/tree-prof/lipo/comdat_fixup_0.C: New test. >> * g++.dg/tree-prof/lipo/comdat_fixup_1.C: Ditto. >> * g++.dg/tree-prof/lipo/comdat_fixup_2.C: Ditto. >> * g++.dg/tree-prof/lipo/comdat_fixup.h: Ditto. >> * lib/profopt.exp: Declare srcdir for use in test options. >> >> Index: libgcc/libgcov-driver.c >> =================================================================== >> --- libgcc/libgcov-driver.c (revision 214976) >> +++ libgcc/libgcov-driver.c (working copy) >> @@ -428,13 +428,15 @@ struct gcov_filename_aux{ >> #include "libgcov-driver-system.c" >> >> /* Scan through the current open gcda file corresponding to GI_PTR >> - to locate the end position of the last summary, returned in >> - SUMMARY_END_POS_P. Return 0 on success, -1 on error. */ >> + to locate the end position just before function data should be rewritten, >> + returned in SUMMARY_END_POS_P. E.g. scan past the last summary and other >> + sections that won't be rewritten, like the build info. Return 0 on >> success, >> + -1 on error. */ >> static int >> -gcov_scan_summary_end (struct gcov_info *gi_ptr, >> - gcov_position_t *summary_end_pos_p) >> +gcov_scan_to_function_data (struct gcov_info *gi_ptr, >> + gcov_position_t *summary_end_pos_p) >> { >> - gcov_unsigned_t tag, version, stamp; >> + gcov_unsigned_t tag, version, stamp, i, length; >> tag = gcov_read_unsigned (); >> if (tag != GCOV_DATA_MAGIC) >> { >> @@ -467,6 +469,28 @@ static int >> return -1; >> } >> >> + /* If there is a build info section, scan past it as well. */ >> + if (tag == GCOV_TAG_BUILD_INFO) >> + { >> + length = gcov_read_unsigned (); >> + gcov_unsigned_t num_strings = 0; >> + char **build_info_strings = gcov_read_build_info (length, >> &num_strings); >> + if (!build_info_strings) >> + { >> + gcov_error ("profiling:%s:Error reading build info\n", >> gi_filename); >> + return -1; >> + } >> + >> + for (i = 0; i < num_strings; i++) >> + free (build_info_strings[i]); >> + free (build_info_strings); >> + >> + *summary_end_pos_p = gcov_position (); >> + tag = gcov_read_unsigned (); >> + } >> + /* The next section should be the function counters. */ >> + gcc_assert (tag == GCOV_TAG_FUNCTION); >> + >> return 0; >> } >> >> @@ -1031,10 +1055,10 @@ gcov_dump_module_info (struct gcov_filename_aux *g >> >> if (changed) >> { >> - /* Scan file to find the end of the summary section, which is >> + /* Scan file to find the start of the function section, which is >> where we will start re-writing the counters. */ >> gcov_position_t summary_end_pos; >> - if (gcov_scan_summary_end (gi_ptr, &summary_end_pos) == -1) >> + if (gcov_scan_to_function_data (gi_ptr, &summary_end_pos) == -1) >> gcov_error ("profiling:%s:Error scanning summaries\n", >> gi_filename); >> else >> Index: gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt >> =================================================================== >> --- gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt (revision 0) >> +++ gcc/testsuite/g++.dg/tree-prof/lipo/buildinfo.txt (revision 0) >> @@ -0,0 +1 @@ >> +Test -fprofile-generate-buildinfo option >> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C >> =================================================================== >> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C (revision 0) >> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_0.C (revision 0) >> @@ -0,0 +1,9 @@ >> +/* { dg-options "-O2 -fno-inline >> -fprofile-generate-buildinfo=$srcdir/g++.dg/tree-prof/lipo/buildinfo.txt" >> } */ >> +#include <stdio.h> >> + >> +extern int foo1(int x); >> +extern int foo2(int x); >> +int main() >> +{ >> + printf ("Result = %d\n", foo1(1) + foo2(1)); >> +} >> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C >> =================================================================== >> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C (revision 0) >> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_1.C (revision 0) >> @@ -0,0 +1,7 @@ >> +/* { dg-options "-O2 -fno-inline" } */ >> +#include "comdat_fixup.h" >> +int foo1(int x) >> +{ >> + Foo f; >> + return f.foo(x); >> +} >> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C >> =================================================================== >> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C (revision 0) >> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup_2.C (revision 0) >> @@ -0,0 +1,7 @@ >> +/* { dg-options "-O2 -fno-inline" } */ >> +#include "comdat_fixup.h" >> +int foo2(int x) >> +{ >> + Foo f; >> + return f.foo(x); >> +} >> Index: gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h >> =================================================================== >> --- gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h (revision 0) >> +++ gcc/testsuite/g++.dg/tree-prof/lipo/comdat_fixup.h (revision 0) >> @@ -0,0 +1,5 @@ >> +class Foo >> +{ >> + public: >> + int foo(int x) { return x; } >> +}; >> Index: gcc/testsuite/lib/profopt.exp >> =================================================================== >> --- gcc/testsuite/lib/profopt.exp (revision 214976) >> +++ gcc/testsuite/lib/profopt.exp (working copy) >> @@ -169,6 +169,8 @@ proc profopt-final-code { which final_code name } >> # SRC is the full pathname of the testcase. >> # >> proc profopt-get-options { src } { >> + global srcdir >> + >> # dg-options sets a variable called dg-extra-tool-flags. >> set dg-extra-tool-flags "" >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413