Message ID | 20220817121534.1825108-1-richard.purdie@linuxfoundation.org |
---|---|
State | New |
Headers | show |
Series | [1/2] gcc/file-prefix-map: Allow remapping of relative paths | expand |
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
> 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
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
> 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.
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 >
> 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.
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
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; }
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(-)