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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Tom de Vries
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  <[hidden email]>

        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"
+ }
+    }
+
     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.
Reply | Threaded
Open this post in threaded view
|

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

Alan Hayward


> On 23 Jul 2019, at 08:22, Tom de Vries <[hidden email]> 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  <[hidden email]>
>
> 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.

Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
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
Reply | Threaded
Open this post in threaded view
|

[committed][gdb/testsuite] Fail in gdb_compile if pie results in non-PIE executable

Tom de Vries
[ was: Re: [PATCH][gdb/testsuite] Fail in gdb_compile if nopie results
in PIE executable ]

On 30-07-19 03:20, Simon Marchi wrote:
> 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.

Hi,

this follow-up patch deals with the opposite: Fail in gdb_compile if pie
results in non-PIE executable.

Thanks,
- Tom

[gdb/testsuite] Fail in gdb_compile if pie results in non-PIE executable

When running gdb.base/break-idempotent.exp with
--target_board=unix/-fno-PIE/-no-pie, we get:
...
nr of expected passes            140
...

The test-case is compiled once with nopie and once with pie, but in both cases
we end up with a non-PIE executable.  The "-fno-PIE -no-pie" options specified
using the target_board are interpreted by dejagnu as multilib_flags, and end up
overriding the pie flags.

Fix this by checking in gdb_compile if the resulting exec is non-PIE despite of
a pie setting, and if so return an error:
...
Running gdb/testsuite/gdb.base/break-idempotent.exp ...
gdb compile failed, pie failed to generate PIE executable

                === gdb Summary ===

nr of expected passes            70
nr of untested testcases         1
...

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

2019-08-05  Tom de Vries  <[hidden email]>

        * lib/gdb.exp (version_at_least): Factor out of ...
        (tcl_version_at_least): ... here.
        (gdb_compile): Fail if pie results in non-PIE executable.
        (readelf_version, readelf_prints_pie): New proc.
        (exec_is_pie): Return -1 if unknown.

---
 gdb/testsuite/lib/gdb.exp | 78 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 14 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 6d16217f3b..529b6f6030 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1103,21 +1103,27 @@ proc gdb_test { args } {
      }]
 }
 
-# Return 1 if tcl version used is at least MAJOR.MINOR
-proc tcl_version_at_least { major minor } {
-    global tcl_version
-    regexp {^([0-9]+)\.([0-9]+)$} $tcl_version \
- dummy tcl_version_major tcl_version_minor
-    if { $tcl_version_major > $major } {
+# Return 1 if version MAJOR.MINOR is at least AT_LEAST_MAJOR.AT_LEAST_MINOR.
+proc version_at_least { major minor at_least_major at_least_minor} {
+    if { $major > $at_least_major } {
         return 1
-    } elseif { $tcl_version_major == $major \
-   && $tcl_version_minor >= $minor } {
+    } elseif { $major == $at_least_major \
+   && $minor >= $at_least_minor } {
         return 1
     } else {
         return 0
     }
 }
 
+# Return 1 if tcl version used is at least MAJOR.MINOR
+proc tcl_version_at_least { major minor } {
+    global tcl_version
+    regexp {^([0-9]+)\.([0-9]+)$} $tcl_version \
+ dummy tcl_version_major tcl_version_minor
+    return [version_at_least $tcl_version_major $tcl_version_minor \
+ $major $minor]
+}
+
 if { [tcl_version_at_least 8 5] == 0 } {
     # lrepeat was added in tcl 8.5.  Only add if missing.
     proc lrepeat { n element } {
@@ -3803,9 +3809,13 @@ 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"] } {
+    if { $type == "executable" && $result == "" \
+     && ($nopie != -1 || $pie != -1) } {
+ set is_pie [exec_is_pie "$dest"]
+ if { $nopie != -1 && $is_pie == 1 } {
     set result "nopie failed to prevent PIE executable"
+ } elseif { $pie != -1 && $is_pie == 0 } {
+    set result "pie failed to generate PIE executable"
  }
     }
 
@@ -5209,13 +5219,53 @@ proc exec_has_index_section { executable } {
     return 0
 }
 
-# Return true if EXECUTABLE is a Position Independent Executable.
+# Return list with major and minor version of readelf, or an empty list.
+gdb_caching_proc readelf_version {
+    set readelf_program [gdb_find_readelf]
+    set res [catch {exec $readelf_program --version} output]
+    if { $res != 0 } {
+ return [list]
+    }
+    set lines [split $output \n]
+    set line [lindex $lines 0]
+    set res [regexp {[ \t]+([0-9]+)[.]([0-9]+)[^ \t]*$} \
+ $line dummy major minor]
+    if { $res != 1 } {
+ return [list]
+    }
+    return [list $major $minor]
+}
+
+# Return 1 if readelf prints the PIE flag, 0 if is doesn't, and -1 if unknown.
+proc readelf_prints_pie { } {
+    set version [readelf_version]
+    if { [llength $version] == 0 } {
+ return -1
+    }
+    set major [lindex $version 0]
+    set minor [lindex $version 1]
+    # It would be better to construct a PIE executable and test if the PIE
+    # flag is printed by readelf, but we cannot reliably construct a PIE
+    # executable if the multilib_flags dictate otherwise
+    # (--target_board=unix/-no-pie/-fno-PIE).
+    return [version_at_least $major $minor 2 26]
+}
+
+# Return 1 if EXECUTABLE is a Position Independent Executable, 0 if it is not,
+# and -1 if unknown.
 
 proc exec_is_pie { executable } {
+    set res [readelf_prints_pie]
+    if { $res != 1 } {
+ return -1
+    }
     set readelf_program [gdb_find_readelf]
-    set res [catch {exec $readelf_program -d $executable \
- | grep -E "(FLAGS_1).*Flags:.* PIE($| )" }]
-    if { $res == 0 } {
+    set res [catch {exec $readelf_program -d $executable} output]
+    if { $res != 0 } {
+ return -1
+    }
+    set res [regexp -line {\(FLAGS_1\).*Flags:.* PIE($| )} $output]
+    if { $res == 1 } {
  return 1
     }
     return 0
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Tom de Vries
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
Reply | Threaded
Open this post in threaded view
|

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

Tom de Vries
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