Re: [Google] Fix test failure after porting __gcov_get_profile_prefix from google/4_7

[Teresa Johnson <tejohnson at google dot com>]

Testing passes, is the below patch ok for google/4_8?
Thanks, Teresa

On Thu, Sep 12, 2013 at 10:18 PM, Teresa Johnson <tejohnson@google.com> wrote:
> On Thu, Sep 12, 2013 at 2:32 PM, Xinliang David Li <davidxl@google.com> wrote:
>> 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.
>
> Yes I think I agree with you now.
>
> Basically, for non-lto compilations, you get the following:
> -fprofile-generate={path}
>    -> no auxbase-strip and profile_data_prefix={path}
> -fprofile-generate -o relative/path/to/file.o
>    -> no auxbase-strip and profile_data_prefix=getpwd()
> -fprofile-generate -o /absolute/path/to/file.o
>    -> auxbase-strip /absolute/path/to/file.o and profile_data_prefix=NULL
>
> But with -flto and -fprofile-generate -o relative/path/to/file.o
>    -> auxbase-strip /tmp/file.ltrans.out and profile_data_prefix=NULL
>
> In the LTO case the gcda files will go into cwd, but not in the case
> just above where the absolute path is given to the object file.
> However, for our purposes we rely on the path being specified to
> -fprofile-generate={path} in places where we query
> __gcov_profile_prefix in order to find the dump directory. Therefore,
> I think it is best to simply record a NULL string as the
> profile_data_prefix value in all cases where profile_data_prefix=NULL.
>
> Here is the patch I am regression testing:
>
> Index: tree-profile.c
> ===================================================================
> --- tree-profile.c (revision 202500)
> +++ tree-profile.c (working copy)
> @@ -470,8 +470,15 @@ 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 null_prefix[] = "\0";
> +      const char *prefix = null_prefix;
> +      prefix_len = 0;
> +      if (profile_data_prefix)
> +        {
> +          prefix_len = strlen (profile_data_prefix);
> +          prefix = profile_data_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)));
>
>>
>> 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
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413