[PATCH] Testsuite: Remove pie from trace tests

[PATCH] Testsuite: Remove pie from trace tests

[Alan Hayward]

[I don't know enough about tracing to know if this is a bug or
 expected behaviour. If this is a bug then maybe we should add
 some additional pie specific tests, along with a bugzilla entry?
 Either way, seems sensible to me to disable pie for the existing
 tests given that they were never written with pie in mind?]

Ubuntu/Debian defaults PIE to enabled.  This causes the trace tests
to fall over due to variables being returned as "unavailable".  The
tests were never designed to work with pie.

Simply ensure the nopie flag is always used for the failing tests.

This removes 100+ failures when running native-gdbserver on Ubuntu 18.04.

gdb/testsuite/ChangeLog:

2019-04-10  Alan Hayward  <alan.hayward@arm.com>

	* gdb.trace/backtrace.exp: Use nopie flag.
	* gdb.trace/circ.exp: Likewise.
	* gdb.trace/collection.exp: Likewise.
	* gdb.trace/ftrace.exp: Likewise.
	* gdb.trace/mi-trace-unavailable.exp: Likewise.
	* gdb.trace/mi-traceframe-changed.exp: Likewise.
	* gdb.trace/qtro.exp: Likewise.
	* gdb.trace/read-memory.exp: Likewise.
	* gdb.trace/report.exp: Likewise.
	* gdb.trace/tfile.exp: Likewise.
	* gdb.trace/tfind.exp: Likewise.
	* gdb.trace/unavailable.exp: Likewise.
---
 gdb/testsuite/gdb.trace/backtrace.exp             | 2 +-
 gdb/testsuite/gdb.trace/circ.exp                  | 2 +-
 gdb/testsuite/gdb.trace/collection.exp            | 2 +-
 gdb/testsuite/gdb.trace/ftrace.exp                | 2 +-
 gdb/testsuite/gdb.trace/mi-trace-unavailable.exp  | 2 +-
 gdb/testsuite/gdb.trace/mi-traceframe-changed.exp | 2 +-
 gdb/testsuite/gdb.trace/qtro.exp                  | 2 +-
 gdb/testsuite/gdb.trace/read-memory.exp           | 2 +-
 gdb/testsuite/gdb.trace/report.exp                | 2 +-
 gdb/testsuite/gdb.trace/tfile.exp                 | 2 +-
 gdb/testsuite/gdb.trace/tfind.exp                 | 2 +-
 gdb/testsuite/gdb.trace/unavailable.exp           | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/backtrace.exp b/gdb/testsuite/gdb.trace/backtrace.exp
index 0f60153cb5..24e097135f 100644
--- a/gdb/testsuite/gdb.trace/backtrace.exp
+++ b/gdb/testsuite/gdb.trace/backtrace.exp
@@ -27,7 +27,7 @@ if ![gdb_trace_common_supports_arch] {
 }
 
 if [prepare_for_testing "failed to prepare" $executable $srcfile \
-	[list debug nowarnings]] {
+	[list debug nowarnings nopie]] {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.trace/circ.exp b/gdb/testsuite/gdb.trace/circ.exp
index d48eca5deb..30ec9b47fc 100644
--- a/gdb/testsuite/gdb.trace/circ.exp
+++ b/gdb/testsuite/gdb.trace/circ.exp
@@ -17,7 +17,7 @@ load_lib "trace-support.exp"
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nowarnings}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nowarnings nopie}]} {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index 8c064385ba..44eccbe865 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -19,7 +19,7 @@ load_lib "trace-support.exp"
 standard_testfile
 set executable $testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nowarnings}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nowarnings nopie}]} {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index f97c2c6193..702efe76bb 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -53,7 +53,7 @@ set remote_libipa [gdb_load_shlib $libipa]
 # file unused because linking not done" when building the object.
 
 if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
-	  executable [list debug $additional_flags shlib=$libipa] ] != "" } {
+	  executable [list debug nopie $additional_flags shlib=$libipa] ] != "" } {
     untested "failed to compile"
     return -1
 }
diff --git a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
index 5ec4bbc152..f0b3c52728 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
@@ -17,7 +17,7 @@ load_lib trace-support.exp
 
 standard_testfile trace-unavailable.c
 
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug}] } {
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug nopie}] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
index 781d3646cb..1244b5b273 100644
--- a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
@@ -32,7 +32,7 @@ if {![is_remote host] && ![is_remote target]} {
 
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	   executable \
-	   [list debug nowarnings \
+	   [list debug nowarnings nopie\
 		"additional_flags=-DTFILE_DIR=\"$tfile_dir\""]] \
 	  != "" } {
      untested "failed to compile"
diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
index 9eabfeb51e..d5d912b5b9 100644
--- a/gdb/testsuite/gdb.trace/qtro.exp
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -22,7 +22,7 @@ load_lib trace-support.exp
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nopie}]} {
     return -1
 }
 clean_restart $testfile
diff --git a/gdb/testsuite/gdb.trace/read-memory.exp b/gdb/testsuite/gdb.trace/read-memory.exp
index 61fc137916..4d19f0074d 100644
--- a/gdb/testsuite/gdb.trace/read-memory.exp
+++ b/gdb/testsuite/gdb.trace/read-memory.exp
@@ -17,7 +17,7 @@ load_lib "trace-support.exp"
 
 standard_testfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug nopie}]} {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
index f43fbb7c62..c847ab0c5b 100644
--- a/gdb/testsuite/gdb.trace/report.exp
+++ b/gdb/testsuite/gdb.trace/report.exp
@@ -27,7 +27,7 @@ if ![gdb_trace_common_supports_arch] {
     return -1
 }
 if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
-	  executable {debug nowarnings}] != "" } {
+	  executable {debug nowarnings nopie}] != "" } {
     untested "failed to compile"
     return -1
 }
