Yes, that is what I meant.
David
On Thu, Sep 11, 2014 at 10:09 AM, Teresa Johnson <tejohn...@google.com> wrote:
> 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
