| Message ID | CAAe5K+WpR5m9Gdf+4frtbT6R3=gJPX9rmExSZxfSPfkgGrnnsA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Sep 12, 2013 at 1:06 PM, Teresa Johnson <tejohnson@google.com> wrote: > After porting r198033 from google/4_7 to google/4_8 a test case failed > with an assert when trying to take the strlen of profile_data_prefix. > > In most cases this is either set from the directory specified to > -fprofile-generate=, or to getpwd when a directory is not specified. > However, the exception is when no directory is specified for > -fprofile-generate and -auxbase-strip option is used with the absolute > pathname. In that case the code does not set profile_data_prefix since > the filenames already have the full path. > > In the code that sets __gcov_get_profile_prefix, the fix is to simply > check if profile_data_prefix is still NULL, and if so just set via > getpwd. Why setting it to getpwd() val? Should it be set to null instead? David > > Passes regression tests and failure I reproduced. Ok for google branches? > > Thanks, > Teresa > > 2013-09-12 Teresa Johnson <tejohnson@google.com> > > * tree-profile.c (tree_init_instrumentation): Handle the case > where profile_data_prefix is NULL. > > Index: tree-profile.c > =================================================================== > --- tree-profile.c (revision 202500) > +++ tree-profile.c (working copy) > @@ -470,8 +470,11 @@ tree_init_instrumentation (void) > DECL_ASSEMBLER_NAME (gcov_profile_prefix_decl)); > TREE_STATIC (gcov_profile_prefix_decl) = 1; > > - prefix_len = strlen (profile_data_prefix); > - prefix_string = build_string (prefix_len + 1, profile_data_prefix); > + const char *prefix = profile_data_prefix; > + if (!prefix) > + prefix = getpwd (); > + prefix_len = strlen (prefix); > + prefix_string = build_string (prefix_len + 1, prefix); > TREE_TYPE (prefix_string) = build_array_type > (char_type_node, build_index_type > (build_int_cst (NULL_TREE, prefix_len))); > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Thu, Sep 12, 2013 at 1:20 PM, Xinliang David Li <davidxl@google.com> wrote: > On Thu, Sep 12, 2013 at 1:06 PM, Teresa Johnson <tejohnson@google.com> wrote: >> After porting r198033 from google/4_7 to google/4_8 a test case failed >> with an assert when trying to take the strlen of profile_data_prefix. >> >> In most cases this is either set from the directory specified to >> -fprofile-generate=, or to getpwd when a directory is not specified. >> However, the exception is when no directory is specified for >> -fprofile-generate and -auxbase-strip option is used with the absolute >> pathname. In that case the code does not set profile_data_prefix since >> the filenames already have the full path. >> >> In the code that sets __gcov_get_profile_prefix, the fix is to simply >> check if profile_data_prefix is still NULL, and if so just set via >> getpwd. > > Why setting it to getpwd() val? Should it be set to null instead? The specified behavior when no path is given to -fprofile-generate (or -fprofile-dir) is to use the current directory. The case where this was happening was an lto test case, where lto1 was first run in WPA (-fwpa) mode and was emitting the ltrans output to a /tmp/ path (-fltrans-output-list=/tmp/cciR1m1o.ltrans.out). Then lto1 was run again in LTRANS mode (-fltrans) with -auxbase-strip /tmp/cciR1m1o.ltrans0.ltrans.o, triggering the problem. Teresa > > David > >> >> Passes regression tests and failure I reproduced. Ok for google branches? >> >> Thanks, >> Teresa >> >> 2013-09-12 Teresa Johnson <tejohnson@google.com> >> >> * tree-profile.c (tree_init_instrumentation): Handle the case >> where profile_data_prefix is NULL. >> >> Index: tree-profile.c >> =================================================================== >> --- tree-profile.c (revision 202500) >> +++ tree-profile.c (working copy) >> @@ -470,8 +470,11 @@ tree_init_instrumentation (void) >> DECL_ASSEMBLER_NAME (gcov_profile_prefix_decl)); >> TREE_STATIC (gcov_profile_prefix_decl) = 1; >> >> - prefix_len = strlen (profile_data_prefix); >> - prefix_string = build_string (prefix_len + 1, profile_data_prefix); >> + const char *prefix = profile_data_prefix; >> + if (!prefix) >> + prefix = getpwd (); >> + prefix_len = strlen (prefix); >> + prefix_string = build_string (prefix_len + 1, prefix); >> TREE_TYPE (prefix_string) = build_array_type >> (char_type_node, build_index_type >> (build_int_cst (NULL_TREE, prefix_len))); >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
When absolute path is specified for the object file, no prefix will be prepended to the gcda path. If you record the cwd as in the _gcov_profile_prefix variable, at profile dump time, the prefix will be wrong -- as it is never used. David On Thu, Sep 12, 2013 at 2:07 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Thu, Sep 12, 2013 at 1:20 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Thu, Sep 12, 2013 at 1:06 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> After porting r198033 from google/4_7 to google/4_8 a test case failed >>> with an assert when trying to take the strlen of profile_data_prefix. >>> >>> In most cases this is either set from the directory specified to >>> -fprofile-generate=, or to getpwd when a directory is not specified. >>> However, the exception is when no directory is specified for >>> -fprofile-generate and -auxbase-strip option is used with the absolute >>> pathname. In that case the code does not set profile_data_prefix since >>> the filenames already have the full path. >>> >>> In the code that sets __gcov_get_profile_prefix, the fix is to simply >>> check if profile_data_prefix is still NULL, and if so just set via >>> getpwd. >> >> Why setting it to getpwd() val? Should it be set to null instead? > > The specified behavior when no path is given to -fprofile-generate (or > -fprofile-dir) is to use the current directory. > > The case where this was happening was an lto test case, where lto1 was > first run in WPA (-fwpa) mode and was emitting the ltrans output to a > /tmp/ path (-fltrans-output-list=/tmp/cciR1m1o.ltrans.out). Then lto1 > was run again in LTRANS mode (-fltrans) with -auxbase-strip > /tmp/cciR1m1o.ltrans0.ltrans.o, triggering the problem. > > Teresa > >> >> David >> >>> >>> Passes regression tests and failure I reproduced. Ok for google branches? >>> >>> Thanks, >>> Teresa >>> >>> 2013-09-12 Teresa Johnson <tejohnson@google.com> >>> >>> * tree-profile.c (tree_init_instrumentation): Handle the case >>> where profile_data_prefix is NULL. >>> >>> Index: tree-profile.c >>> =================================================================== >>> --- tree-profile.c (revision 202500) >>> +++ tree-profile.c (working copy) >>> @@ -470,8 +470,11 @@ tree_init_instrumentation (void) >>> DECL_ASSEMBLER_NAME (gcov_profile_prefix_decl)); >>> TREE_STATIC (gcov_profile_prefix_decl) = 1; >>> >>> - prefix_len = strlen (profile_data_prefix); >>> - prefix_string = build_string (prefix_len + 1, profile_data_prefix); >>> + const char *prefix = profile_data_prefix; >>> + if (!prefix) >>> + prefix = getpwd (); >>> + prefix_len = strlen (prefix); >>> + prefix_string = build_string (prefix_len + 1, prefix); >>> TREE_TYPE (prefix_string) = build_array_type >>> (char_type_node, build_index_type >>> (build_int_cst (NULL_TREE, prefix_len))); >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Index: tree-profile.c =================================================================== --- tree-profile.c (revision 202500) +++ tree-profile.c (working copy) @@ -470,8 +470,11 @@ tree_init_instrumentation (void) DECL_ASSEMBLER_NAME (gcov_profile_prefix_decl)); TREE_STATIC (gcov_profile_prefix_decl) = 1; - prefix_len = strlen (profile_data_prefix); - prefix_string = build_string (prefix_len + 1, profile_data_prefix); + const char *prefix = profile_data_prefix; + if (!prefix) + prefix = getpwd (); + prefix_len = strlen (prefix); + prefix_string = build_string (prefix_len + 1, prefix); TREE_TYPE (prefix_string) = build_array_type (char_type_node, build_index_type (build_int_cst (NULL_TREE, prefix_len)));
After porting r198033 from google/4_7 to google/4_8 a test case failed with an assert when trying to take the strlen of profile_data_prefix. In most cases this is either set from the directory specified to -fprofile-generate=, or to getpwd when a directory is not specified. However, the exception is when no directory is specified for -fprofile-generate and -auxbase-strip option is used with the absolute pathname. In that case the code does not set profile_data_prefix since the filenames already have the full path. In the code that sets __gcov_get_profile_prefix, the fix is to simply check if profile_data_prefix is still NULL, and if so just set via getpwd. Passes regression tests and failure I reproduced. Ok for google branches? Thanks, Teresa 2013-09-12 Teresa Johnson <tejohnson@google.com> * tree-profile.c (tree_init_instrumentation): Handle the case where profile_data_prefix is NULL.