public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
@ 2022-08-17 12:15 Richard Purdie
  2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Richard Purdie @ 2022-08-17 12:15 UTC (permalink / raw)
  To: gcc-patches

Relative paths currently aren't remapped by -ffile-prefix-map and friends.
When cross compiling with separate 'source' and 'build' directories, the same
relative paths between directories may not be available on target as compared
to build time.

In order to be able to remap these relative build paths to paths that would
work on target, resolve paths within the file-prefix-map function using
realpath().

This does cause a change of behaviour if users were previously relying upon
symlinks or absolute paths not being resolved.

Use basename to ensure plain filenames don't have paths added.

gcc/ChangeLog:

    * file-prefix-map.cc (remap_filename): Allow remapping of relative paths

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 gcc/file-prefix-map.cc | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gcc/file-prefix-map.cc b/gcc/file-prefix-map.cc
index 24733f831d6..50d5d724a8f 100644
--- a/gcc/file-prefix-map.cc
+++ b/gcc/file-prefix-map.cc
@@ -70,19 +70,28 @@ remap_filename (file_prefix_map *maps, const char *filename)
   file_prefix_map *map;
   char *s;
   const char *name;
+  char *realname;
   size_t name_len;
 
+  if (lbasename (filename) == filename)
+    return filename;
+
+  realname = lrealpath (filename);
+
   for (map = maps; map; map = map->next)
-    if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
+    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
       break;
-  if (!map)
+  if (!map) {
+    free (realname);
     return filename;
-  name = filename + map->old_len;
+  }
+  name = realname + map->old_len;
   name_len = strlen (name) + 1;
 
   s = (char *) ggc_alloc_atomic (name_len + map->new_len);
   memcpy (s, map->new_prefix, map->new_len);
   memcpy (s + map->new_len, name, name_len);
+  free (realname);
   return s;
 }
 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/2] libcpp: Avoid remapping filenames within directives
  2022-08-17 12:15 [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Richard Purdie
@ 2022-08-17 12:15 ` Richard Purdie
  2022-08-17 14:19   ` Richard Purdie
  2022-11-01 19:32   ` Jeff Law
  2022-11-01 19:46 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Jeff Law
  2022-11-04  9:12 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Eric Botcazou
  2 siblings, 2 replies; 17+ messages in thread
From: Richard Purdie @ 2022-08-17 12:15 UTC (permalink / raw)
  To: gcc-patches


Code such as:

can interact poorly with file-prefix-map options when cross compiling. In
general you're after to remap filenames for use in target context but the
local paths should be used to find include files at compile time. Ingoring
filename remapping for directives is one way to avoid such failures.

libcpp/ChangeLog:

    * macro.cc (_cpp_builtin_macro_text): Don't remap filenames within directives

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 libcpp/macro.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I wasn't sure if this should apply to all directives or whether this may need
filtering to includes (along the lines of pfile->directive.flags & INCL). That
gets a little more complex as the flag isn't available outside of directives.cc
and would need a helper function.

diff --git a/libcpp/macro.cc b/libcpp/macro.cc
index 8ebf360c03c..7d5a0d0fd2e 100644
--- a/libcpp/macro.cc
+++ b/libcpp/macro.cc
@@ -563,7 +563,7 @@ _cpp_builtin_macro_text (cpp_reader *pfile, cpp_hashnode *node,
 	    if (!name)
 	      abort ();
 	  }
-	if (pfile->cb.remap_filename)
+	if (pfile->cb.remap_filename && !pfile->state.in_directive)
 	  name = pfile->cb.remap_filename (name);
 	len = strlen (name);
 	buf = _cpp_unaligned_alloc (pfile, len * 2 + 3);


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] libcpp: Avoid remapping filenames within directives
  2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
@ 2022-08-17 14:19   ` Richard Purdie
  2022-11-01 19:32   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Richard Purdie @ 2022-08-17 14:19 UTC (permalink / raw)
  To: gcc-patches

On Wed, 2022-08-17 at 13:15 +0100, Richard Purdie via Gcc-patches
wrote:
> Code such as:
> 
> can interact poorly with file-prefix-map options when cross compiling. In
> general you're after to remap filenames for use in target context but the
> local paths should be used to find include files at compile time. Ingoring
> filename remapping for directives is one way to avoid such failures.
> 
> libcpp/ChangeLog:
> 
>     * macro.cc (_cpp_builtin_macro_text): Don't remap filenames within directives
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  libcpp/macro.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> I wasn't sure if this should apply to all directives or whether this may need
> filtering to includes (along the lines of pfile->directive.flags & INCL). That
> gets a little more complex as the flag isn't available outside of directives.cc
> and would need a helper function.

The above should have said 

Code such as:

#include __FILE__

can interact poorly with...

Cheers,

Richard

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] libcpp: Avoid remapping filenames within directives
  2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
  2022-08-17 14:19   ` Richard Purdie
@ 2022-11-01 19:32   ` Jeff Law
  2022-11-02 10:52     ` Richard Purdie
  1 sibling, 1 reply; 17+ messages in thread
From: Jeff Law @ 2022-11-01 19:32 UTC (permalink / raw)
  To: Richard Purdie, gcc-patches


On 8/17/22 06:15, Richard Purdie via Gcc-patches wrote:
> Code such as:
#include __FILE__
>
> can interact poorly with file-prefix-map options when cross compiling. In
> general you're after to remap filenames for use in target context but the
> local paths should be used to find include files at compile time. Ingoring
> filename remapping for directives is one way to avoid such failures.
>
> libcpp/ChangeLog:
>
>      * macro.cc (_cpp_builtin_macro_text): Don't remap filenames within directives

So I went back and reviewed the old PR which introduced this code.  It 
was actually the Yocto project that got this code in to begin with :-)  
There wasn't really any discussion AFAICT about whether or not to remap 
in directives that I saw in the PR.


ISTM that given the change in behavior, we should probably document that 
we don't remap in directives.  Probably doc/invoke.texi.

With suitable documentation, this should be fine.  It seems like it 
ought to be independent of the first patch in this series which adds 
support for remapping relative paths.


jeff


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-08-17 12:15 [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Richard Purdie
  2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
@ 2022-11-01 19:46 ` Jeff Law
  2023-01-19 13:06   ` Jakub Jelinek
  2023-01-20 15:05   ` [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464] Jakub Jelinek
  2022-11-04  9:12 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Eric Botcazou
  2 siblings, 2 replies; 17+ messages in thread
From: Jeff Law @ 2022-11-01 19:46 UTC (permalink / raw)
  To: Richard Purdie, gcc-patches


On 8/17/22 06:15, Richard Purdie via Gcc-patches wrote:
> Relative paths currently aren't remapped by -ffile-prefix-map and friends.
> When cross compiling with separate 'source' and 'build' directories, the same
> relative paths between directories may not be available on target as compared
> to build time.
>
> In order to be able to remap these relative build paths to paths that would
> work on target, resolve paths within the file-prefix-map function using
> realpath().

Understood.


>
> This does cause a change of behaviour if users were previously relying upon
> symlinks or absolute paths not being resolved.

I'm not too worried about this scenario.


>
> Use basename to ensure plain filenames don't have paths added.
>
> gcc/ChangeLog:
>
>      * file-prefix-map.cc (remap_filename): Allow remapping of relative paths

Basically OK.  Just formatting nit:



>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   gcc/file-prefix-map.cc | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/file-prefix-map.cc b/gcc/file-prefix-map.cc
> index 24733f831d6..50d5d724a8f 100644
> --- a/gcc/file-prefix-map.cc
> +++ b/gcc/file-prefix-map.cc
> @@ -70,19 +70,28 @@ remap_filename (file_prefix_map *maps, const char *filename)
>     file_prefix_map *map;
>     char *s;
>     const char *name;
> +  char *realname;
>     size_t name_len;
>   
> +  if (lbasename (filename) == filename)
> +    return filename;
> +
> +  realname = lrealpath (filename);
> +
>     for (map = maps; map; map = map->next)
> -    if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
> +    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
>         break;
> -  if (!map)
> +  if (!map) {
> +    free (realname);
>       return filename;
> -  name = filename + map->old_len;
> +  }


Put the the curley braces go on their own lines, indented two 
positions.  The code inside the curleys is indented two more 
positions.   I fixed that and pushed this change to the trunk.


THanks,

jeff



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] libcpp: Avoid remapping filenames within directives
  2022-11-01 19:32   ` Jeff Law
@ 2022-11-02 10:52     ` Richard Purdie
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Purdie @ 2022-11-02 10:52 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

On Tue, 2022-11-01 at 13:32 -0600, Jeff Law wrote:
> On 8/17/22 06:15, Richard Purdie via Gcc-patches wrote:
> > Code such as:
> #include __FILE__
> > 
> > can interact poorly with file-prefix-map options when cross compiling. In
> > general you're after to remap filenames for use in target context but the
> > local paths should be used to find include files at compile time. Ingoring
> > filename remapping for directives is one way to avoid such failures.
> > 
> > libcpp/ChangeLog:
> > 
> >      * macro.cc (_cpp_builtin_macro_text): Don't remap filenames within directives
> 
> So I went back and reviewed the old PR which introduced this code.  It 
> was actually the Yocto project that got this code in to begin with :-)

Thanks for the review!

That sounds right, we use it heavily and originally had a few issues in
this area. It now generally works really well, we just found this
corner case :)
  
> There wasn't really any discussion AFAICT about whether or not to remap 
> in directives that I saw in the PR.

I don't think we'd realised there was this corner case. Now we have
found code doing it, I think the behaviour we should have is fairly
clear which is why we're sending the patch.

> ISTM that given the change in behavior, we should probably document that 
> we don't remap in directives.  Probably doc/invoke.texi.
> 
> With suitable documentation, this should be fine.  It seems like it 
> ought to be independent of the first patch in this series which adds 
> support for remapping relative paths.

Thanks for merging 1/2, it was independent, just related as a path
mapping issue we found. I'll keep the coding style in mind in future.

I've sent a new version of this patch which updates doc/invoke.texi,
just adding to file-prefix-map as the others all reference it.

Cheers,

Richard





