[gdb/testsuite] Fail in gdb_compile if nopie results in PIE executable

Message ID 20190723072210.GA24180@delia
State New, archived
Headers

Commit Message

Tom de Vries July 23, 2019, 7:22 a.m. UTC
  Hi,

When running gdb.base/dump.exp with --target_board=unix/-fPIE/-pie, we get:
...
Running gdb/testsuite/gdb.base/dump.exp ...
FAIL: gdb.base/dump.exp: dump array as value, intel hex
...

The FAIL happens because although the test specifies nopie, the exec is
in fact compiled as PIE.  The "-fPIE -pie" options specified using the
target_board are interpreted by dejagnu as multilib_flags, and end up
overriding the nopie flags.

Fix this by checking in gdb_compile if the resulting exec is PIE despite of
a nopie setting, and if so return an error:
...
Running gdb/testsuite/gdb.base/dump.exp ...
gdb compile failed, nopie failed to prevent PIE executable

                === gdb Summary ===

nr of untested testcases         1
...

Tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb/testsuite] Fail in gdb_compile if nopie results in PIE executable

2019-07-23  Tom de Vries  <tdevries@suse.de>

	PR testsuite/24834
	* lib/gdb.exp (gdb_compile): Fail if nopie results in PIE executable.
	(exec_is_pie): New proc.

---
 gdb/testsuite/lib/gdb.exp | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
  

Comments

Alan Hayward July 29, 2019, 9:57 a.m. UTC | #1
> On 23 Jul 2019, at 08:22, Tom de Vries <tdevries@suse.de> wrote:

> 

> Hi,

> 

> When running gdb.base/dump.exp with --target_board=unix/-fPIE/-pie, we get:

> ...

> Running gdb/testsuite/gdb.base/dump.exp ...

> FAIL: gdb.base/dump.exp: dump array as value, intel hex

> ...

> 

> The FAIL happens because although the test specifies nopie, the exec is

> in fact compiled as PIE.  The "-fPIE -pie" options specified using the

> target_board are interpreted by dejagnu as multilib_flags, and end up

> overriding the nopie flags.

> 

> Fix this by checking in gdb_compile if the resulting exec is PIE despite of

> a nopie setting, and if so return an error:

> ...

> Running gdb/testsuite/gdb.base/dump.exp ...

> gdb compile failed, nopie failed to prevent PIE executable

> 

>                === gdb Summary ===

> 

> nr of untested testcases         1

> ...

> 

> Tested on x86_64-linux.

> 

> OK for trunk?


LGTM (but I’m not a global maintainer).

I tried this on an Ubuntu18.04 (which defaults gcc to use PIE), and everything
looks fine.


> 

> Thanks,

> - Tom

> 

> [gdb/testsuite] Fail in gdb_compile if nopie results in PIE executable

> 

> 2019-07-23  Tom de Vries  <tdevries@suse.de>

> 

> 	PR testsuite/24834

> 	* lib/gdb.exp (gdb_compile): Fail if nopie results in PIE executable.

> 	(exec_is_pie): New proc.

> 

> ---

> gdb/testsuite/lib/gdb.exp | 18 ++++++++++++++++++

> 1 file changed, 18 insertions(+)

> 

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

> index 49ec8b2a55..5ec0912325 100644

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

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

