Re: [PATCH v2] Strip build path directories in tools and hypervisor

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 05, 2025 at 10:15:12AM +0200, Jan Beulich wrote:
> On 04.09.2025 16:27, Marek Marczykowski-Górecki wrote:
> > On Thu, Sep 04, 2025 at 02:58:20PM +0200, Jan Beulich wrote:
> >> On 04.09.2025 13:41, Marek Marczykowski-Górecki wrote:
> >>> Use -fdebug-prefix-map in preference to -ffile-prefix-map, as it's
> >>> available in earlier toolchain versions. But use it together with
> >>> -fmacro-prefix-map (if available) for hypervisor build, otherwise it
> >>> still contains some paths in out-of-tree builds.
> >>
> >> I consider it wrong not to use -ffile-prefix-map when available. That
> >> already covers more than "debug" and "macro", and it may gain further
> >> functionality.
> > 
> > I asked about that on v1 and got ambiguous answer suggesting the opposite:
> > https://lore.kernel.org/xen-devel/0370c0eb1fd9ac00acab016792132fa0b943d384.1742317309.git-series.marmarek@xxxxxxxxxxxxxxxxxxxxxx/T/#m74a8883835e30fb74a85b07a7b14507ee52e7c65
> 
> Ambiguous answer(s)? There's no reply to that mail of yours,

I mean your email to which I responded.

> and I don't
> see how the conclusion drawn fits my earlier comment. That was more
> towards what I did in v1 of my patch - fall back to the more widely
> supported option when the less widely available one can't be used.
> 
> >>> --- a/tools/Makefile
> >>> +++ b/tools/Makefile
> >>> @@ -1,4 +1,4 @@
> >>> -XEN_ROOT = $(CURDIR)/..
> >>> +XEN_ROOT = $(realpath $(CURDIR)/..)
> >>>  
> >>>  export PKG_CONFIG_DIR = $(CURDIR)/pkg-config
> >>>  
> >>> diff --git a/tools/Rules.mk b/tools/Rules.mk
> >>> index 725c3c32e9a2..428fce094819 100644
> >>> --- a/tools/Rules.mk
> >>> +++ b/tools/Rules.mk
> >>> @@ -166,6 +166,8 @@ endif
> >>>  CFLAGS-$(CONFIG_X86_32) += $(call 
> >>> cc-option,$(CC),-mno-tls-direct-seg-refs)
> >>>  CFLAGS += $(CFLAGS-y)
> >>>  
> >>> +$(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath 
> >>> $(XEN_ROOT))=.)
> >>
> >> Here and below - no need to use cc-option-add for -fdebug-prefix-map,
> >> which all permissible compilers support.
> > 
> > Ok.
> > 
> >> Further, again as per reply to Andrew on the thread hanging off of my
> >> patch - I don't view it as desirable to leave the tools/ prefix in
> >> place, or e.g. for libraries, the entire tools/libs/<subdir>/ part.
> >> Imo every binary should have only the path (if any) from its own source
> >> root left. (And yes, how to deal with e.g. shared include files isn't
> >> quite clear to me, yet. Maybe we actually need to pass two options.)
> > 
> > I don't think it's valid to strip arbitrary prefixes from debug symbols,
> > especially in tools. This will break some automated tools that try to match
> > coredumps (and similar) to source code and sometimes even debug symbols
> > too. But even for manual usage, having to jump between directories (I'm
> > not sure if gdb supports multiple source dirs at once?)
> 
> Pretty necessarily: When debugging you might easily cross project boundaries.
> 
> > just because you
> > happen to debug a binary that use more of libraries isn't exactly
> > desirable.
> > I think the paths in debug symbols and similar should match the layout
> > in the source repository, not a subset of it.
> 
> Well, okay, we disagree here. To me, xen.git really is an agglomeration of
> too many things in a single repo. If things were properly split, you'd end
> up with what I'm asking for anyway.

To give specific example: Fedora installs source files in
/usr/src/debug/(package name) and then does debuginfo postprocessing to
point at that path. Debian does pretty much the same, and I'm sure many
other distributions too. Now, if you strip part of the path from debug
symbols, they will not point at the correct source location.
Of course Fedora/Debian/etc package can apply a patch to adjust it (as
it's currently supplying -fdebug-prefix-map via CFLAGS), but IMO forcing
every distribution to basically undo upstream change is a wrong move.

> > Theoretically this doesn't apply to the hypervisor yet, as I'm not aware
> > of any tool processing xen memory dumps automatically (and those for
> > manual usage are quite unstable, to say the least...). But I don't think
> > it's an excuse to have incomplete paths in there, just to save few
> > bytes?
> > The only case where I can see it would make some sense is out of tree
> > build, where indeed it's about just the hypervisor, not the toolstack
> > (IMO due to the build system limitation, but well...). But at the same
> > time, having different path variant depending on it-tree/out-of-tree
> > build feels weird.
> 
> Which is why I'm arguing for the dropping of the xen/ prefixes, as that's
> how things come out in in-tree builds.
> 
> Jan

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature