Fix various tests to use -no-pie linker flag when needed

Message ID 20180828193643.15530-1-jan.vrany@fit.cvut.cz
State New, archived
Headers

Commit Message

Jan Vrany Aug. 28, 2018, 7:36 p.m. UTC
  Various test use test code written in i385 / x86_64 assembly that cannot
be used to create PIE executables. Therefore compilation of test programs
failed on systems where the compiler default is to create PIE executable.

To fix this, force -no-pie linker flag.

gdb/testsuite/ChangeLog:
2018-08-28  Jan Vrany  <jan.vrany@fit.cvut.cz>

	* gdb.arch/amd64-disp-step.exp: Use -no-pie linker flag
	to enforce non-PIE executable.
	* gdb.arch/amd64-entry-value.exp: Likewise.
	* gdb.arch/amd64-invalid-stack-middle.exp: Likewise.
	* gdb.arch/i386-float.exp: Likewise.
	* gdb.arch/i386-signal.exp: Likewise.
	* gdb.mi/mi-reg-undefined.exp: Likewise.
---
 gdb/testsuite/ChangeLog                               | 10 ++++++++++
 gdb/testsuite/gdb.arch/amd64-disp-step.exp            |  3 ++-
 gdb/testsuite/gdb.arch/amd64-entry-value.exp          |  2 +-
 gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp |  3 ++-
 gdb/testsuite/gdb.arch/i386-float.exp                 |  2 +-
 gdb/testsuite/gdb.arch/i386-signal.exp                |  2 +-
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp             |  2 +-
 7 files changed, 18 insertions(+), 6 deletions(-)
  

Comments

Tom Tromey Sept. 7, 2018, 8:43 p.m. UTC | #1
>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:

Jan> Various test use test code written in i385 / x86_64 assembly that cannot
Jan> be used to create PIE executables. Therefore compilation of test programs
Jan> failed on systems where the compiler default is to create PIE executable.

Jan> To fix this, force -no-pie linker flag.

I guess you're on an OS that enables PIE by default?

These tests don't seem to be linux- or gcc-specific.
Do you know how universal -no-pie is?  My worry is that this would fix
the test for some people but break it for others.

Tom
  
Simon Marchi Sept. 7, 2018, 10:18 p.m. UTC | #2
On 2018-09-07 21:43, Tom Tromey wrote:
>>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> 
> Jan> Various test use test code written in i385 / x86_64 assembly that 
> cannot
> Jan> be used to create PIE executables. Therefore compilation of test 
> programs
> Jan> failed on systems where the compiler default is to create PIE 
> executable.
> 
> Jan> To fix this, force -no-pie linker flag.
> 
> I guess you're on an OS that enables PIE by default?

I know recent-ish Ubuntus do that, I don't know if it's a patch specific 
to that distro or if it's the default value in gcc that changed.

> These tests don't seem to be linux- or gcc-specific.
> Do you know how universal -no-pie is?  My worry is that this would fix
> the test for some people but break it for others.
> 
> Tom

If needed, we could add a new option understood by gdb_compile that adds 
the right flag to achieve the result of -no-pie.

Simon
  
Jan Vrany Oct. 14, 2018, 9:59 a.m. UTC | #3
Hi, 

I'm sorry for a long delay, I've been very, very busy lately.
Hopefully I'd have more time for GDB now...


On Fri, 2018-09-07 at 23:18 +0100, Simon Marchi wrote:
> On 2018-09-07 21:43, Tom Tromey wrote:
> > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> > 
> > Jan> Various test use test code written in i385 / x86_64 assembly that 
> > cannot
> > Jan> be used to create PIE executables. Therefore compilation of test 
> > programs
> > Jan> failed on systems where the compiler default is to create PIE 
> > executable.
> > 
> > Jan> To fix this, force -no-pie linker flag.
> > 
> > I guess you're on an OS that enables PIE by default?
> 
> I know recent-ish Ubuntus do that, I don't know if it's a patch specific 
> to that distro or if it's the default value in gcc that changed.

Neither I know. I'm using Debian Buster which seems to have -no-pie 
by default too. 

> 
> > These tests don't seem to be linux- or gcc-specific.
> > Do you know how universal -no-pie is?  My worry is that this would fix
> > the test for some people but break it for others.
> > 

