diff mbox

x86emul/test: disable pie for 64-bit builds

Message ID 20170925104914.28975-1-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu Sept. 25, 2017, 10:49 a.m. UTC
PIE may (and commonly will) result in the binary being loaded above
the 4Gb boundary, which can't work with at least the VZEROUPPER compat
mode test.

Reported-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

With this patch, vzeroupper passes, but one other test fails.
Testing SSE packed single 64-bit code sequence...[line 368] failed!
---
 tools/tests/x86_emulator/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jan Beulich Sept. 25, 2017, 11:35 a.m. UTC | #1
>>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
> PIE may (and commonly will) result in the binary being loaded above
> the 4Gb boundary, which can't work with at least the VZEROUPPER compat
> mode test.
> 
> Reported-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> With this patch, vzeroupper passes, but one other test fails.
> Testing SSE packed single 64-bit code sequence...[line 368] failed!

Feel free to mail me the binary again, albeit that one's going to
be more difficult to debug without being able to see it myself.

> @@ -98,7 +98,9 @@ asm:
>  
>  asm/%: asm ;
>  
> -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
> +HOSTCFLAGS-x86_64 :=
> +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
> +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))

I don't understand this change to my original patch: You now
conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
addition needs to be conditional: In order to be able to build the
entire test, a reasonably new tool chain is needed anyway (much
newer than what we require for building everything else). And finally
- is there a difference between -no-pie and -fno-PIE / -fno-pie?

Jan
Wei Liu Sept. 25, 2017, 11:43 a.m. UTC | #2
On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
> > PIE may (and commonly will) result in the binary being loaded above
> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
> > mode test.
> > 
> > Reported-by: Wei Liu <wei.liu2@citrix.com>
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > With this patch, vzeroupper passes, but one other test fails.
> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
> 
> Feel free to mail me the binary again, albeit that one's going to
> be more difficult to debug without being able to see it myself.
> 
> > @@ -98,7 +98,9 @@ asm:
> >  
> >  asm/%: asm ;
> >  
> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
> > +HOSTCFLAGS-x86_64 :=
> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
> 
> I don't understand this change to my original patch: You now
> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the

My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.

> addition needs to be conditional: In order to be able to build the
> entire test, a reasonably new tool chain is needed anyway (much
> newer than what we require for building everything else). And finally

It needs to be conditional because not all gcc versions support -no-pie.

> - is there a difference between -no-pie and -fno-PIE / -fno-pie?
> 

I can't tell the difference by reading the manpage TBH. I only know one
works while the other doesn't by trial and error.
Jan Beulich Sept. 25, 2017, 11:54 a.m. UTC | #3
>>> On 25.09.17 at 13:43, <wei.liu2@citrix.com> wrote:
> On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
>> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
>> > PIE may (and commonly will) result in the binary being loaded above
>> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
>> > mode test.
>> > 
>> > Reported-by: Wei Liu <wei.liu2@citrix.com>
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > ---
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> > 
>> > With this patch, vzeroupper passes, but one other test fails.
>> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
>> 
>> Feel free to mail me the binary again, albeit that one's going to
>> be more difficult to debug without being able to see it myself.
>> 
>> > @@ -98,7 +98,9 @@ asm:
>> >  
>> >  asm/%: asm ;
>> >  
>> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
>> > +HOSTCFLAGS-x86_64 :=
>> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
>> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
>> 
>> I don't understand this change to my original patch: You now
>> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
>> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
> 
> My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.
> 
>> addition needs to be conditional: In order to be able to build the
>> entire test, a reasonably new tool chain is needed anyway (much
>> newer than what we require for building everything else). And finally
> 
> It needs to be conditional because not all gcc versions support -no-pie.

