public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599
@ 2023-01-19 13:00 jakub at gcc dot gnu.org
2023-01-19 13:01 ` [Bug other/108464] " jakub at gcc dot gnu.org
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-19 13:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
Bug ID: 108464
Summary: [13 Regression] Broken -fdebug-prefix-map since
r13-3599
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: jakub at gcc dot gnu.org
Target Milestone: ---
Starting with r13-3599-ge5c15eb183f17e806ad -fdebug-prefix-map stopped working
in some cases (breaks ccache testsuite among other things).
Reproducer:
mkdir -p foobar1/include foobar1/src
echo 'int test;' > foobar1/include/test.h
echo '#include <test.h>' > foobar1/src/test.c
ln -sf foobar1 foobar2
cd foobar2
gcc -S -I include/ -fdebug-prefix-map=`pwd`=XXXX -g -dA src/test.c -o test.s;
grep 'XXXX\|foobar' test.s
Before r13-3599, all the way back from r0-82686-gc8aea42ce2c691e4e8 when
-fdebug-prefix-map= has been introduced, this prints XXXX (once or multiple
times depending on exact version) and doesn't print any foobar strings.
Even r13-3598 prints
.long .LASF1 # DW_AT_comp_dir: "XXXX"
.long .LASF1 # Directory Entry: 0: "XXXX"
.string "XXXX"
but starting with r13-3599 we print
.long .LASF1 # DW_AT_comp_dir: "/home/jakub/foobar2"
.long .LASF1 # Directory Entry: 0: "/home/jakub/foobar2"
.string "/home/jakub/foobar2"
instead, which is wrong because user asked for remapping of /home/jakub/foobar2
and it wasn't done.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
@ 2023-01-19 13:01 ` jakub at gcc dot gnu.org
2023-01-19 13:21 ` richard.purdie at linuxfoundation dot org
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-19 13:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Priority|P3 |P1
Target Milestone|--- |13.0
CC| |law at gcc dot gnu.org,
| |richard.purdie@linuxfoundat
| |ion.org
Ever confirmed|0 |1
Last reconfirmed| |2023-01-19
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
2023-01-19 13:01 ` [Bug other/108464] " jakub at gcc dot gnu.org
@ 2023-01-19 13:21 ` richard.purdie at linuxfoundation dot org
2023-01-19 13:29 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: richard.purdie at linuxfoundation dot org @ 2023-01-19 13:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #1 from Richard Purdie <richard.purdie at linuxfoundation dot org> ---
I can see why that would change behaviour with the patch :(.
It is a tough dilemma since without the patch, files might have prefixes
removed some of the time but not all the time, depending on whether they were
accessed via a symlink (or similar) or not. It was effectively impossible to
remap some prefix values meaning in some cases output files couldn't be build
deterministically.
One possible way to fix this issue might be to call realpath on DW_AT_comp_dir
so that we're always working on canonicalized paths? I'm not sure if that would
be acceptable or not, the challenge right now is that we're making comparisons
against things which aren't always definitive.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
2023-01-19 13:01 ` [Bug other/108464] " jakub at gcc dot gnu.org
2023-01-19 13:21 ` richard.purdie at linuxfoundation dot org
@ 2023-01-19 13:29 ` jakub at gcc dot gnu.org
2023-01-19 13:43 ` richard.purdie at linuxfoundation dot org
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-19 13:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Dunno about other uses of remap_filename, but for remap_debug_filename what
that commit changed doesn't seem to be ever appropriate. Users pass often pass
what they get from pwd to -fdebug-prefix-map=, and that is usually not the
canonicalized path. And DW_AT_comp_dir was that same path as well and IMHO it
would be a very bad idea to change that. Debug info contains relative paths in
many cases, that also shouldn't be canonicalized, should be whatever user used
in -I etc. options or #include paths etc.
If one is mixing absolute paths and some are canonical and others aren't, one
can use multiple -fdebug-prefix-map= options. But ignoring what user asked for
is just wrong.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (2 preceding siblings ...)
2023-01-19 13:29 ` jakub at gcc dot gnu.org
@ 2023-01-19 13:43 ` richard.purdie at linuxfoundation dot org
2023-01-19 13:44 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: richard.purdie at linuxfoundation dot org @ 2023-01-19 13:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #3 from Richard Purdie <richard.purdie at linuxfoundation dot org> ---
(In reply to Jakub Jelinek from comment #2)
> Dunno about other uses of remap_filename, but for remap_debug_filename what
> that commit changed doesn't seem to be ever appropriate.
I think it depends on your use case. For example if via debug-prefix-map you
say you want:
/somepath/somesources translated to /somewhere
and then some code references ../somesources and ../../somesources from it's
build tree, would you want the user to have to add mappings for those and any
other combination some build system might end up generating due to it's source
layout?
> Users pass often
> pass what they get from pwd to -fdebug-prefix-map=, and that is usually not
> the canonicalized path. And DW_AT_comp_dir was that same path as well and
> IMHO it would be a very bad idea to change that. Debug info contains
> relative paths in many cases, that also shouldn't be canonicalized, should
> be whatever user used in -I etc. options or #include paths etc.
> If one is mixing absolute paths and some are canonical and others aren't,
> one can use multiple -fdebug-prefix-map= options. But ignoring what user
> asked for is just wrong.
It can be argued that if you ask for my above example and it doesn't remap the
relative accesses, it is also just wrong.
I can say that with all the cross compiling we do (in Yocto Project), making
this handle the corner cases improved our binary reproducibility quite
significantly. gcc can revert the change but we'd just have to patch something
in to handle this.
Another possible option is to resolve both sides of the equation before
deciding to remap or not (i.e. the directory in debug-prefix-map and the
potential target)?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (3 preceding siblings ...)
2023-01-19 13:43 ` richard.purdie at linuxfoundation dot org
@ 2023-01-19 13:44 ` jakub at gcc dot gnu.org
2023-01-19 15:08 ` richard.purdie at linuxfoundation dot org
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-19 13:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Though, even for the other options the change doesn't seem to be a good idea as
is.
I think instead the syntax of those remapping options should be extended, so
that for each of the pair one can choose the behavior which is wanted. Using
the OLD=NEW syntax
with the previous behavior, and some other to mark pairs where you want new
behavior
(dunno whether to mark those using say extra = at the end after NEW or whatever
else).
Then when populating file_prefix_map entries, one would record this
non-canonicalizing vs. canonicalizing flag and in the latter case canonicalize
the old_prefix path in the structure.
remap_filename then would ideally lazily lrealpath when seeing first map entry
with the bool flag indicating canonicalization.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (4 preceding siblings ...)
2023-01-19 13:44 ` jakub at gcc dot gnu.org
@ 2023-01-19 15:08 ` richard.purdie at linuxfoundation dot org
2023-01-31 9:01 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: richard.purdie at linuxfoundation dot org @ 2023-01-19 15:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #5 from Richard Purdie <richard.purdie at linuxfoundation dot org> ---
(In reply to Jakub Jelinek from comment #4)
> Though, even for the other options the change doesn't seem to be a good idea
> as is.
> I think instead the syntax of those remapping options should be extended, so
> that for each of the pair one can choose the behavior which is wanted.
> Using the OLD=NEW syntax
> with the previous behavior, and some other to mark pairs where you want new
> behavior
> (dunno whether to mark those using say extra = at the end after NEW or
> whatever else).
> Then when populating file_prefix_map entries, one would record this
> non-canonicalizing vs. canonicalizing flag and in the latter case
> canonicalize the old_prefix path in the structure.
> remap_filename then would ideally lazily lrealpath when seeing first map
> entry with the bool flag indicating canonicalization.
Having some way to enable the behaviour would be very valuable to the places we
use prefix mapping so we could make that work FWIW.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (5 preceding siblings ...)
2023-01-19 15:08 ` richard.purdie at linuxfoundation dot org
@ 2023-01-31 9:01 ` jakub at gcc dot gnu.org
2023-02-20 15:16 ` orion at cora dot nwra.com
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-31 9:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
URL| |https://gcc.gnu.org/piperma
| |il/gcc-patches/2023-January
| |/610285.html
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (6 preceding siblings ...)
2023-01-31 9:01 ` jakub at gcc dot gnu.org
@ 2023-02-20 15:16 ` orion at cora dot nwra.com
2023-02-20 15:26 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: orion at cora dot nwra.com @ 2023-02-20 15:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #6 from Orion Poplawski <orion at cora dot nwra.com> ---
So, there was quite the initial flurry of activity here and then nothing. Any
chance we could get some more movement here? It's breaking a ccache test for
one. Thank you.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (7 preceding siblings ...)
2023-02-20 15:16 ` orion at cora dot nwra.com
@ 2023-02-20 15:26 ` jakub at gcc dot gnu.org
2023-03-10 9:23 ` cvs-commit at gcc dot gnu.org
2023-03-10 9:24 ` jakub at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-02-20 15:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The patches (3 variants) have been posted and are awaiting review. I've pinged
them twice.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (8 preceding siblings ...)
2023-02-20 15:26 ` jakub at gcc dot gnu.org
@ 2023-03-10 9:23 ` cvs-commit at gcc dot gnu.org
2023-03-10 9:24 ` jakub at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-03-10 9:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:2eb0191aa104badf3cab127f7f87d371c0fef92b
commit r13-6576-g2eb0191aa104badf3cab127f7f87d371c0fef92b
Author: Jakub Jelinek <jakub@redhat.com>
Date: Fri Mar 10 10:21:23 2023 +0100
file-prefix-map: Fix up -f*-prefix-map= [PR108464]
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've created 3 patches for this.
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.
This commit is the third patch described above.
2023-03-10 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug other/108464] [13 Regression] Broken -fdebug-prefix-map since r13-3599
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
` (9 preceding siblings ...)
2023-03-10 9:23 ` cvs-commit at gcc dot gnu.org
@ 2023-03-10 9:24 ` jakub at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-03-10 9:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108464
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed now.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-03-10 9:24 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19 13:00 [Bug other/108464] New: [13 Regression] Broken -fdebug-prefix-map since r13-3599 jakub at gcc dot gnu.org
2023-01-19 13:01 ` [Bug other/108464] " jakub at gcc dot gnu.org
2023-01-19 13:21 ` richard.purdie at linuxfoundation dot org
2023-01-19 13:29 ` jakub at gcc dot gnu.org
2023-01-19 13:43 ` richard.purdie at linuxfoundation dot org
2023-01-19 13:44 ` jakub at gcc dot gnu.org
2023-01-19 15:08 ` richard.purdie at linuxfoundation dot org
2023-01-31 9:01 ` jakub at gcc dot gnu.org
2023-02-20 15:16 ` orion at cora dot nwra.com
2023-02-20 15:26 ` jakub at gcc dot gnu.org
2023-03-10 9:23 ` cvs-commit at gcc dot gnu.org
2023-03-10 9:24 ` jakub at gcc dot gnu.org
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).