[PATCH v1 0/2] Improve reproducibility of build artifacts

[PATCH v1 0/2] Improve reproducibility of build artifacts

[Marek Marczykowski-Górecki]

Few patches from qubes patch queue that improve reproducible builds.

Frédéric Pierret (fepitre) (2):
  docs/xen-headers: use alphabetical sorting for @incontents
  Strip build path directories in tools, xen and xen/arch/x86

 docs/xen-headers      | 2 +-
 tools/Rules.mk        | 2 ++
 xen/Makefile          | 2 ++
 xen/arch/x86/Makefile | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

base-commit: 77be740e8182fa6b31291a8ae983d253187e9b50
-- 
git-series 0.9.1

[PATCH v1 1/2] docs/xen-headers: use alphabetical sorting for @incontents

[Marek Marczykowski-Górecki]

From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>

It makes the build reproducible with fileordering flags

Signed-off-by: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
---
 docs/xen-headers | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/xen-headers b/docs/xen-headers
index 8c434d77e20e..98ffe814500b 100755
--- a/docs/xen-headers
+++ b/docs/xen-headers
@@ -331,7 +331,7 @@ sub output_index () {
 <h2>Starting points</h2>
 <ul>
 END
-    foreach my $ic (sort { $a->{Seq} <=> $b->{Seq} or $a->{Title} cmp $b->{Title} } @incontents) {
+    foreach my $ic (sort { $a->{Href} cmp $b->{Href} } @incontents) {
         $o .= "<li><a href=\"$ic->{Href}\">$ic->{Title}</a></li>\n";
     }
     $o .= "</ul>\n";
-- 
git-series 0.9.1

Re: [PATCH v1 1/2] docs/xen-headers: use alphabetical sorting for @incontents

[Anthony PERARD]

On Tue, Mar 18, 2025 at 06:01:56PM +0100, Marek Marczykowski-Górecki wrote:
> From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
>
> It makes the build reproducible with fileordering flags
>
> Signed-off-by: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
> ---
>  docs/xen-headers | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/docs/xen-headers b/docs/xen-headers
> index 8c434d77e20e..98ffe814500b 100755
> --- a/docs/xen-headers
> +++ b/docs/xen-headers
> @@ -331,7 +331,7 @@ sub output_index () {
>  <h2>Starting points</h2>
>  <ul>
>  END
> -    foreach my $ic (sort { $a->{Seq} <=> $b->{Seq} or $a->{Title} cmp $b->{Title} } @incontents) {
> +    foreach my $ic (sort { $a->{Href} cmp $b->{Href} } @incontents) {

Why is `Seq` been ignored? As I understand, the index is supposed to use
it as first ordering key.

Comment in that same file:
    #  extra syntax:
    #   `incontents <seq> <shortname> <anchor text html>...
    #                              make a table of contents entry; they
    #                              will be sorted by increasing seq, and
    #                              shortname will be used as the anchor target

Also, we already have a fix for reproducible build:
    e18dadc5b709 ("docs: use predictable ordering in generated documentation")

Would it be enough to replace `Title` by `Href` for the second sorting
key instead?

Thanks,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v1 1/2] docs/xen-headers: use alphabetical sorting for @incontents

[Marek Marczykowski-Górecki]

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

On Wed, Mar 19, 2025 at 11:19:33AM +0000, Anthony PERARD wrote:
> On Tue, Mar 18, 2025 at 06:01:56PM +0100, Marek Marczykowski-Górecki wrote:
> > From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
> >
> > It makes the build reproducible with fileordering flags
> >
> > Signed-off-by: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
> > ---
> >  docs/xen-headers | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/docs/xen-headers b/docs/xen-headers
> > index 8c434d77e20e..98ffe814500b 100755
> > --- a/docs/xen-headers
> > +++ b/docs/xen-headers
> > @@ -331,7 +331,7 @@ sub output_index () {
> >  <h2>Starting points</h2>
> >  <ul>
> >  END
> > -    foreach my $ic (sort { $a->{Seq} <=> $b->{Seq} or $a->{Title} cmp $b->{Title} } @incontents) {
> > +    foreach my $ic (sort { $a->{Href} cmp $b->{Href} } @incontents) {
> 
> Why is `Seq` been ignored? As I understand, the index is supposed to use
> it as first ordering key.
> 
> Comment in that same file:
>     #  extra syntax:
>     #   `incontents <seq> <shortname> <anchor text html>...
>     #                              make a table of contents entry; they
>     #                              will be sorted by increasing seq, and
>     #                              shortname will be used as the anchor target
> 
> Also, we already have a fix for reproducible build:
>     e18dadc5b709 ("docs: use predictable ordering in generated documentation")
> 
> Would it be enough to replace `Title` by `Href` for the second sorting
> key instead?

Hmm, right. It looks like this may be not needed anymore, as long as
title is unique (it looks like it is right now).

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>

Ensure to have a realpath for XEN_ROOT else it fails to substitute
properly pathes in strings sections

Signed-off-by: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
[use cc-option-add]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/Rules.mk        | 2 ++
 xen/Makefile          | 2 ++
 xen/arch/x86/Makefile | 1 +
 3 files changed, 5 insertions(+)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 6bd636709ff7..9ed0336c07d5 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -169,6 +169,8 @@ endif
 CFLAGS-$(CONFIG_X86_32) += $(call cc-option,$(CC),-mno-tls-direct-seg-refs)
 CFLAGS += $(CFLAGS-y)
 
+$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
+
 CFLAGS += $(EXTRA_CFLAGS_XEN_TOOLS)
 
 INSTALL_PYTHON_PROG = \
diff --git a/xen/Makefile b/xen/Makefile
index 58fafab33d6f..0d79e259a33e 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
+$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
+
 AFLAGS += -D__ASSEMBLY__
 
 $(call cc-option-add,AFLAGS,CC,-Wa$$(comma)--noexecstack)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index f59c9665fdd0..70d0653257d7 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -137,6 +137,7 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
 	mv $(TMP) $(TARGET)
 
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
+$(call cc-option-add CFLAGS-$(XEN_BUILD_EFI),CC,-ffile-prefix-map=$(XEN_ROOT)=.)
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
-- 
git-series 0.9.1

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
>  CFLAGS += -Wa,--strip-local-absolute
>  endif
>  
> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)

With this, ...

> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -137,6 +137,7 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
>  	mv $(TMP) $(TARGET)
>  
>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
> +$(call cc-option-add CFLAGS-$(XEN_BUILD_EFI),CC,-ffile-prefix-map=$(XEN_ROOT)=.)

... why is this also needed? CFLAGS-y ought to be folded into CFLAGS.

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
>  CFLAGS += -Wa,--strip-local-absolute
>  endif
>  
> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)

This is lacking a comma:

$(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)

Makes me wonder whether you tested this after wrapping in cc-option-add.

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 19.03.2025 10:15, Jan Beulich wrote:
> On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
>>  CFLAGS += -Wa,--strip-local-absolute
>>  endif
>>  
>> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
> 
> This is lacking a comma:
> 
> $(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)

And then, having tried the correct form (seeing the option then is passed
to the compiler), I can't spot any difference in the resulting
xen-syms.map. There were a few absolute paths there before (for
arch/x86/x86_64/kexec_reloc.S and arch/x86/acpi/wakeup_prot.S), and the
exact same ones are present afterwards.

I've tried this with both an in-tree build and an out-of-tree one. Under
what (extra?) conditions would a behavioral change to be expected?

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Wed, Mar 19, 2025 at 10:43:12AM +0100, Jan Beulich wrote:
> On 19.03.2025 10:15, Jan Beulich wrote:
> > On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
> >> --- a/xen/Makefile
> >> +++ b/xen/Makefile
> >> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
> >>  CFLAGS += -Wa,--strip-local-absolute
> >>  endif
> >>  
> >> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
> > 
> > This is lacking a comma:
> > 
> > $(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
> 
> And then, having tried the correct form (seeing the option then is passed
> to the compiler), I can't spot any difference in the resulting
> xen-syms.map. There were a few absolute paths there before (for
> arch/x86/x86_64/kexec_reloc.S and arch/x86/acpi/wakeup_prot.S), and the
> exact same ones are present afterwards.

I'm not sure about xen-syms.map, it's about build path included in
xen-syms. It appears at least once in .debug_str and once in
.debug_line_str.

But also, I see the patch lost a chunk during rebase (from before
4.17...), that adjusts XEN_ROOT to use $(realpath ...). That's the part
even mentioned in the commit message...

I'll send v2 shortly.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 19.03.2025 12:58, Marek Marczykowski-Górecki wrote:
> On Wed, Mar 19, 2025 at 10:43:12AM +0100, Jan Beulich wrote:
>> On 19.03.2025 10:15, Jan Beulich wrote:
>>> On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
>>>> --- a/xen/Makefile
>>>> +++ b/xen/Makefile
>>>> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
>>>>  CFLAGS += -Wa,--strip-local-absolute
>>>>  endif
>>>>  
>>>> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
>>>
>>> This is lacking a comma:
>>>
>>> $(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
>>
>> And then, having tried the correct form (seeing the option then is passed
>> to the compiler), I can't spot any difference in the resulting
>> xen-syms.map. There were a few absolute paths there before (for
>> arch/x86/x86_64/kexec_reloc.S and arch/x86/acpi/wakeup_prot.S), and the
>> exact same ones are present afterwards.
> 
> I'm not sure about xen-syms.map, it's about build path included in
> xen-syms. It appears at least once in .debug_str and once in
> .debug_line_str.

In which case -fdebug-prefix-map= may suffice, which is available on
more compiler versions? And then only if DEBUG_INFO=y?

> But also, I see the patch lost a chunk during rebase (from before
> 4.17...), that adjusts XEN_ROOT to use $(realpath ...). That's the part
> even mentioned in the commit message...
> 
> I'll send v2 shortly.

Provided there's actually a need. I was in fact wondering whether this
was known to have significant effect prior to Anthony's work to make
out-of-tree builds possible (plus less related adjustments), but may
have lost most of its functionality since then (yet was still carried
forward for all the time).

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Wed, Mar 19, 2025 at 01:43:59PM +0100, Jan Beulich wrote:
> On 19.03.2025 12:58, Marek Marczykowski-Górecki wrote:
> > On Wed, Mar 19, 2025 at 10:43:12AM +0100, Jan Beulich wrote:
> >> On 19.03.2025 10:15, Jan Beulich wrote:
> >>> On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
> >>>> --- a/xen/Makefile
> >>>> +++ b/xen/Makefile
> >>>> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
> >>>>  CFLAGS += -Wa,--strip-local-absolute
> >>>>  endif
> >>>>  
> >>>> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
> >>>
> >>> This is lacking a comma:
> >>>
> >>> $(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
> >>
> >> And then, having tried the correct form (seeing the option then is passed
> >> to the compiler), I can't spot any difference in the resulting
> >> xen-syms.map. There were a few absolute paths there before (for
> >> arch/x86/x86_64/kexec_reloc.S and arch/x86/acpi/wakeup_prot.S), and the
> >> exact same ones are present afterwards.
> > 
> > I'm not sure about xen-syms.map, it's about build path included in
> > xen-syms. It appears at least once in .debug_str and once in
> > .debug_line_str.
> 
> In which case -fdebug-prefix-map= may suffice, which is available on
> more compiler versions? And then only if DEBUG_INFO=y?

Oh, and xen.efi is full of build path. Binary on plain staging has 790
occurrences. But there, -fdebug-prefix-map= also helps.

But also I don't think -fdebug-prefix-map= will be enough for tools, it
looks like at least libxl has build path embedded in .rodata too.

> > But also, I see the patch lost a chunk during rebase (from before
> > 4.17...), that adjusts XEN_ROOT to use $(realpath ...). That's the part
> > even mentioned in the commit message...
> > 
> > I'll send v2 shortly.
> 
> Provided there's actually a need. I was in fact wondering whether this
> was known to have significant effect prior to Anthony's work to make
> out-of-tree builds possible (plus less related adjustments), but may
> have lost most of its functionality since then (yet was still carried
> forward for all the time).

There are clearly some build path embedding left. And
-ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
XEN_ROOT having xen/.. at the end.
BTW, would it be acceptable to have this?

    $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)

It may be less efficient (if make doesn't cache result), but helps
especially in tools, where XEN_ROOT is set in _a lot_ of places.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 19.03.2025 14:40, Marek Marczykowski-Górecki wrote:
> On Wed, Mar 19, 2025 at 01:43:59PM +0100, Jan Beulich wrote:
>> On 19.03.2025 12:58, Marek Marczykowski-Górecki wrote:
>>> On Wed, Mar 19, 2025 at 10:43:12AM +0100, Jan Beulich wrote:
>>>> On 19.03.2025 10:15, Jan Beulich wrote:
>>>>> On 18.03.2025 18:01, Marek Marczykowski-Górecki wrote:
>>>>>> --- a/xen/Makefile
>>>>>> +++ b/xen/Makefile
>>>>>> @@ -411,6 +411,8 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
>>>>>>  CFLAGS += -Wa,--strip-local-absolute
>>>>>>  endif
>>>>>>  
>>>>>> +$(call cc-option-add CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
>>>>>
>>>>> This is lacking a comma:
>>>>>
>>>>> $(call cc-option-add,CFLAGS,CC,-ffile-prefix-map=$(XEN_ROOT)=.)
>>>>
>>>> And then, having tried the correct form (seeing the option then is passed
>>>> to the compiler), I can't spot any difference in the resulting
>>>> xen-syms.map. There were a few absolute paths there before (for
>>>> arch/x86/x86_64/kexec_reloc.S and arch/x86/acpi/wakeup_prot.S), and the
>>>> exact same ones are present afterwards.
>>>
>>> I'm not sure about xen-syms.map, it's about build path included in
>>> xen-syms. It appears at least once in .debug_str and once in
>>> .debug_line_str.
>>
>> In which case -fdebug-prefix-map= may suffice, which is available on
>> more compiler versions? And then only if DEBUG_INFO=y?
> 
> Oh, and xen.efi is full of build path. Binary on plain staging has 790
> occurrences. But there, -fdebug-prefix-map= also helps.
> 
> But also I don't think -fdebug-prefix-map= will be enough for tools, it
> looks like at least libxl has build path embedded in .rodata too.

And _all_ of them go away with -ffile-prefix-map=?

>>> But also, I see the patch lost a chunk during rebase (from before
>>> 4.17...), that adjusts XEN_ROOT to use $(realpath ...). That's the part
>>> even mentioned in the commit message...
>>>
>>> I'll send v2 shortly.
>>
>> Provided there's actually a need. I was in fact wondering whether this
>> was known to have significant effect prior to Anthony's work to make
>> out-of-tree builds possible (plus less related adjustments), but may
>> have lost most of its functionality since then (yet was still carried
>> forward for all the time).
> 
> There are clearly some build path embedding left. And
> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> XEN_ROOT having xen/.. at the end.
> BTW, would it be acceptable to have this?
> 
>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> 
> It may be less efficient (if make doesn't cache result),

What do you mean here? Variable evaluation depends solely on how we
use variables. I don't think there's any caching make does on its own?

As to $(realpath ...) - make 3.80 doesn't support that. We do provide a
fallback, but for that you need to use $(call realpath,...).

Jan

> but helps
> especially in tools, where XEN_ROOT is set in _a lot_ of places.
> 

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Anthony PERARD]

On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> There are clearly some build path embedding left. And
> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> XEN_ROOT having xen/.. at the end.
> BTW, would it be acceptable to have this?
>
>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)

Hi,

Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
in "tools/"). In "xen/", there's a few variables you can use if they are
needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
$(objtree) for relative path. That also should avoid the need to use
$(realpath ).

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> > There are clearly some build path embedding left. And
> > -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> > XEN_ROOT having xen/.. at the end.
> > BTW, would it be acceptable to have this?
> >
> >     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> 
> Hi,
> 
> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> in "tools/"). In "xen/", there's a few variables you can use if they are
> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> $(objtree) for relative path. That also should avoid the need to use
> $(realpath ).

XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
not have /.. for prefix-map to work correctly. Would it be better to use
literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
have paths in debug symbols relative to hypervisor source dir, instead
of xen repo root? I'm not sure if that wouldn't confuse some tools...

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
>>> There are clearly some build path embedding left. And
>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
>>> XEN_ROOT having xen/.. at the end.
>>> BTW, would it be acceptable to have this?
>>>
>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
>>
>> Hi,
>>
>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
>> in "tools/"). In "xen/", there's a few variables you can use if they are
>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
>> $(objtree) for relative path. That also should avoid the need to use
>> $(realpath ).
> 
> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> not have /.. for prefix-map to work correctly. Would it be better to use
> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> have paths in debug symbols relative to hypervisor source dir, instead
> of xen repo root? I'm not sure if that wouldn't confuse some tools...

abs_srctree being computed using realpath, can't we replace

export XEN_ROOT := $(abs_srctree)/..

by something as simpl{e,istic} as

export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))

?

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> > On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> >> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> >>> There are clearly some build path embedding left. And
> >>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> >>> XEN_ROOT having xen/.. at the end.
> >>> BTW, would it be acceptable to have this?
> >>>
> >>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> >>
> >> Hi,
> >>
> >> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> >> in "tools/"). In "xen/", there's a few variables you can use if they are
> >> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> >> $(objtree) for relative path. That also should avoid the need to use
> >> $(realpath ).
> > 
> > XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> > not have /.. for prefix-map to work correctly. Would it be better to use
> > literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> > have paths in debug symbols relative to hypervisor source dir, instead
> > of xen repo root? I'm not sure if that wouldn't confuse some tools...
> 
> abs_srctree being computed using realpath, can't we replace
> 
> export XEN_ROOT := $(abs_srctree)/..
> 
> by something as simpl{e,istic} as
> 
> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> 
> ?

That works too. It's slightly less robust, but I don't expect "xen"
directory to be renamed, so shouldn't be an issue. I'll leave also a
comment there why not /.. .

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Anthony PERARD]

On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> > On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> > > On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> > >> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> > >>> There are clearly some build path embedding left. And
> > >>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> > >>> XEN_ROOT having xen/.. at the end.
> > >>> BTW, would it be acceptable to have this?
> > >>>
> > >>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> > >>
> > >> Hi,
> > >>
> > >> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> > >> in "tools/"). In "xen/", there's a few variables you can use if they are
> > >> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> > >> $(objtree) for relative path. That also should avoid the need to use
> > >> $(realpath ).
> > >
> > > XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> > > not have /.. for prefix-map to work correctly. Would it be better to use
> > > literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> > > have paths in debug symbols relative to hypervisor source dir, instead
> > > of xen repo root? I'm not sure if that wouldn't confuse some tools...
> >
> > abs_srctree being computed using realpath, can't we replace
> >
> > export XEN_ROOT := $(abs_srctree)/..
> >
> > by something as simpl{e,istic} as
> >
> > export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> >
> > ?
>
> That works too. It's slightly less robust, but I don't expect "xen"
> directory to be renamed, so shouldn't be an issue. I'll leave also a
> comment there why not /.. .

