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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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