Re: [PATCH] Arm: do a 4th linking pass if necessary

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.05.2026 18:03, Anthony PERARD wrote:
> On Wed, May 20, 2026 at 01:53:34PM +0200, Jan Beulich wrote:
>> Once we generalize linking, we may want to introduce an equivalent of
>> Linux'es KALLSYMS_EXTRA_PASS as well. I don't think doing this right here
>> would make overly much sense, though.
> 
> If you generalise linking, you are going to put it in a shell script,
> right? Because this recipe is getting very complicated, for within a
> makefile.

Well. Prior to this change I was definitely hoping to spit the big rule up
into small ones. Whether that's still feasible with an optional path I'll
have to see; I very much would prefer if I could keep everything in make
logic.

>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -99,9 +99,20 @@ $(TARGET)-syms: $(objtree)/prelink.o $(o
>>              | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>>              > $(dot-target).2.S
>>      $(MAKE) $(build)=$(@D) $(dot-target).2.o
>> -    $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o)
>> +    if ! { $(call compare-symbol-tables, $(dot-target).1.o, 
>> $(dot-target).2.o) >/dev/null; }; \
> 
> This `>/dev/null` seems to only suppress the output of the `diff` of the
> macro, is it what is intended?

Yes. All errors and alike should appear normally.

>> +    then \
>> +            $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> +                $(dot-target).2.o -o $(dot-target).2; \
>> +            $(NM) -pa --format=sysv $(dot-target).2 \
>> +                    | $(objtree)/tools/symbols $(all_symbols) --sysv --sort 
>> \
>> +                    > $(dot-target).3.S; \
>> +            $(MAKE) $(build)=$(@D) $(dot-target).3.o; \
> 
> This new block ignore all errors, from LD, NM and MAKE. We want
> a `set -e` before the if.

Hmm, perhaps I should add that, yes, albeit ...

>> +            $(call compare-symbol-tables, $(dot-target).2.o, 
>> $(dot-target).3.o); \
> 
> At least, an error returned by `diff` in that macro should be taken into
> account, for now.

... I expect this would fail if there was an earlier error.

>> --- a/xen/scripts/Kbuild.include
>> +++ b/xen/scripts/Kbuild.include
>> @@ -65,7 +65,7 @@ define compare-symbol-tables
>>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym; \
>>      ln -f $(2) $(@D)/.cst.$$$$; \
>>      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(2).sym; \
>> -    rm -f $(@D)/.cst.$$$$
>> +    rm -f $(@D)/.cst.$$$$; \
>>      diff -u $(1).sym $(2).sym
> 
> This macro is missing `set -e`, if both OBJDUMP command fails and create
> an empty file, `diff` will return success.

Whether to have "set -e" here is an independent question, I guess. To avoid
the case you mention, maybe better

      $(OBJDUMP) -t $(@D)/.cst.$$$$ > $(1).sym || rm -f $(1).sym; \

?

> But looks like `set -e` in
> this macro isn't going to work in the condition of the `if`.

Whereas the above would be compatible with both uses, I think.

Jan