Testsuite: Remove pie from trace tests
Commit Message
[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(-)
--
2.20.1 (Apple Git-117)
Comments
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
>>>>> "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
> 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.
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
@@ -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
}
@@ -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
}
@@ -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
}
@@ -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
}
@@ -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
}
@@ -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"
@@ -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
@@ -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
}
@@ -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
}
@@ -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"
@@ -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
}
@@ -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
}