No, I don't know how universal -no-pie is. From what I have understood
from Simon (https://sourceware.org/ml/gdb-patches/2018-08/msg00662.html),
out of the options available -no-pie is still the best bet. 

> > Tom
> 
> If needed, we could add a new option understood by gdb_compile that adds 
> the right flag to achieve the result of -no-pie.

Indeed we can. Tom, would that be OK with you? 

Best Jan
  
Simon Marchi Oct. 16, 2018, 10:18 p.m. UTC | #4
On 2018-10-14 05:59, Jan Vrany wrote:
> Hi,
> 
> I'm sorry for a long delay, I've been very, very busy lately.
> Hopefully I'd have more time for GDB now...
> 
> 
> On Fri, 2018-09-07 at 23:18 +0100, Simon Marchi wrote:
>> On 2018-09-07 21:43, Tom Tromey wrote:
>> > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
>> >
>> > Jan> Various test use test code written in i385 / x86_64 assembly that
>> > cannot
>> > Jan> be used to create PIE executables. Therefore compilation of test
>> > programs
>> > Jan> failed on systems where the compiler default is to create PIE
>> > executable.
>> >
>> > Jan> To fix this, force -no-pie linker flag.
>> >
>> > I guess you're on an OS that enables PIE by default?
>> 
>> I know recent-ish Ubuntus do that, I don't know if it's a patch 
>> specific
>> to that distro or if it's the default value in gcc that changed.
> 
> Neither I know. I'm using Debian Buster which seems to have -no-pie
> by default too.
> 
>> 
>> > These tests don't seem to be linux- or gcc-specific.
>> > Do you know how universal -no-pie is?  My worry is that this would fix
>> > the test for some people but break it for others.
>> >
> 
> No, I don't know how universal -no-pie is. From what I have understood
> from Simon 
> (https://sourceware.org/ml/gdb-patches/2018-08/msg00662.html),
> out of the options available -no-pie is still the best bet.

gcc supports it, icc supports it [1], clang supports it starting at 
version 6 according to my testing.  I think this is universal enough 
that we can use it.  If somebody stumbles on a compiler that does not 
understand -no-pie and they really need to use it for their testing, 
they can quite easily add an option for gdb_compile.  But for now, I 
don't think it's worth the extra work.

[1] https://software.intel.com/en-us/node/523278

I tested the patch and amd64-disp-step.exp fails for me.  When you 
specify options to prepare_for_testing explicitly, you have to include 
the "debug" option (if you do want debug symbols).

With this fix the patch LGTM.

Simon
  
Tom Tromey Oct. 17, 2018, 1:55 p.m. UTC | #5
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> No, I don't know how universal -no-pie is. From what I have understood
>> from Simon
>> (https://sourceware.org/ml/gdb-patches/2018-08/msg00662.html),
>> out of the options available -no-pie is still the best bet.

Simon> gcc supports it, icc supports it [1], clang supports it starting at
Simon> version 6 according to my testing.  I think this is universal enough
Simon> that we can use it.

Sounds reasonable to me.

Tom
  
Pedro Alves Oct. 17, 2018, 3:47 p.m. UTC | #6
On 10/16/2018 11:18 PM, Simon Marchi wrote:
> On 2018-10-14 05:59, Jan Vrany wrote:

>> No, I don't know how universal -no-pie is. From what I have understood
>> from Simon (https://sourceware.org/ml/gdb-patches/2018-08/msg00662.html),
>> out of the options available -no-pie is still the best bet.
> 
> gcc supports it, icc supports it [1], clang supports it starting at version 6 according to my testing.  I think this is universal 
> enough that we can use it.  If somebody stumbles on a compiler that does not understand -no-pie and they really need to use it for 
> their testing, they can quite easily add an option for gdb_compile.  

Not sure it's that universal.  See:

  https://github.com/xd009642/tarpaulin/issues/7#issuecomment-317180523

For example, the gcc 4.8 I have handy (our minimum supported version) does
not support it:

 $ /opt/gcc-4.8/bin/gcc /home/pedro/gdb/tests/main.c -o main -no-pie
 gcc: error: unrecognized command line option ‘-no-pie’

> But for now, I don't think it's worth the extra work.

I disagree.

Thanks,
Pedro Alves
  
Simon Marchi Oct. 17, 2018, 3:51 p.m. UTC | #7
On 2018-10-17 11:47, Pedro Alves wrote:
> On 10/16/2018 11:18 PM, Simon Marchi wrote:
>> On 2018-10-14 05:59, Jan Vrany wrote:
> 
>>> No, I don't know how universal -no-pie is. From what I have 
>>> understood
>>> from Simon 
>>> (https://sourceware.org/ml/gdb-patches/2018-08/msg00662.html),
>>> out of the options available -no-pie is still the best bet.
>> 
>> gcc supports it, icc supports it [1], clang supports it starting at 
>> version 6 according to my testing.  I think this is universal
>> enough that we can use it.  If somebody stumbles on a compiler that 
>> does not understand -no-pie and they really need to use it for
>> their testing, they can quite easily add an option for gdb_compile.
> 
> Not sure it's that universal.  See:
> 
>   https://github.com/xd009642/tarpaulin/issues/7#issuecomment-317180523
> 
> For example, the gcc 4.8 I have handy (our minimum supported version) 
> does
> not support it:
> 
>  $ /opt/gcc-4.8/bin/gcc /home/pedro/gdb/tests/main.c -o main -no-pie
>  gcc: error: unrecognized command line option ‘-no-pie’

Ah, I just assumed that gcc supported it since virtually forever, but 
no.

>> But for now, I don't think it's worth the extra work.
> 
> I disagree.

Given that new information, I agree with you disagreeing.

Simon
  

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6b5275a4cb..409843ff27 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@ 
+2018-08-28  Jan Vrany  <jan.vrany@fit.cvut.cz>
+
+	* gdb.arch/amd64-disp-step.exp: Use -no-pie linker flag
+	to enforce non-PIE executable.
+	* gdb.arch/amd64-entry-value.exp: Likewise.
+	* gdb.arch/amd64-invalid-stack-middle.exp: Likewise.
+	* gdb.arch/i386-float.exp: Likewise.
+	* gdb.arch/i386-signal.exp: Likewise.
+	* gdb.mi/mi-reg-undefined.exp: Likewise.
+
 2018-08-23  Kevin Buettner  <kevinb@redhat.com>
 
 	* gdb.dwarf2/dw2-ranges-func.c: New file.
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index 782b75896c..8ddd48a1e8 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
@@ -25,9 +25,10 @@  if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
 
 set newline "\[\r\n\]*"
 
+set opts {ldflags=-no-pie}
 standard_testfile .S
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.arch/amd64-entry-value.exp b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
index 72700d55c2..eeb22c8066 100644
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
@@ -14,7 +14,7 @@ 
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 standard_testfile .s
-set opts {}
+set opts {ldflags=-no-pie}
 
 if [info exists COMPILE] {
     # make check RUNTESTFLAGS="gdb.arch/amd64-entry-value.exp COMPILE=1"
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index c00bfa10bd..b63fec9a1f 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -27,7 +27,8 @@ 
 # run twice, and we restart gdb before testing each different command to
 # ensure that nothing is being cached.
 
-set opts {}
+set opts {ldflags=-no-pie}
+
 standard_testfile .S
 
 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
diff --git a/gdb/testsuite/gdb.arch/i386-float.exp b/gdb/testsuite/gdb.arch/i386-float.exp
index 87c90c372e..3fccf5e81b 100644
--- a/gdb/testsuite/gdb.arch/i386-float.exp
+++ b/gdb/testsuite/gdb.arch/i386-float.exp
@@ -28,7 +28,7 @@  standard_testfile .S
 # some targets have leading underscores on assembly symbols.
 set additional_flags [gdb_target_symbol_prefix_flags_asm]
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile [list debug $additional_flags]] } {
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile [list debug ldflags=-no-pie $additional_flags]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.arch/i386-signal.exp b/gdb/testsuite/gdb.arch/i386-signal.exp
index 38046a13cb..a046c06c9d 100644
--- a/gdb/testsuite/gdb.arch/i386-signal.exp
+++ b/gdb/testsuite/gdb.arch/i386-signal.exp
@@ -23,7 +23,7 @@  if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
 standard_testfile
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
-	  executable { debug }] != "" } {
+	  executable { debug ldflags=-no-pie }] != "" } {
     untested "failed to compile"
     return -1
 }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 83abab1a0e..bc560ad9e5 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -33,7 +33,7 @@  if [mi_gdb_start] {
 
 standard_testfile .S
 
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug ldflags=-no-pie}] != "" } {
      untested "failed to compile"
      return -1
 }