I don't think $(XEN_ROOT) is present in the binaries produce by the
hypervisor's build system. There's only a few use if that variable: to
load some makefile, to execute makefile that build xsm policy and to
generate cpuid-autogen.h. Otherwise I don't think the compile have this
path in the command line. What is going to be in the binary is
$(abs_srctree), which you can replace by "./xen" if you want; which mean
it doesn't matter if the directory is renamed or not. You might want to
also take care of $(abs_objtree) which seems to also be in `xen-syms`
when doing out-of-tree build.

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 20.03.2025 15:36, Anthony PERARD wrote:
> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
>>>>>> There are clearly some build path embedding left. And
>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
>>>>>> XEN_ROOT having xen/.. at the end.
>>>>>> BTW, would it be acceptable to have this?
>>>>>>
>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
>>>>>
>>>>> Hi,
>>>>>
>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
>>>>> $(objtree) for relative path. That also should avoid the need to use
>>>>> $(realpath ).
>>>>
>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
>>>> not have /.. for prefix-map to work correctly. Would it be better to use
>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
>>>> have paths in debug symbols relative to hypervisor source dir, instead
>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
>>>
>>> abs_srctree being computed using realpath, can't we replace
>>>
>>> export XEN_ROOT := $(abs_srctree)/..
>>>
>>> by something as simpl{e,istic} as
>>>
>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
>>>
>>> ?
>>
>> That works too. It's slightly less robust, but I don't expect "xen"
>> directory to be renamed, so shouldn't be an issue. I'll leave also a
>> comment there why not /.. .
> 
> I don't think $(XEN_ROOT) is present in the binaries produce by the
> hypervisor's build system.

