[PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Jan Beulich]

I haven't been able to find evidence of "-nopie" ever having been a
supported compiler option. The correct spelling is "-no-pie".
Furthermore like "-pie" this is an option which is solely passed to the
linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
it doesn't infer these options from "-pie" / "-no-pie".

Add the compiler recognized form, but for the possible case of the
variable also being used somewhere for linking keep the linker option as
well (with corrected spelling).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- unstable.orig/Config.mk	2022-04-07 12:23:27.000000000 +0200
+++ unstable/Config.mk	2022-08-25 08:58:00.044287451 +0200
@@ -188,7 +188,7 @@ endif
 APPEND_LDFLAGS += $(foreach i, $(APPEND_LIB), -L$(i))
 APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
 
-EMBEDDED_EXTRA_CFLAGS := -nopie -fno-stack-protector -fno-stack-protector-all
+EMBEDDED_EXTRA_CFLAGS := -fno-pie -no-pie -fno-stack-protector -fno-stack-protector-all
 EMBEDDED_EXTRA_CFLAGS += -fno-exceptions -fno-asynchronous-unwind-tables
 
 XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles

Re: [PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Julien Grall]

Hi Jan,

On 25/08/2022 08:17, Jan Beulich wrote:
> I haven't been able to find evidence of "-nopie" ever having been a
> supported compiler option. The correct spelling is "-no-pie".
> Furthermore like "-pie" this is an option which is solely passed to the
> linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
> it doesn't infer these options from "-pie" / "-no-pie".

OOI, how did you find out this issue?

> 
> Add the compiler recognized form, but for the possible case of the
> variable also being used somewhere for linking keep the linker option as
> well (with corrected spelling).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Jan Beulich]

On 07.09.2022 16:33, Julien Grall wrote:
> On 25/08/2022 08:17, Jan Beulich wrote:
>> I haven't been able to find evidence of "-nopie" ever having been a
>> supported compiler option. The correct spelling is "-no-pie".
>> Furthermore like "-pie" this is an option which is solely passed to the
>> linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
>> it doesn't infer these options from "-pie" / "-no-pie".
> 
> OOI, how did you find out this issue?

By reviewing Andrew's "x86/hvmloader: Don't build as PIC/PIE".

>> Add the compiler recognized form, but for the possible case of the
>> variable also being used somewhere for linking keep the linker option as
>> well (with corrected spelling).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks.

Jan

Re: [PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Andrew Cooper]

On 08/09/2022 07:10, Jan Beulich wrote:
> On 07.09.2022 16:33, Julien Grall wrote:
>> On 25/08/2022 08:17, Jan Beulich wrote:
>>> I haven't been able to find evidence of "-nopie" ever having been a
>>> supported compiler option. The correct spelling is "-no-pie".
>>> Furthermore like "-pie" this is an option which is solely passed to the
>>> linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
>>> it doesn't infer these options from "-pie" / "-no-pie".
>> OOI, how did you find out this issue?
> By reviewing Andrew's "x86/hvmloader: Don't build as PIC/PIE".

It was actually first discussed here:
https://lore.kernel.org/xen-devel/7b129a01-07c7-e856-fb5b-0c7b44a8dac5@citrix.com/

The reason why I hadn't got back around to this patch yet is because the
commit message is wrong (not helped to some appalling GCC/Binutils
documentation).

The -f forms are to do with GCC code generation.  These are CFLAGS, but
they want want specifying (or not) together, and not split across
EMBEDDED_EXTRA_CFLAGS and something else like this.

The non-f forms are LDFLAGS but do behave as described.  Passing -no-pie
causes GCC to cancel passing -pie to LD; it does not pass -no-pie.  But
it does other things too, because different cr0 objects get passed. 
This matters for hosted binaries, but not for freestanding.

~Andrew

Gitlab breakage: [PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Stefano Stabellini]

Hi Jan,

This patch breaks the gitlab-ci pipeline, specifically it breaks the
hvmloader build with clang:


https://gitlab.com/xen-project/xen/-/pipelines/634274727
https://gitlab.com/xen-project/xen/-/jobs/2996114313

