[PATCH 2/2] symbols/x86: don't use symbols-dummy

Jan Beulich posted 2 patches 2 weeks, 4 days ago
[PATCH 2/2] symbols/x86: don't use symbols-dummy
Posted by Jan Beulich 2 weeks, 4 days ago
In particular when linking with lld, which converts hidden symbols to
local ones, the ELF symbol table can change in unhelpful ways between the
first two linking passes, resulting in the .rodata contributions to change
between the 2nd and 3rd pass. That, however, renders our embedded symbol
table pretty much unusable; the recently introduced self-test may then
also fail. (Another difference between compiling a C file and assembling
the generated ones is that - with -fdata-sections in use - the .rodata
contributions move between passes 1 and 2, when we'd prefer them not to.)

Make tools/symbols capable of producing an "empty" assembly file, such
that unwanted differences between passes 1 and 2 go away when then using
the corresponding objects in place of common/symbols-dummy.o.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
May want mirroring to other arch-es.

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -134,8 +134,10 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
+	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
+	$(MAKE) $(build)=$(@D) $(dot-target).0.o
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
-	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	      $(dot-target).0.o -o $(dot-target).0
 	$(NM) -pa --format=sysv $(dot-target).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		> $(dot-target).1.S
@@ -207,9 +209,11 @@ $(TARGET).efi: $(objtree)/prelink.o $(no
 ifeq ($(CONFIG_DEBUG_INFO),y)
 	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),echo,:) "Will strip debug info from $(@F)"
 endif
+	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0s.S
+	$(MAKE) $(build)=$(@D) .$(@F).0s.o
 	$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
 	          $(LD) $(call EFI_LDFLAGS,$(base)) -T $(obj)/efi.lds $< $(relocs-dummy) \
-	                $(objtree)/common/symbols-dummy.o $(note_file_option) \
+	                $(dot-target).0s.o $(note_file_option) \
 	                -o $(dot-target).$(base).0 &&) :
 	$(MKRELOC) $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(dot-target).$(base).0) \
 		> $(dot-target).1r.S
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -672,7 +672,10 @@ int main(int argc, char **argv)
 				warn_dup = true;
 			else if (strcmp(argv[i], "--error-dup") == 0)
 				warn_dup = error_dup = true;
-			else if (strcmp(argv[i], "--xensyms") == 0)
+			else if (strcmp(argv[i], "--empty") == 0) {
+				write_src();
+				return 0;
+			} else if (strcmp(argv[i], "--xensyms") == 0)
 				map_only = true;
 			else
 				usage();