It is, in the symbol table that tools/symbols produces. In a random
out-of-tree build I can see various static symbols being prefixed by
the full paths to the source files. I can't quite spot a pattern
between when this is the case and when it's not. In in-tree builds I
can't spot any such occurrences.

I also think Marek said debug info may contain full paths.

Jan

> There's only a few use if that variable: to
> load some makefile, to execute makefile that build xsm policy and to
> generate cpuid-autogen.h. Otherwise I don't think the compile have this
> path in the command line. What is going to be in the binary is
> $(abs_srctree), which you can replace by "./xen" if you want; which mean
> it doesn't matter if the directory is renamed or not. You might want to
> also take care of $(abs_objtree) which seems to also be in `xen-syms`
> when doing out-of-tree build.
> 
> Cheers,
> 

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
> > On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> > > On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> > > > On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> > > >> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> > > >>> There are clearly some build path embedding left. And
> > > >>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> > > >>> XEN_ROOT having xen/.. at the end.
> > > >>> BTW, would it be acceptable to have this?
> > > >>>
> > > >>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> > > >>
> > > >> Hi,
> > > >>
> > > >> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> > > >> in "tools/"). In "xen/", there's a few variables you can use if they are
> > > >> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> > > >> $(objtree) for relative path. That also should avoid the need to use
> > > >> $(realpath ).
> > > >
> > > > XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> > > > not have /.. for prefix-map to work correctly. Would it be better to use
> > > > literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> > > > have paths in debug symbols relative to hypervisor source dir, instead
> > > > of xen repo root? I'm not sure if that wouldn't confuse some tools...
> > >
> > > abs_srctree being computed using realpath, can't we replace
> > >
> > > export XEN_ROOT := $(abs_srctree)/..
> > >
> > > by something as simpl{e,istic} as
> > >
> > > export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> > >
> > > ?
> >
> > That works too. It's slightly less robust, but I don't expect "xen"
> > directory to be renamed, so shouldn't be an issue. I'll leave also a
> > comment there why not /.. .
> 
> I don't think $(XEN_ROOT) is present in the binaries produce by the
> hypervisor's build system. There's only a few use if that variable: to
> load some makefile, to execute makefile that build xsm policy and to
> generate cpuid-autogen.h. Otherwise I don't think the compile have this
> path in the command line. What is going to be in the binary is
> $(abs_srctree), which you can replace by "./xen" if you want; which mean
> it doesn't matter if the directory is renamed or not. You might want to
> also take care of $(abs_objtree) which seems to also be in `xen-syms`
> when doing out-of-tree build.