make[7]: Entering directory '/builds/xen-project/xen/tools/firmware/hvmloader'
clang   -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-local-typedefs   -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .hvmloader.o.d -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs  -DNDEBUG -Werror -fno-pie -no-pie -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -fcf-protection=none -ffreestanding -msoft-float -nostdinc -I/builds/xen-project/xen/tools/firmware/hvmloader/../../../tools/firmware/include -I/builds/xen-project/xen/tools/firmware/hvmloader/../../../tools/include -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -I../../libacpi  -c -o hvmloader.o hvmloader.c 
clang: error: argument unused during compilation: '-nopie' [-Werror,-Wunused-command-line-argument]

Cheers,

Stefano


On Thu, 25 Aug 2022, Jan Beulich wrote:
> I haven't been able to find evidence of "-nopie" ever having been a
> supported compiler option. The correct spelling is "-no-pie".
> Furthermore like "-pie" this is an option which is solely passed to the
> linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
> it doesn't infer these options from "-pie" / "-no-pie".
> 
> Add the compiler recognized form, but for the possible case of the
> variable also being used somewhere for linking keep the linker option as
> well (with corrected spelling).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- unstable.orig/Config.mk	2022-04-07 12:23:27.000000000 +0200
> +++ unstable/Config.mk	2022-08-25 08:58:00.044287451 +0200
> @@ -188,7 +188,7 @@ endif
>  APPEND_LDFLAGS += $(foreach i, $(APPEND_LIB), -L$(i))
>  APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
>  
> -EMBEDDED_EXTRA_CFLAGS := -nopie -fno-stack-protector -fno-stack-protector-all
> +EMBEDDED_EXTRA_CFLAGS := -fno-pie -no-pie -fno-stack-protector -fno-stack-protector-all
>  EMBEDDED_EXTRA_CFLAGS += -fno-exceptions -fno-asynchronous-unwind-tables
>  
>  XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
> 

Re: Gitlab breakage: [PATCH] Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

[Jan Beulich]

On 09.09.2022 01:34, Stefano Stabellini wrote:
> This patch breaks the gitlab-ci pipeline, specifically it breaks the
> hvmloader build with clang:
> 
> 
> https://gitlab.com/xen-project/xen/-/pipelines/634274727
> https://gitlab.com/xen-project/xen/-/jobs/2996114313
> 
> make[7]: Entering directory '/builds/xen-project/xen/tools/firmware/hvmloader'
> clang   -m32 -march=i686 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-local-typedefs   -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .hvmloader.o.d -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -mno-tls-direct-seg-refs  -DNDEBUG -Werror -fno-pie -no-pie -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -fcf-protection=none -ffreestanding -msoft-float -nostdinc -I/builds/xen-project/xen/tools/firmware/hvmloader/../../../tools/firmware/include -I/builds/xen-project/xen/tools/firmware/hvmloader/../../../tools/include -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -I../../libacpi  -c -o hvmloader.o hvmloader.c 
> clang: error: argument unused during compilation: '-nopie' [-Werror,-Wunused-command-line-argument]

First of all I'm puzzled by the error message: Now that we don't (try to)
use -nopie anymore, it complains about this option? We're clearly passing
-no-pie now as the command line shows.

But then - yes, I was actually expecting a similar diagnostic from gcc,
and I was surprised that there was none. Yet I have to admit I should
have tried a clang build of the hypervisor, where the issue also surfaces.

What's important though here - it's not really clear to me what the best
course of action is: We could filter out -no-pie everywhere that CFLAGS
has EMBEDDED_EXTRA_CFLAGS folded in, but isn't used for linking, but
that's odd to have in multiple places. We could also simply drop -no-pie
on the assumption that it's LDFLAGS which is supposed to be used for
linking, not CFLAGS. But that would be wrong for cases where compilation
and linking is done all in one go. Looks like we do such only with
HOSTCC / HOSTCFLAGS right now, but relying on this appears fragile.

I'll see if using the former approach promises to address the issue,
but I'll be happy to take suggestions towards better ways of dealing
with this.

Jan