Re: [PATCH v2 3/7] RISC-V: split xen-syms linking rule

On 8/26/26 2:01 PM, Jan Beulich wrote:
> Doing so, besides (hopefully) adding clarity (not the least by way of
> [re-]using pattern rules where possible), also avoids explicit recursive
> $(MAKE) invocations.
> 
> By re-using the generic rules introduced when the respective x86 rule was
> split,
> - the .map file now isn't created after the final binary anymore,
> - --strip-debug is passed to $(LD) during early linking passes (for
>    consistency the option is also explicitly added to the optional linking
>    pass rule),
> - CONFIG_{SUPPRESS_DUPLICATE_SYMBOL_WARNINGS,ENFORCE_UNIQUE_SYMBOLS} are
>    now properly respected.
> Orphan section checking, otoh, is getting suppressed for now, until the
> about a dozen warnings which would result have been taken care of.
> 
> While the 4th linking step continues to be avoided when possible, a
> redundant invocation of $(NM) and tools/symbols (plus the assembling of
> the resulting .S file) is hopefully deemed acceptable.
> 
> Signed-off-by: Jan Beulich <[email protected]>
> 
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -31,40 +31,12 @@ obj-y += vtimer.o
>   $(TARGET): $(TARGET)-syms
>   	$(OBJCOPY) -O binary -S $< $@
>   
> -$(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) \
> -	      $(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
> -	$(MAKE) $(build)=$(@D) $(dot-target).1.o
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(dot-target).1.o -o $(dot-target).1
> -	$(NM) -pa --format=sysv $(dot-target).1 \
> -		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
> -		> $(dot-target).2.S
> -	$(MAKE) $(build)=$(@D) $(dot-target).2.o
> -	if ! { $(call compare-symbol-tables, $(dot-target).1.o, $(dot-target).2.o) >/dev/null; }; \
> -	then \
> -		set -e; \
> -		$(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; \
> -		$(call compare-symbol-tables, $(dot-target).2.o, $(dot-target).3.o); \
> -	else \
> -		ln -sf $(dot-target).2.o $(dot-target).3.o; \
> -	fi
> -	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
> -	    $(dot-target).3.o -o $@
> -	$(NM) -pa --format=sysv $@ \
> -		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
> -		> [email protected]
> -	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> +LAST_LINKING_PASS := 3
> +
> +include scripts/Makefile.link
> +
> +# Suppress orphan section checking for the time being.
> +orphan-handling-y :=

This works, but I think it's worth reconsidering the shape of it.

It works only by virtue of deferred expansion: $(orphan-handling-y) is
referenced solely inside the recipe of the final-pass rule in 
Makefile.link, so the value that matters is the one in effect when that
recipe is expanded, not when the rule was defined. Nothing states that
requirement, and nothing enforces it.

What makes me uneasy is that the ordering is not merely undocumented,
it's inverted with respect to the obvious reading. Makefile.link has

   orphan-handling-$(call ld-option,--orphan-handling=warn) := 
--orphan-handling=warn

i.e. an unconditional := to orphan-handling-y whenever the linker
supports the option. So an arch that sets orphan-handling-y *before*
the include has its setting silently discarded and ends up with orphan
checking enabled after all: no warning, no error, just a dozen new
linker diagnostics appearing at some later point. And "before the
include" is exactly where one would naturally put it: right next to
LAST_LINKING_PASS, which is the one knob the arch Makefile does set up
front.

I am not insisting on reworking but probably a small comment (in the 
commit mesage at least?) somewhere about that "+orphan-handling-y :=" 
should go after include will be useful.


Thanks.

~ Oleksii