So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
appears to work for in-tree builds too.
But now I actually tested how it looks with out-of-tree builds, and
indeed $(abs_objtree) is embedded there too. Adding
-fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
-fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
+ -fmacro-prefix-map. Is there any preference which one to use? It
appears as -fmacro-prefix-map and -ffile-prefix-map have the same
availability in both GCC (8) and Clang (10).

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
>> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
>>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
>>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
>>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
>>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
>>>>>>> There are clearly some build path embedding left. And
>>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
>>>>>>> XEN_ROOT having xen/.. at the end.
>>>>>>> BTW, would it be acceptable to have this?
>>>>>>>
>>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
>>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
>>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
>>>>>> $(objtree) for relative path. That also should avoid the need to use
>>>>>> $(realpath ).
>>>>>
>>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
>>>>> not have /.. for prefix-map to work correctly. Would it be better to use
>>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
>>>>> have paths in debug symbols relative to hypervisor source dir, instead
>>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
>>>>
>>>> abs_srctree being computed using realpath, can't we replace
>>>>
>>>> export XEN_ROOT := $(abs_srctree)/..
>>>>
>>>> by something as simpl{e,istic} as
>>>>
>>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
>>>>
>>>> ?
>>>
>>> That works too. It's slightly less robust, but I don't expect "xen"
>>> directory to be renamed, so shouldn't be an issue. I'll leave also a
>>> comment there why not /.. .
>>
>> I don't think $(XEN_ROOT) is present in the binaries produce by the
>> hypervisor's build system. There's only a few use if that variable: to
>> load some makefile, to execute makefile that build xsm policy and to
>> generate cpuid-autogen.h. Otherwise I don't think the compile have this
>> path in the command line. What is going to be in the binary is
>> $(abs_srctree), which you can replace by "./xen" if you want; which mean
>> it doesn't matter if the directory is renamed or not. You might want to
>> also take care of $(abs_objtree) which seems to also be in `xen-syms`
>> when doing out-of-tree build.
> 
> So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
> appears to work for in-tree builds too.

