| Message ID | 0370c0eb1fd9ac00acab016792132fa0b943d384.1742317309.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Improve reproducibility of build artifacts | expand |
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
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
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
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.
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
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.
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. >
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,
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...
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
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 /.. .
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,
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, >
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).
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
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.
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
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) \