^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-08-17 12:15 [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Richard Purdie
  2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
  2022-11-01 19:46 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Jeff Law
@ 2022-11-04  9:12 ` Eric Botcazou
  2022-11-04 17:21   ` Richard Purdie
  2 siblings, 1 reply; 17+ messages in thread
From: Eric Botcazou @ 2022-11-04  9:12 UTC (permalink / raw)
  To: Richard Purdie; +Cc: gcc-patches

> gcc/ChangeLog:
> 
>     * file-prefix-map.cc (remap_filename): Allow remapping of relative paths

Small regression in Ada though, probably a missing guard somewhere:

                === gnat tests ===


Running target unix
FAIL: gnat.dg/specs/coverage1.ads (test for excess errors)

In order to reproduce, configure the compiler with Ada enabled, build it, and 
copy $[srcdir)/gcc/testsuite/gnat.dg/specs/coverage1.ads into the build 
directory, then just issue:

gcc/gnat1 -quiet coverage1.ads -ftest-coverage -Igcc/ada/rts

raised STORAGE_ERROR : stack overflow or erroneous memory access

-- 
Eric Botcazou



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-11-04  9:12 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Eric Botcazou
@ 2022-11-04 17:21   ` Richard Purdie
  2022-11-07  8:01     ` Eric Botcazou
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Purdie @ 2022-11-04 17:21 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 2022-11-04 at 10:12 +0100, Eric Botcazou wrote:
> > gcc/ChangeLog:
> > 
> >     * file-prefix-map.cc (remap_filename): Allow remapping of
> > relative paths
> 
> Small regression in Ada though, probably a missing guard somewhere:
> 
>                 === gnat tests ===
> 
> 
> Running target unix
> FAIL: gnat.dg/specs/coverage1.ads (test for excess errors)
> 
> In order to reproduce, configure the compiler with Ada enabled, build
> it, and 
> copy $[srcdir)/gcc/testsuite/gnat.dg/specs/coverage1.ads into the
> build 
> directory, then just issue:
> 
> gcc/gnat1 -quiet coverage1.ads -ftest-coverage -Igcc/ada/rts
> 
> raised STORAGE_ERROR : stack overflow or erroneous memory access

It took me a while to work out how to get ada to build. When I did I
found it was faulting due to a NULL filename being passed to lbasename:

Program received signal SIGSEGV, Segmentation fault.
lbasename (name=0x0) at gcc/libiberty/lbasename.c:82
82	  return unix_lbasename (name);
(gdb) bt
#0  lbasename (name=0x0) at gcc/libiberty/lbasename.c:82
#1  0x0000000000f3d566 in remap_filename (maps=0x0, filename=filename@entry=0x0) at gcc/gcc/file-prefix-map.cc:76
#2  0x0000000000f3d6df in remap_profile_filename (filename=filename@entry=0x0) at gcc/gcc/file-prefix-map.cc:158
#3  0x0000000000e59f59 in coverage_begin_function (lineno_checksum=lineno_checksum@entry=595732889, cfg_checksum=cfg_checksum@entry=2754642872)
    at gcc/gcc/coverage.cc:650
#4  0x00000000012263c0 in branch_prob (thunk=thunk@entry=false) at gcc/gcc/profile.cc:1400
#5  0x00000000013b1205 in tree_profiling () at gcc/gcc/tree-profile.cc:782
#6  (anonymous namespace)::pass_ipa_tree_profile::execute (this=<optimised out>) at gcc/gcc/tree-profile.cc:888
#7  0x00000000011ef30b in execute_one_pass (pass=0x36ebea0) at gcc/gcc/passes.cc:2644
#8  0x00000000011f0697 in execute_ipa_pass_list (pass=0x36ebea0) at gcc/gcc/passes.cc:3093
#9  0x0000000000e4c28d in ipa_passes () at gcc/gcc/cgraphunit.cc:2170
#10 symbol_table::compile (this=0x7ffff718f000) at gcc/gcc/cgraphunit.cc:2291
#11 0x0000000000e4ec08 in symbol_table::compile (this=0x7ffff718f000) at gcc/gcc/cgraphunit.cc:2271
#12 symbol_table::finalize_compilation_unit (this=0x7ffff718f000) at gcc/gcc/cgraphunit.cc:2543
#13 0x00000000012cfea0 in compile_file () at gcc/gcc/toplev.cc:471
#14 0x00000000009a727c in do_compile (no_backend=false) at gcc/gcc/toplev.cc:2125
#15 toplev::main (this=this@entry=0x7fffffffe21e, argc=<optimised out>, argc@entry=4, argv=<optimised out>, argv@entry=0x7fffffffe348) at gcc/gcc/toplev.cc:2277
#16 0x00000000009a8a8b in main (argc=4, argv=0x7fffffffe348) at gcc/gcc/main.cc:39

I can send the obvious patch to make it work as before and ignore the
NULL which fixes this. I'm not sure if it should really be passing NULL
around in the first place or not which is why I'm sharing the
backtrace.

Cheers,

Richard

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-11-04 17:21   ` Richard Purdie
@ 2022-11-07  8:01     ` Eric Botcazou
  2022-11-07 16:21       ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Botcazou @ 2022-11-07  8:01 UTC (permalink / raw)
  To: Richard Purdie; +Cc: gcc-patches

> I can send the obvious patch to make it work as before and ignore the
> NULL which fixes this. I'm not sure if it should really be passing NULL
> around in the first place or not which is why I'm sharing the backtrace.

Thanks for the investigation.  That's because the DECL_SOURCE_LOCATION of the 
function is UNKNOWN_LOCATION since:

2021-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR debug/101598
	* gcc-interface/trans.c (Subprogram_Body_to_gnu): Set the
	DECL_SOURCE_LOCATION of DECL_IGNORED_P gnu_subprog_decl to
	UNKNOWN_LOCATION.

That's indeed quite irregular but we have to live with it now.

-- 
Eric Botcazou



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-11-07  8:01     ` Eric Botcazou
@ 2022-11-07 16:21       ` Jeff Law
  2022-11-07 16:36         ` Eric Botcazou
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2022-11-07 16:21 UTC (permalink / raw)
  To: Eric Botcazou, Richard Purdie; +Cc: gcc-patches


On 11/7/22 01:01, Eric Botcazou via Gcc-patches wrote:
>> I can send the obvious patch to make it work as before and ignore the
>> NULL which fixes this. I'm not sure if it should really be passing NULL
>> around in the first place or not which is why I'm sharing the backtrace.
> Thanks for the investigation.  That's because the DECL_SOURCE_LOCATION of the
> function is UNKNOWN_LOCATION since:
>
> 2021-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	PR debug/101598
> 	* gcc-interface/trans.c (Subprogram_Body_to_gnu): Set the
> 	DECL_SOURCE_LOCATION of DECL_IGNORED_P gnu_subprog_decl to
> 	UNKNOWN_LOCATION.
>
> That's indeed quite irregular but we have to live with it now.

Eric, can you push the approved fix for this issue (look for a message 
from Richard P day or two back, approved by Richi)?  I'm dealing with a 
medical issue and heading offline again momentarily.


jeff

>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-11-07 16:21       ` Jeff Law
@ 2022-11-07 16:36         ` Eric Botcazou
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Botcazou @ 2022-11-07 16:36 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Purdie, gcc-patches

> Eric, can you push the approved fix for this issue (look for a message
> from Richard P day or two back, approved by Richi)?  I'm dealing with a
> medical issue and heading offline again momentarily.

Sure, done.

-- 
Eric Botcazou



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths
  2022-11-01 19:46 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Jeff Law
@ 2023-01-19 13:06   ` Jakub Jelinek
  2023-01-20 15:05   ` [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464] Jakub Jelinek
  1 sibling, 0 replies; 17+ messages in thread
From: Jakub Jelinek @ 2023-01-19 13:06 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Purdie, gcc-patches

On Tue, Nov 01, 2022 at 01:46:20PM -0600, Jeff Law via Gcc-patches wrote:
> 
> On 8/17/22 06:15, Richard Purdie via Gcc-patches wrote:
> > Relative paths currently aren't remapped by -ffile-prefix-map and friends.
> > When cross compiling with separate 'source' and 'build' directories, the same
> > relative paths between directories may not be available on target as compared
> > to build time.
> > 
> > In order to be able to remap these relative build paths to paths that would
> > work on target, resolve paths within the file-prefix-map function using
> > realpath().
> 
> Understood.
> 
> 
> > 
> > This does cause a change of behaviour if users were previously relying upon
> > symlinks or absolute paths not being resolved.
> 
> I'm not too worried about this scenario.

This breaks ccache testsuite and -fdebug-prefix-map behavior in directories
which are symlinks, see PR108464/  I can't see how the new behavior would be
correct in that case, user is asking to remap say /home/jakub/foobar2 to
some other path, but exactly /home/jakub/foobar2 appears in the debug info,
rather than the other path.

	Jakub


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464]
  2022-11-01 19:46 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Jeff Law
  2023-01-19 13:06   ` Jakub Jelinek
@ 2023-01-20 15:05   ` Jakub Jelinek
  2023-01-23 15:39     ` Jakub Jelinek
  2023-03-10  8:49     ` Patch ping - " Jakub Jelinek
  1 sibling, 2 replies; 17+ messages in thread
From: Jakub Jelinek @ 2023-01-20 15:05 UTC (permalink / raw)
  To: Jeff Law, Richard Biener; +Cc: Richard Purdie, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]

Hi!

On Tue, Nov 01, 2022 at 01:46:20PM -0600, Jeff Law via Gcc-patches wrote:
> > This does cause a change of behaviour if users were previously relying upon
> > symlinks or absolute paths not being resolved.
> 
> I'm not too worried about this scenario.

As mentioned in the PR, this patch breaks e.g. ccache testsuite.

I strongly doubt most of the users want such a behavior, because it
makes all filenames absolute when -f*-prefix-map= options remap one
absolute path to another one.
Say if I'm in /tmp and /tmp is the canonical path and there is
src/test.c file, with -fdebug-prefix-map=/tmp=/blah
previously there would be DW_AT_comp_dir "/blah" and it is still there,
but DW_AT_name which was previouly "src/test.c" (relative against
DW_AT_comp_dir) is now "/blah/src/test.c" instead.

Even worse, the canonicalization is only done on the remap_filename
argument, but not on the old_prefix side.  That is e.g. what breaks
ccache.  If there is
/tmp/foobar1 directory and
ln -sf foobar1 /tmp/foobar2
cd /tmp/foobar2
then -fdebug-prefix-map=`pwd`:/blah will just not work, while
src/test.c will be canonicalized to /tmp/foobar1/src/test.c,
old_prefix is still what the user provided which is /tmp/foobar2.
User would need to change their uses to use -fdebug-prefix-map=`realpath $(pwd)`=/blah

I'm attaching 3 so far just compile tested patches.

The first patch just reverts the patch (and its follow-up patch).

The second introduces a new option, -f{,no}-canon-prefix-map which affects
the behavior of -f{file,macro,debug,profile}-prefix-map=, if on it
canonicalizes the old path of the prefix map option and compares that
against the canonicalized filename for absolute paths but not relative.

And last is like the second, but does that also for relative paths except
for filenames with no / (or / or \ on DOS based fs).  So, the third patch
gets an optional behavior of what has been on the trunk lately with the
difference that the old_prefix is canonicalized by the compiler.

Initially I've thought I'd just add some magic syntax to the OLD=NEW
argument of those options (because there are 4 of them), but as noted
in the comments, = is valid char in OLD (just not new), so it would
be hard to figure out some syntax.  So instead a new option, which one
can turn on and off for different -f*-prefix-map= options if needed.

-fdebug-prefix-map=/path1=/mypath1 -fcanon-prefix-map \
-fdebug-prefix-map=/path2=/mypath2 -fno-canon-prefix-map \
-fdebug-prefix-map=/path3=/mypath3

will use the old behavior for the /path1 and /path3 handling and
the new one only for /path2 handling.

Thoughts on this?

	Jakub

[-- Attachment #2: R757a --]
[-- Type: text/plain, Size: 1088 bytes --]

2023-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR other/108464
	* file-prefix-map.cc (remap_filename): Revert 2022-11-01 and 2022-11-07
	changes.

--- gcc/file-prefix-map.cc
+++ gcc/file-prefix-map.cc
@@ -70,29 +70,19 @@ remap_filename (file_prefix_map *maps, const char *filename)
   file_prefix_map *map;
   char *s;
   const char *name;
-  char *realname;
   size_t name_len;
 
-  if (!filename || lbasename (filename) == filename)
-    return filename;
-
-  realname = lrealpath (filename);
-
   for (map = maps; map; map = map->next)
-    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
+    if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
       break;
   if (!map)
-    {
-      free (realname);
-      return filename;
-    }
-  name = realname + map->old_len;
+    return filename;
+  name = filename + map->old_len;
   name_len = strlen (name) + 1;
 
   s = (char *) ggc_alloc_atomic (name_len + map->new_len);
   memcpy (s, map->new_prefix, map->new_len);
   memcpy (s + map->new_len, name, name_len);
-  free (realname);
   return s;
 }
 

[-- Attachment #3: R757b --]
[-- Type: text/plain, Size: 9758 bytes --]

2023-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR other/108464
	* common.opt (fcanon-prefix-map): New option.
	* opts.cc: Include file-prefix-map.h.
	(flag_canon_prefix_map): New variable.
	(common_handle_option): Handle OPT_fcanon_prefix_map.
	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
	* file-prefix-map.h (flag_canon_prefix_map): Declare.
	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
	member.
	(add_prefix_map): Initialize canonicalize member from
	flag_canon_prefix_map, and if true and old_prefix is absolute
	pathname, canonicalize it using lrealpath.
	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
	use lrealpath result only for absolute filenames and only for
	map->canonicalize map entries.
	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
	* opts-global.cc (handle_common_deferred_options): Clear
	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
	* doc/invoke.texi (-fcanon-prefix-map): Document.
	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
	see also for -fcanon-prefix-map.
	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.

--- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
+++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
@@ -1204,6 +1204,10 @@ fchecking=
 Common Joined RejectNegative UInteger Var(flag_checking)
 Perform internal consistency checkings.
 
+fcanon-prefix-map
+Common Var(common_deferred_options) Defer
+For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
+
 fcode-hoisting
 Common Var(flag_code_hoisting) Optimization
 Enable code hoisting.
--- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
+++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
@@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
 #include "diagnostic-color.h"
 #include "version.h"
 #include "selftest.h"
+#include "file-prefix-map.h"
 
 /* In this file all option sets are explicit.  */
 #undef OPTION_SET_P
 
+/* Set by -fcanon-prefix-map.  */
+bool flag_canon_prefix_map;
+
 static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
 
 /* Names of fundamental debug info formats indexed by enum
@@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
       /* Deferred.  */
       break;
 
+    case OPT_fcanon_prefix_map:
+      flag_canon_prefix_map = value;
+      break;
+
     case OPT_fcallgraph_info:
       opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
       break;
@@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
       case OPT_fmacro_prefix_map_:
       case OPT_ffile_prefix_map_:
       case OPT_fprofile_prefix_map_:
+      case OPT_fcanon_prefix_map:
       case OPT_fcompare_debug:
       case OPT_fchecking:
       case OPT_fchecking_:
--- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
+++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
@@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
 void add_debug_prefix_map (const char *);
 void add_file_prefix_map (const char *);
 void add_profile_prefix_map (const char *);
+extern bool flag_canon_prefix_map;
 
 const char *remap_macro_filename (const char *);
 const char *remap_debug_filename (const char *);
--- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
+++ gcc/file-prefix-map.cc	2023-01-20 13:53:31.568896433 +0100
@@ -30,6 +30,7 @@ struct file_prefix_map
   const char *new_prefix;
   size_t old_len;
   size_t new_len;
+  bool canonicalize;
   struct file_prefix_map *next;
 };
 
@@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
       return;
     }
   map = XNEW (file_prefix_map);
+  map->canonicalize = flag_canon_prefix_map;
   map->old_prefix = xstrndup (arg, p - arg);
   map->old_len = p - arg;
+  if (map->canonicalize && IS_ABSOLUTE_PATH (map->old_prefix))
+    {
+      char *realname = lrealpath (map->old_prefix);
+      free (const_cast <char *> (map->old_prefix));
+      map->old_prefix = realname;
+      map->old_len = strlen (realname);
+    }
   p++;
   map->new_prefix = xstrdup (p);
   map->new_len = strlen (p);
@@ -70,23 +79,32 @@ remap_filename (file_prefix_map *maps, c
   file_prefix_map *map;
   char *s;
   const char *name;
-  char *realname;
+  char *realname = NULL;
   size_t name_len;
 
-  if (!filename || lbasename (filename) == filename)
+  if (!filename)
     return filename;
 
-  realname = lrealpath (filename);
-
+  bool absolute = IS_ABSOLUTE_PATH (filename);
   for (map = maps; map; map = map->next)
-    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
+    if (map->canonicalize && absolute)
+      {
+	if (realname == NULL)
+	  realname = lrealpath (filename);
+	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
+	  break;
+      }
+    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
       break;
   if (!map)
     {
       free (realname);
       return filename;
     }
-  name = realname + map->old_len;
+  if (map->canonicalize && absolute)
+    name = realname + map->old_len;
+  else
+    name = filename + map->old_len;
   name_len = strlen (name) + 1;
 
   s = (char *) ggc_alloc_atomic (name_len + map->new_len);
@@ -97,7 +115,7 @@ remap_filename (file_prefix_map *maps, c
 }
 
 /* NOTE: if adding another -f*-prefix-map option then don't forget to
-   ignore it in DW_AT_producer (dwarf2out.cc).  */
+   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
 
 /* Linked lists of file_prefix_map structures.  */
 static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
--- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
+++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
@@ -150,6 +150,7 @@ lto_write_options (void)
 	case OPT_ffile_prefix_map_:
 	case OPT_fmacro_prefix_map_:
 	case OPT_fprofile_prefix_map_:
+	case OPT_fcanon_prefix_map:
 	case OPT_fwhole_program:
 	  continue;
 
--- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
+++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
@@ -364,6 +364,7 @@ handle_common_deferred_options (void)
   if (flag_opt_info)
     opt_info_switch_p (NULL);
 
+  flag_canon_prefix_map = false;
   FOR_EACH_VEC_ELT (v, i, opt)
     {
       switch (opt->opt_index)
@@ -392,6 +393,10 @@ handle_common_deferred_options (void)
 	  add_profile_prefix_map (opt->arg);
 	  break;
 
+	case OPT_fcanon_prefix_map:
+	  flag_canon_prefix_map = opt->value;
+	  break;
+
 	case OPT_fdump_:
 	  g->get_dumps ()->dump_switch_p (opt->arg);
 	  break;
--- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
+++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
@@ -191,7 +191,7 @@ in the following sections.
 -dumpdir @var{dumppfx}  -x @var{language}  @gol
 -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
 -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
-@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
+@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
 -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
 -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
 
@@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
 option is equivalent to specifying all the individual
 @option{-f*-prefix-map} options.  This can be used to make reproducible
 builds that are location independent.  Directories referenced by
-directives are not affected by these options. See also
-@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
-@option{-fprofile-prefix-map}.
+directives are not affected by these options.  See also
+@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
+@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
+
+@item -fcanon-prefix-map
+@opindex fcanon-prefix-map
+For the @option{-f*-prefix-map} options normally comparison
+of @file{@var{old}} prefix against the filename that would be normally
+referenced in the result of the compilation is done using textual
+comparison of the prefixes, or ignoring character case for case insensitive
+filesystems and considering slashes and backslashes as equal on DOS based
+filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
+to be done for absolute pathnames on canonicalized paths of @file{@var{old}}
+and the referenced filename.
 
 @item -fplugin=@var{name}.so
 @opindex fplugin
@@ -11289,7 +11300,8 @@ build-time path with an install-time pat
 also be used to change an absolute path to a relative path by using
 @file{.} for @var{new}.  This can give more reproducible builds, which
 are location independent, but may require an extra command to tell GDB
-where to find the source files. See also @option{-ffile-prefix-map}.
+where to find the source files. See also @option{-ffile-prefix-map}
+and @option{-fcanon-prefix-map}.
 
 @item -fvar-tracking
 @opindex fvar-tracking
@@ -16466,7 +16478,7 @@ When compiling files residing in directo
 profiling information (with @option{--coverage})
 describing them as if the files resided in
 directory @file{@var{new}} instead.
-See also @option{-ffile-prefix-map}.
+See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
 
 @item -fprofile-update=@var{method}
 @opindex fprofile-update
--- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
+++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
@@ -305,7 +305,7 @@ to change an absolute path to a relative
 @var{new} which can result in more reproducible builds that are
 location independent.  This option also affects
 @code{__builtin_FILE()} during compilation.  See also
-@option{-ffile-prefix-map}.
+@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
 
 @item -fexec-charset=@var{charset}
 @opindex fexec-charset

[-- Attachment #4: R757c --]
[-- Type: text/plain, Size: 9872 bytes --]

2023-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR other/108464
	* common.opt (fcanon-prefix-map): New option.
	* opts.cc: Include file-prefix-map.h.
	(flag_canon_prefix_map): New variable.
	(common_handle_option): Handle OPT_fcanon_prefix_map.
	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
	* file-prefix-map.h (flag_canon_prefix_map): Declare.
	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
	member.
	(add_prefix_map): Initialize canonicalize member from
	flag_canon_prefix_map, and if true canonicalize it using lrealpath.
	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
	use lrealpath result only for map->canonicalize map entries.
	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
	* opts-global.cc (handle_common_deferred_options): Clear
	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
	* doc/invoke.texi (-fcanon-prefix-map): Document.
	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
	see also for -fcanon-prefix-map.
	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.

--- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
+++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
@@ -1204,6 +1204,10 @@ fchecking=
 Common Joined RejectNegative UInteger Var(flag_checking)
 Perform internal consistency checkings.
 
+fcanon-prefix-map
+Common Var(common_deferred_options) Defer
+For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
+
 fcode-hoisting
 Common Var(flag_code_hoisting) Optimization
 Enable code hoisting.
--- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
+++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
@@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
 #include "diagnostic-color.h"
 #include "version.h"
 #include "selftest.h"
+#include "file-prefix-map.h"
 
 /* In this file all option sets are explicit.  */
 #undef OPTION_SET_P
 
+/* Set by -fcanon-prefix-map.  */
+bool flag_canon_prefix_map;
+
 static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
 
 /* Names of fundamental debug info formats indexed by enum
@@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
       /* Deferred.  */
       break;
 
+    case OPT_fcanon_prefix_map:
+      flag_canon_prefix_map = value;
+      break;
+
     case OPT_fcallgraph_info:
       opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
       break;
@@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
       case OPT_fmacro_prefix_map_:
       case OPT_ffile_prefix_map_:
       case OPT_fprofile_prefix_map_:
+      case OPT_fcanon_prefix_map:
       case OPT_fcompare_debug:
       case OPT_fchecking:
       case OPT_fchecking_:
--- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
+++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
@@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
 void add_debug_prefix_map (const char *);
 void add_file_prefix_map (const char *);
 void add_profile_prefix_map (const char *);
+extern bool flag_canon_prefix_map;
 
 const char *remap_macro_filename (const char *);
 const char *remap_debug_filename (const char *);
--- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
+++ gcc/file-prefix-map.cc	2023-01-20 15:42:30.661088219 +0100
@@ -30,6 +30,7 @@ struct file_prefix_map
   const char *new_prefix;
   size_t old_len;
   size_t new_len;
+  bool canonicalize;
   struct file_prefix_map *next;
 };
 
@@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
       return;
     }
   map = XNEW (file_prefix_map);
+  map->canonicalize = flag_canon_prefix_map;
   map->old_prefix = xstrndup (arg, p - arg);
   map->old_len = p - arg;
+  if (map->canonicalize)
+    {
+      char *realname = lrealpath (map->old_prefix);
+      free (const_cast <char *> (map->old_prefix));
+      map->old_prefix = realname;
+      map->old_len = strlen (realname);
+    }
   p++;
   map->new_prefix = xstrdup (p);
   map->new_len = strlen (p);
@@ -70,34 +79,49 @@ remap_filename (file_prefix_map *maps, c
   file_prefix_map *map;
   char *s;
   const char *name;
-  char *realname;
+  const char *realname = NULL;
   size_t name_len;
 
-  if (!filename || lbasename (filename) == filename)
+  if (!filename)
     return filename;
 
-  realname = lrealpath (filename);
-
   for (map = maps; map; map = map->next)
-    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
+    if (map->canonicalize)
+      {
+	if (realname == NULL)
+	  {
+	    if (lbasename (filename) == filename)
+	      realname = filename;
+	    else
+	      realname = lrealpath (filename);
+	  }
+	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
+	  break;
+      }
+    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
       break;
   if (!map)
     {
-      free (realname);
+      if (realname != filename)
+	free (const_cast <char *> (realname));
       return filename;
     }
-  name = realname + map->old_len;
+  if (map->canonicalize)
+    name = realname + map->old_len;
+  else
+    name = filename + map->old_len;
   name_len = strlen (name) + 1;
 
   s = (char *) ggc_alloc_atomic (name_len + map->new_len);
   memcpy (s, map->new_prefix, map->new_len);
   memcpy (s + map->new_len, name, name_len);
-  free (realname);
+  if (realname != filename)
+    free (const_cast <char *> (realname));
   return s;
 }
 
 /* NOTE: if adding another -f*-prefix-map option then don't forget to
-   ignore it in DW_AT_producer (dwarf2out.cc).  */
+   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
 
 /* Linked lists of file_prefix_map structures.  */
 static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
--- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
+++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
@@ -150,6 +150,7 @@ lto_write_options (void)
 	case OPT_ffile_prefix_map_:
 	case OPT_fmacro_prefix_map_:
 	case OPT_fprofile_prefix_map_:
+	case OPT_fcanon_prefix_map:
 	case OPT_fwhole_program:
 	  continue;
 
--- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
+++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
@@ -364,6 +364,7 @@ handle_common_deferred_options (void)
   if (flag_opt_info)
     opt_info_switch_p (NULL);
 
+  flag_canon_prefix_map = false;
   FOR_EACH_VEC_ELT (v, i, opt)
     {
       switch (opt->opt_index)
@@ -392,6 +393,10 @@ handle_common_deferred_options (void)
 	  add_profile_prefix_map (opt->arg);
 	  break;
 
+	case OPT_fcanon_prefix_map:
+	  flag_canon_prefix_map = opt->value;
+	  break;
+
 	case OPT_fdump_:
 	  g->get_dumps ()->dump_switch_p (opt->arg);
 	  break;
--- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
+++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
@@ -191,7 +191,7 @@ in the following sections.
 -dumpdir @var{dumppfx}  -x @var{language}  @gol
 -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
 -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
-@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
+@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
 -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
 -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
 
@@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
 option is equivalent to specifying all the individual
 @option{-f*-prefix-map} options.  This can be used to make reproducible
 builds that are location independent.  Directories referenced by
-directives are not affected by these options. See also
-@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
-@option{-fprofile-prefix-map}.
+directives are not affected by these options.  See also
+@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
+@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
+
+@item -fcanon-prefix-map
+@opindex fcanon-prefix-map
+For the @option{-f*-prefix-map} options normally comparison
+of @file{@var{old}} prefix against the filename that would be normally
+referenced in the result of the compilation is done using textual
+comparison of the prefixes, or ignoring character case for case insensitive
+filesystems and considering slashes and backslashes as equal on DOS based
+filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
+to be done on canonicalized paths of @file{@var{old}}
+and the referenced filename.
 
 @item -fplugin=@var{name}.so
 @opindex fplugin
@@ -11289,7 +11300,8 @@ build-time path with an install-time pat
 also be used to change an absolute path to a relative path by using
 @file{.} for @var{new}.  This can give more reproducible builds, which
 are location independent, but may require an extra command to tell GDB
-where to find the source files. See also @option{-ffile-prefix-map}.
+where to find the source files. See also @option{-ffile-prefix-map}
+and @option{-fcanon-prefix-map}.
 
 @item -fvar-tracking
 @opindex fvar-tracking
@@ -16466,7 +16478,7 @@ When compiling files residing in directo
 profiling information (with @option{--coverage})
 describing them as if the files resided in
 directory @file{@var{new}} instead.
-See also @option{-ffile-prefix-map}.
+See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
 
 @item -fprofile-update=@var{method}
 @opindex fprofile-update
--- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
+++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
@@ -305,7 +305,7 @@ to change an absolute path to a relative
 @var{new} which can result in more reproducible builds that are
 location independent.  This option also affects
 @code{__builtin_FILE()} during compilation.  See also
-@option{-ffile-prefix-map}.
+@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
 
 @item -fexec-charset=@var{charset}
 @opindex fexec-charset

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464]
  2023-01-20 15:05   ` [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464] Jakub Jelinek
@ 2023-01-23 15:39     ` Jakub Jelinek
  2023-03-10  8:49     ` Patch ping - " Jakub Jelinek
  1 sibling, 0 replies; 17+ messages in thread
From: Jakub Jelinek @ 2023-01-23 15:39 UTC (permalink / raw)
  To: Jeff Law, Richard Biener, Richard Purdie, gcc-patches

On Fri, Jan 20, 2023 at 04:05:55PM +0100, Jakub Jelinek via Gcc-patches wrote:
> I'm attaching 3 so far just compile tested patches.

So far successfully bootstrapped/regtested the first and third patches,
both on x86_64-linux and i686-linux.

	Jakub


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Patch ping - [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464]
  2023-01-20 15:05   ` [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464] Jakub Jelinek
  2023-01-23 15:39     ` Jakub Jelinek
@ 2023-03-10  8:49     ` Jakub Jelinek
  2023-03-10  9:05       ` Richard Biener
  1 sibling, 1 reply; 17+ messages in thread
From: Jakub Jelinek @ 2023-03-10  8:49 UTC (permalink / raw)
  To: Jeff Law, Richard Biener; +Cc: gcc-patches, Richard Purdie

Hi!

I'd like to ping these patches.  All 3 variants have been
bootstrapped/regtested on x86_64-linux and i686-linux, the last
one is my preference I guess.  The current state breaks e.g. ccache.

https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610285.html
  - PR108464 - P1 - file-prefix-map: Fix up -f*-prefix-map= (3 variants)

Thanks
	Jakub

On Fri, Jan 20, 2023 at 04:05:55PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Nov 01, 2022 at 01:46:20PM -0600, Jeff Law via Gcc-patches wrote:
> > > This does cause a change of behaviour if users were previously relying upon
> > > symlinks or absolute paths not being resolved.
> > 
> > I'm not too worried about this scenario.
> 
> As mentioned in the PR, this patch breaks e.g. ccache testsuite.
> 
> I strongly doubt most of the users want such a behavior, because it
> makes all filenames absolute when -f*-prefix-map= options remap one
> absolute path to another one.
> Say if I'm in /tmp and /tmp is the canonical path and there is
> src/test.c file, with -fdebug-prefix-map=/tmp=/blah
> previously there would be DW_AT_comp_dir "/blah" and it is still there,
> but DW_AT_name which was previouly "src/test.c" (relative against
> DW_AT_comp_dir) is now "/blah/src/test.c" instead.
> 
> Even worse, the canonicalization is only done on the remap_filename
> argument, but not on the old_prefix side.  That is e.g. what breaks
> ccache.  If there is
> /tmp/foobar1 directory and
> ln -sf foobar1 /tmp/foobar2
> cd /tmp/foobar2
> then -fdebug-prefix-map=`pwd`:/blah will just not work, while
> src/test.c will be canonicalized to /tmp/foobar1/src/test.c,
> old_prefix is still what the user provided which is /tmp/foobar2.
> User would need to change their uses to use -fdebug-prefix-map=`realpath $(pwd)`=/blah
> 
> I'm attaching 3 so far just compile tested patches.
> 
> The first patch just reverts the patch (and its follow-up patch).
> 
> The second introduces a new option, -f{,no}-canon-prefix-map which affects
> the behavior of -f{file,macro,debug,profile}-prefix-map=, if on it
> canonicalizes the old path of the prefix map option and compares that
> against the canonicalized filename for absolute paths but not relative.
> 
> And last is like the second, but does that also for relative paths except
> for filenames with no / (or / or \ on DOS based fs).  So, the third patch
> gets an optional behavior of what has been on the trunk lately with the
> difference that the old_prefix is canonicalized by the compiler.
> 
> Initially I've thought I'd just add some magic syntax to the OLD=NEW
> argument of those options (because there are 4 of them), but as noted
> in the comments, = is valid char in OLD (just not new), so it would
> be hard to figure out some syntax.  So instead a new option, which one
> can turn on and off for different -f*-prefix-map= options if needed.
> 
> -fdebug-prefix-map=/path1=/mypath1 -fcanon-prefix-map \
> -fdebug-prefix-map=/path2=/mypath2 -fno-canon-prefix-map \
> -fdebug-prefix-map=/path3=/mypath3
> 
> will use the old behavior for the /path1 and /path3 handling and
> the new one only for /path2 handling.
> 
> Thoughts on this?
> 
> 	Jakub

> 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR other/108464
> 	* file-prefix-map.cc (remap_filename): Revert 2022-11-01 and 2022-11-07
> 	changes.
> 
> --- gcc/file-prefix-map.cc
> +++ gcc/file-prefix-map.cc
> @@ -70,29 +70,19 @@ remap_filename (file_prefix_map *maps, const char *filename)
>    file_prefix_map *map;
>    char *s;
>    const char *name;
> -  char *realname;
>    size_t name_len;
>  
> -  if (!filename || lbasename (filename) == filename)
> -    return filename;
> -
> -  realname = lrealpath (filename);
> -
>    for (map = maps; map; map = map->next)
> -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> +    if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
>        break;
>    if (!map)
> -    {
> -      free (realname);
> -      return filename;
> -    }
> -  name = realname + map->old_len;
> +    return filename;
> +  name = filename + map->old_len;
>    name_len = strlen (name) + 1;
>  
>    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
>    memcpy (s, map->new_prefix, map->new_len);
>    memcpy (s + map->new_len, name, name_len);
> -  free (realname);
>    return s;
>  }
>  