And why ./xen (question to Anthony)? Just . is quite fine, isn't it?

> But now I actually tested how it looks with out-of-tree builds, and
> indeed $(abs_objtree) is embedded there too. Adding
> -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
> -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
> for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
> + -fmacro-prefix-map. Is there any preference which one to use? It
> appears as -fmacro-prefix-map and -ffile-prefix-map have the same
> availability in both GCC (8) and Clang (10).

Then the simpler -ffile-prefix-map is better, imo. Question then is
whether any of the options is actually needed at all for in-tree builds.

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 04:21:18PM +0100, Jan Beulich wrote:
> On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
> > On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
> >> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
> >>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> >>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> >>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> >>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> >>>>>>> There are clearly some build path embedding left. And
> >>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> >>>>>>> XEN_ROOT having xen/.. at the end.
> >>>>>>> BTW, would it be acceptable to have this?
> >>>>>>>
> >>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> >>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
> >>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> >>>>>> $(objtree) for relative path. That also should avoid the need to use
> >>>>>> $(realpath ).
> >>>>>
> >>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> >>>>> not have /.. for prefix-map to work correctly. Would it be better to use
> >>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> >>>>> have paths in debug symbols relative to hypervisor source dir, instead
> >>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
> >>>>
> >>>> abs_srctree being computed using realpath, can't we replace
> >>>>
> >>>> export XEN_ROOT := $(abs_srctree)/..
> >>>>
> >>>> by something as simpl{e,istic} as
> >>>>
> >>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> >>>>
> >>>> ?
> >>>
> >>> That works too. It's slightly less robust, but I don't expect "xen"
> >>> directory to be renamed, so shouldn't be an issue. I'll leave also a
> >>> comment there why not /.. .
> >>
> >> I don't think $(XEN_ROOT) is present in the binaries produce by the
> >> hypervisor's build system. There's only a few use if that variable: to
> >> load some makefile, to execute makefile that build xsm policy and to
> >> generate cpuid-autogen.h. Otherwise I don't think the compile have this
> >> path in the command line. What is going to be in the binary is
> >> $(abs_srctree), which you can replace by "./xen" if you want; which mean
> >> it doesn't matter if the directory is renamed or not. You might want to
> >> also take care of $(abs_objtree) which seems to also be in `xen-syms`
> >> when doing out-of-tree build.
> > 
> > So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
> > appears to work for in-tree builds too.
> 
> And why ./xen (question to Anthony)? Just . is quite fine, isn't it?

It makes paths in debug symbols relative to xen/ subdir, not the
repository root. I'm not sure if that is a problem, but it may be for
some tools.

> > But now I actually tested how it looks with out-of-tree builds, and
> > indeed $(abs_objtree) is embedded there too. Adding
> > -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
> > -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
> > for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
> > + -fmacro-prefix-map. Is there any preference which one to use? It
> > appears as -fmacro-prefix-map and -ffile-prefix-map have the same
> > availability in both GCC (8) and Clang (10).
> 
> Then the simpler -ffile-prefix-map is better, imo. Question then is
> whether any of the options is actually needed at all for in-tree builds.