You mean older one (which would be no problem, as said) or even
up-to-date ones (due to the way they're being configured)?

>> - is there a difference between -no-pie and -fno-PIE / -fno-pie?
>> 
> 
> I can't tell the difference by reading the manpage TBH. I only know one
> works while the other doesn't by trial and error.

Oh, that's certainly worthwhile adding to the description then.

Jan
Wei Liu Sept. 25, 2017, 12:55 p.m. UTC | #4
On Mon, Sep 25, 2017 at 05:54:41AM -0600, Jan Beulich wrote:
> >>> On 25.09.17 at 13:43, <wei.liu2@citrix.com> wrote:
> > On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
> >> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
> >> > PIE may (and commonly will) result in the binary being loaded above
> >> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
> >> > mode test.
> >> > 
> >> > Reported-by: Wei Liu <wei.liu2@citrix.com>
> >> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> > ---
> >> > Cc: Jan Beulich <jbeulich@suse.com>
> >> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >> > 
> >> > With this patch, vzeroupper passes, but one other test fails.
> >> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
> >> 
> >> Feel free to mail me the binary again, albeit that one's going to
> >> be more difficult to debug without being able to see it myself.
> >> 
> >> > @@ -98,7 +98,9 @@ asm:
> >> >  
> >> >  asm/%: asm ;
> >> >  
> >> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
> >> > +HOSTCFLAGS-x86_64 :=
> >> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
> >> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
> >> 
> >> I don't understand this change to my original patch: You now
> >> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
> >> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
> > 
> > My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.
> > 
> >> addition needs to be conditional: In order to be able to build the
> >> entire test, a reasonably new tool chain is needed anyway (much
> >> newer than what we require for building everything else). And finally
> > 
> > It needs to be conditional because not all gcc versions support -no-pie.
> 
> You mean older one (which would be no problem, as said) or even
> up-to-date ones (due to the way they're being configured)?

Let me be precise because I don't know which version you count as old or
up-to-date.

Gcc <5.4 has -pie but no -no-pie. IIRC passing -no-pie will cause the
linker to return an error. I don't have a machine that old to verify it
though.

> 
> >> - is there a difference between -no-pie and -fno-PIE / -fno-pie?
> >> 
> > 
> > I can't tell the difference by reading the manpage TBH. I only know one
> > works while the other doesn't by trial and error.
> 
> Oh, that's certainly worthwhile adding to the description then.

After reading a bit more into the manual: the -no-pie option is for
the linker, while the -fno-pie option is for code generation.

The build rune is in fact using HOSTCC to link the executable, hence we
need -fno-pie.

I'm not sure why omitting -fno-PIE is not a problem, but it works.
Wei Liu Sept. 25, 2017, 1:01 p.m. UTC | #5
On Mon, Sep 25, 2017 at 01:55:03PM +0100, Wei Liu wrote:
> On Mon, Sep 25, 2017 at 05:54:41AM -0600, Jan Beulich wrote:
> > >>> On 25.09.17 at 13:43, <wei.liu2@citrix.com> wrote:
> > > On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
> > >> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
> > >> > PIE may (and commonly will) result in the binary being loaded above
> > >> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
> > >> > mode test.
> > >> > 
> > >> > Reported-by: Wei Liu <wei.liu2@citrix.com>
> > >> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > >> > ---
> > >> > Cc: Jan Beulich <jbeulich@suse.com>
> > >> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > >> > 
> > >> > With this patch, vzeroupper passes, but one other test fails.
> > >> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
> > >> 
> > >> Feel free to mail me the binary again, albeit that one's going to
> > >> be more difficult to debug without being able to see it myself.
> > >> 
> > >> > @@ -98,7 +98,9 @@ asm:
> > >> >  
> > >> >  asm/%: asm ;
> > >> >  
> > >> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
> > >> > +HOSTCFLAGS-x86_64 :=
> > >> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
> > >> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
> > >> 
> > >> I don't understand this change to my original patch: You now
> > >> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
> > >> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
> > > 
> > > My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.
> > > 
> > >> addition needs to be conditional: In order to be able to build the
> > >> entire test, a reasonably new tool chain is needed anyway (much
> > >> newer than what we require for building everything else). And finally
> > > 
> > > It needs to be conditional because not all gcc versions support -no-pie.
> > 
> > You mean older one (which would be no problem, as said) or even
> > up-to-date ones (due to the way they're being configured)?
> 
> Let me be precise because I don't know which version you count as old or
> up-to-date.
> 
> Gcc <5.4 has -pie but no -no-pie. IIRC passing -no-pie will cause the
> linker to return an error. I don't have a machine that old to verify it
> though.
> 

Actually I do have a wheezy chroot to verify that:

(wheezy)wei@zion:/local/work$ gcc --version
gcc (Debian 4.6.3-14) 4.6.3

(wheezy)wei@zion:/local/work$ gcc -no-pie
gcc: error: unrecognized option '-no-pie'
gcc: fatal error: no input files
compilation terminated.
Jan Beulich Sept. 25, 2017, 1:09 p.m. UTC | #6
>>> On 25.09.17 at 14:55, <wei.liu2@citrix.com> wrote:
> On Mon, Sep 25, 2017 at 05:54:41AM -0600, Jan Beulich wrote:
>> >>> On 25.09.17 at 13:43, <wei.liu2@citrix.com> wrote:
>> > On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
>> >> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
>> >> > PIE may (and commonly will) result in the binary being loaded above
>> >> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
>> >> > mode test.
>> >> > 
>> >> > Reported-by: Wei Liu <wei.liu2@citrix.com>
>> >> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> >> > ---
>> >> > Cc: Jan Beulich <jbeulich@suse.com>
>> >> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> >> > 
>> >> > With this patch, vzeroupper passes, but one other test fails.
>> >> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
>> >> 
>> >> Feel free to mail me the binary again, albeit that one's going to
>> >> be more difficult to debug without being able to see it myself.
>> >> 
>> >> > @@ -98,7 +98,9 @@ asm:
>> >> >  
>> >> >  asm/%: asm ;
>> >> >  
>> >> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
>> >> > +HOSTCFLAGS-x86_64 :=
>> >> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
>> >> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
>> >> 
>> >> I don't understand this change to my original patch: You now
>> >> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
>> >> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
>> > 
>> > My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.
>> > 
>> >> addition needs to be conditional: In order to be able to build the
>> >> entire test, a reasonably new tool chain is needed anyway (much
>> >> newer than what we require for building everything else). And finally
>> > 
>> > It needs to be conditional because not all gcc versions support -no-pie.
>> 
>> You mean older one (which would be no problem, as said) or even
>> up-to-date ones (due to the way they're being configured)?
> 
> Let me be precise because I don't know which version you count as old or
> up-to-date.
> 
> Gcc <5.4 has -pie but no -no-pie. IIRC passing -no-pie will cause the
> linker to return an error. I don't have a machine that old to verify it
> though.

Okay, gcc below 5 counts as "old" here for me; iirc I had to update
to that version to actually be able to properly build and run all
current tests.

>> >> - is there a difference between -no-pie and -fno-PIE / -fno-pie?
>> >> 
>> > 
>> > I can't tell the difference by reading the manpage TBH. I only know one
>> > works while the other doesn't by trial and error.
>> 
>> Oh, that's certainly worthwhile adding to the description then.
> 
> After reading a bit more into the manual: the -no-pie option is for
> the linker, while the -fno-pie option is for code generation.
> 
> The build rune is in fact using HOSTCC to link the executable, hence we
> need -fno-pie.
> 
> I'm not sure why omitting -fno-PIE is not a problem, but it works.

All pretty confusing. Are you going to send a cleaned up patch
then?

Jan
Jan Beulich Sept. 25, 2017, 1:16 p.m. UTC | #7
>>> On 25.09.17 at 15:01, <wei.liu2@citrix.com> wrote:
> On Mon, Sep 25, 2017 at 01:55:03PM +0100, Wei Liu wrote:
>> On Mon, Sep 25, 2017 at 05:54:41AM -0600, Jan Beulich wrote:
>> > >>> On 25.09.17 at 13:43, <wei.liu2@citrix.com> wrote:
>> > > On Mon, Sep 25, 2017 at 05:35:05AM -0600, Jan Beulich wrote:
>> > >> >>> On 25.09.17 at 12:49, <wei.liu2@citrix.com> wrote:
>> > >> > PIE may (and commonly will) result in the binary being loaded above
>> > >> > the 4Gb boundary, which can't work with at least the VZEROUPPER compat
>> > >> > mode test.
>> > >> > 
>> > >> > Reported-by: Wei Liu <wei.liu2@citrix.com>
>> > >> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> > >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > >> > ---
>> > >> > Cc: Jan Beulich <jbeulich@suse.com>
>> > >> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> > >> > 
>> > >> > With this patch, vzeroupper passes, but one other test fails.
>> > >> > Testing SSE packed single 64-bit code sequence...[line 368] failed!
>> > >> 
>> > >> Feel free to mail me the binary again, albeit that one's going to
>> > >> be more difficult to debug without being able to see it myself.
>> > >> 
>> > >> > @@ -98,7 +98,9 @@ asm:
>> > >> >  
>> > >> >  asm/%: asm ;
>> > >> >  
>> > >> > -HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
>> > >> > +HOSTCFLAGS-x86_64 :=
>> > >> > +$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
>> > >> > +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
>> > >> 
>> > >> I don't understand this change to my original patch: You now
>> > >> conditionally add -no-pie to HOSTCFLAGS (i.e. also for 32-bit builds),
>> > >> and HOSTCFLAGS-x86_64 remains empty. I also don't see why the
>> > > 
>> > > My bad. The flag should be conditionally added to HOSTCFLAGS-x86_64.
>> > > 
>> > >> addition needs to be conditional: In order to be able to build the
>> > >> entire test, a reasonably new tool chain is needed anyway (much
>> > >> newer than what we require for building everything else). And finally
>> > > 
>> > > It needs to be conditional because not all gcc versions support -no-pie.
>> > 
>> > You mean older one (which would be no problem, as said) or even
>> > up-to-date ones (due to the way they're being configured)?
>> 
>> Let me be precise because I don't know which version you count as old or
>> up-to-date.
>> 
>> Gcc <5.4 has -pie but no -no-pie. IIRC passing -no-pie will cause the
>> linker to return an error. I don't have a machine that old to verify it
>> though.
>> 
> 
> Actually I do have a wheezy chroot to verify that:
> 
> (wheezy)wei@zion:/local/work$ gcc --version
> gcc (Debian 4.6.3-14) 4.6.3
> 
> (wheezy)wei@zion:/local/work$ gcc -no-pie
> gcc: error: unrecognized option '-no-pie'
> gcc: fatal error: no input files
> compilation terminated.

But you said gcc would want to be passed -fno-pie anyway - does
it choke on that one too?

Jan
Wei Liu Sept. 25, 2017, 1:21 p.m. UTC | #8
On Mon, Sep 25, 2017 at 07:16:43AM -0600, Jan Beulich wrote:
> > 
> > Actually I do have a wheezy chroot to verify that:
> > 
> > (wheezy)wei@zion:/local/work$ gcc --version
> > gcc (Debian 4.6.3-14) 4.6.3
> > 
> > (wheezy)wei@zion:/local/work$ gcc -no-pie
> > gcc: error: unrecognized option '-no-pie'
> > gcc: fatal error: no input files
> > compilation terminated.
> 
> But you said gcc would want to be passed -fno-pie anyway - does

That was a rather unfortunate typo. Sorry.

"The build rune is in fact using HOSTCC to link the executable, hence we
need -fno-pie." Should be "we need -no-pie".

> it choke on that one too?
> 

No, it doesn't.

For gcc 4.6, -fno-pie is OK but -no-pie is not, because 4.6 already has
-fno-pie.
Wei Liu Sept. 25, 2017, 1:22 p.m. UTC | #9
On Mon, Sep 25, 2017 at 07:09:51AM -0600, Jan Beulich wrote:
> > The build rune is in fact using HOSTCC to link the executable, hence we
> > need -fno-pie.
> > 

This should be "we need -no-pie", sorry.

> > I'm not sure why omitting -fno-PIE is not a problem, but it works.
> 
> All pretty confusing. Are you going to send a cleaned up patch
> then?

I'm going to send out v2 with updated commit message.
diff mbox

Patch

diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index fd13ab53b1..819a29f731 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -76,7 +76,7 @@  $(addsuffix .c,$(SIMD)) $(addsuffix -avx.c,$(filter sse%,$(SIMD))):
 	ln -sf simd.c $@
 
 $(TARGET): x86_emulate.o test_x86_emulator.o
-	$(HOSTCC) -o $@ $^
+	$(HOSTCC) $(HOSTCFLAGS) -o $@ $^
 
 .PHONY: clean
 clean:
@@ -98,7 +98,9 @@  asm:
 
 asm/%: asm ;
 
-HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
+HOSTCFLAGS-x86_64 :=
+$(call cc-option-add,HOSTCFLAGS,HOSTCC,-no-pie)
+HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
 
 x86.h := asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h
 x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h $(x86.h)