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

Message ID 20181213152049.7702-1-jan.vrany@fit.cvut.cz
State New
Headers show
Series
  • Fix various tests to use -no-pie linker flag when needed
Related show

Commit Message

Jan Vrany Dec. 13, 2018, 3:20 p.m.
Various test use test code written in i386 / 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.

The solution is to use -no-pie linker flag, however, such flag may not
(is not) supported by all compilers GDB needs to support. To handle this,
introduce a new flag to gdb_compile - nopie - which inserts -no-pie linker
flag where supported and is no-op where it is not. By default, -no-pie flag
is inserted since most modern compiler do support it.

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                             | 12 ++++++++++++
 gdb/testsuite/gdb.arch/amd64-disp-step.exp          |  3 ++-
 gdb/testsuite/gdb.arch/amd64-entry-value.exp        |  2 +-
 .../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 +-
 gdb/testsuite/lib/gdb.exp                           | 13 +++++++++++++
 8 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.19.2

Comments

Simon Marchi Dec. 13, 2018, 5:13 p.m. | #1
On 2018-12-13 10:20, Jan Vrany wrote:
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp

> index 5a5713b114..f25df0a772 100644

> --- a/gdb/testsuite/lib/gdb.exp

> +++ b/gdb/testsuite/lib/gdb.exp

> @@ -3482,6 +3482,7 @@ set gdb_saved_set_unbuffered_mode_obj ""

>  #     dynamically load libraries at runtime.  For example, on Linux, 

> this adds

>  #     -ldl so that the test can use dlopen.

>  #   - nowarnings:  Inhibit all compiler warnings.

> +#   - nopie: Prevent creation of PIE executables.

>  #

>  # And here are some of the not too obscure options understood by 

> DejaGnu that

>  # influence the compilation:

> @@ -3603,6 +3604,18 @@ proc gdb_compile {source dest type options} {

>  	set options [lreplace $options $nowarnings $nowarnings $flag]

>      }

> 

> +    # Replace the "nopie" option with the appropriate additional_flags

> +    # to disable PIE executables.

> +    set nopie [lsearch -exact $options nopie]

> +    if {$nopie != -1} {

> +	if [target_info exists gdb,nowarnings_flag] {


s/nowarnings_flag/nopie_flag/

> +	    set flag "ldflags=[target_info gdb,nopie_flag]"

> +	} else {

> +	    set flag "ldflags=-no-pie"

> +	}

> +	set options [lreplace $options $nopie $nopie $flag]

> +    }

> +


With this, people who run tests against gcc 4.8 will be forced to create 
a specific board file to override the nopie_flag, whereas currently it's 
possible to do it all from the command line:

$ make check TESTS="gdb.arch/amd64-disp-step.exp" 
RUNTESTFLAGS='CC_FOR_TARGET="gcc-4.8"'

One way to avoid that would be to auto-detect whether the toolchain 
generates PIE executables by default.  If the nopie option is requested 
but the toolchain generates non-PIE by default, we would not pass any 
additional flags.  On Linux, we can generate an executable and use 
"readelf -h" to check for the type of ELF artifact generated.

Another way would be to check whether the toolchain knows the -no-pie 
flag, and assume that if it doesn't, it's because it probably generates 
non-PIE by default, so we don't need to pass anything.  That would break 
with toolchains that generate PIE by default but use another flag to 
disable PIE (I am not aware of any that fits in this category), but 
people would still have the possibiliy of overriding it with a board 
file.

Or maybe this just fine, and we can require the few people who test 
against gcc 4.8 to use a board file.

Simon
Simon Marchi Dec. 21, 2018, 8:56 p.m. | #2
On 2018-12-13 12:13 p.m., Simon Marchi wrote:
> On 2018-12-13 10:20, Jan Vrany wrote:

>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp

>> index 5a5713b114..f25df0a772 100644

>> --- a/gdb/testsuite/lib/gdb.exp

>> +++ b/gdb/testsuite/lib/gdb.exp

>> @@ -3482,6 +3482,7 @@ set gdb_saved_set_unbuffered_mode_obj ""

>>  #     dynamically load libraries at runtime.  For example, on Linux, this adds

>>  #     -ldl so that the test can use dlopen.

>>  #   - nowarnings:  Inhibit all compiler warnings.

>> +#   - nopie: Prevent creation of PIE executables.

>>  #

>>  # And here are some of the not too obscure options understood by DejaGnu that