> @@ -3760,6 +3760,12 @@ proc gdb_compile {source dest type options} {

>     regsub "\[\r\n\]*$" "$result" "" result

>     regsub "^\[\r\n\]*" "$result" "" result

> 

> +    if { $type == "executable" && $result == "" && $nopie != -1 } {

> +	if { [exec_is_pie "$dest"] } {

> +	    set result "nopie failed to prevent PIE executable"

> +	}

> +    }

> +


As a side point, I just spotted that if a script specifies both pie and nopie, then
I think it ends up as nopie because the nopie flag gets set second.
Maybe it should error if both options are sent to gdb_compile.
Not something that needs fixing with this patch though.


>     if {[lsearch $options quiet] < 0} {

> 	# We shall update this on a per language basis, to avoid

> 	# changing the entire testsuite in one go.

> @@ -5160,6 +5166,18 @@ proc exec_has_index_section { executable } {

>     return 0

> }

> 

> +# Return true if EXECUTABLE is a Position Independent Executable.

> +

> +proc exec_is_pie { executable } {

> +    set readelf_program [gdb_find_readelf]

> +    set res [catch {exec $readelf_program -d $executable \

> +			| grep -E "(FLAGS_1).*Flags:.* PIE($| )" }]

> +    if { $res == 0 } {

> +	return 1

> +    }

> +    return 0

> +}

> +

> # Return true if a test should be skipped due to lack of floating

> # point support or GDB can't fetch the contents from floating point

> # registers.
  
Simon Marchi July 30, 2019, 1:20 a.m. UTC | #2
On 2019-07-29 5:57 a.m., Alan Hayward wrote:
>> OK for trunk?
> 
> LGTM (but I’m not a global maintainer).
> 
> I tried this on an Ubuntu18.04 (which defaults gcc to use PIE), and everything
> looks fine.
Thanks, LGTM too, please push.

Simon
  
Pedro Alves Aug. 16, 2019, 6:48 p.m. UTC | #3
On 7/23/19 8:22 AM, Tom de Vries wrote:
> When running gdb.base/dump.exp with --target_board=unix/-fPIE/-pie, we get:
> ...
> Running gdb/testsuite/gdb.base/dump.exp ...
> FAIL: gdb.base/dump.exp: dump array as value, intel hex
> ...
> 
> The FAIL happens because although the test specifies nopie, the exec is
> in fact compiled as PIE.  The "-fPIE -pie" options specified using the
> target_board are interpreted by dejagnu as multilib_flags, and end up
> overriding the nopie flags.

I'd think it would be better to temporarily strip out -fPIE/-pie (*)
from multilib_flags if nopie is set?

(*) - and/or the contents of gdb,pie_ldflag gdb,pie_flag.
 
Thanks,
Pedro Alves
  
Tom de Vries Aug. 17, 2019, 7:09 a.m. UTC | #4
On 16-08-19 20:48, Pedro Alves wrote:
> On 7/23/19 8:22 AM, Tom de Vries wrote:
>> When running gdb.base/dump.exp with --target_board=unix/-fPIE/-pie, we get:
>> ...
>> Running gdb/testsuite/gdb.base/dump.exp ...
>> FAIL: gdb.base/dump.exp: dump array as value, intel hex
>> ...
>>
>> The FAIL happens because although the test specifies nopie, the exec is
>> in fact compiled as PIE.  The "-fPIE -pie" options specified using the
>> target_board are interpreted by dejagnu as multilib_flags, and end up
>> overriding the nopie flags.
> 
> I'd think it would be better to temporarily strip out -fPIE/-pie (*)
> from multilib_flags if nopie is set?
> 
> (*) - and/or the contents of gdb,pie_ldflag gdb,pie_flag.
>  

I'm not sure about that.

I think the rationale you're applying here is to test as much as
possible in a single run: testing more is better.

But AFAIU, the implicit assumption about multilib flags is that a range
of multilib flags is tested (because multilib flags select different
libraries, so in order to claim complete testing you'd have to test all
libraries). In other words, taking the -m32/-m64 example, that you'd
test both with --target_board='unix/-m64' and
--target_board='unix/-m32'. So, if a test-case only works for -m32, then
forcing it to -m32 for unix/-m64 only makes sure you run the test twice
in identical fashion. In which case testing more is not better, just longer.

[ The same approach is used in gcc testing: a testcase can require a
certain effective target, and multilib flags influence whether the
effective target is available or not, and if not, the test is skipped as
unsupported. ]

Thanks,
- Tom
  

Patch

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 49ec8b2a55..5ec0912325 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3760,6 +3760,12 @@  proc gdb_compile {source dest type options} {
     regsub "\[\r\n\]*$" "$result" "" result
     regsub "^\[\r\n\]*" "$result" "" result
     
+    if { $type == "executable" && $result == "" && $nopie != -1 } {
+	if { [exec_is_pie "$dest"] } {
+	    set result "nopie failed to prevent PIE executable"
+	}
+    }
+
     if {[lsearch $options quiet] < 0} {
 	# We shall update this on a per language basis, to avoid
 	# changing the entire testsuite in one go.
@@ -5160,6 +5166,18 @@  proc exec_has_index_section { executable } {
     return 0
 }
 
+# Return true if EXECUTABLE is a Position Independent Executable.
+
+proc exec_is_pie { executable } {
+    set readelf_program [gdb_find_readelf]
+    set res [catch {exec $readelf_program -d $executable \
+			| grep -E "(FLAGS_1).*Flags:.* PIE($| )" }]
+    if { $res == 0 } {
+	return 1
+    }
+    return 0
+}
+
 # Return true if a test should be skipped due to lack of floating
 # point support or GDB can't fetch the contents from floating point
 # registers.