Yes, without any of those options, both xen-syms and xen.efi contain
full source path.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 20.03.2025 16:32, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 20, 2025 at 04:21:18PM +0100, Jan Beulich wrote:
>> On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
>>> On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
>>>> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
>>>>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
>>>>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
>>>>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
>>>>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
>>>>>>>>> There are clearly some build path embedding left. And
>>>>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
>>>>>>>>> XEN_ROOT having xen/.. at the end.
>>>>>>>>> BTW, would it be acceptable to have this?
>>>>>>>>>
>>>>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
>>>>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
>>>>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
>>>>>>>> $(objtree) for relative path. That also should avoid the need to use
>>>>>>>> $(realpath ).
>>>>>>>
>>>>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
>>>>>>> not have /.. for prefix-map to work correctly. Would it be better to use
>>>>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
>>>>>>> have paths in debug symbols relative to hypervisor source dir, instead
>>>>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
>>>>>>
>>>>>> abs_srctree being computed using realpath, can't we replace
>>>>>>
>>>>>> export XEN_ROOT := $(abs_srctree)/..
>>>>>>
>>>>>> by something as simpl{e,istic} as
>>>>>>
>>>>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
>>>>>>
>>>>>> ?
>>>>>
>>>>> That works too. It's slightly less robust, but I don't expect "xen"
>>>>> directory to be renamed, so shouldn't be an issue. I'll leave also a
>>>>> comment there why not /.. .
>>>>
>>>> I don't think $(XEN_ROOT) is present in the binaries produce by the
>>>> hypervisor's build system. There's only a few use if that variable: to
>>>> load some makefile, to execute makefile that build xsm policy and to
>>>> generate cpuid-autogen.h. Otherwise I don't think the compile have this
>>>> path in the command line. What is going to be in the binary is
>>>> $(abs_srctree), which you can replace by "./xen" if you want; which mean
>>>> it doesn't matter if the directory is renamed or not. You might want to
>>>> also take care of $(abs_objtree) which seems to also be in `xen-syms`
>>>> when doing out-of-tree build.
>>>
>>> So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
>>> appears to work for in-tree builds too.
>>
>> And why ./xen (question to Anthony)? Just . is quite fine, isn't it?
> 
> It makes paths in debug symbols relative to xen/ subdir, not the
> repository root. I'm not sure if that is a problem, but it may be for
> some tools.

Yet especially in the symbol table (and hence in strack traces) that's
unnecessary extra space it takes up.

>>> But now I actually tested how it looks with out-of-tree builds, and
>>> indeed $(abs_objtree) is embedded there too. Adding
>>> -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
>>> -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
>>> for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
>>> + -fmacro-prefix-map. Is there any preference which one to use? It
>>> appears as -fmacro-prefix-map and -ffile-prefix-map have the same
>>> availability in both GCC (8) and Clang (10).
>>
>> Then the simpler -ffile-prefix-map is better, imo. Question then is
>> whether any of the options is actually needed at all for in-tree builds.
> 
> Yes, without any of those options, both xen-syms and xen.efi contain
> full source path.

Even in builds without debug info? Imo a goal ought to be to specify the
weakest possible of these options for any particular build mode. I.e.
possibly -ffile-prefix-map= for out of tree builds, else
-fdebug-prefix-map= when DEBUG_INFO=y, else nothing (if possible).

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 04:48:02PM +0100, Jan Beulich wrote:
> On 20.03.2025 16:32, Marek Marczykowski-Górecki wrote:
> > On Thu, Mar 20, 2025 at 04:21:18PM +0100, Jan Beulich wrote:
> >> On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
> >>> On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
> >>>> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
> >>>>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> >>>>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> >>>>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> >>>>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> >>>>>>>>> There are clearly some build path embedding left. And
> >>>>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> >>>>>>>>> XEN_ROOT having xen/.. at the end.
> >>>>>>>>> BTW, would it be acceptable to have this?
> >>>>>>>>>
> >>>>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> >>>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> >>>>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
> >>>>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> >>>>>>>> $(objtree) for relative path. That also should avoid the need to use
> >>>>>>>> $(realpath ).
> >>>>>>>
> >>>>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> >>>>>>> not have /.. for prefix-map to work correctly. Would it be better to use
> >>>>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> >>>>>>> have paths in debug symbols relative to hypervisor source dir, instead
> >>>>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
> >>>>>>
> >>>>>> abs_srctree being computed using realpath, can't we replace
> >>>>>>
> >>>>>> export XEN_ROOT := $(abs_srctree)/..
> >>>>>>
> >>>>>> by something as simpl{e,istic} as
> >>>>>>
> >>>>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> >>>>>>
> >>>>>> ?
> >>>>>
> >>>>> That works too. It's slightly less robust, but I don't expect "xen"
> >>>>> directory to be renamed, so shouldn't be an issue. I'll leave also a
> >>>>> comment there why not /.. .
> >>>>
> >>>> I don't think $(XEN_ROOT) is present in the binaries produce by the
> >>>> hypervisor's build system. There's only a few use if that variable: to
> >>>> load some makefile, to execute makefile that build xsm policy and to
> >>>> generate cpuid-autogen.h. Otherwise I don't think the compile have this
> >>>> path in the command line. What is going to be in the binary is
> >>>> $(abs_srctree), which you can replace by "./xen" if you want; which mean
> >>>> it doesn't matter if the directory is renamed or not. You might want to
> >>>> also take care of $(abs_objtree) which seems to also be in `xen-syms`
> >>>> when doing out-of-tree build.
> >>>
> >>> So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
> >>> appears to work for in-tree builds too.
> >>
> >> And why ./xen (question to Anthony)? Just . is quite fine, isn't it?
> > 
> > It makes paths in debug symbols relative to xen/ subdir, not the
> > repository root. I'm not sure if that is a problem, but it may be for
> > some tools.
> 
> Yet especially in the symbol table (and hence in strack traces) that's
> unnecessary extra space it takes up.
> 
> >>> But now I actually tested how it looks with out-of-tree builds, and
> >>> indeed $(abs_objtree) is embedded there too. Adding
> >>> -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
> >>> -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
> >>> for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
> >>> + -fmacro-prefix-map. Is there any preference which one to use? It
> >>> appears as -fmacro-prefix-map and -ffile-prefix-map have the same
> >>> availability in both GCC (8) and Clang (10).
> >>
> >> Then the simpler -ffile-prefix-map is better, imo. Question then is
> >> whether any of the options is actually needed at all for in-tree builds.
> > 
> > Yes, without any of those options, both xen-syms and xen.efi contain
> > full source path.
> 
> Even in builds without debug info? 

For in-tree build without debug info, it appears no. But with debug
info, something is needed even for in-tree build.
And BTW, IIUC out-of-tree builds will become relevant even for in-tree
build at some point, due to pvshim.

> Imo a goal ought to be to specify the
> weakest possible of these options for any particular build mode. I.e.
> possibly -ffile-prefix-map= for out of tree builds, else
> -fdebug-prefix-map= when DEBUG_INFO=y, else nothing (if possible).

Is it? I don't really see why making the selection overly complex if the
option is supported (and cc-option-add covers that case).

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Jan Beulich]

On 20.03.2025 16:58, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 20, 2025 at 04:48:02PM +0100, Jan Beulich wrote:
>> On 20.03.2025 16:32, Marek Marczykowski-Górecki wrote:
>>> On Thu, Mar 20, 2025 at 04:21:18PM +0100, Jan Beulich wrote:
>>>> On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
>>>>> On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
>>>>>> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
>>>>>>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
>>>>>>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
>>>>>>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
>>>>>>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
>>>>>>>>>>> There are clearly some build path embedding left. And
>>>>>>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
>>>>>>>>>>> XEN_ROOT having xen/.. at the end.
>>>>>>>>>>> BTW, would it be acceptable to have this?
>>>>>>>>>>>
>>>>>>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
>>>>>>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
>>>>>>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
>>>>>>>>>> $(objtree) for relative path. That also should avoid the need to use
>>>>>>>>>> $(realpath ).
>>>>>>>>>
>>>>>>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
>>>>>>>>> not have /.. for prefix-map to work correctly. Would it be better to use
>>>>>>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
>>>>>>>>> have paths in debug symbols relative to hypervisor source dir, instead
>>>>>>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
>>>>>>>>
>>>>>>>> abs_srctree being computed using realpath, can't we replace
>>>>>>>>
>>>>>>>> export XEN_ROOT := $(abs_srctree)/..
>>>>>>>>
>>>>>>>> by something as simpl{e,istic} as
>>>>>>>>
>>>>>>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
>>>>>>>>
>>>>>>>> ?
>>>>>>>
>>>>>>> That works too. It's slightly less robust, but I don't expect "xen"
>>>>>>> directory to be renamed, so shouldn't be an issue. I'll leave also a
>>>>>>> comment there why not /.. .
>>>>>>
>>>>>> I don't think $(XEN_ROOT) is present in the binaries produce by the
>>>>>> hypervisor's build system. There's only a few use if that variable: to
>>>>>> load some makefile, to execute makefile that build xsm policy and to
>>>>>> generate cpuid-autogen.h. Otherwise I don't think the compile have this
>>>>>> path in the command line. What is going to be in the binary is
>>>>>> $(abs_srctree), which you can replace by "./xen" if you want; which mean
>>>>>> it doesn't matter if the directory is renamed or not. You might want to
>>>>>> also take care of $(abs_objtree) which seems to also be in `xen-syms`
>>>>>> when doing out-of-tree build.
>>>>>
>>>>> So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
>>>>> appears to work for in-tree builds too.
>>>>
>>>> And why ./xen (question to Anthony)? Just . is quite fine, isn't it?
>>>
>>> It makes paths in debug symbols relative to xen/ subdir, not the
>>> repository root. I'm not sure if that is a problem, but it may be for
>>> some tools.
>>
>> Yet especially in the symbol table (and hence in strack traces) that's
>> unnecessary extra space it takes up.
>>
>>>>> But now I actually tested how it looks with out-of-tree builds, and
>>>>> indeed $(abs_objtree) is embedded there too. Adding
>>>>> -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
>>>>> -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
>>>>> for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
>>>>> + -fmacro-prefix-map. Is there any preference which one to use? It
>>>>> appears as -fmacro-prefix-map and -ffile-prefix-map have the same
>>>>> availability in both GCC (8) and Clang (10).
>>>>
>>>> Then the simpler -ffile-prefix-map is better, imo. Question then is
>>>> whether any of the options is actually needed at all for in-tree builds.
>>>
>>> Yes, without any of those options, both xen-syms and xen.efi contain
>>> full source path.
>>
>> Even in builds without debug info? 
> 
> For in-tree build without debug info, it appears no. But with debug
> info, something is needed even for in-tree build.
> And BTW, IIUC out-of-tree builds will become relevant even for in-tree
> build at some point, due to pvshim.

That hasn't happened yet because it's not quite straightforward to arrange
for.

>> Imo a goal ought to be to specify the
>> weakest possible of these options for any particular build mode. I.e.
>> possibly -ffile-prefix-map= for out of tree builds, else
>> -fdebug-prefix-map= when DEBUG_INFO=y, else nothing (if possible).
> 
> Is it? I don't really see why making the selection overly complex if the
> option is supported (and cc-option-add covers that case).

Yes, cc-option-add might cover the case where nothing is needed. But the
two options mentioned have appeared in gcc at different versions. People
using e.g. gcc7 may still benefit from -fdebug-prefix-map=.

Jan

Re: [PATCH v1 2/2] Strip build path directories in tools, xen and xen/arch/x86

[Marek Marczykowski-Górecki]

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