>>  # influence the compilation:

>> @@ -3603,6 +3604,18 @@ proc gdb_compile {source dest type options} {

>>      set options [lreplace $options $nowarnings $nowarnings $flag]

>>      }

>>

>> +    # Replace the "nopie" option with the appropriate additional_flags

>> +    # to disable PIE executables.

>> +    set nopie [lsearch -exact $options nopie]

>> +    if {$nopie != -1} {

>> +    if [target_info exists gdb,nowarnings_flag] {

> 

> s/nowarnings_flag/nopie_flag/

> 

>> +        set flag "ldflags=[target_info gdb,nopie_flag]"

>> +    } else {

>> +        set flag "ldflags=-no-pie"

>> +    }

>> +    set options [lreplace $options $nopie $nopie $flag]

>> +    }

>> +

> 

> With this, people who run tests against gcc 4.8 will be forced to create a specific board file to override the nopie_flag, whereas currently it's possible to do it all from the command line:

> 

> $ make check TESTS="gdb.arch/amd64-disp-step.exp" RUNTESTFLAGS='CC_FOR_TARGET="gcc-4.8"'

> 

> One way to avoid that would be to auto-detect whether the toolchain generates PIE executables by default.  If the nopie option is requested but the toolchain generates non-PIE by default, we would not pass any additional flags.  On Linux, we can generate an executable and use "readelf -h" to check for the type of ELF artifact generated.

> 

> Another way would be to check whether the toolchain knows the -no-pie flag, and assume that if it doesn't, it's because it probably generates non-PIE by default, so we don't need to pass anything.  That would break with toolchains that generate PIE by default but use another flag to disable PIE (I am not aware of any that fits in this category), but people would still have the possibiliy of overriding it with a board file.

> 

> Or maybe this just fine, and we can require the few people who test against gcc 4.8 to use a board file.

> 

> Simon


Hi Jan,

I adjusted the "nowarnings_flag" copy-pasto, added an entry in testsuite/README
and pushed your patch (as below).

Thanks!

Simon

From 6e8b1ab2fd4c692146eb14295866920f83dc5403 Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@fit.cvut.cz>

Date: Thu, 13 Dec 2018 15:20:49 +0000
Subject: [PATCH] Fix various tests to use -no-pie linker flag when needed

Various tests use test code written in i386 / 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.

The solution is to use -no-pie linker flag, however, such flag may not
(is not) supported by all compilers GDB needs to support (e.g. gcc 4.8).
To handle this, introduce a new flag to gdb_compile - nopie - which
inserts -no-pie linker flag where supported and is no-op where it is
not. By default, -no-pie flag is inserted since most modern compiler do
support it.
---
 gdb/testsuite/ChangeLog                             | 13 +++++++++++++
 gdb/testsuite/README                                |  5 +++++
 gdb/testsuite/gdb.arch/amd64-disp-step.exp          |  3 ++-
 gdb/testsuite/gdb.arch/amd64-entry-value.exp        |  2 +-
 .../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 +-
 gdb/testsuite/lib/gdb.exp                           | 13 +++++++++++++
 9 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f3224d4b4e4..1ac687e17d0 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,16 @@
+2018-12-21  Jan Vrany  <jan.vrany@fit.cvut.cz>
+
+	* lib/gdb.exp (gdb_compile): Add new nopie flag enforce
+	non-PIE executables.
+	* README: Mention the gdb,nopie_flag board setting.
+	* gdb.arch/amd64-disp-step.exp: Use nopie 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-12-21  Paul Marechal  <paul.marechal@ericsson.com>

 	PR gdb/23974
diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 723d8ba5eb5..b5e75b9a79a 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -482,6 +482,11 @@ gdb,no_thread_names

   The target doesn't support thread names.

+gdb,nopie_flag
+
+  The flag required to force the compiler to produce non-position-independent
+  executables.
+
 Testsuite Organization
 **********************

diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index 782b75896cc..0b941e7f63f 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 {debug nopie}
 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 72700d55c23..a8d7d4e2f6c 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 {nopie}

 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 2bb0344052f..db4d9d70bbb 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -27,7 +27,6 @@
 # run twice, and we restart gdb before testing each different command to
 # ensure that nothing is being cached.

-set opts {}
 standard_testfile .S

 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
@@ -35,7 +34,7 @@ if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     return
 }

-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} $opts] } {
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {nopie}] } {
     return -1
 }

diff --git a/gdb/testsuite/gdb.arch/i386-float.exp b/gdb/testsuite/gdb.arch/i386-float.exp
index 87c90c372ee..f0d05f340f3 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 nopie $additional_flags]] } {
     return -1
 }

diff --git a/gdb/testsuite/gdb.arch/i386-signal.exp b/gdb/testsuite/gdb.arch/i386-signal.exp
index 38046a13cb4..d7acb0497c0 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 nopie }] != "" } {
     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 83abab1a0e3..e7f0b548604 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 nopie}] != "" } {
      untested "failed to compile"
      return -1
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 5a5713b1148..c5fda7477e0 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3482,6 +3482,7 @@ set gdb_saved_set_unbuffered_mode_obj ""
 #     dynamically load libraries at runtime.  For example, on Linux, this adds
 #     -ldl so that the test can use dlopen.
 #   - nowarnings:  Inhibit all compiler warnings.
+#   - nopie: Prevent creation of PIE executables.
 #
 # And here are some of the not too obscure options understood by DejaGnu that
 # influence the compilation:
@@ -3603,6 +3604,18 @@ proc gdb_compile {source dest type options} {
 	set options [lreplace $options $nowarnings $nowarnings $flag]
     }

+    # Replace the "nopie" option with the appropriate additional_flags
+    # to disable PIE executables.
+    set nopie [lsearch -exact $options nopie]
+    if {$nopie != -1} {
+	if [target_info exists gdb,nopie_flag] {
+	    set flag "ldflags=[target_info gdb,nopie_flag]"
+	} else {
+	    set flag "ldflags=-no-pie"
+	}
+	set options [lreplace $options $nopie $nopie $flag]
+    }
+
     if { $type == "executable" } {
 	if { ([istarget "*-*-mingw*"]
 	      || [istarget "*-*-*djgpp"]
-- 
2.20.1

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ab3a74fffd..34e10ea3cb 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,15 @@ 
+2018-12-13  Jan Vrany  <jan.vrany@fit.cvut.cz>
+
+	* lib/gdb.exp (gdb_compile): Add new nopie flag enforce
+	non-PIE executables.
+	* gdb.arch/amd64-disp-step.exp: Use nopie 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-12-12  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.base/annota1.exp: Update a test regexp.
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index 782b75896c..0b941e7f63 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 {debug nopie}
 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..a8d7d4e2f6 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 {nopie}
 
 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 2bb0344052..db4d9d70bb 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -27,7 +27,6 @@ 
 # run twice, and we restart gdb before testing each different command to
 # ensure that nothing is being cached.
 
-set opts {}
 standard_testfile .S
 
 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
@@ -35,7 +34,7 @@  if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     return
 }
 
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} $opts] } {
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {nopie}] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.arch/i386-float.exp b/gdb/testsuite/gdb.arch/i386-float.exp
index 87c90c372e..f0d05f340f 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 nopie $additional_flags]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.arch/i386-signal.exp b/gdb/testsuite/gdb.arch/i386-signal.exp
index 38046a13cb..d7acb0497c 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 nopie }] != "" } {
     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..e7f0b54860 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 nopie}] != "" } {
      untested "failed to compile"
      return -1
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 5a5713b114..f25df0a772 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3482,6 +3482,7 @@  set gdb_saved_set_unbuffered_mode_obj ""
 #     dynamically load libraries at runtime.  For example, on Linux, this adds
 #     -ldl so that the test can use dlopen.
 #   - nowarnings:  Inhibit all compiler warnings.
+#   - nopie: Prevent creation of PIE executables.
 #
 # And here are some of the not too obscure options understood by DejaGnu that
 # influence the compilation:
@@ -3603,6 +3604,18 @@  proc gdb_compile {source dest type options} {
 	set options [lreplace $options $nowarnings $nowarnings $flag]
     }
 
+    # Replace the "nopie" option with the appropriate additional_flags
+    # to disable PIE executables.
+    set nopie [lsearch -exact $options nopie]
+    if {$nopie != -1} {
+	if [target_info exists gdb,nowarnings_flag] {
+	    set flag "ldflags=[target_info gdb,nopie_flag]"
+	} else {
+	    set flag "ldflags=-no-pie"
+	}
+	set options [lreplace $options $nopie $nopie $flag]
+    }
+
     if { $type == "executable" } {
 	if { ([istarget "*-*-mingw*"]
 	      || [istarget "*-*-*djgpp"]