Re: [PATCH] zap linking-only option from EMBEDDED_EXTRA_CFLAGS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.09.2022 16:14, Roger Pau Monné wrote:
> On Fri, Sep 09, 2022 at 09:22:52AM +0200, Jan Beulich wrote:
>> While I was suspicious of the compiler issuing a diagnostic about an
>> unused linking-only option when not doing any linking, I did check this
>> with a couple of gcc versions only, but not with Clang. (Oddly enough at
>> least older Clang versions complain about the use of '-nopie' now that
>> we actually use '-no-pie'.) Filter out the problematic option in all
>> cases where the variable is consumed for compilation only (which right
>> now is everywhere).
>>
>> Fixes: ecd6b9759919 ("Config.mk: correct PIE-related option(s) in 
>> EMBEDDED_EXTRA_CFLAGS")
>> Reported-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Arguably with all users of EMBEDDED_EXTRA_CFLAGS using these just for
>> compiling, the option could be omitted from that variable right away.
>> But if any compile-and-link-in-one-go use appeared, there would be an
>> issue.
> 
> Is it feasible to have compile-and-link-in-one-go in one use feasible
> with what we consider embedded (firmware or kernel like binaries).  I
> would expect those to always require a linker script and a separate
> linking step.

A separate linking step doesn't mean this needs doing via $(LD) - it
could also be done via $(CC). There's also no connection between using
a separate linking step and using a linker script - aiui the linker
script could also be handed to $(CC) for it to pass on the option to
the linker.

>> --- a/tools/tests/x86_emulator/testcase.mk
>> +++ b/tools/tests/x86_emulator/testcase.mk
>> @@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
>>  CFLAGS :=
>>  include $(XEN_ROOT)/tools/Rules.mk
>>  
>> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>> +$(call cc-options-add,CFLAGS,CC,$(filter-out 
>> -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
> 
> Is the x86 emulator harness correct in using EMBEDDED_EXTRA_CFLAGS?

Yes, I think it is (here): This is the script to build the blobs we
then have the emulator process. Of course it wouldn't be right to
use for building the actual harness executable.

> TBH I'm not sure the naming and usage of the variable is very
> helpful, maybe it would better be STANDALONE_EXTRA_CFLAGS, and drop
> it's usage from the x86 emulator test harness, open code the needed
> flags for that use-case.

I agree the naming is, well, odd. I would be okay with the proposed
alternative name, but I also don't view that as all-so-much-better.

Jan