Xen devel - Aug 2012 - [PATCH] x86-64/EFI: add -fno-stack-protector to EFI build

Otherwise, the build fails due to a missing __stack_chk_fail symbol.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/arch/x86/efi/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 005e3e0..b727757 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,11 +1,11 @@
-CFLAGS += -fshort-wchar
+CFLAGS += -fshort-wchar -fno-stack-protector
 
 obj-y += stub.o
 
 create = test -e $(1) || touch -t 199901010000 $(1)
 
 efi := $(filter y,$(x86_64)$(shell rm -f disabled))
-efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled &&
echo y))
+efi := $(if $(efi),$(shell $(CC) -fno-stack-protector -c -Werror check.c
2>disabled && echo y))
 efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
 efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
 
-- 
1.7.11.2

>>> On 14.08.12 at 15:52, Daniel De Graaf <dgdegra@tycho.nsa.gov>
wrote:
> Otherwise, the build fails due to a missing __stack_chk_fail symbol.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  xen/arch/x86/efi/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
> index 005e3e0..b727757 100644
> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -1,11 +1,11 @@
> -CFLAGS += -fshort-wchar
> +CFLAGS += -fshort-wchar -fno-stack-protector
It shouldn''t be needed here (or else the rest of the Xen build
should fail too). Or where would that symbol magically come
from?
>  obj-y += stub.o
>  
>  create = test -e $(1) || touch -t 199901010000 $(1)
>  
>  efi := $(filter y,$(x86_64)$(shell rm -f disabled))
> -efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled
&& echo y))
> +efi := $(if $(efi),$(shell $(CC) -fno-stack-protector -c -Werror check.c
2>disabled && echo y))
I can see why it might be needed here, albeit I''m curious why
this is not a problem for our builds: Are you having a compiler
in use that had been configured in a non-standard way? Plus
I first want to understand why this special option isn''t needed
for the rest of the build tree.

Jan
>  efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi
check.o 2>disabled && echo y))
>  efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call
create,boot.init.o); $(call create,runtime.o)))
>

On 08/14/2012 10:19 AM, Jan Beulich wrote:
>>>> On 14.08.12 at 15:52, Daniel De Graaf
<dgdegra@tycho.nsa.gov> wrote:
>> Otherwise, the build fails due to a missing __stack_chk_fail symbol.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> ---
>>  xen/arch/x86/efi/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
>> index 005e3e0..b727757 100644
>> --- a/xen/arch/x86/efi/Makefile
>> +++ b/xen/arch/x86/efi/Makefile
>> @@ -1,11 +1,11 @@
>> -CFLAGS += -fshort-wchar
>> +CFLAGS += -fshort-wchar -fno-stack-protector
> 
> It shouldn''t be needed here (or else the rest of the Xen build
> should fail too). Or where would that symbol magically come
> from?
You are correct, this change is not needed. The Xen build already adds
-fno-stack-protector to CFLAGS, so this change would just duplicate it.
>>  obj-y += stub.o
>>  
>>  create = test -e $(1) || touch -t 199901010000 $(1)
>>  
>>  efi := $(filter y,$(x86_64)$(shell rm -f disabled))
>> -efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled
&& echo y))
>> +efi := $(if $(efi),$(shell $(CC) -fno-stack-protector -c -Werror
check.c 2>disabled && echo y))
> 
> I can see why it might be needed here, albeit I''m curious why
> this is not a problem for our builds: Are you having a compiler
> in use that had been configured in a non-standard way? Plus
> I first want to understand why this special option isn''t needed
> for the rest of the build tree.
> 
> Jan
This build was done on Gentoo using a hardened profile, which I assume
adds -fstack-protector to the default compiler flags. Compiler version
string is "gcc version 4.6.3 (Gentoo Hardened 4.6.3 p1.6, pie-0.5.2)"