> 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR other/108464
> 	* common.opt (fcanon-prefix-map): New option.
> 	* opts.cc: Include file-prefix-map.h.
> 	(flag_canon_prefix_map): New variable.
> 	(common_handle_option): Handle OPT_fcanon_prefix_map.
> 	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
> 	* file-prefix-map.h (flag_canon_prefix_map): Declare.
> 	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
> 	member.
> 	(add_prefix_map): Initialize canonicalize member from
> 	flag_canon_prefix_map, and if true and old_prefix is absolute
> 	pathname, canonicalize it using lrealpath.
> 	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
> 	use lrealpath result only for absolute filenames and only for
> 	map->canonicalize map entries.
> 	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
> 	* opts-global.cc (handle_common_deferred_options): Clear
> 	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
> 	* doc/invoke.texi (-fcanon-prefix-map): Document.
> 	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
> 	see also for -fcanon-prefix-map.
> 	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.
> 
> --- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
> +++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
> @@ -1204,6 +1204,10 @@ fchecking=
>  Common Joined RejectNegative UInteger Var(flag_checking)
>  Perform internal consistency checkings.
>  
> +fcanon-prefix-map
> +Common Var(common_deferred_options) Defer
> +For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
> +
>  fcode-hoisting
>  Common Var(flag_code_hoisting) Optimization
>  Enable code hoisting.
> --- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
> +++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
> @@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
>  #include "diagnostic-color.h"
>  #include "version.h"
>  #include "selftest.h"
> +#include "file-prefix-map.h"
>  
>  /* In this file all option sets are explicit.  */
>  #undef OPTION_SET_P
>  
> +/* Set by -fcanon-prefix-map.  */
> +bool flag_canon_prefix_map;
> +
>  static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
>  
>  /* Names of fundamental debug info formats indexed by enum
> @@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
>        /* Deferred.  */
>        break;
>  
> +    case OPT_fcanon_prefix_map:
> +      flag_canon_prefix_map = value;
> +      break;
> +
>      case OPT_fcallgraph_info:
>        opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
>        break;
> @@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
>        case OPT_fmacro_prefix_map_:
>        case OPT_ffile_prefix_map_:
>        case OPT_fprofile_prefix_map_:
> +      case OPT_fcanon_prefix_map:
>        case OPT_fcompare_debug:
>        case OPT_fchecking:
>        case OPT_fchecking_:
> --- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
> +++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
> @@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
>  void add_debug_prefix_map (const char *);
>  void add_file_prefix_map (const char *);
>  void add_profile_prefix_map (const char *);
> +extern bool flag_canon_prefix_map;
>  
>  const char *remap_macro_filename (const char *);
>  const char *remap_debug_filename (const char *);
> --- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
> +++ gcc/file-prefix-map.cc	2023-01-20 13:53:31.568896433 +0100
> @@ -30,6 +30,7 @@ struct file_prefix_map
>    const char *new_prefix;
>    size_t old_len;
>    size_t new_len;
> +  bool canonicalize;
>    struct file_prefix_map *next;
>  };
>  
> @@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
>        return;
>      }
>    map = XNEW (file_prefix_map);
> +  map->canonicalize = flag_canon_prefix_map;
>    map->old_prefix = xstrndup (arg, p - arg);
>    map->old_len = p - arg;
> +  if (map->canonicalize && IS_ABSOLUTE_PATH (map->old_prefix))
> +    {
> +      char *realname = lrealpath (map->old_prefix);
> +      free (const_cast <char *> (map->old_prefix));
> +      map->old_prefix = realname;
> +      map->old_len = strlen (realname);
> +    }
>    p++;
>    map->new_prefix = xstrdup (p);
>    map->new_len = strlen (p);
> @@ -70,23 +79,32 @@ remap_filename (file_prefix_map *maps, c
>    file_prefix_map *map;
>    char *s;
>    const char *name;
> -  char *realname;
> +  char *realname = NULL;
>    size_t name_len;
>  
> -  if (!filename || lbasename (filename) == filename)
> +  if (!filename)
>      return filename;
>  
> -  realname = lrealpath (filename);
> -
> +  bool absolute = IS_ABSOLUTE_PATH (filename);
>    for (map = maps; map; map = map->next)
> -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> +    if (map->canonicalize && absolute)
> +      {
> +	if (realname == NULL)
> +	  realname = lrealpath (filename);
> +	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> +	  break;
> +      }
> +    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
>        break;
>    if (!map)
>      {
>        free (realname);
>        return filename;
>      }
> -  name = realname + map->old_len;
> +  if (map->canonicalize && absolute)
> +    name = realname + map->old_len;
> +  else
> +    name = filename + map->old_len;
>    name_len = strlen (name) + 1;
>  
>    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
> @@ -97,7 +115,7 @@ remap_filename (file_prefix_map *maps, c
>  }
>  
>  /* NOTE: if adding another -f*-prefix-map option then don't forget to
> -   ignore it in DW_AT_producer (dwarf2out.cc).  */
> +   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
>  
>  /* Linked lists of file_prefix_map structures.  */
>  static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
> --- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
> +++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
> @@ -150,6 +150,7 @@ lto_write_options (void)
>  	case OPT_ffile_prefix_map_:
>  	case OPT_fmacro_prefix_map_:
>  	case OPT_fprofile_prefix_map_:
> +	case OPT_fcanon_prefix_map:
>  	case OPT_fwhole_program:
>  	  continue;
>  
> --- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
> +++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
> @@ -364,6 +364,7 @@ handle_common_deferred_options (void)
>    if (flag_opt_info)
>      opt_info_switch_p (NULL);
>  
> +  flag_canon_prefix_map = false;
>    FOR_EACH_VEC_ELT (v, i, opt)
>      {
>        switch (opt->opt_index)
> @@ -392,6 +393,10 @@ handle_common_deferred_options (void)
>  	  add_profile_prefix_map (opt->arg);
>  	  break;
>  
> +	case OPT_fcanon_prefix_map:
> +	  flag_canon_prefix_map = opt->value;
> +	  break;
> +
>  	case OPT_fdump_:
>  	  g->get_dumps ()->dump_switch_p (opt->arg);
>  	  break;
> --- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
> +++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
> @@ -191,7 +191,7 @@ in the following sections.
>  -dumpdir @var{dumppfx}  -x @var{language}  @gol
>  -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
>  -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
> -@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
> +@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
>  -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
>  -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
>  
> @@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
>  option is equivalent to specifying all the individual
>  @option{-f*-prefix-map} options.  This can be used to make reproducible
>  builds that are location independent.  Directories referenced by
> -directives are not affected by these options. See also
> -@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
> -@option{-fprofile-prefix-map}.
> +directives are not affected by these options.  See also
> +@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
> +@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
> +
> +@item -fcanon-prefix-map
> +@opindex fcanon-prefix-map
> +For the @option{-f*-prefix-map} options normally comparison
> +of @file{@var{old}} prefix against the filename that would be normally
> +referenced in the result of the compilation is done using textual
> +comparison of the prefixes, or ignoring character case for case insensitive
> +filesystems and considering slashes and backslashes as equal on DOS based
> +filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
> +to be done for absolute pathnames on canonicalized paths of @file{@var{old}}
> +and the referenced filename.
>  
>  @item -fplugin=@var{name}.so
>  @opindex fplugin
> @@ -11289,7 +11300,8 @@ build-time path with an install-time pat
>  also be used to change an absolute path to a relative path by using
>  @file{.} for @var{new}.  This can give more reproducible builds, which
>  are location independent, but may require an extra command to tell GDB
> -where to find the source files. See also @option{-ffile-prefix-map}.
> +where to find the source files. See also @option{-ffile-prefix-map}
> +and @option{-fcanon-prefix-map}.
>  
>  @item -fvar-tracking
>  @opindex fvar-tracking
> @@ -16466,7 +16478,7 @@ When compiling files residing in directo
>  profiling information (with @option{--coverage})
>  describing them as if the files resided in
>  directory @file{@var{new}} instead.
> -See also @option{-ffile-prefix-map}.
> +See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
>  
>  @item -fprofile-update=@var{method}
>  @opindex fprofile-update
> --- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
> +++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
> @@ -305,7 +305,7 @@ to change an absolute path to a relative
>  @var{new} which can result in more reproducible builds that are
>  location independent.  This option also affects
>  @code{__builtin_FILE()} during compilation.  See also
> -@option{-ffile-prefix-map}.
> +@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
>  
>  @item -fexec-charset=@var{charset}
>  @opindex fexec-charset

> 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR other/108464
> 	* common.opt (fcanon-prefix-map): New option.
> 	* opts.cc: Include file-prefix-map.h.
> 	(flag_canon_prefix_map): New variable.
> 	(common_handle_option): Handle OPT_fcanon_prefix_map.
> 	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
> 	* file-prefix-map.h (flag_canon_prefix_map): Declare.
> 	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
> 	member.
> 	(add_prefix_map): Initialize canonicalize member from
> 	flag_canon_prefix_map, and if true canonicalize it using lrealpath.
> 	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
> 	use lrealpath result only for map->canonicalize map entries.
> 	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
> 	* opts-global.cc (handle_common_deferred_options): Clear
> 	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
> 	* doc/invoke.texi (-fcanon-prefix-map): Document.
> 	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
> 	see also for -fcanon-prefix-map.
> 	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.
> 
> --- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
> +++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
> @@ -1204,6 +1204,10 @@ fchecking=
>  Common Joined RejectNegative UInteger Var(flag_checking)
>  Perform internal consistency checkings.
>  
> +fcanon-prefix-map
> +Common Var(common_deferred_options) Defer
> +For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
> +
>  fcode-hoisting
>  Common Var(flag_code_hoisting) Optimization
>  Enable code hoisting.
> --- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
> +++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
> @@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
>  #include "diagnostic-color.h"
>  #include "version.h"
>  #include "selftest.h"
> +#include "file-prefix-map.h"
>  
>  /* In this file all option sets are explicit.  */
>  #undef OPTION_SET_P
>  
> +/* Set by -fcanon-prefix-map.  */
> +bool flag_canon_prefix_map;
> +
>  static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
>  
>  /* Names of fundamental debug info formats indexed by enum
> @@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
>        /* Deferred.  */
>        break;
>  
> +    case OPT_fcanon_prefix_map:
> +      flag_canon_prefix_map = value;
> +      break;
> +
>      case OPT_fcallgraph_info:
>        opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
>        break;
> @@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
>        case OPT_fmacro_prefix_map_:
>        case OPT_ffile_prefix_map_:
>        case OPT_fprofile_prefix_map_:
> +      case OPT_fcanon_prefix_map:
>        case OPT_fcompare_debug:
>        case OPT_fchecking:
>        case OPT_fchecking_:
> --- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
> +++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
> @@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
>  void add_debug_prefix_map (const char *);
>  void add_file_prefix_map (const char *);
>  void add_profile_prefix_map (const char *);
> +extern bool flag_canon_prefix_map;
>  
>  const char *remap_macro_filename (const char *);
>  const char *remap_debug_filename (const char *);
> --- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
> +++ gcc/file-prefix-map.cc	2023-01-20 15:42:30.661088219 +0100
> @@ -30,6 +30,7 @@ struct file_prefix_map
>    const char *new_prefix;
>    size_t old_len;
>    size_t new_len;
> +  bool canonicalize;
>    struct file_prefix_map *next;
>  };
>  
> @@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
>        return;
>      }
>    map = XNEW (file_prefix_map);
> +  map->canonicalize = flag_canon_prefix_map;
>    map->old_prefix = xstrndup (arg, p - arg);
>    map->old_len = p - arg;
> +  if (map->canonicalize)
> +    {
> +      char *realname = lrealpath (map->old_prefix);
> +      free (const_cast <char *> (map->old_prefix));
> +      map->old_prefix = realname;
> +      map->old_len = strlen (realname);
> +    }
>    p++;
>    map->new_prefix = xstrdup (p);
>    map->new_len = strlen (p);
> @@ -70,34 +79,49 @@ remap_filename (file_prefix_map *maps, c
>    file_prefix_map *map;
>    char *s;
>    const char *name;
> -  char *realname;
> +  const char *realname = NULL;
>    size_t name_len;
>  
> -  if (!filename || lbasename (filename) == filename)
> +  if (!filename)
>      return filename;
>  
> -  realname = lrealpath (filename);
> -
>    for (map = maps; map; map = map->next)
> -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> +    if (map->canonicalize)
> +      {
> +	if (realname == NULL)
> +	  {
> +	    if (lbasename (filename) == filename)
> +	      realname = filename;
> +	    else
> +	      realname = lrealpath (filename);
> +	  }
> +	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> +	  break;
> +      }
> +    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
>        break;
>    if (!map)
>      {
> -      free (realname);
> +      if (realname != filename)
> +	free (const_cast <char *> (realname));
>        return filename;
>      }
> -  name = realname + map->old_len;
> +  if (map->canonicalize)
> +    name = realname + map->old_len;
> +  else
> +    name = filename + map->old_len;
>    name_len = strlen (name) + 1;
>  
>    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
>    memcpy (s, map->new_prefix, map->new_len);
>    memcpy (s + map->new_len, name, name_len);
> -  free (realname);
> +  if (realname != filename)
> +    free (const_cast <char *> (realname));
>    return s;
>  }
>  
>  /* NOTE: if adding another -f*-prefix-map option then don't forget to
> -   ignore it in DW_AT_producer (dwarf2out.cc).  */
> +   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
>  
>  /* Linked lists of file_prefix_map structures.  */
>  static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
> --- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
> +++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
> @@ -150,6 +150,7 @@ lto_write_options (void)
>  	case OPT_ffile_prefix_map_:
>  	case OPT_fmacro_prefix_map_:
>  	case OPT_fprofile_prefix_map_:
> +	case OPT_fcanon_prefix_map:
>  	case OPT_fwhole_program:
>  	  continue;
>  
> --- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
> +++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
> @@ -364,6 +364,7 @@ handle_common_deferred_options (void)
>    if (flag_opt_info)
>      opt_info_switch_p (NULL);
>  
> +  flag_canon_prefix_map = false;
>    FOR_EACH_VEC_ELT (v, i, opt)
>      {
>        switch (opt->opt_index)
> @@ -392,6 +393,10 @@ handle_common_deferred_options (void)
>  	  add_profile_prefix_map (opt->arg);
>  	  break;
>  
> +	case OPT_fcanon_prefix_map:
> +	  flag_canon_prefix_map = opt->value;
> +	  break;
> +
>  	case OPT_fdump_:
>  	  g->get_dumps ()->dump_switch_p (opt->arg);
>  	  break;
> --- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
> +++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
> @@ -191,7 +191,7 @@ in the following sections.
>  -dumpdir @var{dumppfx}  -x @var{language}  @gol
>  -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
>  -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
> -@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
> +@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
>  -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
>  -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
>  
> @@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
>  option is equivalent to specifying all the individual
>  @option{-f*-prefix-map} options.  This can be used to make reproducible
>  builds that are location independent.  Directories referenced by
> -directives are not affected by these options. See also
> -@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
> -@option{-fprofile-prefix-map}.
> +directives are not affected by these options.  See also
> +@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
> +@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
> +
> +@item -fcanon-prefix-map
> +@opindex fcanon-prefix-map
> +For the @option{-f*-prefix-map} options normally comparison
> +of @file{@var{old}} prefix against the filename that would be normally
> +referenced in the result of the compilation is done using textual
> +comparison of the prefixes, or ignoring character case for case insensitive
> +filesystems and considering slashes and backslashes as equal on DOS based
> +filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
> +to be done on canonicalized paths of @file{@var{old}}
> +and the referenced filename.
>  
>  @item -fplugin=@var{name}.so
>  @opindex fplugin
> @@ -11289,7 +11300,8 @@ build-time path with an install-time pat
>  also be used to change an absolute path to a relative path by using
>  @file{.} for @var{new}.  This can give more reproducible builds, which
>  are location independent, but may require an extra command to tell GDB
> -where to find the source files. See also @option{-ffile-prefix-map}.
> +where to find the source files. See also @option{-ffile-prefix-map}
> +and @option{-fcanon-prefix-map}.
>  
>  @item -fvar-tracking
>  @opindex fvar-tracking
> @@ -16466,7 +16478,7 @@ When compiling files residing in directo
>  profiling information (with @option{--coverage})
>  describing them as if the files resided in
>  directory @file{@var{new}} instead.
> -See also @option{-ffile-prefix-map}.
> +See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
>  
>  @item -fprofile-update=@var{method}
>  @opindex fprofile-update
> --- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
> +++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
> @@ -305,7 +305,7 @@ to change an absolute path to a relative
>  @var{new} which can result in more reproducible builds that are
>  location independent.  This option also affects
>  @code{__builtin_FILE()} during compilation.  See also
> -@option{-ffile-prefix-map}.
> +@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
>  
>  @item -fexec-charset=@var{charset}
>  @opindex fexec-charset


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Patch ping - [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464]
  2023-03-10  8:49     ` Patch ping - " Jakub Jelinek
@ 2023-03-10  9:05       ` Richard Biener
  2023-03-10 10:14         ` Richard Purdie
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2023-03-10  9:05 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, gcc-patches, Richard Purdie

On Fri, 10 Mar 2023, Jakub Jelinek wrote:

> Hi!
> 
> I'd like to ping these patches.  All 3 variants have been
> bootstrapped/regtested on x86_64-linux and i686-linux, the last
> one is my preference I guess.  The current state breaks e.g. ccache.
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610285.html
>   - PR108464 - P1 - file-prefix-map: Fix up -f*-prefix-map= (3 variants)

Let's go with your preference - I was hoping on comments from Richard 
Purdie as of his preference but then ..

Thanks,
Richard.

> Thanks
> 	Jakub
> 
> On Fri, Jan 20, 2023 at 04:05:55PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > On Tue, Nov 01, 2022 at 01:46:20PM -0600, Jeff Law via Gcc-patches wrote:
> > > > This does cause a change of behaviour if users were previously relying upon
> > > > symlinks or absolute paths not being resolved.
> > > 
> > > I'm not too worried about this scenario.
> > 
> > As mentioned in the PR, this patch breaks e.g. ccache testsuite.
> > 
> > I strongly doubt most of the users want such a behavior, because it
> > makes all filenames absolute when -f*-prefix-map= options remap one
> > absolute path to another one.
> > Say if I'm in /tmp and /tmp is the canonical path and there is
> > src/test.c file, with -fdebug-prefix-map=/tmp=/blah
> > previously there would be DW_AT_comp_dir "/blah" and it is still there,
> > but DW_AT_name which was previouly "src/test.c" (relative against
> > DW_AT_comp_dir) is now "/blah/src/test.c" instead.
> > 
> > Even worse, the canonicalization is only done on the remap_filename
> > argument, but not on the old_prefix side.  That is e.g. what breaks
> > ccache.  If there is
> > /tmp/foobar1 directory and
> > ln -sf foobar1 /tmp/foobar2
> > cd /tmp/foobar2
> > then -fdebug-prefix-map=`pwd`:/blah will just not work, while
> > src/test.c will be canonicalized to /tmp/foobar1/src/test.c,
> > old_prefix is still what the user provided which is /tmp/foobar2.
> > User would need to change their uses to use -fdebug-prefix-map=`realpath $(pwd)`=/blah
> > 
> > I'm attaching 3 so far just compile tested patches.
> > 
> > The first patch just reverts the patch (and its follow-up patch).
> > 
> > The second introduces a new option, -f{,no}-canon-prefix-map which affects
> > the behavior of -f{file,macro,debug,profile}-prefix-map=, if on it
> > canonicalizes the old path of the prefix map option and compares that
> > against the canonicalized filename for absolute paths but not relative.
> > 
> > And last is like the second, but does that also for relative paths except
> > for filenames with no / (or / or \ on DOS based fs).  So, the third patch
> > gets an optional behavior of what has been on the trunk lately with the
> > difference that the old_prefix is canonicalized by the compiler.
> > 
> > Initially I've thought I'd just add some magic syntax to the OLD=NEW
> > argument of those options (because there are 4 of them), but as noted
> > in the comments, = is valid char in OLD (just not new), so it would
> > be hard to figure out some syntax.  So instead a new option, which one
> > can turn on and off for different -f*-prefix-map= options if needed.
> > 
> > -fdebug-prefix-map=/path1=/mypath1 -fcanon-prefix-map \
> > -fdebug-prefix-map=/path2=/mypath2 -fno-canon-prefix-map \
> > -fdebug-prefix-map=/path3=/mypath3
> > 
> > will use the old behavior for the /path1 and /path3 handling and
> > the new one only for /path2 handling.
> > 
> > Thoughts on this?
> > 
> > 	Jakub
> 
> > 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR other/108464
> > 	* file-prefix-map.cc (remap_filename): Revert 2022-11-01 and 2022-11-07
> > 	changes.
> > 
> > --- gcc/file-prefix-map.cc
> > +++ gcc/file-prefix-map.cc
> > @@ -70,29 +70,19 @@ remap_filename (file_prefix_map *maps, const char *filename)
> >    file_prefix_map *map;
> >    char *s;
> >    const char *name;
> > -  char *realname;
> >    size_t name_len;
> >  
> > -  if (!filename || lbasename (filename) == filename)
> > -    return filename;
> > -
> > -  realname = lrealpath (filename);
> > -
> >    for (map = maps; map; map = map->next)
> > -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> > +    if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
> >        break;
> >    if (!map)
> > -    {
> > -      free (realname);
> > -      return filename;
> > -    }
> > -  name = realname + map->old_len;
> > +    return filename;
> > +  name = filename + map->old_len;
> >    name_len = strlen (name) + 1;
> >  
> >    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
> >    memcpy (s, map->new_prefix, map->new_len);
> >    memcpy (s + map->new_len, name, name_len);
> > -  free (realname);
> >    return s;
> >  }
> >  
> 
> > 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR other/108464
> > 	* common.opt (fcanon-prefix-map): New option.
> > 	* opts.cc: Include file-prefix-map.h.
> > 	(flag_canon_prefix_map): New variable.
> > 	(common_handle_option): Handle OPT_fcanon_prefix_map.
> > 	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
> > 	* file-prefix-map.h (flag_canon_prefix_map): Declare.
> > 	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
> > 	member.
> > 	(add_prefix_map): Initialize canonicalize member from
> > 	flag_canon_prefix_map, and if true and old_prefix is absolute
> > 	pathname, canonicalize it using lrealpath.
> > 	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
> > 	use lrealpath result only for absolute filenames and only for
> > 	map->canonicalize map entries.
> > 	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
> > 	* opts-global.cc (handle_common_deferred_options): Clear
> > 	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
> > 	* doc/invoke.texi (-fcanon-prefix-map): Document.
> > 	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
> > 	see also for -fcanon-prefix-map.
> > 	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.
> > 
> > --- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
> > +++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
> > @@ -1204,6 +1204,10 @@ fchecking=
> >  Common Joined RejectNegative UInteger Var(flag_checking)
> >  Perform internal consistency checkings.
> >  
> > +fcanon-prefix-map
> > +Common Var(common_deferred_options) Defer
> > +For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
> > +
> >  fcode-hoisting
> >  Common Var(flag_code_hoisting) Optimization
> >  Enable code hoisting.
> > --- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
> > +++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
> > @@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
> >  #include "diagnostic-color.h"
> >  #include "version.h"
> >  #include "selftest.h"
> > +#include "file-prefix-map.h"
> >  
> >  /* In this file all option sets are explicit.  */
> >  #undef OPTION_SET_P
> >  
> > +/* Set by -fcanon-prefix-map.  */
> > +bool flag_canon_prefix_map;
> > +
> >  static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
> >  
> >  /* Names of fundamental debug info formats indexed by enum
> > @@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
> >        /* Deferred.  */
> >        break;
> >  
> > +    case OPT_fcanon_prefix_map:
> > +      flag_canon_prefix_map = value;
> > +      break;
> > +
> >      case OPT_fcallgraph_info:
> >        opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
> >        break;
> > @@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
> >        case OPT_fmacro_prefix_map_:
> >        case OPT_ffile_prefix_map_:
> >        case OPT_fprofile_prefix_map_:
> > +      case OPT_fcanon_prefix_map:
> >        case OPT_fcompare_debug:
> >        case OPT_fchecking:
> >        case OPT_fchecking_:
> > --- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
> > +++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
> > @@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
> >  void add_debug_prefix_map (const char *);
> >  void add_file_prefix_map (const char *);
> >  void add_profile_prefix_map (const char *);
> > +extern bool flag_canon_prefix_map;
> >  
> >  const char *remap_macro_filename (const char *);
> >  const char *remap_debug_filename (const char *);
> > --- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
> > +++ gcc/file-prefix-map.cc	2023-01-20 13:53:31.568896433 +0100
> > @@ -30,6 +30,7 @@ struct file_prefix_map
> >    const char *new_prefix;
> >    size_t old_len;
> >    size_t new_len;
> > +  bool canonicalize;
> >    struct file_prefix_map *next;
> >  };
> >  
> > @@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
> >        return;
> >      }
> >    map = XNEW (file_prefix_map);
> > +  map->canonicalize = flag_canon_prefix_map;
> >    map->old_prefix = xstrndup (arg, p - arg);
> >    map->old_len = p - arg;
> > +  if (map->canonicalize && IS_ABSOLUTE_PATH (map->old_prefix))
> > +    {
> > +      char *realname = lrealpath (map->old_prefix);
> > +      free (const_cast <char *> (map->old_prefix));
> > +      map->old_prefix = realname;
> > +      map->old_len = strlen (realname);
> > +    }
> >    p++;
> >    map->new_prefix = xstrdup (p);
> >    map->new_len = strlen (p);
> > @@ -70,23 +79,32 @@ remap_filename (file_prefix_map *maps, c
> >    file_prefix_map *map;
> >    char *s;
> >    const char *name;
> > -  char *realname;
> > +  char *realname = NULL;
> >    size_t name_len;
> >  
> > -  if (!filename || lbasename (filename) == filename)
> > +  if (!filename)
> >      return filename;
> >  
> > -  realname = lrealpath (filename);
> > -
> > +  bool absolute = IS_ABSOLUTE_PATH (filename);
> >    for (map = maps; map; map = map->next)
> > -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> > +    if (map->canonicalize && absolute)
> > +      {
> > +	if (realname == NULL)
> > +	  realname = lrealpath (filename);
> > +	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> > +	  break;
> > +      }
> > +    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
> >        break;
> >    if (!map)
> >      {
> >        free (realname);
> >        return filename;
> >      }
> > -  name = realname + map->old_len;
> > +  if (map->canonicalize && absolute)
> > +    name = realname + map->old_len;
> > +  else
> > +    name = filename + map->old_len;
> >    name_len = strlen (name) + 1;
> >  
> >    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
> > @@ -97,7 +115,7 @@ remap_filename (file_prefix_map *maps, c
> >  }
> >  
> >  /* NOTE: if adding another -f*-prefix-map option then don't forget to
> > -   ignore it in DW_AT_producer (dwarf2out.cc).  */
> > +   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
> >  
> >  /* Linked lists of file_prefix_map structures.  */
> >  static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
> > --- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
> > +++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
> > @@ -150,6 +150,7 @@ lto_write_options (void)
> >  	case OPT_ffile_prefix_map_:
> >  	case OPT_fmacro_prefix_map_:
> >  	case OPT_fprofile_prefix_map_:
> > +	case OPT_fcanon_prefix_map:
> >  	case OPT_fwhole_program:
> >  	  continue;
> >  
> > --- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
> > +++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
> > @@ -364,6 +364,7 @@ handle_common_deferred_options (void)
> >    if (flag_opt_info)
> >      opt_info_switch_p (NULL);
> >  
> > +  flag_canon_prefix_map = false;
> >    FOR_EACH_VEC_ELT (v, i, opt)
> >      {
> >        switch (opt->opt_index)
> > @@ -392,6 +393,10 @@ handle_common_deferred_options (void)
> >  	  add_profile_prefix_map (opt->arg);
> >  	  break;
> >  
> > +	case OPT_fcanon_prefix_map:
> > +	  flag_canon_prefix_map = opt->value;
> > +	  break;
> > +
> >  	case OPT_fdump_:
> >  	  g->get_dumps ()->dump_switch_p (opt->arg);
> >  	  break;
> > --- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
> > +++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
> > @@ -191,7 +191,7 @@ in the following sections.
> >  -dumpdir @var{dumppfx}  -x @var{language}  @gol
> >  -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
> >  -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
> > -@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
> > +@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
> >  -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
> >  -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
> >  
> > @@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
> >  option is equivalent to specifying all the individual
> >  @option{-f*-prefix-map} options.  This can be used to make reproducible
> >  builds that are location independent.  Directories referenced by
> > -directives are not affected by these options. See also
> > -@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
> > -@option{-fprofile-prefix-map}.
> > +directives are not affected by these options.  See also
> > +@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
> > +@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
> > +
> > +@item -fcanon-prefix-map
> > +@opindex fcanon-prefix-map
> > +For the @option{-f*-prefix-map} options normally comparison
> > +of @file{@var{old}} prefix against the filename that would be normally
> > +referenced in the result of the compilation is done using textual
> > +comparison of the prefixes, or ignoring character case for case insensitive
> > +filesystems and considering slashes and backslashes as equal on DOS based
> > +filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
> > +to be done for absolute pathnames on canonicalized paths of @file{@var{old}}
> > +and the referenced filename.
> >  
> >  @item -fplugin=@var{name}.so
> >  @opindex fplugin
> > @@ -11289,7 +11300,8 @@ build-time path with an install-time pat
> >  also be used to change an absolute path to a relative path by using
> >  @file{.} for @var{new}.  This can give more reproducible builds, which
> >  are location independent, but may require an extra command to tell GDB
> > -where to find the source files. See also @option{-ffile-prefix-map}.
> > +where to find the source files. See also @option{-ffile-prefix-map}
> > +and @option{-fcanon-prefix-map}.
> >  
> >  @item -fvar-tracking
> >  @opindex fvar-tracking
> > @@ -16466,7 +16478,7 @@ When compiling files residing in directo
> >  profiling information (with @option{--coverage})
> >  describing them as if the files resided in
> >  directory @file{@var{new}} instead.
> > -See also @option{-ffile-prefix-map}.
> > +See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
> >  
> >  @item -fprofile-update=@var{method}
> >  @opindex fprofile-update
> > --- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
> > +++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
> > @@ -305,7 +305,7 @@ to change an absolute path to a relative
> >  @var{new} which can result in more reproducible builds that are
> >  location independent.  This option also affects
> >  @code{__builtin_FILE()} during compilation.  See also
> > -@option{-ffile-prefix-map}.
> > +@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
> >  
> >  @item -fexec-charset=@var{charset}
> >  @opindex fexec-charset
> 
> > 2023-01-20  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR other/108464
> > 	* common.opt (fcanon-prefix-map): New option.
> > 	* opts.cc: Include file-prefix-map.h.
> > 	(flag_canon_prefix_map): New variable.
> > 	(common_handle_option): Handle OPT_fcanon_prefix_map.
> > 	(gen_command_line_string): Ignore OPT_fcanon_prefix_map.
> > 	* file-prefix-map.h (flag_canon_prefix_map): Declare.
> > 	* file-prefix-map.cc (struct file_prefix_map): Add canonicalize
> > 	member.
> > 	(add_prefix_map): Initialize canonicalize member from
> > 	flag_canon_prefix_map, and if true canonicalize it using lrealpath.
> > 	(remap_filename): Revert 2022-11-01 and 2022-11-07 changes,
> > 	use lrealpath result only for map->canonicalize map entries.
> > 	* lto-opts.cc (lto_write_options): Ignore OPT_fcanon_prefix_map.
> > 	* opts-global.cc (handle_common_deferred_options): Clear
> > 	flag_canon_prefix_map at the start and handle OPT_fcanon_prefix_map.
> > 	* doc/invoke.texi (-fcanon-prefix-map): Document.
> > 	(-ffile-prefix-map, -fdebug-prefix-map, -fprofile-prefix-map): Add
> > 	see also for -fcanon-prefix-map.
> > 	* doc/cppopts.texi (-fmacro-prefix-map): Likewise.
> > 
> > --- gcc/common.opt.jj	2023-01-09 13:18:28.298019442 +0100
> > +++ gcc/common.opt	2023-01-20 12:43:17.152178057 +0100
> > @@ -1204,6 +1204,10 @@ fchecking=
> >  Common Joined RejectNegative UInteger Var(flag_checking)
> >  Perform internal consistency checkings.
> >  
> > +fcanon-prefix-map
> > +Common Var(common_deferred_options) Defer
> > +For -f*-prefix-map= options compare canonicalized pathnames rather than just strings.
> > +
> >  fcode-hoisting
> >  Common Var(flag_code_hoisting) Optimization
> >  Enable code hoisting.
> > --- gcc/opts.cc.jj	2023-01-02 09:32:52.073856323 +0100
> > +++ gcc/opts.cc	2023-01-20 13:54:53.526705571 +0100
> > @@ -34,10 +34,14 @@ along with GCC; see the file COPYING3.
> >  #include "diagnostic-color.h"
> >  #include "version.h"
> >  #include "selftest.h"
> > +#include "file-prefix-map.h"
> >  
> >  /* In this file all option sets are explicit.  */
> >  #undef OPTION_SET_P
> >  
> > +/* Set by -fcanon-prefix-map.  */
> > +bool flag_canon_prefix_map;
> > +
> >  static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
> >  
> >  /* Names of fundamental debug info formats indexed by enum
> > @@ -2812,6 +2816,10 @@ common_handle_option (struct gcc_options
> >        /* Deferred.  */
> >        break;
> >  
> > +    case OPT_fcanon_prefix_map:
> > +      flag_canon_prefix_map = value;
> > +      break;
> > +
> >      case OPT_fcallgraph_info:
> >        opts->x_flag_callgraph_info = CALLGRAPH_INFO_NAKED;
> >        break;
> > @@ -3718,6 +3726,7 @@ gen_command_line_string (cl_decoded_opti
> >        case OPT_fmacro_prefix_map_:
> >        case OPT_ffile_prefix_map_:
> >        case OPT_fprofile_prefix_map_:
> > +      case OPT_fcanon_prefix_map:
> >        case OPT_fcompare_debug:
> >        case OPT_fchecking:
> >        case OPT_fchecking_:
> > --- gcc/file-prefix-map.h.jj	2023-01-02 09:32:33.258128185 +0100
> > +++ gcc/file-prefix-map.h	2023-01-20 12:44:31.542094469 +0100
> > @@ -22,6 +22,7 @@ void add_macro_prefix_map (const char *)
> >  void add_debug_prefix_map (const char *);
> >  void add_file_prefix_map (const char *);
> >  void add_profile_prefix_map (const char *);
> > +extern bool flag_canon_prefix_map;
> >  
> >  const char *remap_macro_filename (const char *);
> >  const char *remap_debug_filename (const char *);
> > --- gcc/file-prefix-map.cc.jj	2023-01-02 09:32:52.787846007 +0100
> > +++ gcc/file-prefix-map.cc	2023-01-20 15:42:30.661088219 +0100
> > @@ -30,6 +30,7 @@ struct file_prefix_map
> >    const char *new_prefix;
> >    size_t old_len;
> >    size_t new_len;
> > +  bool canonicalize;
> >    struct file_prefix_map *next;
> >  };
> >  
> > @@ -51,8 +52,16 @@ add_prefix_map (file_prefix_map *&maps,
> >        return;
> >      }
> >    map = XNEW (file_prefix_map);
> > +  map->canonicalize = flag_canon_prefix_map;
> >    map->old_prefix = xstrndup (arg, p - arg);
> >    map->old_len = p - arg;
> > +  if (map->canonicalize)
> > +    {
> > +      char *realname = lrealpath (map->old_prefix);
> > +      free (const_cast <char *> (map->old_prefix));
> > +      map->old_prefix = realname;
> > +      map->old_len = strlen (realname);
> > +    }
> >    p++;
> >    map->new_prefix = xstrdup (p);
> >    map->new_len = strlen (p);
> > @@ -70,34 +79,49 @@ remap_filename (file_prefix_map *maps, c
> >    file_prefix_map *map;
> >    char *s;
> >    const char *name;
> > -  char *realname;
> > +  const char *realname = NULL;
> >    size_t name_len;
> >  
> > -  if (!filename || lbasename (filename) == filename)
> > +  if (!filename)
> >      return filename;
> >  
> > -  realname = lrealpath (filename);
> > -
> >    for (map = maps; map; map = map->next)
> > -    if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> > +    if (map->canonicalize)
> > +      {
> > +	if (realname == NULL)
> > +	  {
> > +	    if (lbasename (filename) == filename)
> > +	      realname = filename;
> > +	    else
> > +	      realname = lrealpath (filename);
> > +	  }
> > +	if (filename_ncmp (realname, map->old_prefix, map->old_len) == 0)
> > +	  break;
> > +      }
> > +    else if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
> >        break;
> >    if (!map)
> >      {
> > -      free (realname);
> > +      if (realname != filename)
> > +	free (const_cast <char *> (realname));
> >        return filename;
> >      }
> > -  name = realname + map->old_len;
> > +  if (map->canonicalize)
> > +    name = realname + map->old_len;
> > +  else
> > +    name = filename + map->old_len;
> >    name_len = strlen (name) + 1;
> >  
> >    s = (char *) ggc_alloc_atomic (name_len + map->new_len);
> >    memcpy (s, map->new_prefix, map->new_len);
> >    memcpy (s + map->new_len, name, name_len);
> > -  free (realname);
> > +  if (realname != filename)
> > +    free (const_cast <char *> (realname));
> >    return s;
> >  }
> >  
> >  /* NOTE: if adding another -f*-prefix-map option then don't forget to
> > -   ignore it in DW_AT_producer (dwarf2out.cc).  */
> > +   ignore it in DW_AT_producer (gen_command_line_string in opts.cc).  */
> >  
> >  /* Linked lists of file_prefix_map structures.  */
> >  static file_prefix_map *macro_prefix_maps; /* -fmacro-prefix-map  */
> > --- gcc/lto-opts.cc.jj	2023-01-02 09:32:29.547181803 +0100
> > +++ gcc/lto-opts.cc	2023-01-20 13:55:35.010102584 +0100
> > @@ -150,6 +150,7 @@ lto_write_options (void)
> >  	case OPT_ffile_prefix_map_:
> >  	case OPT_fmacro_prefix_map_:
> >  	case OPT_fprofile_prefix_map_:
> > +	case OPT_fcanon_prefix_map:
> >  	case OPT_fwhole_program:
> >  	  continue;
> >  
> > --- gcc/opts-global.cc.jj	2023-01-02 09:32:35.169100574 +0100
> > +++ gcc/opts-global.cc	2023-01-20 13:38:31.174977423 +0100
> > @@ -364,6 +364,7 @@ handle_common_deferred_options (void)
> >    if (flag_opt_info)
> >      opt_info_switch_p (NULL);
> >  
> > +  flag_canon_prefix_map = false;
> >    FOR_EACH_VEC_ELT (v, i, opt)
> >      {
> >        switch (opt->opt_index)
> > @@ -392,6 +393,10 @@ handle_common_deferred_options (void)
> >  	  add_profile_prefix_map (opt->arg);
> >  	  break;
> >  
> > +	case OPT_fcanon_prefix_map:
> > +	  flag_canon_prefix_map = opt->value;
> > +	  break;
> > +
> >  	case OPT_fdump_:
> >  	  g->get_dumps ()->dump_switch_p (opt->arg);
> >  	  break;
> > --- gcc/doc/invoke.texi.jj	2023-01-16 11:52:16.115733593 +0100
> > +++ gcc/doc/invoke.texi	2023-01-20 14:44:47.514203801 +0100
> > @@ -191,7 +191,7 @@ in the following sections.
> >  -dumpdir @var{dumppfx}  -x @var{language}  @gol
> >  -v  -###  --help@r{[}=@var{class}@r{[},@dots{}@r{]]}  --target-help  --version @gol
> >  -pass-exit-codes  -pipe  -specs=@var{file}  -wrapper  @gol
> > -@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  @gol
> > +@@@var{file}  -ffile-prefix-map=@var{old}=@var{new}  -fcanon-prefix-map  @gol
> >  -fplugin=@var{file}  -fplugin-arg-@var{name}=@var{arg}  @gol
> >  -fdump-ada-spec@r{[}-slim@r{]}  -fada-spec-parent=@var{unit}  -fdump-go-spec=@var{file}}
> >  
> > @@ -2203,9 +2203,20 @@ files resided in directory @file{@var{ne
> >  option is equivalent to specifying all the individual
> >  @option{-f*-prefix-map} options.  This can be used to make reproducible
> >  builds that are location independent.  Directories referenced by
> > -directives are not affected by these options. See also
> > -@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map} and
> > -@option{-fprofile-prefix-map}.
> > +directives are not affected by these options.  See also
> > +@option{-fmacro-prefix-map}, @option{-fdebug-prefix-map},
> > +@option{-fprofile-prefix-map} and @option{-fcanon-prefix-map}.
> > +
> > +@item -fcanon-prefix-map
> > +@opindex fcanon-prefix-map
> > +For the @option{-f*-prefix-map} options normally comparison
> > +of @file{@var{old}} prefix against the filename that would be normally
> > +referenced in the result of the compilation is done using textual
> > +comparison of the prefixes, or ignoring character case for case insensitive
> > +filesystems and considering slashes and backslashes as equal on DOS based
> > +filesystems.  The @option{-fcanon-prefix-map} causes such comparisons
> > +to be done on canonicalized paths of @file{@var{old}}
> > +and the referenced filename.
> >  
> >  @item -fplugin=@var{name}.so
> >  @opindex fplugin
> > @@ -11289,7 +11300,8 @@ build-time path with an install-time pat
> >  also be used to change an absolute path to a relative path by using
> >  @file{.} for @var{new}.  This can give more reproducible builds, which
> >  are location independent, but may require an extra command to tell GDB
> > -where to find the source files. See also @option{-ffile-prefix-map}.
> > +where to find the source files. See also @option{-ffile-prefix-map}
> > +and @option{-fcanon-prefix-map}.
> >  
> >  @item -fvar-tracking
> >  @opindex fvar-tracking
> > @@ -16466,7 +16478,7 @@ When compiling files residing in directo
> >  profiling information (with @option{--coverage})
> >  describing them as if the files resided in
> >  directory @file{@var{new}} instead.
> > -See also @option{-ffile-prefix-map}.
> > +See also @option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
> >  
> >  @item -fprofile-update=@var{method}
> >  @opindex fprofile-update
> > --- gcc/doc/cppopts.texi.jj	2023-01-16 11:52:16.102733784 +0100
> > +++ gcc/doc/cppopts.texi	2023-01-20 14:45:09.106890233 +0100
> > @@ -305,7 +305,7 @@ to change an absolute path to a relative
> >  @var{new} which can result in more reproducible builds that are
> >  location independent.  This option also affects
> >  @code{__builtin_FILE()} during compilation.  See also
> > -@option{-ffile-prefix-map}.
> > +@option{-ffile-prefix-map} and @option{-fcanon-prefix-map}.
> >  
> >  @item -fexec-charset=@var{charset}
> >  @opindex fexec-charset
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Patch ping - [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464]
  2023-03-10  9:05       ` Richard Biener
@ 2023-03-10 10:14         ` Richard Purdie
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Purdie @ 2023-03-10 10:14 UTC (permalink / raw)
  To: Richard Biener, Jakub Jelinek; +Cc: Jeff Law, gcc-patches

On Fri, 2023-03-10 at 09:05 +0000, Richard Biener wrote:
> On Fri, 10 Mar 2023, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > I'd like to ping these patches.  All 3 variants have been
> > bootstrapped/regtested on x86_64-linux and i686-linux, the last
> > one is my preference I guess.  The current state breaks e.g. ccache.
> > 
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610285.html
> >   - PR108464 - P1 - file-prefix-map: Fix up -f*-prefix-map= (3 variants)
> 
> Let's go with your preference - I was hoping on comments from Richard 
> Purdie as of his preference but then ..
> > 

Sorry, I hadn't realised you were waiting on me :(. 

I'd commented on the bug and thought that covered things. We should be
fine with the third option, sorry about the issue and thanks for
resolving it!

Cheers,

Richard

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-03-10 10:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 12:15 [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Richard Purdie
2022-08-17 12:15 ` [PATCH 2/2] libcpp: Avoid remapping filenames within directives Richard Purdie
2022-08-17 14:19   ` Richard Purdie
2022-11-01 19:32   ` Jeff Law
2022-11-02 10:52     ` Richard Purdie
2022-11-01 19:46 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Jeff Law
2023-01-19 13:06   ` Jakub Jelinek
2023-01-20 15:05   ` [PATCH] file-prefix-map: Fix up -f*-prefix-map= [PR108464] Jakub Jelinek
2023-01-23 15:39     ` Jakub Jelinek
2023-03-10  8:49     ` Patch ping - " Jakub Jelinek
2023-03-10  9:05       ` Richard Biener
2023-03-10 10:14         ` Richard Purdie
2022-11-04  9:12 ` [PATCH 1/2] gcc/file-prefix-map: Allow remapping of relative paths Eric Botcazou
2022-11-04 17:21   ` Richard Purdie
2022-11-07  8:01     ` Eric Botcazou
2022-11-07 16:21       ` Jeff Law
2022-11-07 16:36         ` Eric Botcazou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).