Re: [PATCH 2/2] symbols/x86: don't use symbols-dummy
Posted by Roger Pau Monné 2 weeks, 4 days ago
On Tue, Nov 25, 2025 at 03:14:27PM +0100, Jan Beulich wrote:
> In particular when linking with lld, which converts hidden symbols to
> local ones, the ELF symbol table can change in unhelpful ways between the
> first two linking passes, resulting in the .rodata contributions to change
> between the 2nd and 3rd pass. That, however, renders our embedded symbol
> table pretty much unusable; the recently introduced self-test may then
> also fail. (Another difference between compiling a C file and assembling
> the generated ones is that - with -fdata-sections in use - the .rodata
> contributions move between passes 1 and 2, when we'd prefer them not to.)
> 
> Make tools/symbols capable of producing an "empty" assembly file, such
> that unwanted differences between passes 1 and 2 go away when then using
> the corresponding objects in place of common/symbols-dummy.o.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

LGTM, not sure whether you want to extend to other arches in this
same patch, or simply guard the build of symbols-dummy.o for non-x86
arches.

> ---
> May want mirroring to other arch-es.
> 
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -134,8 +134,10 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj
>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>  
>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> +	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
> +	$(MAKE) $(build)=$(@D) $(dot-target).0.o
>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0

It would be good if we could now stop building symbols-dummy.o as part
of extra-y.  Maybe you could guard it with ifneq ($(CONFIG_X86),y) in
the Makefile?

Or otherwise remove this from all arches thus removing
common/symbols-dummy.c.

> +	      $(dot-target).0.o -o $(dot-target).0
>  	$(NM) -pa --format=sysv $(dot-target).0 \
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>  		> $(dot-target).1.S
> @@ -207,9 +209,11 @@ $(TARGET).efi: $(objtree)/prelink.o $(no
>  ifeq ($(CONFIG_DEBUG_INFO),y)
>  	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),echo,:) "Will strip debug info from $(@F)"
>  endif
> +	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0s.S
> +	$(MAKE) $(build)=$(@D) .$(@F).0s.o
>  	$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
>  	          $(LD) $(call EFI_LDFLAGS,$(base)) -T $(obj)/efi.lds $< $(relocs-dummy) \
> -	                $(objtree)/common/symbols-dummy.o $(note_file_option) \
> +	                $(dot-target).0s.o $(note_file_option) \
>  	                -o $(dot-target).$(base).0 &&) :
>  	$(MKRELOC) $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(dot-target).$(base).0) \
>  		> $(dot-target).1r.S
> --- a/xen/tools/symbols.c
> +++ b/xen/tools/symbols.c
> @@ -672,7 +672,10 @@ int main(int argc, char **argv)
>  				warn_dup = true;
>  			else if (strcmp(argv[i], "--error-dup") == 0)
>  				warn_dup = error_dup = true;
> -			else if (strcmp(argv[i], "--xensyms") == 0)
> +			else if (strcmp(argv[i], "--empty") == 0) {
> +				write_src();
> +				return 0;

Oh, that was easier than I was expecting for symbols to generate a
working empty assembly file.

Thanks, Roger.

Re: [PATCH 2/2] symbols/x86: don't use symbols-dummy
Posted by Jan Beulich 2 weeks, 3 days ago
On 25.11.2025 18:36, Roger Pau Monné wrote:
> On Tue, Nov 25, 2025 at 03:14:27PM +0100, Jan Beulich wrote:
>> In particular when linking with lld, which converts hidden symbols to
>> local ones, the ELF symbol table can change in unhelpful ways between the
>> first two linking passes, resulting in the .rodata contributions to change
>> between the 2nd and 3rd pass. That, however, renders our embedded symbol
>> table pretty much unusable; the recently introduced self-test may then
>> also fail. (Another difference between compiling a C file and assembling
>> the generated ones is that - with -fdata-sections in use - the .rodata
>> contributions move between passes 1 and 2, when we'd prefer them not to.)
>>
>> Make tools/symbols capable of producing an "empty" assembly file, such
>> that unwanted differences between passes 1 and 2 go away when then using
>> the corresponding objects in place of common/symbols-dummy.o.
>>
>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> LGTM, not sure whether you want to extend to other arches in this
> same patch, or simply guard the build of symbols-dummy.o for non-x86
> arches.

I think I'd prefer to mirror it to other arch-es, but in separate
patches (so they can go in independently). Then once all are in, ...

>> ---
>> May want mirroring to other arch-es.
>>
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -134,8 +134,10 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj
>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>  
>>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
>> +	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
>> +	$(MAKE) $(build)=$(@D) $(dot-target).0.o
>>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> -	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
> 
> It would be good if we could now stop building symbols-dummy.o as part
> of extra-y.  Maybe you could guard it with ifneq ($(CONFIG_X86),y) in
> the Makefile?
> 
> Or otherwise remove this from all arches thus removing
> common/symbols-dummy.c.

... I'd remove symbols-dummy altogether. I don't think there's a need
to transiently disable building of symbols-dummy.o for just some of the
arch-es (like foe x86 right here).

>> --- a/xen/tools/symbols.c
>> +++ b/xen/tools/symbols.c
>> @@ -672,7 +672,10 @@ int main(int argc, char **argv)
>>  				warn_dup = true;
>>  			else if (strcmp(argv[i], "--error-dup") == 0)
>>  				warn_dup = error_dup = true;
>> -			else if (strcmp(argv[i], "--xensyms") == 0)
>> +			else if (strcmp(argv[i], "--empty") == 0) {
>> +				write_src();
>> +				return 0;
> 
> Oh, that was easier than I was expecting for symbols to generate a
> working empty assembly file.

Yeah, I also expected it to be more intrusive. The file isn't exactly,
though - two of the tables (the ones with 256 entries) are emitted
nevertheless. I didn't consider this an issue, though.

Jan

Re: [PATCH 2/2] symbols/x86: don't use symbols-dummy
Posted by Roger Pau Monné 2 weeks, 3 days ago
On Wed, Nov 26, 2025 at 07:21:09AM +0100, Jan Beulich wrote:
> On 25.11.2025 18:36, Roger Pau Monné wrote:
> > On Tue, Nov 25, 2025 at 03:14:27PM +0100, Jan Beulich wrote:
> >> In particular when linking with lld, which converts hidden symbols to
> >> local ones, the ELF symbol table can change in unhelpful ways between the
> >> first two linking passes, resulting in the .rodata contributions to change
> >> between the 2nd and 3rd pass. That, however, renders our embedded symbol
> >> table pretty much unusable; the recently introduced self-test may then
> >> also fail. (Another difference between compiling a C file and assembling
> >> the generated ones is that - with -fdata-sections in use - the .rodata
> >> contributions move between passes 1 and 2, when we'd prefer them not to.)
> >>
> >> Make tools/symbols capable of producing an "empty" assembly file, such
> >> that unwanted differences between passes 1 and 2 go away when then using
> >> the corresponding objects in place of common/symbols-dummy.o.
> >>
> >> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > LGTM, not sure whether you want to extend to other arches in this
> > same patch, or simply guard the build of symbols-dummy.o for non-x86
> > arches.
> 
> I think I'd prefer to mirror it to other arch-es, but in separate
> patches (so they can go in independently). Then once all are in, ...

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> >> ---
> >> May want mirroring to other arch-es.
> >>
> >> --- a/xen/arch/x86/Makefile
> >> +++ b/xen/arch/x86/Makefile
> >> @@ -134,8 +134,10 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj
> >>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
> >>  
> >>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> >> +	$(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
> >> +	$(MAKE) $(build)=$(@D) $(dot-target).0.o
> >>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> >> -	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
> > 
> > It would be good if we could now stop building symbols-dummy.o as part
> > of extra-y.  Maybe you could guard it with ifneq ($(CONFIG_X86),y) in
> > the Makefile?
> > 
> > Or otherwise remove this from all arches thus removing
> > common/symbols-dummy.c.
> 
> ... I'd remove symbols-dummy altogether. I don't think there's a need
> to transiently disable building of symbols-dummy.o for just some of the
> arch-es (like foe x86 right here).

OK, as long as those other patches don't get stuck that's fine for me.

> >> --- a/xen/tools/symbols.c
> >> +++ b/xen/tools/symbols.c
> >> @@ -672,7 +672,10 @@ int main(int argc, char **argv)
> >>  				warn_dup = true;
> >>  			else if (strcmp(argv[i], "--error-dup") == 0)
> >>  				warn_dup = error_dup = true;
> >> -			else if (strcmp(argv[i], "--xensyms") == 0)
> >> +			else if (strcmp(argv[i], "--empty") == 0) {
> >> +				write_src();
> >> +				return 0;
> > 
> > Oh, that was easier than I was expecting for symbols to generate a
> > working empty assembly file.
> 
> Yeah, I also expected it to be more intrusive. The file isn't exactly,
> though - two of the tables (the ones with 256 entries) are emitted
> nevertheless. I didn't consider this an issue, though.

As long as it builds and has the same symbols defined, it's all fine.
IMO,  We don't care about the sizes at this point, just using the same
sections for the defined symbols.

Thanks, Roger.