On Thu, Mar 20, 2025 at 05:09:12PM +0100, Jan Beulich wrote:
> On 20.03.2025 16:58, Marek Marczykowski-Górecki wrote:
> > On Thu, Mar 20, 2025 at 04:48:02PM +0100, Jan Beulich wrote:
> >> On 20.03.2025 16:32, Marek Marczykowski-Górecki wrote:
> >>> On Thu, Mar 20, 2025 at 04:21:18PM +0100, Jan Beulich wrote:
> >>>> On 20.03.2025 16:17, Marek Marczykowski-Górecki wrote:
> >>>>> On Thu, Mar 20, 2025 at 02:35:59PM +0000, Anthony PERARD wrote:
> >>>>>> On Thu, Mar 20, 2025 at 02:59:04PM +0100, Marek Marczykowski-Górecki wrote:
> >>>>>>> On Thu, Mar 20, 2025 at 02:49:27PM +0100, Jan Beulich wrote:
> >>>>>>>> On 20.03.2025 13:51, Marek Marczykowski-Górecki wrote:
> >>>>>>>>> On Thu, Mar 20, 2025 at 10:18:28AM +0000, Anthony PERARD wrote:
> >>>>>>>>>> On Wed, Mar 19, 2025 at 02:40:33PM +0100, Marek Marczykowski-Górecki wrote:
> >>>>>>>>>>> There are clearly some build path embedding left. And
> >>>>>>>>>>> -ffile-prefix-map=/-fdebug-prefix-map= doesn't work correctly with
> >>>>>>>>>>> XEN_ROOT having xen/.. at the end.
> >>>>>>>>>>> BTW, would it be acceptable to have this?
> >>>>>>>>>>>
> >>>>>>>>>>>     $(call cc-option-add,CFLAGS,CC,-fdebug-prefix-map=$(realpath $(XEN_ROOT))=.)
> >>>>>>>>>>
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> Could you avoid using $(XEN_ROOT) in hypervisor build system? (It's fine
> >>>>>>>>>> in "tools/"). In "xen/", there's a few variables you can use if they are
> >>>>>>>>>> needed: $(abs_objtree) $(abs_srctree) for absolutes path, and $(srctree)
> >>>>>>>>>> $(objtree) for relative path. That also should avoid the need to use
> >>>>>>>>>> $(realpath ).
> >>>>>>>>>
> >>>>>>>>> XEN_ROOT is literally "$(abs_srctree)/..". And I need to resolve it to
> >>>>>>>>> not have /.. for prefix-map to work correctly. Would it be better to use
> >>>>>>>>> literal $(realpath $(abs_srctree)/..)? Or use just $(abs_srctree) and
> >>>>>>>>> have paths in debug symbols relative to hypervisor source dir, instead
> >>>>>>>>> of xen repo root? I'm not sure if that wouldn't confuse some tools...
> >>>>>>>>
> >>>>>>>> abs_srctree being computed using realpath, can't we replace
> >>>>>>>>
> >>>>>>>> export XEN_ROOT := $(abs_srctree)/..
> >>>>>>>>
> >>>>>>>> by something as simpl{e,istic} as
> >>>>>>>>
> >>>>>>>> export XEN_ROOT := $(patsubst %/xen,%,$(abs_srctree))
> >>>>>>>>
> >>>>>>>> ?
> >>>>>>>
> >>>>>>> That works too. It's slightly less robust, but I don't expect "xen"
> >>>>>>> directory to be renamed, so shouldn't be an issue. I'll leave also a
> >>>>>>> comment there why not /.. .
> >>>>>>
> >>>>>> I don't think $(XEN_ROOT) is present in the binaries produce by the
> >>>>>> hypervisor's build system. There's only a few use if that variable: to
> >>>>>> load some makefile, to execute makefile that build xsm policy and to
> >>>>>> generate cpuid-autogen.h. Otherwise I don't think the compile have this
> >>>>>> path in the command line. What is going to be in the binary is
> >>>>>> $(abs_srctree), which you can replace by "./xen" if you want; which mean
> >>>>>> it doesn't matter if the directory is renamed or not. You might want to
> >>>>>> also take care of $(abs_objtree) which seems to also be in `xen-syms`
> >>>>>> when doing out-of-tree build.
> >>>>>
> >>>>> So, you suggest to do -fdebug-prefix-map=$(abs_srctree)=./xen ? That
> >>>>> appears to work for in-tree builds too.
> >>>>
> >>>> And why ./xen (question to Anthony)? Just . is quite fine, isn't it?
> >>>
> >>> It makes paths in debug symbols relative to xen/ subdir, not the
> >>> repository root. I'm not sure if that is a problem, but it may be for
> >>> some tools.
> >>
> >> Yet especially in the symbol table (and hence in strack traces) that's
> >> unnecessary extra space it takes up.
> >>
> >>>>> But now I actually tested how it looks with out-of-tree builds, and
> >>>>> indeed $(abs_objtree) is embedded there too. Adding
> >>>>> -fdebug-prefix-map=$(abs_objtree)=./xen appears to help for this. But,
> >>>>> -fdebug-prefix-map doesn't help with abs_srctree in out-of-tree builds
> >>>>> for some reason. -ffile-prefix-map does. And so does -fdebug-prefix-map
> >>>>> + -fmacro-prefix-map. Is there any preference which one to use? It
> >>>>> appears as -fmacro-prefix-map and -ffile-prefix-map have the same
> >>>>> availability in both GCC (8) and Clang (10).
> >>>>
> >>>> Then the simpler -ffile-prefix-map is better, imo. Question then is
> >>>> whether any of the options is actually needed at all for in-tree builds.
> >>>
> >>> Yes, without any of those options, both xen-syms and xen.efi contain
> >>> full source path.
> >>
> >> Even in builds without debug info? 
> > 
> > For in-tree build without debug info, it appears no. But with debug
> > info, something is needed even for in-tree build.
> > And BTW, IIUC out-of-tree builds will become relevant even for in-tree
> > build at some point, due to pvshim.
> 
> That hasn't happened yet because it's not quite straightforward to arrange
> for.

Sure, but if it will happen at some point, even users doing in-tree
build would benefit from options that normally would be relevant only
for out-of-tree builds. So, it's IMO valuable to attempt make
out-of-tree builds reproducible too.

> >> Imo a goal ought to be to specify the
> >> weakest possible of these options for any particular build mode. I.e.
> >> possibly -ffile-prefix-map= for out of tree builds, else
> >> -fdebug-prefix-map= when DEBUG_INFO=y, else nothing (if possible).
> > 
> > Is it? I don't really see why making the selection overly complex if the
> > option is supported (and cc-option-add covers that case).
> 
> Yes, cc-option-add might cover the case where nothing is needed. But the
> two options mentioned have appeared in gcc at different versions. People
> using e.g. gcc7 may still benefit from -fdebug-prefix-map=.

That sounds like an argument to use -fdebug-prefix-map= +
-fmacro-prefix-map (with separate cc-option-add), instead of just
-ffile-prefix-map. I'm fine with that.


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]