public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [committed][gdb/testsuite] Disable vgdb tests if xml not supported
@ 2021-09-29 13:55 Tom de Vries
  2021-09-30 17:30 ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2021-09-29 13:55 UTC (permalink / raw)
  To: gdb-patches

Hi,

I build gdb without xml support using --without-expat, and ran into:
...
(gdb) target remote | vgdb --wait=2 --max-invoke-ms=2500 --pid=22032^M
Remote debugging using | vgdb --wait=2 --max-invoke-ms=2500 --pid=22032^M
relaying data between gdb and process 22032^M
warning: Can not parse XML target description; XML support was disabled at \
  compile time^M
  ...
(gdb) PASS: gdb.base/valgrind-infcall.exp: continue #1
p gdb_test_infcall ()^M
Remote 'g' packet reply is too long (expected 560 bytes, got 800 bytes): ...^M
(gdb) FAIL: gdb.base/valgrind-infcall.exp: p gdb_test_infcall ()
...

After googling the error message with context valgrind gdbserver, I found
indications that the Remote 'g' packet reply error is due to missing xml
support.

And here ( https://www.valgrind.org/docs/manual/manual-core-adv.html ) I
found:
...
GDB version needed for ARM and PPC32/64.

You must use a GDB version which is able to read XML target description sent
by a gdbserver.  This is the standard setup if GDB was configured and built
with the "expat" library.  If your GDB was not configured with XML support, it
will report an error message when using the "target" command.  Debugging will
not work because GDB will then not be able to fetch the registers from the
Valgrind gdbserver.
...

So I guess I'm running into the same problem for x86_64.

Fix this by skipping all gdb.base/valgrind-*.exp tests if xml support is not
available.  Although only the gdb.base/valgrind-infcall*.exp produce fails,
the Remote 'g' packet reply error occurs in all tests, so it seems prudent to
disable them all.

Tested on x86_64-linux.

Committed to trunk.

Thanks,
- Tom

[gdb/testsuite] Disable vgdb tests if xml not supported

---
 gdb/testsuite/gdb.base/valgrind-bt.exp        | 5 +++++
 gdb/testsuite/gdb.base/valgrind-disp-step.exp | 5 +++++
 gdb/testsuite/gdb.base/valgrind-infcall-2.exp | 5 +++++
 gdb/testsuite/gdb.base/valgrind-infcall.exp   | 5 +++++
 4 files changed, 20 insertions(+)

diff --git a/gdb/testsuite/gdb.base/valgrind-bt.exp b/gdb/testsuite/gdb.base/valgrind-bt.exp
index b559e433d72..4b84a45d333 100644
--- a/gdb/testsuite/gdb.base/valgrind-bt.exp
+++ b/gdb/testsuite/gdb.base/valgrind-bt.exp
@@ -13,6 +13,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if { [gdb_skip_xml_test] } {
+    # Valgrind gdbserver requires gdb with xml support.
+    return 0
+}
+
 load_lib valgrind.exp
 
 if [is_remote target] {
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
index 3aecf00a291..f9429fbd69a 100644
--- a/gdb/testsuite/gdb.base/valgrind-disp-step.exp
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
@@ -18,6 +18,11 @@
 # really tests is that GDB falls back to in-line stepping
 # automatically instead of getting stuck or crashing.
 
+if { [gdb_skip_xml_test] } {
+    # Valgrind gdbserver requires gdb with xml support.
+    return 0
+}
+
 load_lib valgrind.exp
 
 if [is_remote target] {
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
index 82b3f4e5c49..30881ca72ea 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
@@ -29,6 +29,11 @@
 # terminate called after throwing an instance of 'gdb_exception_error'
 # Aborted (core dumped)
 
+if { [gdb_skip_xml_test] } {
+    # Valgrind gdbserver requires gdb with xml support.
+    return 0
+}
+
 load_lib valgrind.exp
 
 if [is_remote target] {
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall.exp b/gdb/testsuite/gdb.base/valgrind-infcall.exp
index 658d2bdb3de..0108f92ceaf 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall.exp
@@ -13,6 +13,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if { [gdb_skip_xml_test] } {
+    # Valgrind gdbserver requires gdb with xml support.
+    return 0
+}
+
 load_lib valgrind.exp
 
 if [is_remote target] {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [committed][gdb/testsuite] Disable vgdb tests if xml not supported
  2021-09-29 13:55 [committed][gdb/testsuite] Disable vgdb tests if xml not supported Tom de Vries
@ 2021-09-30 17:30 ` Tom Tromey
  2021-09-30 21:41   ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2021-09-30 17:30 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches

Tom> +if { [gdb_skip_xml_test] } {
Tom> +    # Valgrind gdbserver requires gdb with xml support.
Tom> +    return 0
Tom> +}

Do we want/need an 'untested' in there?

I wonder if there's a better way to handle these checks, and also
checking for whether compilation succeeded -- that is, remove the
repeated code blocks in favor of some kind of wrapper that uses "return
-code" (but not an exception) to short-circuit the calling .exp file on
failure.  Probably a big task though.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [committed][gdb/testsuite] Disable vgdb tests if xml not supported
  2021-09-30 17:30 ` Tom Tromey
@ 2021-09-30 21:41   ` Tom de Vries
  2021-09-30 21:47     ` Tom de Vries
  2021-10-05 18:15     ` Tom Tromey
  0 siblings, 2 replies; 9+ messages in thread
From: Tom de Vries @ 2021-09-30 21:41 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

On 9/30/21 7:30 PM, Tom Tromey wrote:
> Tom> +if { [gdb_skip_xml_test] } {
> Tom> +    # Valgrind gdbserver requires gdb with xml support.
> Tom> +    return 0
> Tom> +}
> 
> Do we want/need an 'untested' in there?
> 

Ack, will do.

> I wonder if there's a better way to handle these checks, and also
> checking for whether compilation succeeded -- that is, remove the
> repeated code blocks in favor of some kind of wrapper that uses "return
> -code" (but not an exception) to short-circuit the calling .exp file on
> failure.  Probably a big task though.

I suppose we could do in lib/gdb.exp:
...
proc require_xml_support {} {
    if { [gdb_skip_xml_test] } {
        untested "missing xml support"
        return -code return 0
    }
}
...
and then in the test-case we're left with the relatively brief:
...
require_xml_support
...

Or perhaps you mean more generically (which wouldn't require us to write
a proc for each type of require):
...
proc require { fn val } {
    if { [$fn] != $val } {
        untested "$fn != $val"
        return -code return 0
    }
}
...
and:
...
require gdb_skip_xml_test 0
...
?

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [committed][gdb/testsuite] Disable vgdb tests if xml not supported
  2021-09-30 21:41   ` Tom de Vries
@ 2021-09-30 21:47     ` Tom de Vries
  2021-10-05 18:15     ` Tom Tromey
  1 sibling, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2021-09-30 21:47 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On 9/30/21 11:41 PM, Tom de Vries wrote:
> On 9/30/21 7:30 PM, Tom Tromey wrote:
>> Tom> +if { [gdb_skip_xml_test] } {
>> Tom> +    # Valgrind gdbserver requires gdb with xml support.
>> Tom> +    return 0
>> Tom> +}
>>
>> Do we want/need an 'untested' in there?
>>
> 
> Ack, will do.
> 

Pushed this patch.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Add-untested-for-missing-xml-support-in-gdb.base-valgrind-.exp.patch --]
[-- Type: text/x-patch, Size: 2056 bytes --]

[gdb/testsuite] Add untested for missing xml support in gdb.base/valgrind*.exp

Add untested in case missing xml support is detected in test-cases
gdb.base/valgrind*.exp.

Tested on x86_64-linux.

---
 gdb/testsuite/gdb.base/valgrind-bt.exp        | 1 +
 gdb/testsuite/gdb.base/valgrind-disp-step.exp | 1 +
 gdb/testsuite/gdb.base/valgrind-infcall-2.exp | 1 +
 gdb/testsuite/gdb.base/valgrind-infcall.exp   | 1 +
 4 files changed, 4 insertions(+)

diff --git a/gdb/testsuite/gdb.base/valgrind-bt.exp b/gdb/testsuite/gdb.base/valgrind-bt.exp
index 4b84a45d333..440c6e403e9 100644
--- a/gdb/testsuite/gdb.base/valgrind-bt.exp
+++ b/gdb/testsuite/gdb.base/valgrind-bt.exp
@@ -15,6 +15,7 @@
 
 if { [gdb_skip_xml_test] } {
     # Valgrind gdbserver requires gdb with xml support.
+    untested "missing xml support"
     return 0
 }
 
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
index f9429fbd69a..88ac848ae2d 100644
--- a/gdb/testsuite/gdb.base/valgrind-disp-step.exp
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
@@ -20,6 +20,7 @@
 
 if { [gdb_skip_xml_test] } {
     # Valgrind gdbserver requires gdb with xml support.
+    untested "missing xml support"
     return 0
 }
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
index 30881ca72ea..a368717c1e8 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
@@ -31,6 +31,7 @@
 
 if { [gdb_skip_xml_test] } {
     # Valgrind gdbserver requires gdb with xml support.
+    untested "missing xml support"
     return 0
 }
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall.exp b/gdb/testsuite/gdb.base/valgrind-infcall.exp
index 0108f92ceaf..9f49a320f52 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall.exp
@@ -15,6 +15,7 @@
 
 if { [gdb_skip_xml_test] } {
     # Valgrind gdbserver requires gdb with xml support.
+    untested "missing xml support"
     return 0
 }
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [committed][gdb/testsuite] Disable vgdb tests if xml not supported
  2021-09-30 21:41   ` Tom de Vries
  2021-09-30 21:47     ` Tom de Vries
@ 2021-10-05 18:15     ` Tom Tromey
  2021-10-06  7:40       ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2021-10-05 18:15 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Tom Tromey, Tom de Vries

Tom> Or perhaps you mean more generically (which wouldn't require us to write
Tom> a proc for each type of require):
Tom> ...
Tom> proc require { fn val } {
Tom>     if { [$fn] != $val } {
Tom>         untested "$fn != $val"
Tom>         return -code return 0
Tom>     }
Tom> }
Tom> ...
Tom> and:
Tom> ...
Tom> require gdb_skip_xml_test 0
Tom> ...
Tom> ?

Either seems fine but this one does read nicely to me.
Or maybe even just requiring 'fn' to return 0 would be good enough.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp
  2021-10-05 18:15     ` Tom Tromey
@ 2021-10-06  7:40       ` Tom de Vries
  2021-10-07 11:00         ` [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie Tom de Vries
  2021-10-11 10:28         ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
  0 siblings, 2 replies; 9+ messages in thread
From: Tom de Vries @ 2021-10-06  7:40 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

[ was: Re: [committed][gdb/testsuite] Disable vgdb tests if xml not
supported ]

On 10/5/21 8:15 PM, Tom Tromey wrote:
> Tom> Or perhaps you mean more generically (which wouldn't require us to write
> Tom> a proc for each type of require):
> Tom> ...
> Tom> proc require { fn val } {
> Tom>     if { [$fn] != $val } {
> Tom>         untested "$fn != $val"
> Tom>         return -code return 0
> Tom>     }
> Tom> }
> Tom> ...
> Tom> and:
> Tom> ...
> Tom> require gdb_skip_xml_test 0
> Tom> ...
> Tom> ?
> 
> Either seems fine but this one does read nicely to me.
> Or maybe even just requiring 'fn' to return 0 would be good enough.

I've also thought about that, but decided against it:
- it does not take into account a proc support_foo that returns 1, which
  in the current form can be required using "require support_foo 1"
- it does not support the cases where we require no support for a
  feature, say "require gdb_skip_xml_test 1"

Also, I considered making the test freeform, as in gdb_assert, but
decided against it because:
- it does not enforce uniform usage
- makes it harder to do the fn+value to message matching I've added in
  this version.

Any comment?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Add-proc-require-in-lib-gdb.exp.patch --]
[-- Type: text/x-patch, Size: 3925 bytes --]

[gdb/testsuite] Add proc require in lib/gdb.exp

Add a new proc require in lib/gdb.exp, and use to shorten:
...
if { [gdb_skip_xml_test] } {
    # Valgrind gdbserver requires gdb with xml support.
    untested "missing xml support"
    return 0
}
...
into:
...
require gdb_skip_xml_test 0
...

Tested on x86_64-linux, both with and without a trigger patch that forces
gdb_skip_xml_test to return 1.

---
 gdb/testsuite/gdb.base/valgrind-bt.exp        |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-disp-step.exp |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-infcall-2.exp |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-infcall.exp   |  7 ++-----
 gdb/testsuite/lib/gdb.exp                     | 16 ++++++++++++++++
 5 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/gdb/testsuite/gdb.base/valgrind-bt.exp b/gdb/testsuite/gdb.base/valgrind-bt.exp
index 440c6e403e9..546701498b2 100644
--- a/gdb/testsuite/gdb.base/valgrind-bt.exp
+++ b/gdb/testsuite/gdb.base/valgrind-bt.exp
@@ -13,11 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
index 88ac848ae2d..c1ed5a3e69b 100644
--- a/gdb/testsuite/gdb.base/valgrind-disp-step.exp
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
@@ -18,11 +18,8 @@
 # really tests is that GDB falls back to in-line stepping
 # automatically instead of getting stuck or crashing.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
index a368717c1e8..064cf719712 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
@@ -29,11 +29,8 @@
 # terminate called after throwing an instance of 'gdb_exception_error'
 # Aborted (core dumped)
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall.exp b/gdb/testsuite/gdb.base/valgrind-infcall.exp
index 9f49a320f52..68bc1b8e4e7 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall.exp
@@ -13,11 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 5642db4334d..89733c59605 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8215,5 +8215,21 @@ gdb_caching_proc have_avx {
     return $status
 }
 
+# Require [FN] to return VAL.  If not, return in the caller's context.
+
+proc require { fn val } {
+    if { [$fn] == $val } {
+	return
+    }
+
+    switch $fn-$val {
+	gdb_skip_xml_test-0 { set msg "missing xml support" }
+	default { set msg "$fn != $val" }
+    }
+
+    untested $msg
+    return -code return 0
+}
+
 # Always load compatibility stuff.
 load_lib future.exp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie
  2021-10-06  7:40       ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
@ 2021-10-07 11:00         ` Tom de Vries
  2021-10-08 14:37           ` Tom de Vries
  2021-10-11 10:28         ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2021-10-07 11:00 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 147 bytes --]

[ was: Re: [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp ]

This follow-up patch uses the new proc require.

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0002-gdb-testsuite-Prevent-compilation-fails-with-unix-fPIE-pie.patch --]
[-- Type: text/x-patch, Size: 10056 bytes --]

[gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie

A regular test-case will produce an executable, and depending on the compiler
default, it will be a PIE or not.  A test-case can force one or the other
using the pie and nopie options.

However, when running with target board unix/-fPIE/-pie, the nopie option will
have no effect, and likewise for target board unix/-fno-PIE/-no-pie and the
pie option.

When say we run test-case gdb.base/attach-pie-noexec.exp, which passes the pie
option with target board unix/-fno-PIE/-no-pie we get:
...
 Running src/gdb/testsuite/gdb.base/attach-pie-noexec.exp ...
 gdb compile failed, pie failed to generate PIE executable

                 === gdb Summary ===

 # of untested testcases         1
...

However, this works only when we actually manage to generate an executable.

There are other test-cases, like f.i. gdb.arch/amd64-disp-step.exp that
specify nopie, but will generate a compilation failure with target board
unix/-fPIE/-pie due to using a hard-coded .S file:
...
 Running src/gdb/testsuite/gdb.arch/amd64-disp-step.exp ...
 gdb compile failed, ld: outputs/gdb.arch/amd64-disp-step/amd64-disp-step0.o: \
   relocation R_X86_64_32S against `.text' can not be used when making a PIE \
   object; recompile with -fPIE
 collect2: error: ld returned 1 exit status

                 === gdb Summary ===

 # of untested testcases         1
...

Hide this compilation error by:
- adding a gdb_caching_proc pie_forced, and
- adding "require pie_forced 0" in all affected test-cases.
such that we simply have:
...
UNTESTED: gdb.arch/amd64-disp-step.exp: nopie failed to prevent PIE executable
...

Likewise, add nopie_forced.

Tested on x86_64-linux.

---
 gdb/testsuite/gdb.arch/amd64-disp-step.exp         |  2 ++
 gdb/testsuite/gdb.arch/amd64-entry-value.exp       |  2 ++
 .../gdb.arch/amd64-invalid-stack-middle.exp        |  2 ++
 gdb/testsuite/gdb.arch/i386-float.exp              |  2 ++
 gdb/testsuite/gdb.arch/i386-signal.exp             |  2 ++
 gdb/testsuite/gdb.dwarf2/clztest.exp               |  2 ++
 gdb/testsuite/gdb.dwarf2/dw2-common-block.exp      |  2 ++
 gdb/testsuite/gdb.dwarf2/dw2-dup-frame.exp         |  3 +++
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp     |  3 +++
 .../gdb.dwarf2/dw2-single-line-discriminators.exp  |  2 ++
 .../gdb.dwarf2/dw2-undefined-ret-addr.exp          |  3 +++
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp          |  3 +++
 gdb/testsuite/lib/gdb.exp                          | 30 ++++++++++++++++++++++
 13 files changed, 58 insertions(+)

diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
index f30f29ea578..15fbcbf4051 100644
--- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
@@ -23,6 +23,8 @@ if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     return
 }
 
+require pie_forced 0
+
 set newline "\[\r\n\]*"
 
 set opts {debug nopie}
diff --git a/gdb/testsuite/gdb.arch/amd64-entry-value.exp b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
index fdfa4a01b58..b9468f04897 100644
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+require pie_forced 0
+
 standard_testfile .s
 set opts {nopie}
 
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..57bb1de9850 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -27,6 +27,8 @@
 # run twice, and we restart gdb before testing each different command to
 # ensure that nothing is being cached.
 
+require pie_forced 0
+
 standard_testfile .S
 
 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
diff --git a/gdb/testsuite/gdb.arch/i386-float.exp b/gdb/testsuite/gdb.arch/i386-float.exp
index b96ff2ba13e..91b9142dbba 100644
--- a/gdb/testsuite/gdb.arch/i386-float.exp
+++ b/gdb/testsuite/gdb.arch/i386-float.exp
@@ -18,6 +18,8 @@
 
 # Test the x87 floating point information printout.
 
+require pie_forced 0
+
 if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
     verbose "Skipping i386 tests for x87 floating point support."
     return
diff --git a/gdb/testsuite/gdb.arch/i386-signal.exp b/gdb/testsuite/gdb.arch/i386-signal.exp
index aff796325c9..4354a956d28 100644
--- a/gdb/testsuite/gdb.arch/i386-signal.exp
+++ b/gdb/testsuite/gdb.arch/i386-signal.exp
@@ -15,6 +15,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+require pie_forced 0
+
 if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
     verbose "Skipping i386 unwinder tests."
     return
diff --git a/gdb/testsuite/gdb.dwarf2/clztest.exp b/gdb/testsuite/gdb.dwarf2/clztest.exp
index 0f6d18f2c41..a038d3546f6 100644
--- a/gdb/testsuite/gdb.dwarf2/clztest.exp
+++ b/gdb/testsuite/gdb.dwarf2/clztest.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+require pie_forced 0
+
 load_lib dwarf.exp
 
 standard_testfile .S
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-common-block.exp b/gdb/testsuite/gdb.dwarf2/dw2-common-block.exp
index 5c78f07943d..afd9c87e286 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-common-block.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-common-block.exp
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+require pie_forced 0
+
 load_lib dwarf.exp
 
 # This test can only be run on targets which support DWARF-2 and use gas.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dup-frame.exp b/gdb/testsuite/gdb.dwarf2/dw2-dup-frame.exp
index 4864658a2a1..64a8e462c73 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dup-frame.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dup-frame.exp
@@ -12,6 +12,9 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+require pie_forced 0
+
 load_lib dwarf.exp
 
 # This test can only be run on targets which support DWARF-2 and use gas.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..195fb683553 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -12,6 +12,9 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+require pie_forced 0
+
 load_lib dwarf.exp
 
 # This test can only be run on targets which support DWARF-2 and use gas.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-single-line-discriminators.exp b/gdb/testsuite/gdb.dwarf2/dw2-single-line-discriminators.exp
index 752c4842467..c1af8090e32 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-single-line-discriminators.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-single-line-discriminators.exp
@@ -16,6 +16,8 @@
 # Test gdb's coalescing of multiple line number entries for the same line
 # but with different discriminators.  PR 17276.
 
+require pie_forced 0
+
 load_lib dwarf.exp
 
 # This test can only be run on targets which support DWARF-2 and use gas.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-undefined-ret-addr.exp b/gdb/testsuite/gdb.dwarf2/dw2-undefined-ret-addr.exp
index f130ce784ee..d60d6da12ab 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-undefined-ret-addr.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-undefined-ret-addr.exp
@@ -12,6 +12,9 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+require pie_forced 0
+
 load_lib dwarf.exp
 
 standard_testfile .S
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..3732f084b7a 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -12,6 +12,9 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+require pie_forced 0
+
 load_lib dwarf.exp
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 89733c59605..ad0902f5a22 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8073,6 +8073,34 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Return 1 if nopie fails to prevent a PIE, 0 if nopie prevented a PIE,
+# and -1 if an error occurred.
+gdb_caching_proc pie_forced {
+    set me "pie_forced"
+    set src { int main() { return 0; } }
+    gdb_simple_compile $me $src executable nopie
+    set res [exec_is_pie $obj]
+    if { $res == -1 } {
+	return -1
+    }
+    set res [expr $res == 1]
+    return $res
+}
+
+# Return 1 if pie fails to generated a PIE, 0 if pie generated a PIE,
+# and -1 if an error occurred.
+gdb_caching_proc nopie_forced {
+    set me "nopie_forced"
+    set src { int main() { return 0; } }
+    gdb_simple_compile $me $src executable pie]
+    set res [exec_is_pie $obj]
+    if { $res == -1 } {
+	return -1
+    }
+    set res [expr $res == 0]
+    return $res
+}
+
 # Return 1 if compiler supports scalar_storage_order attribute, otherwise
 # return 0.
 gdb_caching_proc supports_scalar_storage_order_attribute {
@@ -8224,6 +8252,8 @@ proc require { fn val } {
 
     switch $fn-$val {
 	gdb_skip_xml_test-0 { set msg "missing xml support" }
+	pie_forced-0 { set msg "nopie failed to prevent PIE executable" }
+	nopie_forced-0 { set msg "pie failed to generate PIE executable" }
 	default { set msg "$fn != $val" }
     }
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie
  2021-10-07 11:00         ` [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie Tom de Vries
@ 2021-10-08 14:37           ` Tom de Vries
  0 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2021-10-08 14:37 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

On 10/7/21 1:00 PM, Tom de Vries via Gdb-patches wrote:
> [ was: Re: [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp ]
> 
> This follow-up patch uses the new proc require.
> 

And this version of the patch doesn't use proc require, but instead
handles things in gdb_compile.

Any comments?

Thanks,
- Tom



[-- Attachment #2: 0001-gdb-testsuite-Prevent-compilation-fails-with-unix-fPIE-pie.patch --]
[-- Type: text/x-patch, Size: 4044 bytes --]

[gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie

A regular test-case will produce an executable, and depending on the compiler
default, it will be a PIE or not.  A test-case can force one or the other
using the pie and nopie options.

However, when running with target board unix/-fPIE/-pie, the nopie option will
have no effect, and likewise for target board unix/-fno-PIE/-no-pie and the
pie option.

When say we run test-case gdb.base/attach-pie-noexec.exp, which passes the pie
option with target board unix/-fno-PIE/-no-pie we get:
...
 Running src/gdb/testsuite/gdb.base/attach-pie-noexec.exp ...
 gdb compile failed, pie failed to generate PIE executable

                 === gdb Summary ===

 # of untested testcases         1
...

However, this works only when we actually manage to generate an executable.

There are other test-cases, like f.i. gdb.arch/amd64-disp-step.exp that
specify nopie, but will generate a compilation failure with target board
unix/-fPIE/-pie due to using a hard-coded .S file:
...
 Running src/gdb/testsuite/gdb.arch/amd64-disp-step.exp ...
 gdb compile failed, ld: outputs/gdb.arch/amd64-disp-step/amd64-disp-step0.o: \
   relocation R_X86_64_32S against `.text' can not be used when making a PIE \
   object; recompile with -fPIE
 collect2: error: ld returned 1 exit status

                 === gdb Summary ===

 # of untested testcases         1
...

Hide this compilation error by:
- adding a gdb_caching_proc pie_forced, and
- using it in gdb_compile to bail out before even trying compilation
such that we simply have:
...
UNTESTED: gdb.arch/amd64-disp-step.exp: failed to prepare
...

Likewise, add nopie_forced.

Tested on x86_64-linux.

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

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 5642db4334d..78019fcd54d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4487,6 +4487,17 @@ proc gdb_compile {source dest type options} {
 	lappend options "$flag"
     }
 
+    if { $pie != -1 && [nopie_forced] } {
+	set result "pie unsupported"
+	verbose -log "gdb_compile: $result"
+	return $result
+    }
+    if { $nopie != -1 && [pie_forced] } {
+	set result "nopie unsupported"
+	verbose -log "gdb_compile: $result"
+	return $result
+    }
+
     if { $type == "executable" } {
 	if { ([istarget "*-*-mingw*"]
 	      || [istarget "*-*-*djgpp"]
@@ -8073,6 +8084,48 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Helper function for pie_forced.
+proc pie_forced_0 { } { return 0 }
+
+# Return 1 if nopie fails to prevent a PIE, 0 if nopie prevented a PIE,
+# and -1 if an error occurred.
+gdb_caching_proc pie_forced {
+    set me "pie_forced"
+    set src { int main() { return 0; } }
+    # gdb_compile calls pie_forced when nopie is passed, so pretend it
+    # returns 0, to allow us to find out the actual pie_forced value.
+    with_override pie_forced pie_forced_0 {
+	gdb_simple_compile $me $src executable nopie
+    }
+    set res [exec_is_pie $obj]
+    if { $res == -1 } {
+	return -1
+    }
+    set res [expr $res == 1]
+    return $res
+}
+
+# Helper function for nopie_forced.
+proc nopie_forced_0 {} { return 0 }
+
+# Return 1 if pie fails to generated a PIE, 0 if pie generated a PIE,
+# and -1 if an error occurred.
+gdb_caching_proc nopie_forced {
+    set me "nopie_forced"
+    set src { int main() { return 0; } }
+    # gdb_compile calls nopie_forced when pie is passed, so pretend it
+    # returns 0, to allow us to find out the actual nopie_forced value.
+    with_override nopie_forced nopie_forced_0 {
+	gdb_simple_compile $me $src executable pie
+    }
+    set res [exec_is_pie $obj]
+    if { $res == -1 } {
+	return -1
+    }
+    set res [expr $res == 0]
+    return $res
+}
+
 # Return 1 if compiler supports scalar_storage_order attribute, otherwise
 # return 0.
 gdb_caching_proc supports_scalar_storage_order_attribute {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp
  2021-10-06  7:40       ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
  2021-10-07 11:00         ` [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie Tom de Vries
@ 2021-10-11 10:28         ` Tom de Vries
  1 sibling, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2021-10-11 10:28 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On 10/6/21 9:40 AM, Tom de Vries wrote:
> [ was: Re: [committed][gdb/testsuite] Disable vgdb tests if xml not
> supported ]
> 
> On 10/5/21 8:15 PM, Tom Tromey wrote:
>> Tom> Or perhaps you mean more generically (which wouldn't require us to write
>> Tom> a proc for each type of require):
>> Tom> ...
>> Tom> proc require { fn val } {
>> Tom>     if { [$fn] != $val } {
>> Tom>         untested "$fn != $val"
>> Tom>         return -code return 0
>> Tom>     }
>> Tom> }
>> Tom> ...
>> Tom> and:
>> Tom> ...
>> Tom> require gdb_skip_xml_test 0
>> Tom> ...
>> Tom> ?
>>
>> Either seems fine but this one does read nicely to me.
>> Or maybe even just requiring 'fn' to return 0 would be good enough.
> 
> I've also thought about that, but decided against it:
> - it does not take into account a proc support_foo that returns 1, which
>   in the current form can be required using "require support_foo 1"
> - it does not support the cases where we require no support for a
>   feature, say "require gdb_skip_xml_test 1"
> 
> Also, I considered making the test freeform, as in gdb_assert, but
> decided against it because:
> - it does not enforce uniform usage
> - makes it harder to do the fn+value to message matching I've added in
>   this version.
> 

I ended up using require to transform:
...
if { [ensure_gdb_index $binfile] == -1 } {
    return -1
}
...
into:
...
require {ensure_gdb_index $binfile} != -1
...
and consequently had to change proc require a bit.

Committed as attached.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Add-proc-require-in-lib-gdb.exp.patch --]
[-- Type: text/x-patch, Size: 4245 bytes --]

[gdb/testsuite] Add proc require in lib/gdb.exp

Add a new proc require in lib/gdb.exp, and use it to shorten:
...
if { [gdb_skip_xml_test] } {
    # Valgrind gdbserver requires gdb with xml support.
    untested "missing xml support"
    return 0
}
...
into:
...
require gdb_skip_xml_test 0
...

Tested on x86_64-linux, both with and without a trigger patch that forces
gdb_skip_xml_test to return 1.

---
 gdb/testsuite/gdb.base/valgrind-bt.exp        |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-disp-step.exp |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-infcall-2.exp |  7 ++-----
 gdb/testsuite/gdb.base/valgrind-infcall.exp   |  7 ++-----
 gdb/testsuite/lib/gdb.exp                     | 30 +++++++++++++++++++++++++++
 5 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/gdb/testsuite/gdb.base/valgrind-bt.exp b/gdb/testsuite/gdb.base/valgrind-bt.exp
index 440c6e403e9..546701498b2 100644
--- a/gdb/testsuite/gdb.base/valgrind-bt.exp
+++ b/gdb/testsuite/gdb.base/valgrind-bt.exp
@@ -13,11 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
index 88ac848ae2d..c1ed5a3e69b 100644
--- a/gdb/testsuite/gdb.base/valgrind-disp-step.exp
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
@@ -18,11 +18,8 @@
 # really tests is that GDB falls back to in-line stepping
 # automatically instead of getting stuck or crashing.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
index a368717c1e8..064cf719712 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
@@ -29,11 +29,8 @@
 # terminate called after throwing an instance of 'gdb_exception_error'
 # Aborted (core dumped)
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall.exp b/gdb/testsuite/gdb.base/valgrind-infcall.exp
index 9f49a320f52..68bc1b8e4e7 100644
--- a/gdb/testsuite/gdb.base/valgrind-infcall.exp
+++ b/gdb/testsuite/gdb.base/valgrind-infcall.exp
@@ -13,11 +13,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-if { [gdb_skip_xml_test] } {
-    # Valgrind gdbserver requires gdb with xml support.
-    untested "missing xml support"
-    return 0
-}
+# Valgrind gdbserver requires gdb with xml support.
+require gdb_skip_xml_test 0
 
 load_lib valgrind.exp
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 5642db4334d..6a5cdc06485 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8215,5 +8215,35 @@ gdb_caching_proc have_avx {
     return $status
 }
 
+# Called as either:
+# - require EXPR VAL
+# - require EXPR OP VAL
+# In the first case, OP is ==.
+#
+# Require EXPR OP VAL, where EXPR is evaluated in caller context.  If not,
+# return in the caller's context.
+
+proc require { fn arg1 {arg2 ""} } {
+    if { $arg2 == "" } {
+	set op ==
+	set val $arg1
+    } else {
+	set op $arg1
+	set val $arg2
+    }
+    set res [uplevel 1 $fn]
+    if { [expr $res $op $val] } {
+	return
+    }
+
+    switch "$fn $op $val" {
+	"gdb_skip_xml_test == 0" { set msg "missing xml support" }
+	default { set msg "$fn != $val" }
+    }
+
+    untested $msg
+    return -code return 0
+}
+
 # Always load compatibility stuff.
 load_lib future.exp

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-10-11 10:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 13:55 [committed][gdb/testsuite] Disable vgdb tests if xml not supported Tom de Vries
2021-09-30 17:30 ` Tom Tromey
2021-09-30 21:41   ` Tom de Vries
2021-09-30 21:47     ` Tom de Vries
2021-10-05 18:15     ` Tom Tromey
2021-10-06  7:40       ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries
2021-10-07 11:00         ` [PATCH][gdb/testsuite] Prevent compilation fails with unix/-fPIE/-pie Tom de Vries
2021-10-08 14:37           ` Tom de Vries
2021-10-11 10:28         ` [PATCH][gdb/testsuite] Add proc require in lib/gdb.exp Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).