diff --git a/gdb/testsuite/gdb.trace/tfile.exp b/gdb/testsuite/gdb.trace/tfile.exp
index 04f3e98e99..23f4fc58d0 100644
--- a/gdb/testsuite/gdb.trace/tfile.exp
+++ b/gdb/testsuite/gdb.trace/tfile.exp
@@ -37,7 +37,7 @@ if {![is_remote host] && ![is_remote target]} {
 standard_testfile
 if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
 	  executable \
-	  [list debug \
+	  [list debug nopie\
 	       "additional_flags=-DTFILE_DIR=\"$tfile_dir\""]] \
 	 != "" } {
     untested "failed to compile"
diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
index 2c9996b040..fb58f80196 100644
--- a/gdb/testsuite/gdb.trace/tfind.exp
+++ b/gdb/testsuite/gdb.trace/tfind.exp
@@ -29,7 +29,7 @@ if ![gdb_trace_common_supports_arch] {
 }
 
 if { [gdb_compile "$srcdir/$subdir/$srcfile" "$binfile" \
-	  executable {debug nowarnings}] != "" } {
+	  executable {debug nowarnings nopie}] != "" } {
     untested "failed to compile"
     return -1
 }
diff --git a/gdb/testsuite/gdb.trace/unavailable.exp b/gdb/testsuite/gdb.trace/unavailable.exp
index 23d593e7d3..f3bb54db34 100644
--- a/gdb/testsuite/gdb.trace/unavailable.exp
+++ b/gdb/testsuite/gdb.trace/unavailable.exp
@@ -19,7 +19,7 @@ standard_testfile unavailable.cc
 set executable $testfile
 
 if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
-	 {debug nowarnings c++}]} {
+	 {debug nowarnings c++ nopie}]} {
     return -1
 }
 
-- 
2.20.1 (Apple Git-117)

Re: [PATCH] Testsuite: Remove pie from trace tests

[Kevin Buettner]

On Wed, 10 Apr 2019 12:25:43 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

> [I don't know enough about tracing to know if this is a bug or
>  expected behaviour. If this is a bug then maybe we should add
>  some additional pie specific tests, along with a bugzilla entry?
>  Either way, seems sensible to me to disable pie for the existing
>  tests given that they were never written with pie in mind?]
> 
> Ubuntu/Debian defaults PIE to enabled.  This causes the trace tests
> to fall over due to variables being returned as "unavailable".  The
> tests were never designed to work with pie.
> 
> Simply ensure the nopie flag is always used for the failing tests.
> 
> This removes 100+ failures when running native-gdbserver on Ubuntu 18.04.
> 
> gdb/testsuite/ChangeLog:
> 
> 2019-04-10  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb.trace/backtrace.exp: Use nopie flag.
> 	* gdb.trace/circ.exp: Likewise.
> 	* gdb.trace/collection.exp: Likewise.
> 	* gdb.trace/ftrace.exp: Likewise.
> 	* gdb.trace/mi-trace-unavailable.exp: Likewise.
> 	* gdb.trace/mi-traceframe-changed.exp: Likewise.
> 	* gdb.trace/qtro.exp: Likewise.
> 	* gdb.trace/read-memory.exp: Likewise.
> 	* gdb.trace/report.exp: Likewise.
> 	* gdb.trace/tfile.exp: Likewise.
> 	* gdb.trace/tfind.exp: Likewise.
> 	* gdb.trace/unavailable.exp: Likewise.

LGTM.

Kevin

Re: [PATCH] Testsuite: Remove pie from trace tests

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> Ubuntu/Debian defaults PIE to enabled.  This causes the trace tests
Alan> to fall over due to variables being returned as "unavailable".  The
Alan> tests were never designed to work with pie.

This seems strange to me -- I would not expect PIE to affect the trace
functionality.  I think it would be better to understand exactly why
this is happening.  Maybe the answer would still be to disable the
tests, but if so then we could have a reason attached to it.

Tom

Re: [PATCH] Testsuite: Remove pie from trace tests

[Alan Hayward]

> On 24 Apr 2019, at 20:22, Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Alan> Ubuntu/Debian defaults PIE to enabled.  This causes the trace tests
> Alan> to fall over due to variables being returned as "unavailable".  The
> Alan> tests were never designed to work with pie.
> 
> This seems strange to me -- I would not expect PIE to affect the trace
> functionality.  I think it would be better to understand exactly why
> this is happening.  Maybe the answer would still be to disable the
> tests, but if so then we could have a reason attached to it.
> 
> Tom

I’ve already pushed because I got a LGTM from Kevin.
It ensures we are essentially running the same test on all systems,
which makes sense.
Do you think it’s worth duplicating one or two of the tests, but with
PIE forced to enabled and the failures XFAIL? (Eg backtrace_pie.exp)


I’ve also raised a bug for it here too:
https://sourceware.org/bugzilla/show_bug.cgi?id=24483

I’ll try and find some time to look a bit deeper into it.


Alan.

Re: [PATCH] Testsuite: Remove pie from trace tests

[Tom Tromey]

Alan> I’ve already pushed because I got a LGTM from Kevin.
[...]
Alan> I’ve also raised a bug for it here too:
Alan> https://sourceware.org/bugzilla/show_bug.cgi?id=24483

Thanks for doing that.  TBH it's probably of minor importance; my sense
is that tracepoints aren't heavily used, and when they are used, it
isn't normally on systems with PIE.

Tom