>>> On 14.08.12 at 16:30, Daniel De Graaf <dgdegra@tycho.nsa.gov>
wrote:
> On 08/14/2012 10:19 AM, Jan Beulich wrote:
>>>>> On 14.08.12 at 15:52, Daniel De Graaf
<dgdegra@tycho.nsa.gov> wrote:
>>> Otherwise, the build fails due to a missing __stack_chk_fail
symbol.
>>>
>>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> ---
>>>  xen/arch/x86/efi/Makefile | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
>>> index 005e3e0..b727757 100644
>>> --- a/xen/arch/x86/efi/Makefile
>>> +++ b/xen/arch/x86/efi/Makefile
>>> @@ -1,11 +1,11 @@
>>> -CFLAGS += -fshort-wchar
>>> +CFLAGS += -fshort-wchar -fno-stack-protector
>> 
>> It shouldn''t be needed here (or else the rest of the Xen build
>> should fail too). Or where would that symbol magically come
>> from?
> 
> You are correct, this change is not needed. The Xen build already adds
> -fno-stack-protector to CFLAGS, so this change would just duplicate it.
> 
>>>  obj-y += stub.o
>>>  
>>>  create = test -e $(1) || touch -t 199901010000 $(1)
>>>  
>>>  efi := $(filter y,$(x86_64)$(shell rm -f disabled))
>>> -efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled
&& echo y))
>>> +efi := $(if $(efi),$(shell $(CC) -fno-stack-protector -c -Werror
check.c 2>disabled && echo y))
>> 
>> I can see why it might be needed here, albeit I''m curious why
>> this is not a problem for our builds: Are you having a compiler
>> in use that had been configured in a non-standard way? Plus
>> I first want to understand why this special option isn''t
needed
>> for the rest of the build tree.
> 
> This build was done on Gentoo using a hardened profile, which I assume
> adds -fstack-protector to the default compiler flags. Compiler version
> string is "gcc version 4.6.3 (Gentoo Hardened 4.6.3 p1.6,
pie-0.5.2)"
So I''d prefer adding $(EMBEDDED_EXTRA_CFLAGS) instead of
the explicit option then, if that''s okay for you too. Mind re-
spinning the patch that way?

Jan

Without this, the compilation of check.c could fail due to compiler
features such as -fstack-protector being enabled, which causes a missing
__stack_chk_fail symbol error.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/arch/x86/efi/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 005e3e0..b32ea66 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -5,7 +5,7 @@ obj-y += stub.o
 create = test -e $(1) || touch -t 199901010000 $(1)
 
 efi := $(filter y,$(x86_64)$(shell rm -f disabled))
-efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled &&
echo y))
+efi := $(if $(efi),$(shell $(CC) $(EMBEDDED_EXTRA_CFLAGS) -c -Werror check.c
2>disabled && echo y))
 efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
 efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
 
-- 
1.7.11.2

Without this, the compilation of check.c could fail due to compiler
features such as -fstack-protector being enabled, which causes a missing
__stack_chk_fail symbol error.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---

Change from v2: EMBEDDED_EXTRA_CFLAGS may include flags not supported by
the compiler; CFLAGS includes a filtered version of these flags.

diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 005e3e0..1ba069d 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -5,7 +5,7 @@ obj-y += stub.o
 create = test -e $(1) || touch -t 199901010000 $(1)
 
 efi := $(filter y,$(x86_64)$(shell rm -f disabled))
-efi := $(if $(efi),$(shell $(CC) -c -Werror check.c 2>disabled &&
echo y))
+efi := $(if $(efi),$(shell $(CC) $(CFLAGS) -c -Werror check.c 2>disabled
&& echo y))
 efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
 efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
 
-- 
1.7.11.2

>>> On 14.08.12 at 21:37, Daniel De Graaf <dgdegra@tycho.nsa.gov>
wrote:
> Change from v2: EMBEDDED_EXTRA_CFLAGS may include flags not supported by
> the compiler; CFLAGS includes a filtered version of these flags.
I noticed this too when trying it out before committing. I''ll commit
a further adjusted version of it though: Using all of $(CFLAGS)
causes a stray file "..d" to be left in that directory, so
I''ll be
filtering out a number of things (specifically, I''ll simply filter out
the whole set of things in $(CFLAGS-y)).

Jan