[PATCH] zap linking-only option from EMBEDDED_EXTRA_CFLAGS

[PATCH] zap linking-only option from EMBEDDED_EXTRA_CFLAGS

[Jan Beulich]

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@kernel.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
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.

--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -13,7 +13,7 @@ endif
 
 CFLAGS += -Werror
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
 
 $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
 
--- 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)))
 
 CFLAGS += -fno-builtin -g0 $($(TESTCASE)-cflags)
 
--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,7 +1,7 @@
 ########################################
 # arm-specific definitions
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
 $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
 
 # Prevent floating-point variables from creeping into Xen.
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -10,7 +10,7 @@ CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFF
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS += -msoft-float
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
 $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
 $(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
 $(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -12,7 +12,7 @@ endif
 $(obj)/head.o: $(head-bin-objs:.o=.bin)
 
 CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
-$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-options-add,CFLAGS_x86_32,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
 CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float
 ifdef building_out_of_srctree
 CFLAGS_x86_32 += -I$(objtree)/include

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

[Stefano Stabellini]

On Fri, 9 Sep 2022, 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@kernel.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I don't think I can review the patch but it made the pipeline pass
again: 

https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/636189844

So:

Tested-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 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.
> 
> --- a/tools/firmware/Rules.mk
> +++ b/tools/firmware/Rules.mk
> @@ -13,7 +13,7 @@ endif
>  
>  CFLAGS += -Werror
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  
>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>  
> --- 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)))
>  
>  CFLAGS += -fno-builtin -g0 $($(TESTCASE)-cflags)
>  
> --- a/xen/arch/arm/arch.mk
> +++ b/xen/arch/arm/arch.mk
> @@ -1,7 +1,7 @@
>  ########################################
>  # arm-specific definitions
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
>  
>  # Prevent floating-point variables from creeping into Xen.
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -10,7 +10,7 @@ CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFF
>  # Prevent floating-point variables from creeping into Xen.
>  CFLAGS += -msoft-float
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
>  $(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
>  $(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -12,7 +12,7 @@ endif
>  $(obj)/head.o: $(head-bin-objs:.o=.bin)
>  
>  CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
> -$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS_x86_32,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float
>  ifdef building_out_of_srctree
>  CFLAGS_x86_32 += -I$(objtree)/include
> 

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

[Roger Pau Monné]

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@kernel.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> 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/tools/firmware/Rules.mk
> +++ b/tools/firmware/Rules.mk
> @@ -13,7 +13,7 @@ endif
>  
>  CFLAGS += -Werror
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  
>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>  
> --- 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?

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.

Thanks, Roger.

>  
>  CFLAGS += -fno-builtin -g0 $($(TESTCASE)-cflags)
>  
> --- a/xen/arch/arm/arch.mk
> +++ b/xen/arch/arm/arch.mk
> @@ -1,7 +1,7 @@
>  ########################################
>  # arm-specific definitions
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
>  
>  # Prevent floating-point variables from creeping into Xen.
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -10,7 +10,7 @@ CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFF
>  # Prevent floating-point variables from creeping into Xen.
>  CFLAGS += -msoft-float
>  
> -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
>  $(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
>  $(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -12,7 +12,7 @@ endif
>  $(obj)/head.o: $(head-bin-objs:.o=.bin)
>  
>  CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
> -$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +$(call cc-options-add,CFLAGS_x86_32,CC,$(filter-out -no-pie,$(EMBEDDED_EXTRA_CFLAGS)))
>  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float
>  ifdef building_out_of_srctree
>  CFLAGS_x86_32 += -I$(objtree)/include

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

[Andrew Cooper]

On 27/09/2022 15:14, Roger Pau Monne 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@kernel.org>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> 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.

As I've expressed several times previously, the entire variable is bogus.

Just because we do have several things which are built as
non-freestanding, doesn't mean they share a common CLFLAGS environment.

PIE is very specific to the intended use, and blindly disabling the
stack protector isn't safe either.  It only works because none of our
freestanding environments have gone to the effort of adding stack
protector support.

-fno-exceptions is furthermore bogus in CLFLAGS, as its a CXXFLAG only.

And no, this mess can't plausibly be used for a compile and link in one
go.  Filtering out a non-CFLAG from the collection at every call site is
nonsense.

~Andrew

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

[Jan Beulich]

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@kernel.org>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> 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

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

[Jan Beulich]

On 27.09.2022 16:29, Andrew Cooper wrote:
> On 27/09/2022 15:14, Roger Pau Monne 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@kernel.org>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> 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.
> 
> As I've expressed several times previously, the entire variable is bogus.
> 
> Just because we do have several things which are built as
> non-freestanding, doesn't mean they share a common CLFLAGS environment.
> 
> PIE is very specific to the intended use, and blindly disabling the
> stack protector isn't safe either.  It only works because none of our
> freestanding environments have gone to the effort of adding stack
> protector support.
> 
> -fno-exceptions is furthermore bogus in CLFLAGS, as its a CXXFLAG only.
> 
> And no, this mess can't plausibly be used for a compile and link in one
> go.  Filtering out a non-CFLAG from the collection at every call site is
> nonsense.

So what do you suggest we do in the short term?

Jan

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

[Roger Pau Monné]

On Tue, Sep 27, 2022 at 04:32:27PM +0200, Jan Beulich wrote:
> 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@kernel.org>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> 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.

There's one thing that puzzles me, if we already pass -fno-pie for
code generation, do we also need the -no-pie linker option explicitly
added?  I would expect the compiler to be clever enough to
automatically pass -no-pie to the linker if -fno-pie is used,
otherwise the code won't be correctly linked?

I would rather prefer to remove the -no-pie option from
EMBEDDED_EXTRA_CFLAGS and just add a note that users wanting to
link-in-place need to review the set of options used.

> >> --- 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.

Oh, OK, didn't gasp it was building blobs of code to pass to the
instruction emulator.

> > 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.

Anyway, it's clearer for me, but I agree it's not much better.  Let's
leave the naming discussion for another time if you don't think it's
much better.

Thanks, Roger.

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

[Jan Beulich]

On 27.09.2022 17:07, Roger Pau Monné wrote:
> On Tue, Sep 27, 2022 at 04:32:27PM +0200, Jan Beulich wrote:
>> 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@kernel.org>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> 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.
> 
> There's one thing that puzzles me, if we already pass -fno-pie for
> code generation, do we also need the -no-pie linker option explicitly
> added?  I would expect the compiler to be clever enough to
> automatically pass -no-pie to the linker if -fno-pie is used,
> otherwise the code won't be correctly linked?

For -pie gcc doc states: "For predictable results, you must also specify
the same set of options used for compilation (‘-fpie’, ‘-fPIE’, or model
suboptions) when you specify this linker option." I infer there's no
deriving along the lines of what you suggest.

I'm also unconvinced of there necessarily being issues with correct
linking. I can see there being executables where mismatched options
simply don't matter.

> I would rather prefer to remove the -no-pie option from
> EMBEDDED_EXTRA_CFLAGS and just add a note that users wanting to
> link-in-place need to review the set of options used.

Which then raises the question why it was added there (in a mis-spelled
manner) in the first place, years ago. That uncertainty was the only
reason why in the earlier change I didn't go and remove the option
altogether.

Jan