Re: [GOOGLE] Fix gcda build info support

[Xinliang David Li <davidxl at google dot com>]

Can you share the buildinfo reader code with the merger by defininig
some hooks for different callbacks?

David

On Wed, Sep 10, 2014 at 10:24 AM, Teresa Johnson <tejohnson@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  <tejohnson@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 | tejohnson@google.com | 408-460-2413