Re: [PATCH] When using -fprofile-generate=/some/path mangle absolute path of file (PR lto/85759).

[Jonathan Wakely <jwakely at redhat dot com>]

On 16/05/18 13:53 +0200, Martin Liška wrote:
> On 12/21/2017 10:13 AM, Martin Liška wrote:
> > On 12/20/2017 06:45 PM, Jakub Jelinek wrote:
> > > Another thing is that the "/" in there is wrong, so
> > >   const char dir_separator_str[] = { DIR_SEPARATOR, '\0' };
> > >   char *b = concat (profile_data_prefix, dir_separator_str, pwd, NULL);
> > > needs to be used instead.
> >
> > This looks much nicer, I forgot about DIR_SEPARATOR.
> >
> > > Does profile_data_prefix have any dir separators stripped from the end?
> >
> > That's easy to achieve..
> >
> > > Is pwd guaranteed to be relative in this case?
> >
> > .. however this is absolute path, which would be problematic on a DOC based FS.
> > Maybe we should do the same path mangling as we do for purpose of gcov:
> >
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/gcov.c#L2424
>
> Hi.
>
> I decided to implement that. Which means for:
>
> $ gcc -fprofile-generate=/tmp/myfolder empty.c -O2 && ./a.out
>
> we get following file:
> /tmp/myfolder/#home#marxin#Programming#testcases#tmp#empty.gcda
>
> That guarantees we have a unique file path. As seen in the PR it
> can produce a funny ICE.
>
> I've been testing the patch.
> Ready after it finishes tests?
>
> Martin
>
> > What do you think about it?
> > Regarding the string manipulation: I'm not an expert, but work with string in C
> > is for me always a pain :)
> >
> > Martin

> > From 386a4561a4d1501e8959871791289e95f6a89af5 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 16 Aug 2017 10:22:57 +0200
> Subject: [PATCH] When using -fprofile-generate=/some/path mangle absolute path
> of file (PR lto/85759).
>
> gcc/ChangeLog:
>
> 2018-05-16  Martin Liska  <mliska@suse.cz>
>
> 	PR lto/85759
> 	* coverage.c (coverage_init): Mangle full path name.
> 	* doc/invoke.texi: Document the change.
> 	* gcov-io.c (mangle_path): New.
> 	* gcov-io.h (mangle_path): Likewise.
> 	* gcov.c (mangle_name): Use mangle_path for path mangling.
> ---
> gcc/coverage.c      | 20 ++++++++++++++++++--
> gcc/doc/invoke.texi |  3 +++
> gcc/gcov-io.c       | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> gcc/gcov-io.h       |  1 +
> gcc/gcov.c          | 37 +------------------------------------
> 5 files changed, 72 insertions(+), 38 deletions(-)
>
> diff --git a/gcc/coverage.c b/gcc/coverage.c
> index 32ef298a11f..6e621c3ff96 100644
> --- a/gcc/coverage.c
> +++ b/gcc/coverage.c
> @@ -1227,8 +1227,24 @@ coverage_init (const char *filename)
>     g->get_passes ()->get_pass_profile ()->static_pass_number;
>   g->get_dumps ()->dump_start (profile_pass_num, NULL);
>
> -  if (!profile_data_prefix && !IS_ABSOLUTE_PATH (filename))
> -    profile_data_prefix = getpwd ();
> +  if (!IS_ABSOLUTE_PATH (filename))
> +    {
> +      /* When a profile_data_prefix is provided, then mangle full path
> +	 of filename in order to prevent file path clashing.  */
> +      if (profile_data_prefix)
> +	{
> +#if HAVE_DOS_BASED_FILE_SYSTEM
> +	  const char separator = "\\";

As mentioned on IRC, this is ill-formed due to the missing *

> +#else
> +	  const char *separator = "/";
> +#endif
> +	  filename = concat (getpwd (), separator, filename, NULL);
> +	  filename = mangle_path (filename);
> +	  len = strlen (filename);
> +	}
> +      else
> +	profile_data_prefix = getpwd ();
> +    }
>
>   if (profile_data_prefix)
>     prefix_len = strlen (profile_data_prefix);
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ca3772bbebf..4859cec0ab5 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11253,6 +11253,9 @@ and used by @option{-fprofile-use} and @option{-fbranch-probabilities}
> and its related options.  Both absolute and relative paths can be used.
> By default, GCC uses the current directory as @var{path}, thus the
> profile data file appears in the same directory as the object file.
> +In order to prevent filename clashing, if object file name is not an absolute
> +path, we mangle absolute path of @file{@var{sourcename}.gcda} file and
> +use it as file name of a @file{.gcda} file.

This is missing several definite articles, and is inconsistent about
using "filename" and "file name", i.e.

In order to prevent the file name clashing, if the object file name is
not an absolute path, we mangle the absolute path of the
@file{@var{sourcename}.gcda} file and use it as the file name of a
@file{.gcda} file.