public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
@ 2014-09-11 23:03 Edjunior Barbosa Machado
  2014-09-11 23:21 ` Sergio Durigan Junior
  2014-09-17 10:07 ` [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Pedro Alves
  0 siblings, 2 replies; 20+ messages in thread
From: Edjunior Barbosa Machado @ 2014-09-11 23:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand

Hi,

this patch intends to fix PR tdep/17379:
  https://sourceware.org/bugzilla/show_bug.cgi?id=17379

The problem is that rs6000_frame_cache attempts to read the stack backchain via
read_memory_unsigned_integer, which throws an exception if the stack pointer is
invalid.  With this path, it calls safe_read_memory_integer instead, which
doesn't throw an exception and allows for safe handling of that situation.
Regression tested on ppc64{,le}.  Ok?

Thanks and regards,
--
Edjunior

gdb/
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

---
 gdb/rs6000-tdep.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
-- 
1.7.9.5

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-11 23:03 [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Edjunior Barbosa Machado
@ 2014-09-11 23:21 ` Sergio Durigan Junior
  2014-09-12  2:47   ` Edjunior Barbosa Machado
  2014-09-17 10:07 ` [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Pedro Alves
  1 sibling, 1 reply; 20+ messages in thread
From: Sergio Durigan Junior @ 2014-09-11 23:21 UTC (permalink / raw)
  To: Edjunior Barbosa Machado; +Cc: gdb-patches, Ulrich Weigand

On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:

> The problem is that rs6000_frame_cache attempts to read the stack backchain via
> read_memory_unsigned_integer, which throws an exception if the stack pointer is
> invalid.  With this path, it calls safe_read_memory_integer instead, which
> doesn't throw an exception and allows for safe handling of that situation.
> Regression tested on ppc64{,le}.  Ok?

Heya!

Thanks for the patch.  Not having reviewed the code deeply to understand
if it's the best approach, I would just like to point that a testcase
for this would be awesome.  As it turns out, you actually already have a
testcase almost written in the bug description :-).

Again, thanks for addressing those issues!

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-11 23:21 ` Sergio Durigan Junior
@ 2014-09-12  2:47   ` Edjunior Barbosa Machado
  2014-09-12  3:20     ` Sergio Durigan Junior
  2014-09-12  9:59     ` Pedro Alves
  0 siblings, 2 replies; 20+ messages in thread
From: Edjunior Barbosa Machado @ 2014-09-12  2:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Sergio Durigan Junior

Thanks Sergio for your feedback. I'm resending the patch with an additional
testcase as you suggested.

--
Edjunior

gdb/ChangeLog
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

gdb/testcase/ChangeLog
2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	PR tdep/17379
	* gdb.arch/powerpc-stackless.S: New file.
	* gdb.arch/powerpc-stackless.exp: New file.

---
 gdb/rs6000-tdep.c                            |   11 ++++--
 gdb/testsuite/gdb.arch/powerpc-stackless.S   |   24 +++++++++++++
 gdb/testsuite/gdb.arch/powerpc-stackless.exp |   48 ++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.S
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.exp

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.S b/gdb/testsuite/gdb.arch/powerpc-stackless.S
new file mode 100644
index 0000000..bbf92bb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.S
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <ppc-asm.h>
+
+FUNC_START(main)
+        li      sp,0
+        mtlr    sp
+        blr
+FUNC_END(main)
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
new file mode 100644
index 0000000..f4b2a90
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
@@ -0,0 +1,48 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Testcase for PR tdep/17379.
+
+if {![istarget "powerpc*-*-*"]} then {
+    verbose "Skipping powerpc-stackless.exp"
+    return
+}
+
+standard_testfile powerpc-stackless.S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested powerpc-stackless.exp
+    return -1
+}
+
+# Run until SIGSEGV.
+gdb_run_cmd
+
+gdb_expect {
+    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
+	pass "run until SIGSEGV"
+    }
+    -re ".*$gdb_prompt $" {
+	fail "run until SIGSEGV"
+    }
+    timeout {
+	fail "run until SIGSEGV (timeout)"
+    }
+}
+
+# Ensure that 'info registers' works properly and does not generate
+# an internal-error.
+gdb_test "info registers" "r0.*" "info registers"
-- 
1.7.9.5

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12  2:47   ` Edjunior Barbosa Machado
@ 2014-09-12  3:20     ` Sergio Durigan Junior
  2014-09-12  8:39       ` Ulrich Weigand
  2014-09-12  9:59     ` Pedro Alves
  1 sibling, 1 reply; 20+ messages in thread
From: Sergio Durigan Junior @ 2014-09-12  3:20 UTC (permalink / raw)
  To: Edjunior Barbosa Machado; +Cc: gdb-patches, Ulrich Weigand

On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:

> Thanks Sergio for your feedback. I'm resending the patch with an additional
> testcase as you suggested.

Nice, thanks :-).

> diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
> new file mode 100644
> index 0000000..f4b2a90
[...]
> +standard_testfile powerpc-stackless.S

Just another really minor nit, no need to resubmit the patch because of
this.  You can write:

  standard_testfile .S

Other than that, it is perfect.

Thanks a lot!

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12  3:20     ` Sergio Durigan Junior
@ 2014-09-12  8:39       ` Ulrich Weigand
  0 siblings, 0 replies; 20+ messages in thread
From: Ulrich Weigand @ 2014-09-12  8:39 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: Edjunior Barbosa Machado, gdb-patches

Sergio Durigan Junior wrote:
> On Thursday, September 11 2014, Edjunior Barbosa Machado wrote:
> 
> > Thanks Sergio for your feedback. I'm resending the patch with an additional
> > testcase as you suggested.
> 
> Nice, thanks :-).
> 
> > diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
> > new file mode 100644
> > index 0000000..f4b2a90
> [...]
> > +standard_testfile powerpc-stackless.S
> 
> Just another really minor nit, no need to resubmit the patch because of
> this.  You can write:
> 
>   standard_testfile .S
> 
> Other than that, it is perfect.
> 
> Thanks a lot!

Thanks for the testcase (and the review)!

Edjunior, the patch is OK to check in with the change Sergio suggested.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12  2:47   ` Edjunior Barbosa Machado
  2014-09-12  3:20     ` Sergio Durigan Junior
@ 2014-09-12  9:59     ` Pedro Alves
  2014-09-12 12:31       ` Edjunior Barbosa Machado
  2014-09-12 13:00       ` Joel Brobecker
  1 sibling, 2 replies; 20+ messages in thread
From: Pedro Alves @ 2014-09-12  9:59 UTC (permalink / raw)
  To: Edjunior Barbosa Machado, gdb-patches
  Cc: Ulrich Weigand, Sergio Durigan Junior

On 09/12/2014 03:46 AM, Edjunior Barbosa Machado wrote:
> +# Run until SIGSEGV.
> +gdb_run_cmd
> +
> +gdb_expect {
> +    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
> +	pass "run until SIGSEGV"
> +    }
> +    -re ".*$gdb_prompt $" {
> +	fail "run until SIGSEGV"
> +    }
> +    timeout {
> +	fail "run until SIGSEGV (timeout)"
> +    }
> +}

gdb_expect should only be used when gdb_test or gdb_test_multiple
really can't be used.  Please write instead:

gdb_run_cmd

set test "run until SIGSEGV"
gdb_test_multiple "" $test {
    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
	pass $test
    }
}

gdb_test_multiple will already issue a FAIL if it sees the prompt
or gets a timeout, and in addition will catch other problems,
like internal errors.

Thanks,
Pedro Alves

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12  9:59     ` Pedro Alves
@ 2014-09-12 12:31       ` Edjunior Barbosa Machado
  2014-09-12 13:00       ` Joel Brobecker
  1 sibling, 0 replies; 20+ messages in thread
From: Edjunior Barbosa Machado @ 2014-09-12 12:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Sergio Durigan Junior, Pedro Alves

Thank you all for the review. I've just pushed the following patch with the
suggested additional fixes.

  https://sourceware.org/ml/gdb-cvs/2014-09/msg00055.html

Thanks and regards,
--
Edjunior

gdb/ChangeLog
2014-09-12  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
	    Ulrich Weigand  <uweigand@de.ibm.com>

	PR tdep/17379
	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
	instead of read_memory_unsigned_integer.

gdb/testcase/ChangeLog
2014-09-12  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	PR tdep/17379
	* gdb.arch/powerpc-stackless.S: New file.
	* gdb.arch/powerpc-stackless.exp: New file.

---
 gdb/rs6000-tdep.c                            |   11 +++++--
 gdb/testsuite/gdb.arch/powerpc-stackless.S   |   24 +++++++++++++++
 gdb/testsuite/gdb.arch/powerpc-stackless.exp |   42 ++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.S
 create mode 100644 gdb/testsuite/gdb.arch/powerpc-stackless.exp

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 730afe7..dabf448 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
     }
 
   if (!fdata.frameless)
-    /* Frameless really means stackless.  */
-    cache->base
-      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
+    {
+      /* Frameless really means stackless.  */
+      LONGEST backchain;
+
+      if (safe_read_memory_integer (cache->base, wordsize,
+				    byte_order, &backchain))
+        cache->base = (CORE_ADDR) backchain;
+    }
 
   trad_frame_set_value (cache->saved_regs,
 			gdbarch_sp_regnum (gdbarch), cache->base);
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.S b/gdb/testsuite/gdb.arch/powerpc-stackless.S
new file mode 100644
index 0000000..bbf92bb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.S
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <ppc-asm.h>
+
+FUNC_START(main)
+        li      sp,0
+        mtlr    sp
+        blr
+FUNC_END(main)
diff --git a/gdb/testsuite/gdb.arch/powerpc-stackless.exp b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
new file mode 100644
index 0000000..420bcbc
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-stackless.exp
@@ -0,0 +1,42 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Testcase for PR tdep/17379.
+
+if {![istarget "powerpc*-*-*"]} then {
+    verbose "Skipping powerpc-stackless.exp"
+    return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested powerpc-stackless.exp
+    return -1
+}
+
+# Run until SIGSEGV.
+gdb_run_cmd
+
+set test "run until SIGSEGV"
+gdb_test_multiple "" $test {
+    -re "Program received signal SIGSEGV.*$gdb_prompt $" {
+  pass $test
+    }
+}
+
+# Ensure that 'info registers' works properly and does not generate
+# an internal-error.
+gdb_test "info registers" "r0.*" "info registers"
-- 
1.7.9.5

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12  9:59     ` Pedro Alves
  2014-09-12 12:31       ` Edjunior Barbosa Machado
@ 2014-09-12 13:00       ` Joel Brobecker
  2014-09-12 13:38         ` Pedro Alves
  1 sibling, 1 reply; 20+ messages in thread
From: Joel Brobecker @ 2014-09-12 13:00 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

> set test "run until SIGSEGV"
> gdb_test_multiple "" $test {
>     -re "Program received signal SIGSEGV.*$gdb_prompt $" {
> 	pass $test
>     }
> }

Taking this one step further, wouldn't a simpler gdb_test also work
in this case?

-- 
Joel

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12 13:00       ` Joel Brobecker
@ 2014-09-12 13:38         ` Pedro Alves
  2014-09-12 13:50           ` Joel Brobecker
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2014-09-12 13:38 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

On 09/12/2014 02:00 PM, Joel Brobecker wrote:
>> set test "run until SIGSEGV"
>> gdb_test_multiple "" $test {
>>     -re "Program received signal SIGSEGV.*$gdb_prompt $" {
>> 	pass $test
>>     }
>> }
> 
> Taking this one step further, wouldn't a simpler gdb_test also work
> in this case?

Yeah, good point.

We still need to use gdb_run_cmd to cover remote testing,
so that'd be:

 gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"

ISTR that gdb_test doesn't allow empty command, but I may well
be mistaken.  And if it doesn't, maybe it should.

Thanks,
Pedro Alves

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12 13:38         ` Pedro Alves
@ 2014-09-12 13:50           ` Joel Brobecker
  2014-09-12 14:21             ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Joel Brobecker @ 2014-09-12 13:50 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

> We still need to use gdb_run_cmd to cover remote testing,
> so that'd be:
> 
>  gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"
> 
> ISTR that gdb_test doesn't allow empty command, but I may well
> be mistaken.  And if it doesn't, maybe it should.

This is me pretending that I had noticed that the command was empty
and knowing that this was still OK :-). But once you mentioned it,
I knew I had already done something like that. See gdb.ada/bp_reset.exp:

    gdb_run_cmd
    gdb_test "" "Breakpoint $decimal, foo\\.nested_sub \\(\\).*"

Doing a quick grep, we have a number of occurences where we use
an empty command when calling gdb_test.  And looking at gdb_test's
implementation, it just passes the first argument to gdb_test_multiple,
so it should indeed be equivalent.  (phew, that was close! ;-)).

-- 
Joel

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-12 13:50           ` Joel Brobecker
@ 2014-09-12 14:21             ` Pedro Alves
  2014-09-12 15:27               ` [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2014-09-12 14:21 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

On 09/12/2014 02:50 PM, Joel Brobecker wrote:
>> We still need to use gdb_run_cmd to cover remote testing,
>> so that'd be:
>>
>>  gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"
>>
>> ISTR that gdb_test doesn't allow empty command, but I may well
>> be mistaken.  And if it doesn't, maybe it should.
> 
> This is me pretending that I had noticed that the command was empty
> and knowing that this was still OK :-). But once you mentioned it,
> I knew I had already done something like that. See gdb.ada/bp_reset.exp:
> 
>     gdb_run_cmd
>     gdb_test "" "Breakpoint $decimal, foo\\.nested_sub \\(\\).*"
> 
> Doing a quick grep, we have a number of occurences where we use
> an empty command when calling gdb_test.  And looking at gdb_test's
> implementation, it just passes the first argument to gdb_test_multiple,
> so it should indeed be equivalent.  (phew, that was close! ;-)).

:-)

I'm writing a test that converts all gdb_run_cmd -> gdb_expect
cases to avoid this from spreading further.

Thanks,
Pedro Alves

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

* [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test
  2014-09-12 14:21             ` Pedro Alves
@ 2014-09-12 15:27               ` Pedro Alves
  2014-09-12 17:10                 ` Joel Brobecker
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2014-09-12 15:27 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

On 09/12/2014 03:21 PM, Pedro Alves wrote:
> On 09/12/2014 02:50 PM, Joel Brobecker wrote:
>>> We still need to use gdb_run_cmd to cover remote testing,
>>> so that'd be:
>>>
>>>  gdb_test "" "Program received signal SIGSEGV.*" "run until SIGSEGV"
>>>
>>> ISTR that gdb_test doesn't allow empty command, but I may well
>>> be mistaken.  And if it doesn't, maybe it should.
>>
>> This is me pretending that I had noticed that the command was empty
>> and knowing that this was still OK :-). But once you mentioned it,
>> I knew I had already done something like that. See gdb.ada/bp_reset.exp:
>>
>>     gdb_run_cmd
>>     gdb_test "" "Breakpoint $decimal, foo\\.nested_sub \\(\\).*"
>>
>> Doing a quick grep, we have a number of occurences where we use
>> an empty command when calling gdb_test.  And looking at gdb_test's
>> implementation, it just passes the first argument to gdb_test_multiple,
>> so it should indeed be equivalent.  (phew, that was close! ;-)).
> 
> :-)
> 
> I'm writing a test that converts all gdb_run_cmd -> gdb_expect
> cases to avoid this from spreading further.

Patch pasted below.

In a2-run.exp I converted one vxworks path, but left the others
in place...  I'm tempted to nuke most of that vxworks stuff
out of the testsuite.  Does anyone still care about it?
Most of the vxworks tweaks seem like either stuff that better belong
in a board file and target testsuite support code, like timeout handling,
missing functions, etc. and out of place for the tests themselves.
And then again, we have a ton of tests likely would need the same
tweak, but don't...

--------------
Subject: [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test

See:
  https://sourceware.org/ml/gdb-patches/2014-09/msg00404.html

We have a number of places that do gdb_run_cmd followed by gdb_expect,
when it would be better to use gdb_test_multiple or gdb_test.

This converts all that "grep gdb_run_cmd -A 2 | grep gdb_expect"
found.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/testsuite/
2014-09-12  Pedro Alves  <palves@redhat.com>

	* gdb.arch/gdb1558.exp: Replace uses of gdb_expect after
	gdb_run_cmd with gdb_test_multiple or gdb_test throughout.
	* gdb.arch/i386-size-overlap.exp: Likewise.
	* gdb.arch/i386-size.exp: Likewise.
	* gdb.arch/i386-unwind.exp: Likewise.
	* gdb.base/a2-run.exp: Likewise.
	* gdb.base/break.exp: Likewise.
	* gdb.base/charset.exp: Likewise.
	* gdb.base/chng-syms.exp: Likewise.
	* gdb.base/commands.exp: Likewise.
	* gdb.base/dbx.exp: Likewise.
	* gdb.base/find.exp: Likewise.
	* gdb.base/funcargs.exp: Likewise.
	* gdb.base/jit-simple.exp: Likewise.
	* gdb.base/reread.exp: Likewise.
	* gdb.base/sepdebug.exp: Likewise.
	* gdb.base/step-bt.exp: Likewise.
	* gdb.cp/mb-inline.exp: Likewise.
	* gdb.cp/mb-templates.exp: Likewise.
	* gdb.objc/basicclass.exp: Likewise.
	* gdb.threads/killed.exp: Likewise.
---
 gdb/testsuite/gdb.arch/gdb1558.exp           | 13 ++--
 gdb/testsuite/gdb.arch/i386-size-overlap.exp | 13 +---
 gdb/testsuite/gdb.arch/i386-size.exp         | 13 +---
 gdb/testsuite/gdb.arch/i386-unwind.exp       | 13 +---
 gdb/testsuite/gdb.base/a2-run.exp            | 78 +++++-------------------
 gdb/testsuite/gdb.base/break.exp             | 48 +++++----------
 gdb/testsuite/gdb.base/charset.exp           | 13 +---
 gdb/testsuite/gdb.base/chng-syms.exp         | 33 +++--------
 gdb/testsuite/gdb.base/commands.exp          | 19 +-----
 gdb/testsuite/gdb.base/dbx.exp               | 13 ++--
 gdb/testsuite/gdb.base/find.exp              | 12 +---
 gdb/testsuite/gdb.base/funcargs.exp          | 89 +++++++---------------------
 gdb/testsuite/gdb.base/jit-simple.exp        |  9 +--
 gdb/testsuite/gdb.base/reread.exp            | 60 ++-----------------
 gdb/testsuite/gdb.base/sepdebug.exp          | 36 +++--------
 gdb/testsuite/gdb.base/step-bt.exp           | 14 +----
 gdb/testsuite/gdb.cp/mb-inline.exp           | 24 +-------
 gdb/testsuite/gdb.cp/mb-templates.exp        | 48 ++-------------
 gdb/testsuite/gdb.objc/basicclass.exp        |  7 +--
 gdb/testsuite/gdb.threads/killed.exp         |  9 +--
 20 files changed, 101 insertions(+), 463 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/gdb1558.exp b/gdb/testsuite/gdb.arch/gdb1558.exp
index 42bb277..3f07a64 100644
--- a/gdb/testsuite/gdb.arch/gdb1558.exp
+++ b/gdb/testsuite/gdb.arch/gdb1558.exp
@@ -50,17 +50,12 @@ gdb_test "b sub2" "Breakpoint 3.*" "set breakpoint at sub2"
 
 gdb_run_cmd
 
-gdb_expect 30 {
+set test "Hits breakpoint at main after function called from main"
+gdb_test_multiple "" $test {
     -re "Breakpoint 1.*main .*$gdb_prompt $" {
-	pass "Hits breakpoint at main after function called from main"
+	pass $test
     }
     -re "Breakpoint 2.*sub1 .*$gdb_prompt $" {
-	kfail "gdb/1558" "Hits breakpoint at main after function called from main"
-    }
-    -re "$gdb_prompt $" { 
-	fail "Hits breakpoint at main after function called from main"
-    }
-    timeout { 
-	fail "Hits breakpoint at main after function called from main (timeout)"
+	kfail "gdb/1558" $test
     }
 }
diff --git a/gdb/testsuite/gdb.arch/i386-size-overlap.exp b/gdb/testsuite/gdb.arch/i386-size-overlap.exp
index 0f9ba75..3b130c2 100644
--- a/gdb/testsuite/gdb.arch/i386-size-overlap.exp
+++ b/gdb/testsuite/gdb.arch/i386-size-overlap.exp
@@ -41,18 +41,7 @@ gdb_load ${binfile}
 # We use gdb_run_cmd so this stands a chance to work for remote
 # targets too.
 gdb_run_cmd
-
-gdb_expect {
-    -re "Program received signal SIGTRAP.*$gdb_prompt $" {
-	pass "run past main"
-    }
-    -re ".*$gdb_prompt $" {
-	fail "run past main"
-    }
-    timeout {
-	fail "run past main (timeout)"
-    }
-}
+gdb_test "" "Program received signal SIGTRAP.*" "run past main"
 
 set message "backtrace shows the outer function"
 gdb_test_multiple "backtrace 10" $message {
diff --git a/gdb/testsuite/gdb.arch/i386-size.exp b/gdb/testsuite/gdb.arch/i386-size.exp
index 6d63976..39e5b50 100644
--- a/gdb/testsuite/gdb.arch/i386-size.exp
+++ b/gdb/testsuite/gdb.arch/i386-size.exp
@@ -46,18 +46,7 @@ gdb_load ${binfile}
 # We use gdb_run_cmd so this stands a chance to work for remote
 # targets too.
 gdb_run_cmd
-
-gdb_expect {
-    -re "Program received signal SIGTRAP.*$gdb_prompt $" {
-	pass "run past main"
-    }
-    -re ".*$gdb_prompt $" {
-	fail "run past main"
-    }
-    timeout {
-	fail "run past main (timeout)"
-    }
-}
+gdb_test "" "Program received signal SIGTRAP.*" "run past main"
 
 set message "backtrace shows no function"
 gdb_test_multiple "backtrace 10" $message {
diff --git a/gdb/testsuite/gdb.arch/i386-unwind.exp b/gdb/testsuite/gdb.arch/i386-unwind.exp
index c329dd6..69f802a 100644
--- a/gdb/testsuite/gdb.arch/i386-unwind.exp
+++ b/gdb/testsuite/gdb.arch/i386-unwind.exp
@@ -46,18 +46,7 @@ gdb_load ${binfile}
 # We use gdb_run_cmd so this stands a chance to work for remote
 # targets too.
 gdb_run_cmd
-
-gdb_expect {
-    -re "Program received signal SIGTRAP.*$gdb_prompt $" {
-	pass "run past gdb1435"
-    }
-    -re ".*$gdb_prompt $" {
-	fail "run past gdb1435"
-    }
-    timeout {
-	fail "run past gdb1435 (timeout)"
-    }
-}
+gdb_test "" "Program received signal SIGTRAP.*" "run past gdb1435"
 
 gdb_test "backtrace 10" \
 	"#1\[ \t]*$hex in gdb1435.*\r\n#2\[ \t\]*$hex in main.*" \
diff --git a/gdb/testsuite/gdb.base/a2-run.exp b/gdb/testsuite/gdb.base/a2-run.exp
index 49fcb92..10aaf67 100644
--- a/gdb/testsuite/gdb.base/a2-run.exp
+++ b/gdb/testsuite/gdb.base/a2-run.exp
@@ -34,48 +34,28 @@ if { [prepare_for_testing ${testfile}.exp $testfile $srcfile] } {
 # On VxWorks this justs make sure the program was run.
 gdb_run_cmd
 
+set test "run \"$testfile\" with no args"
+
 if [istarget "*-*-vxworks*"] then {
-    set timeout 120
-    verbose "Timeout is now $timeout seconds" 2
-    gdb_expect {
-	 "$inferior_exited_re normally" {
-	    unresolved "run \"$testfile\" with no args"
-	}
-	 -re "usage:  factorial <number>" {
-	    pass "run \"$testfile\" with no args"
-	}
-	timeout	{
-	    fail "(timeout) run \"$testfile\" with no args"
-	}
-    }
-    set timeout 10
-    verbose "Timeout is now $timeout seconds" 2
-    gdb_expect -re "$gdb_prompt $" {}
+    gdb_test "" "usage:  factorial <number>.*" $test
 } else {
-    gdb_expect {
+    gdb_test_multiple "" $test {
 	-re ".*usage:  factorial <number>.*$inferior_exited_re with code 01.\r\n$gdb_prompt $" {
-	    pass "run \"$testfile\" with no args"
+	    pass $test
 	    pass "no spurious messages at program exit"
 	}
 	-re ".*usage:  factorial <number>.*$inferior_exited_re with code 01.*$gdb_prompt $" {
-	    pass "run \"$testfile\" with no args"
+	    pass $test
 	    fail "no spurious messages at program exit"
 	}
 	-re ".*usage:  factorial <number>.* EXIT code 1.*$inferior_exited_re normally.\r\n$gdb_prompt $" {
-	    pass "run \"$testfile\" with no args (exit wrapper)"
+	    pass "$test (exit wrapper)"
 	    pass "no spurious messages at program exit"
 	}
 	-re ".*usage:  factorial <number>.* EXIT code 1.*$inferior_exited_re normally.*$gdb_prompt $" {
-	    pass "run \"$testfile\" with no args (exit wrapper)"
+	    pass "$test (exit wrapper)"
 	    fail "no spurious messages at program exit"
 	}
-	-re ".*$gdb_prompt $" {
-	    fail "run \"$testfile\" with no args"
-	    verbose "expect_out is $expect_out(buffer)" 2
-	}
-	timeout	{
-	    fail "(timeout) run \"$testfile\" no args"
-	}
     }
 }
 
@@ -107,14 +87,9 @@ if [istarget "*-*-vxworks*"] then {
     verbose "Timeout is now $timeout seconds" 2
     gdb_expect -re "$gdb_prompt $" {}
 } else {
-	setup_xfail "arm-*-coff"
-	gdb_run_cmd 5
-	gdb_expect {
-	    -re ".*120.*$gdb_prompt $"\
-				{ pass "run \"$testfile\" with arg" }
-	    -re ".*$gdb_prompt $"	{ fail "run \"$testfile\" with arg" }
-	    timeout		{ fail "(timeout) run \"$testfile\" with arg" }
-	}
+    setup_xfail "arm-*-coff"
+    gdb_run_cmd 5
+    gdb_test "" "120.*" "run \"$testfile\" with arg"
 }
 
 # Run again with same arguments.
@@ -135,12 +110,7 @@ if [istarget "*-*-vxworks*"] then {
     gdb_expect -re "$gdb_prompt $" {}
 } else {
     setup_xfail "arm-*-coff"
-    gdb_expect {
-	    -re ".*120.*$gdb_prompt $"\
-				{ pass "run \"$testfile\" again with same args" }
-	    -re ".*$gdb_prompt $"	{ fail "run \"$testfile\" again with same args" }
-	    timeout		{ fail "(timeout) run \"$testfile\" again with same args" }
-	}
+    gdb_test "" "120.*" "run \"$testfile\" again with same args"
 }
 
 # Use "set args" command to specify no arguments as default and run again.
@@ -170,17 +140,7 @@ if [istarget "*-*-vxworks*"] then {
     verbose "Timeout is now $timeout seconds" 2
     gdb_expect -re "$gdb_prompt $" {}
 } else {
-    gdb_expect {
-	-re ".*usage:  factorial <number>.*$gdb_prompt $" {
-	    pass "run after setting args to nil"
-	}
-	-re ".*$gdb_prompt $" {
-	    fail "run after setting args to nil"
-	}
-	timeout {
-	    fail "(timeout) run after setting args to nil"
-	}
-    }
+    gdb_test "" "usage:  factorial <number>.*" "run after setting args to nil"
 }
 
 # Use "set args" command to specify an argument and run again.
@@ -211,17 +171,7 @@ if [istarget "*-*-vxworks*"] then {
     gdb_expect -re "$gdb_prompt $" {}
 } else {
     setup_xfail "arm-*-coff"
-    gdb_expect {
-	    -re ".*720.*$gdb_prompt $" {
-		pass "run \"$testfile\" again after setting args"
-	    }
-	    -re ".*$gdb_prompt $" {
-		fail "run \"$testfile\" again after setting args"
-	    }
-	    timeout {
-		fail "(timeout) run \"$testfile\" again after setting args"
-	    }
-	}
+    gdb_test "" "720.*" "run \"$testfile\" again after setting args"
 }
 
 # GOAL: Test that shell is being used with "run".  For remote debugging
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index beab99a..1870ad5 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -358,17 +358,9 @@ gdb_test "disable \$1foo" \
 # run until the breakpoint at main is hit. For non-stubs-using targets.
 #
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
-	pass "run until function breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "run until function breakpoint"
-    }
-    timeout {
-	fail "run until function breakpoint (timeout)"
-    }
-}
+gdb_test "" \
+    "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*" \
+    "run until function breakpoint"
 
 # Test the 'list' commands sets current file for the 'break LINENO' command.
 set bp_marker1 [gdb_get_line_number "set breakpoint 16 here" ${srcfile1}]
@@ -773,14 +765,7 @@ proc test_next_with_recursion {} {
     # Run until we call factorial with 6
 
     gdb_run_cmd
-    gdb_expect {
-	-re "Break.* factorial .value=6. .*$gdb_prompt $" {}
-	-re ".*$gdb_prompt $" {
-	    fail "run to factorial(6)"
-	    gdb_suppress_tests
-	}
-	timeout { fail "run to factorial(6) (timeout)" ; gdb_suppress_tests }
-    }
+    gdb_test "" "Break.* factorial .value=6. .*" "run to factorial(6)"
 
     # Continue until we call factorial recursively with 5.
 
@@ -871,18 +856,14 @@ gdb_test "break marker4" \
 # run until the breakpoint at main is hit. For non-stubs-using targets.
 #
 gdb_run_cmd
-gdb_expect {
+
+set test "run until function breakpoint, optimized file"
+gdb_test_multiple "" $test {
     -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
-	pass "run until function breakpoint, optimized file"
+	pass $test
     }
     -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$gdb_prompt $" {
-	pass "run until function breakpoint, optimized file (code motion)"
-    }
-    -re "$gdb_prompt $" {
-	fail "run until function breakpoint, optimized file"
-    }
-    timeout {
-	fail "run until function breakpoint, optimized file (timeout)"
+	pass "$test (code motion)"
     }
 }
 
@@ -945,15 +926,14 @@ gdb_test "rbreak main" \
 
 # Run to a breakpoint.  Fail if we see "Junk at end of arguments".
 gdb_run_cmd
-gdb_expect {
+
+set test "rbreak junk"
+gdb_test_multiple "" $test {
     -re "Junk at end of arguments" {
-	fail "rbreak junk"
+	fail $test
     }
     -re ".*Breakpoint \[0-9\]+,.*$gdb_prompt $" {
-	pass "rbreak junk"
-    }
-    timeout {
-	fail "rbreak junk (timeout)"
+	pass $test
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/charset.exp b/gdb/testsuite/gdb.base/charset.exp
index cc3a579..0378a65 100644
--- a/gdb/testsuite/gdb.base/charset.exp
+++ b/gdb/testsuite/gdb.base/charset.exp
@@ -324,18 +324,7 @@ gdb_test "break ${srcfile}:[gdb_get_line_number "all strings initialized"]" \
          ".*Breakpoint.* at .*" \
          "set breakpoint after all strings have been initialized"
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint.*all strings initialized.*$gdb_prompt $" {
-        pass "run until all strings have been initialized"
-    }
-    -re "$gdb_prompt $" {
-        fail "run until all strings have been initialized"
-    }
-    timeout {
-        fail "run until all strings have been initialized (timeout)"
-    }
-}
-
+gdb_test "" "Breakpoint.*all strings initialized.*" "run until all strings have been initialized"
 
 # We only try the wide character tests on machines where the wchar_t
 # typedef in the test case has the right size.
diff --git a/gdb/testsuite/gdb.base/chng-syms.exp b/gdb/testsuite/gdb.base/chng-syms.exp
index ac69c50..b5b7d78 100644
--- a/gdb/testsuite/gdb.base/chng-syms.exp
+++ b/gdb/testsuite/gdb.base/chng-syms.exp
@@ -30,25 +30,10 @@ set timeout 10
 verbose "Timeout is now 10 seconds" 2
 
 proc expect_to_stop_here { ident } {
-    global gdb_prompt
-    global decimal
-
     # the "at foo.c:36" output we get with -g.
     # the "in func" output we get without -g.
-    gdb_expect {
-	-re "Breakpoint \[0-9\]*, stop_here .*$gdb_prompt $" { 
-	    return 1
-	}
-	-re "$gdb_prompt $" { 
-	    fail "running to stop_here $ident"
-	    return 0
-	}
-	timeout { 
-	    fail "running to stop_here $ident (timeout)"
-	    return 0
-	}
-    }
-    return 1
+
+    gdb_test "" "Breakpoint \[0-9\]*, stop_here .*" "running to stop_here $ident"
 }
 
 clean_restart ${binfile}
@@ -83,18 +68,14 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
 } else {
 
     gdb_run_cmd
-    gdb_expect {
+
+    set test "running with invalidated bpt condition after executable changes"
+    gdb_test_multiple "" $test {
 	-re ".*$inferior_exited_re normally.*$gdb_prompt $" {
-	    pass "running with invalidated bpt condition after executable changes" 
+	    pass $test
 	}
 	-re ".*Breakpoint .*,( 0x.* in)? (\[^ \]*)exit .*$gdb_prompt $" {
-	    pass "running with invalidated bpt condition after executable changes" 
-	}
-	-re "$gdb_prompt $" { 
-	    fail "running with invalidated bpt condition after executable changes" 
-	}
-	timeout {
-	    fail "(timeout) running with invalidated bpt condition after executable changes" 
+	    pass $test
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 74eb306..ef86059 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -493,16 +493,7 @@ proc bp_deleted_in_command_test {} {
 	"end commands"
 
     gdb_run_cmd
-    gdb_expect {
-        -re ".*factorial command-list executed.*$gdb_prompt $" {
-	    pass "run factorial until breakpoint"
-        }
-	-re ".*$gdb_prompt $" {
-	    fail "run factorial until breakpoint"
-	}
-	default { fail "(timeout) run factorial until breakpoint" }
-	timeout { fail "(timeout) run factorial until breakpoint" }
-    }
+    gdb_test "" "factorial command-list executed.*" "run factorial until breakpoint"
 }
 
 proc temporary_breakpoint_commands {} {
@@ -551,12 +542,8 @@ proc temporary_breakpoint_commands {} {
 	"end tbreak commands"
 
     gdb_run_cmd
-    gdb_expect {
-	-re ".*factorial tbreak commands executed.*$gdb_prompt $" {
-	    pass "run factorial until temporary breakpoint"
-	}
-	timeout { fail "(timeout) run factorial until temporary breakpoint" }
-    }
+    gdb_test "" "factorial tbreak commands executed.*" \
+	"run factorial until temporary breakpoint"
 }
 
 # Test that GDB can handle $arg0 outside of user functions without
diff --git a/gdb/testsuite/gdb.base/dbx.exp b/gdb/testsuite/gdb.base/dbx.exp
index 06638c7..4383e79 100644
--- a/gdb/testsuite/gdb.base/dbx.exp
+++ b/gdb/testsuite/gdb.base/dbx.exp
@@ -258,11 +258,14 @@ proc test_assign { } {
     global gdb_prompt
 
     gdb_run_cmd
-    gdb_expect 30 {
-        -re "Break.* at .*:$decimal.*$gdb_prompt $" { pass "running to main" }
-        -re "Breakpoint \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" { pass "running to main"  }
-        -re "$gdb_prompt $" { fail "running to main" }
-        timeout { fail "running to main (timeout)" }
+    set test "running to main"
+    gdb_test_multiple "" $test {
+        -re "Break.* at .*:$decimal.*$gdb_prompt $" {
+	    pass $test
+	}
+        -re "Breakpoint \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" {
+	    pass $test
+	}
     }
     send_gdb "assign first=1\n"
     gdb_expect {
diff --git a/gdb/testsuite/gdb.base/find.exp b/gdb/testsuite/gdb.base/find.exp
index edddb3d..d46e421 100644
--- a/gdb/testsuite/gdb.base/find.exp
+++ b/gdb/testsuite/gdb.base/find.exp
@@ -29,17 +29,7 @@ gdb_test "break $srcfile:stop_here" \
     "breakpoint function in file"
 
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*stop_here.* at .*$srcfile:.*$gdb_prompt $" {
-	pass "run until function breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "run until function breakpoint"
-    }
-    timeout {
-	fail "run until function breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*stop_here.* at .*$srcfile:.*" "run until function breakpoint"
 
 # We've now got the target program in a state where we can test "find".
 
diff --git a/gdb/testsuite/gdb.base/funcargs.exp b/gdb/testsuite/gdb.base/funcargs.exp
index b6de3d2..5cbd789 100644
--- a/gdb/testsuite/gdb.base/funcargs.exp
+++ b/gdb/testsuite/gdb.base/funcargs.exp
@@ -54,13 +54,7 @@ proc integral_args {} {
     # Run; should stop at call0a and print actual arguments.
     if {!$gcc_compiled} then { setup_xfail "rs6000-*-*" }
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call0a \\(c=97 'a', s=1, i=2, l=3\\) .*$gdb_prompt $" {
-	    pass "run to call0a"
-	}
-	 -re "$gdb_prompt $"  { fail "run to call0a" ; gdb_suppress_tests }
-	 timeout { fail "(timeout) run to call0a" ; gdb_suppress_tests }
-    }
+    gdb_test "" " call0a \\(c=97 'a', s=1, i=2, l=3\\) .*" "run to call0a"
 
     # Print each arg as a double check to see if we can print
     # them here as well as with backtrace.
@@ -111,13 +105,7 @@ proc unsigned_integral_args {} {
     # Run; should stop at call1a and print actual arguments.
     if {!$gcc_compiled} then { setup_xfail "rs6000-*-*" }
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call1a \\(uc=98 'b', us=6, ui=7, ul=8\\) .*$gdb_prompt $" {
-	    pass "run to call1a"
-	}
-	 -re "$gdb_prompt $" { fail "run to call1a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call1a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " call1a \\(uc=98 'b', us=6, ui=7, ul=8\\) .*" "run to call1a"
 
     # Print each arg as a double check to see if we can print
     # them here as well as with backtrace.
@@ -172,11 +160,14 @@ proc float_and_integral_args {} {
 
     if {!$gcc_compiled} then { setup_xfail "rs6000-*-*" "mips-sgi-irix5*" }
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call2a \\(c=97 'a', f1=4, s=1, d1=5, i=2, f2=4, l=3, d2=5\\) .*$gdb_prompt $" { pass "run to call2a" }
-	 -re ".* call2a \\(c=97 'a', f1=.*, s=1, d1=5, i=2, f2=4, l=3, d2=5\\) .*$gdb_prompt $" { xfail "run to call2a" }
-	 -re "$gdb_prompt $" { fail "run to call2a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call2a" ; gdb_suppress_tests; }
+    set test "run to call2a"
+    gdb_test_multiple "" $test {
+	 -re ".* call2a \\(c=97 'a', f1=4, s=1, d1=5, i=2, f2=4, l=3, d2=5\\) .*$gdb_prompt $" {
+	     pass $test
+	 }
+	 -re ".* call2a \\(c=97 'a', f1=.*, s=1, d1=5, i=2, f2=4, l=3, d2=5\\) .*$gdb_prompt $" {
+	     xfail $test
+	 }
     }
 
     # Print each arg as a double check to see if we can print
@@ -257,10 +248,8 @@ proc complex_args {} {
 
     # Run; should stop at call1a and print actual arguments.
     gdb_run_cmd
-    gdb_expect {
-	-re ".* callca \\(f1=1 \\+ 2 \\* I, f2=1 \\+ 2 \\* I, f3=1 \\+ 2 \\* I\\) .*$gdb_prompt $" { pass "run to call2a" }
-	timeout { fail "(timeout) run to callca" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " callca \\(f1=1 \\+ 2 \\* I, f2=1 \\+ 2 \\* I, f3=1 \\+ 2 \\* I\\) .*" "run to call2a"
+
     gdb_test "cont" ".* callcb \\(d1=3 \\+ 4 \\* I, d2=3 \\+ 4 \\* I, d3=3 \\+ 4 \\* I\\) .*" "continue to callcb"
     gdb_test "cont" ".* callcc \\(ld1=5 \\+ 6 \\* I, ld2=5 \\+ 6 \\* I, ld3=5 \\+ 6 \\* I\\) .*" "continue to callcc"
     gdb_test "cont" ".* callcd \\(fc1=1 \\+ 2 \\* I, dc1=3 \\+ 4 \\* I, ldc1=5 \\+ 6 \\* I\\) .*" "continue to callcd"
@@ -282,10 +271,8 @@ proc complex_integral_args {} {
 
     # Run; should stop at call1a and print actual arguments.
     gdb_run_cmd
-    gdb_expect {
-	-re ".* callc1a \\(c=97 'a', s=1, i=2, ui=7, l=3, fc1=1 \\+ 2 \\* I, dc1=3 \\+ 4 \\* I, ldc1=5 \\+ 6 \\* I\\) .*$gdb_prompt $" { pass "run to callc1a" }
-	timeout { fail "(timeout) run to callc1a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " callc1a \\(c=97 'a', s=1, i=2, ui=7, l=3, fc1=1 \\+ 2 \\* I, dc1=3 \\+ 4 \\* I, ldc1=5 \\+ 6 \\* I\\) .*" "run to callc1a"
+
     gdb_test "cont" ".* callc1b \\(ldc1=5 \\+ 6 \\* I\\, c=97 'a', s=1, i=2, fc1=1 \\+ 2 \\* I, ui=7, l=3, dc1=3 \\+ 4 \\* I\\) .*" "continue to callc1b"
 }
 
@@ -302,10 +289,8 @@ proc complex_float_integral_args {} {
 
     # Run; should stop at call1a and print actual arguments.
     gdb_run_cmd
-    gdb_expect {
-	-re ".* callc2a \\(c=97 'a', s=1, i=2, ui=7, l=3, f=4, d=5, fc1=1 \\+ 2 \\* I, dc1=3 \\+ 4 \\* I, ldc1=5 \\+ 6 \\* I\\) .*$gdb_prompt $" { pass "run to callc2a" }
-	timeout { fail "(timeout) run to callc1a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " callc2a \\(c=97 'a', s=1, i=2, ui=7, l=3, f=4, d=5, fc1=1 \\+ 2 \\* I, dc1=3 \\+ 4 \\* I, ldc1=5 \\+ 6 \\* I\\) .*" "run to callc2a"
+
     gdb_test "cont" ".* callc2b \\(fc1=1 \\+ 2 \\* I, c=97 'a', s=1, i=2, ui=7, ldc1=5 \\+ 6 \\* I\\, l=3, f=4, d=5, dc1=3 \\+ 4 \\* I\\) .*" "continue to callc2b"
 }
 
@@ -328,11 +313,7 @@ proc pointer_args {} {
     # Try dereferencing the arguments.
 
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call3a \\(cp=$hex <c> \"a.*\", sp=$hex <s>, ip=$hex <i>, lp=$hex <l>\\) .*$gdb_prompt $" { pass "run to call3a" }
-	 -re "$gdb_prompt $" { fail "run to call3a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call3a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " call3a \\(cp=$hex <c> \"a.*\", sp=$hex <s>, ip=$hex <i>, lp=$hex <l>\\) .*" "run to call3a"
 
     gdb_test "print *cp" ".* = 97 'a'"
     gdb_test "print *sp" ".* = 1"
@@ -384,13 +365,7 @@ proc structs_by_reference {} {
     # Try dereferencing the arguments.
 
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call4a \\(stp=$hex <st>\\) .*$gdb_prompt $" {
-	    pass "run to call4a"
-	}
-	 -re "$gdb_prompt $" { fail "run to call4a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call4a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " call4a \\(stp=$hex <st>\\) .*" "run to call4a"
 
     gdb_test "print *stp" ".* = \{s1 = 101, s2 = 102\}"
 
@@ -440,13 +415,7 @@ proc structs_by_value {} {
     # Try dereferencing the arguments.
 
     gdb_run_cmd
-    gdb_expect {
-	 -re ".* call5a \\(st=\{s1 = 101, s2 = 102\}\\) .*$gdb_prompt $" {
-	    pass "run to call5a"
-	}
-	 -re "$gdb_prompt $" { fail "run to call5a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call5a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" " call5a \\(st=\{s1 = 101, s2 = 102\}\\) .*" "run to call5a"
 
     gdb_test "print st" ".* = \{s1 = 101, s2 = 102\}"
 
@@ -513,11 +482,7 @@ proc discard_and_shuffle {} {
     # Print backtrace.
 
     gdb_run_cmd
-    gdb_expect {
-	 -re ".*Breakpoint $decimal, call6a .*$gdb_prompt $" { pass "run to call6a" }
-	 -re "$gdb_prompt $" { fail "run to call6a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call6a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" "Breakpoint $decimal, call6a .*" "run to call6a"
 
     setup_xfail "rs6000-*-*"
 
@@ -743,13 +708,7 @@ proc shuffle_round_robin {} {
     # Print backtrace.
 
     gdb_run_cmd
-    gdb_expect {
-	 -re ".*Breakpoint $decimal, call7a .*$gdb_prompt $" {
-	    pass "run to call7a"
-	}
-	 -re "$gdb_prompt $" { fail "run to call7a" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to call7a" ; gdb_suppress_tests; }
-    }
+    gdb_test "" "Breakpoint $decimal, call7a .*" "run to call7a"
 
     if {!$gcc_compiled} then { setup_xfail "rs6000-*-*" "mips-sgi-irix5*" }
     gdb_test_multiple "backtrace 100" "backtrace from call7a" {
@@ -939,11 +898,7 @@ proc recursive_structs_by_value {} {
     # Run; should stop at hitbottom and print actual arguments.
     # Print backtrace.
     gdb_run_cmd
-    gdb_expect {
-	 -re ".*Breakpoint $decimal, hitbottom .*$gdb_prompt $" { pass "run to hitbottom" }
-	 -re "$gdb_prompt $" { fail "run to hitbottom" ; gdb_suppress_tests; }
-	 timeout { fail "(timeout) run to hitbottom" ; gdb_suppress_tests; }
-    }
+    gdb_test "" "Breakpoint $decimal, hitbottom .*" "run to hitbottom"
 
     if ![istarget sparclet-*-*] {
 	gdb_test_sequence "backtrace 100" "recursive passing of structs by value" {
diff --git a/gdb/testsuite/gdb.base/jit-simple.exp b/gdb/testsuite/gdb.base/jit-simple.exp
index 1c815bf..ce65964 100644
--- a/gdb/testsuite/gdb.base/jit-simple.exp
+++ b/gdb/testsuite/gdb.base/jit-simple.exp
@@ -40,14 +40,7 @@ proc jit_run {msg} {
     global decimal gdb_prompt
 
     gdb_run_cmd
-    gdb_expect {
-	-re "Inferior .* exited.*$gdb_prompt $" {
-	    pass $msg
-	}
-	-re ".*$gdb_prompt $" {
-	    fail $msg
-	}
-    }
+    gdb_test "" "Inferior .* exited.*" $msg
 }
 
 # Test re-running an inferior with a JIT descriptor, where the JIT
diff --git a/gdb/testsuite/gdb.base/reread.exp b/gdb/testsuite/gdb.base/reread.exp
index 88abb17..e2900b0 100644
--- a/gdb/testsuite/gdb.base/reread.exp
+++ b/gdb/testsuite/gdb.base/reread.exp
@@ -62,17 +62,7 @@ gdb_test "break foo" \
 # Run, should see "Breakpoint 1, foo () at hello1.c:14"
 
 gdb_run_cmd
-
-gdb_expect {
-    -re ".*Breakpoint.* foo .* at .*$srcfile1:14.*$gdb_prompt $"  {
-	pass "run to foo()"
-    }
-    -re ".*$gdb_prompt $" {
-	fail "run to foo()"
-	gdb_suppress_tests
-    }
-    timeout { fail "run to foo() (timeout)" ; gdb_suppress_tests }
-}
+gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" "run to foo()"
 
 # Restore first executable to its original name, and move
 # second executable into its place.  Ensure that the new
@@ -87,24 +77,12 @@ gdb_touch_execfile ${binfile}
 # and reset the breakpoints correctly.
 # Should see "Breakpoint 1, foo () at reread2.c:9"
 
+set test "run to foo() second time"
 if [is_remote target] {
-    unsupported "run to foo() second time "
+    unsupported $test
 } else {
     gdb_run_cmd
-    gdb_expect {
-	#    -re ".*re-reading symbols.*Breakpoint.* foo .* at .*$srcfile2:9.*$gdb_prompt $" {}
-	-re ".*Breakpoint.* foo .* at .*:9.*$gdb_prompt $" {
-	    pass "run to foo() second time "
-	}
-	-re ".*$gdb_prompt $" {
-	    fail "run to foo() second time"
-	    gdb_suppress_tests
-	}
-	timeout { 
-	    fail "run to foo() second time (timeout)"
-	    gdb_suppress_tests 
-	}
-    }
+    gdb_test "" "Breakpoint.* foo .* at .*:9.*" $test
 }
 
 
@@ -127,19 +105,7 @@ if [is_remote target] {
             "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
             "second pass: breakpoint foo in first file"
     gdb_run_cmd
-    gdb_expect {
-        -re ".*Breakpoint.* foo .* at .*$srcfile1:14.*$gdb_prompt $"  {
-            pass "second pass: run to foo()"
-        }
-        -re ".*$gdb_prompt $" {
-            fail "second pass: run to foo()"
-            gdb_suppress_tests
-        }
-        timeout {
-            fail "second pass: run to foo() (timeout)"
-            gdb_suppress_tests
-        }
-    }
+    gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" "second pass: run to foo()"
 
     # This time, let the program run to completion.  If GDB checks the
     # executable file's timestamp now, it won't notice any change.
@@ -151,23 +117,9 @@ if [is_remote target] {
     gdb_rename_execfile ${binfile} ${binfile1}
     gdb_rename_execfile ${binfile2} ${binfile}
     gdb_run_cmd
-    gdb_expect {
-	-re ".*Breakpoint.* foo .* at .*:9.*$gdb_prompt $" {
-	    pass "second pass: run to foo() second time "
-	}
-	-re ".*$gdb_prompt $" {
-	    fail "second pass: run to foo() second time"
-	    gdb_suppress_tests
-	}
-	timeout { 
-	    fail "second pass: run to foo() second time (timeout)"
-	    gdb_suppress_tests 
-	}
-    }
+    gdb_test "" "Breakpoint.* foo .* at .*:9.*" "second pass: run to foo() second time"
 }
 
 # End of tests.
 
-gdb_stop_suppressing_tests
-
 return 0
diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
index fe16521..c5e019b 100644
--- a/gdb/testsuite/gdb.base/sepdebug.exp
+++ b/gdb/testsuite/gdb.base/sepdebug.exp
@@ -177,17 +177,9 @@ gdb_test "info break" \
 # run until the breakpoint at main is hit. For non-stubs-using targets.
 #
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
-	pass "run until function breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "run until function breakpoint"
-    }
-    timeout {
-	fail "run until function breakpoint (timeout)"
-    }
-}
+gdb_test "" \
+    "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*" \
+    "run until function breakpoint"
 
 #
 # run until the breakpoint at a line number
@@ -574,14 +566,7 @@ proc test_next_with_recursion {} {
     # Run until we call factorial with 6
 
     gdb_run_cmd
-    gdb_expect {
-	-re "Break.* factorial .value=6. .*$gdb_prompt $" {}
-	-re ".*$gdb_prompt $" {
-	    fail "run to factorial(6)"
-	    gdb_suppress_tests
-	}
-	timeout { fail "run to factorial(6) (timeout)" ; gdb_suppress_tests }
-    }
+    gdb_test "" "Break.* factorial .value=6. .*" "run to factorial(6)"
 
     # Continue until we call factorial recursively with 5.
 
@@ -686,18 +671,13 @@ proc test_different_dir {type test_different_dir xfail} {
 	if {$xfail} {
 	    setup_xfail "*-*-*"
 	}
-	gdb_expect {
+	set test "run until function breakpoint, optimized file"
+	gdb_test_multiple "" $test {
 	    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
-		pass "run until function breakpoint, optimized file"
+		pass $test
 	    }
 	    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$gdb_prompt $" {
-		pass "run until function breakpoint, optimized file (code motion)"
-	    }
-	    -re "$gdb_prompt $" {
-		fail "run until function breakpoint, optimized file"
-	    }
-	    timeout {
-		fail "run until function breakpoint, optimized file (timeout)"
+		pass "$test (code motion)"
 	    }
 	}
 
diff --git a/gdb/testsuite/gdb.base/step-bt.exp b/gdb/testsuite/gdb.base/step-bt.exp
index e521456..2469c2b 100644
--- a/gdb/testsuite/gdb.base/step-bt.exp
+++ b/gdb/testsuite/gdb.base/step-bt.exp
@@ -29,19 +29,7 @@ gdb_test "break *hello" \
          "breakpoint at first instruction of hello()"
 
 gdb_run_cmd
-gdb_expect {
-    -re ".*Breakpoint.* hello .* at .*$srcfile:.*$gdb_prompt $"  {
-	pass "run to hello()"
-    }
-    -re ".*$gdb_prompt $" {
-	fail "run to hello()"
-	return -1
-    }
-    timeout {
-        fail "run to hello() (timeout)"
-        return -1
-    }
-}
+gdb_test "" "Breakpoint.* hello .* at .*$srcfile:.*" "run to hello()"
 
 gdb_test "stepi" \
          ".*" \
diff --git a/gdb/testsuite/gdb.cp/mb-inline.exp b/gdb/testsuite/gdb.cp/mb-inline.exp
index 803151a..f83d0bf 100644
--- a/gdb/testsuite/gdb.cp/mb-inline.exp
+++ b/gdb/testsuite/gdb.cp/mb-inline.exp
@@ -46,17 +46,7 @@ gdb_test "info break" \
     "\[\r\n\]1\.1.* y .* at .*$hdrfile:$bp_location.*\[\r\n\]1\.2.* y .* at .*$hdrfile:$bp_location.*"
 
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" {
-	pass "run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "run to breakpoint"
-    }
-    timeout {
-	fail "run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*" "run to breakpoint"
 
 gdb_test "continue" \
     ".*Breakpoint.*foo \\(i=1\\).*" \
@@ -69,17 +59,7 @@ gdb_test "continue" \
 gdb_test_no_output "disable 1.2" "disabling location: disable"
 
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" {
-	pass "disabling location: run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "disabling location: run to breakpoint"
-    }
-    timeout {
-	fail "disabling location: run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*" "disabling location: run to breakpoint"
 
 gdb_test_multiple "info break" "disabled breakpoint 1.2" {
     -re "1\.2.* n .* at .*$hdrfile:$bp_location.*$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.cp/mb-templates.exp b/gdb/testsuite/gdb.cp/mb-templates.exp
index 3a7fbc9..edc1631 100644
--- a/gdb/testsuite/gdb.cp/mb-templates.exp
+++ b/gdb/testsuite/gdb.cp/mb-templates.exp
@@ -72,17 +72,7 @@ gdb_test_no_output {condition $bpnum i==1} \
     "separate condition: set condition"
     
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
-	pass "separate condition: run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "separate condition: run to breakpoint"
-    }
-    timeout {
-	fail "separate condition: run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*" "separate condition: run to breakpoint"
 
 gdb_test "continue" \
     ".*Breakpoint.*foo<double> \\(i=1\\).*" \
@@ -94,17 +84,7 @@ gdb_test "continue" \
 gdb_test_no_output {disable $bpnum.1} "disabling location: disable"
 
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*foo<double> \\(i=1\\).*$gdb_prompt $" {
-	pass "disabling location: run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "disabling location: run to breakpoint"
-    }
-    timeout {
-	fail "disabling location: run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*foo<double> \\(i=1\\).*" "disabling location: run to breakpoint"
 
 # Try disabling entire breakpoint
 gdb_test_no_output {enable $bpnum.1} "disabling location: enable"
@@ -113,17 +93,7 @@ gdb_test_no_output {enable $bpnum.1} "disabling location: enable"
 gdb_test_no_output {disable $bpnum} "disable breakpoint: disable"
 
 gdb_run_cmd
-gdb_expect {
-    -re "$inferior_exited_re normally.*$gdb_prompt $" {
-	pass "disable breakpoint: run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "disable breakpoint: run to breakpoint"
-    }
-    timeout {
-	fail "disable breakpoint: run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "$inferior_exited_re normally.*" "disable breakpoint: run to breakpoint"
 
 # Make sure breakpoint can be set on a specific instantion.
 delete_breakpoints
@@ -132,17 +102,7 @@ gdb_test "break 'void foo<int>(int)'" ".*" \
 
 
 gdb_run_cmd
-gdb_expect {
-    -re ".*Breakpoint \[0-9\]+,.*foo<int> \\(i=0\\).*$gdb_prompt $" {
-	pass "instantiation: run to breakpoint"
-    }
-    -re "$gdb_prompt $" {
-	fail "instantiation: run to breakpoint"
-    }
-    timeout {
-	fail "instantiation: run to breakpoint (timeout)"
-    }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*foo<int> \\(i=0\\).*" "instantiation: run to breakpoint"
 
 gdb_test "continue" \
     ".*Breakpoint.*foo<int> \\(i=1\\).*" \
diff --git a/gdb/testsuite/gdb.objc/basicclass.exp b/gdb/testsuite/gdb.objc/basicclass.exp
index 96badd2..37f1b15 100644
--- a/gdb/testsuite/gdb.objc/basicclass.exp
+++ b/gdb/testsuite/gdb.objc/basicclass.exp
@@ -119,12 +119,7 @@ gdb_test continue \
 # Test resetting breakpoints when re-running program
 #
 gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:.*$gdb_prompt $"\
-                            { pass "resetting breakpoints when rerunning" }
-    -re ".*$gdb_prompt $"       { fail "resetting breakpoints when rerunning" }
-    timeout                 { fail "resetting breakpoints when rerunning" }
-}
+gdb_test "" "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:.*" "resetting breakpoints when r
 
 #
 # Continue until breakpoint (test re-setting breakpoint)
diff --git a/gdb/testsuite/gdb.threads/killed.exp b/gdb/testsuite/gdb.threads/killed.exp
index d29454f..bacd52a 100644
--- a/gdb/testsuite/gdb.threads/killed.exp
+++ b/gdb/testsuite/gdb.threads/killed.exp
@@ -65,14 +65,7 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
 clean_restart ${binfile}
 
 gdb_run_cmd
-gdb_expect {
-  -re "$gdb_prompt $" {
-    pass "run program to completion"
-  }
-  timeout {
-    fail "run program to completion (timeout)"
-  }
-}
+gdb_test "" "" "run program to completion"
 
 # Try to quit.
 send_gdb "quit\n"
-- 
1.9.3


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

* Re: [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test
  2014-09-12 15:27               ` [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test Pedro Alves
@ 2014-09-12 17:10                 ` Joel Brobecker
  2014-09-12 21:44                   ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Joel Brobecker @ 2014-09-12 17:10 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

> > I'm writing a test that converts all gdb_run_cmd -> gdb_expect
> > cases to avoid this from spreading further.
> 
> Patch pasted below.

Thanks Pedro. I started reviewing it, and then switch to scanner-mode
half way through. It looks great!

> In a2-run.exp I converted one vxworks path, but left the others
> in place...  I'm tempted to nuke most of that vxworks stuff
> out of the testsuite.  Does anyone still care about it?

I suspect no one does. I even suspect that no one even cares
about whatever old version of vxworks is supported by GDB.
IIRC, that code pre-dates my involvement in the GDB project,
which started around 2001.

-- 
Joel

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

* Re: [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test
  2014-09-12 17:10                 ` Joel Brobecker
@ 2014-09-12 21:44                   ` Pedro Alves
  2014-09-12 22:45                     ` Pedro Alves
  2014-09-15 13:55                     ` Joel Brobecker
  0 siblings, 2 replies; 20+ messages in thread
From: Pedro Alves @ 2014-09-12 21:44 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

On 09/12/2014 06:10 PM, Joel Brobecker wrote:
>>> I'm writing a test that converts all gdb_run_cmd -> gdb_expect
>>> cases to avoid this from spreading further.
>>
>> Patch pasted below.
> 
> Thanks Pedro. I started reviewing it, and then switch to scanner-mode
> half way through. It looks great!

Thanks Joel.  Now pushed.

>> In a2-run.exp I converted one vxworks path, but left the others
>> in place...  I'm tempted to nuke most of that vxworks stuff
>> out of the testsuite.  Does anyone still care about it?
> 
> I suspect no one does. I even suspect that no one even cares
> about whatever old version of vxworks is supported by GDB.
> IIRC, that code pre-dates my involvement in the GDB project,
> which started around 2001.

Oh, I notice that the vxworks support the testsuite is caring
about must be for a "target vxworks_or_something", not
"target remote", because we have in
gdb/testsuite/gdb.base/default.exp:

 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" ""

     gdb_test "r" "Starting program: .*
 You must specify a function name to run, and arguments if any"\
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                "run \"r\" abbreviation"
     gdb_test "set args main" ".*" ""


And indeed, that target has been removed 10 years ago already ...

 commit e84ecc995d6a5e4e9114d3cea61717b8a573afb6
 Author:     Andrew Cagney <cagney@redhat.com>
 AuthorDate: Sat Nov 13 23:10:02 2004 +0000

    2004-11-13  Andrew Cagney  <cagney@gnu.org>

        * configure.tgt: Delete i[34567]86-*-vxworks*, m68*-netx-*,
        m68*-*-vxworks*, mips*-*-vxworks*, powerpc-*-vxworks*, and
        sparc-*-vxworks*.
        * NEWS: Mention that vxworks was deleted.

Which had:

 -static void
 -vx_create_inferior (char *exec_file, char *args, char **env, int from_tty)
 -{
 -  enum clnt_stat status;
 -  arg_array passArgs;
 -  TASK_START taskStart;
 -
 -  memset ((char *) &passArgs, '\0', sizeof (passArgs));
 -  memset ((char *) &taskStart, '\0', sizeof (taskStart));
 -
 -  /* parse arguments, put them in passArgs */
 -
 -  parse_args (args, &passArgs);
 -
 -  if (passArgs.arg_array_len == 0)
 -    error ("You must specify a function name to run, and arguments if any");
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So I'm going to delete all that cruft.

OOC, how does one debug Ravenscar then?  Does that sit on top of
target remote ?

Thanks,
Pedro Alves

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

* Re: [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test
  2014-09-12 21:44                   ` Pedro Alves
@ 2014-09-12 22:45                     ` Pedro Alves
  2014-09-15 13:55                     ` Joel Brobecker
  1 sibling, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2014-09-12 22:45 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

On 09/12/2014 10:44 PM, Pedro Alves wrote:
> On 09/12/2014 06:10 PM, Joel Brobecker wrote:

>> I suspect no one does. I even suspect that no one even cares
>> about whatever old version of vxworks is supported by GDB.
>> IIRC, that code pre-dates my involvement in the GDB project,
>> which started around 2001.

...

> So I'm going to delete all that cruft.

Sent in a new thread.  Let me know whether I'm deleting something
you may need.

Thanks,
Pedro Alves

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

* Re: [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test
  2014-09-12 21:44                   ` Pedro Alves
  2014-09-12 22:45                     ` Pedro Alves
@ 2014-09-15 13:55                     ` Joel Brobecker
  1 sibling, 0 replies; 20+ messages in thread
From: Joel Brobecker @ 2014-09-15 13:55 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Edjunior Barbosa Machado, gdb-patches, Ulrich Weigand,
	Sergio Durigan Junior

> OOC, how does one debug Ravenscar then?  Does that sit on top of
> target remote ?

The ravenscar target is more like a "thread" target, and depends on
the runtime being used. So, it can sit on top of any execution target,
remote, native, etc. To be able to test it, you need an Ada compiler
with a ravenscar runtime. Not sure if the GCC from FSF has those
runtimes out of the box, but we provide an arm-eabi GNAT toolchain
on libre.adacore.com, if you ever need it.

-- 
Joel

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-11 23:03 [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Edjunior Barbosa Machado
  2014-09-11 23:21 ` Sergio Durigan Junior
@ 2014-09-17 10:07 ` Pedro Alves
  2014-09-17 12:41   ` Ulrich Weigand
  1 sibling, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2014-09-17 10:07 UTC (permalink / raw)
  To: Edjunior Barbosa Machado, gdb-patches; +Cc: Ulrich Weigand

Hi guys,

See https://sourceware.org/bugzilla/show_bug.cgi?id=17384 .

When safe_read_memory_integer call fails, GDB prints a
surprising/confusing error message, more so in case the unwinder
is triggered for some reason other than the "bt" command, like
with "step"/"next".  I take you're now seeing the same errors
with this patch.

IMO, printing the error is not something a low-level helper function
like  safe_read_memory_integer should be doing, as GDB uses it when
probing with heuristics because it can't sure its guesses make sense
(whether there's a frame at all, etc.)  safe_frame_unwind_memory, which is
used in rs6000_in_function_epilogue_p doesn't print the error either.

Thoughts?

Thanks,
Pedro Alves

On 09/12/2014 12:03 AM, Edjunior Barbosa Machado wrote:
> Hi,
> 
> this patch intends to fix PR tdep/17379:
>   https://sourceware.org/bugzilla/show_bug.cgi?id=17379
> 
> The problem is that rs6000_frame_cache attempts to read the stack backchain via
> read_memory_unsigned_integer, which throws an exception if the stack pointer is
> invalid.  With this path, it calls safe_read_memory_integer instead, which
> doesn't throw an exception and allows for safe handling of that situation.
> Regression tested on ppc64{,le}.  Ok?
> 
> Thanks and regards,
> --
> Edjunior
> 
> gdb/
> 2014-09-11  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>
> 	    Ulrich Weigand  <uweigand@de.ibm.com>
> 
> 	PR tdep/17379
> 	* rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer
> 	instead of read_memory_unsigned_integer.
> 
> ---
>  gdb/rs6000-tdep.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index 730afe7..dabf448 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache)
>      }
>  
>    if (!fdata.frameless)
> -    /* Frameless really means stackless.  */
> -    cache->base
> -      = read_memory_unsigned_integer (cache->base, wordsize, byte_order);
> +    {
> +      /* Frameless really means stackless.  */
> +      LONGEST backchain;
> +
> +      if (safe_read_memory_integer (cache->base, wordsize,
> +				    byte_order, &backchain))
> +        cache->base = (CORE_ADDR) backchain;
> +    }
>  
>    trad_frame_set_value (cache->saved_regs,
>  			gdbarch_sp_regnum (gdbarch), cache->base);


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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-17 10:07 ` [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Pedro Alves
@ 2014-09-17 12:41   ` Ulrich Weigand
  2014-09-17 13:03     ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2014-09-17 12:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Edjunior Barbosa Machado, gdb-patches

Pedro Alves wrote:

> See https://sourceware.org/bugzilla/show_bug.cgi?id=17384 .
> 
> When safe_read_memory_integer call fails, GDB prints a
> surprising/confusing error message, more so in case the unwinder
> is triggered for some reason other than the "bt" command, like
> with "step"/"next".  I take you're now seeing the same errors
> with this patch.
> 
> IMO, printing the error is not something a low-level helper function
> like  safe_read_memory_integer should be doing, as GDB uses it when
> probing with heuristics because it can't sure its guesses make sense
> (whether there's a frame at all, etc.)  safe_frame_unwind_memory, which is
> used in rs6000_in_function_epilogue_p doesn't print the error either.

Agreed, it doesn't make sense for safe_read_memory_integer to ever
print an error.  In fact, it doesn't make sense for it to start
using a routine that raises exceptions and then attempt to catch it.
The following patch simplifies the whole logic by just using
target_read_memory directly.   Does this look reasonable?

[ B.t.w. the naming of safe_frame_unwind_memory is a bit weird.  This
should either be "safe_read_memory" in corefile.c, or else something
like safe_get_frame_memory in analogy to get_frame_memory.  ]

Tested on powerpc64le-linux.

Bye,
Ulrich


gdb/ChangeLog:

	* corefile.c (struct captured_read_memory_integer_arguments): Remove.
	(do_captured_read_memory_integer): Remove.
	(safe_read_memory_integer): Use target_read_memory directly instead
	of catching errors in do_captured_read_memory_integer.

diff --git a/gdb/corefile.c b/gdb/corefile.c
index 1617392..a0bb2aa 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -290,40 +290,6 @@ read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     memory_error (status, memaddr);
 }
 
-/* Argument / return result struct for use with
-   do_captured_read_memory_integer().  MEMADDR and LEN are filled in
-   by gdb_read_memory_integer().  RESULT is the contents that were
-   successfully read from MEMADDR of length LEN.  */
-
-struct captured_read_memory_integer_arguments
-{
-  CORE_ADDR memaddr;
-  int len;
-  enum bfd_endian byte_order;
-  LONGEST result;
-};
-
-/* Helper function for gdb_read_memory_integer().  DATA must be a
-   pointer to a captured_read_memory_integer_arguments struct.
-   Return 1 if successful.  Note that the catch_errors() interface
-   will return 0 if an error occurred while reading memory.  This
-   choice of return code is so that we can distinguish between
-   success and failure.  */
-
-static int
-do_captured_read_memory_integer (void *data)
-{
-  struct captured_read_memory_integer_arguments *args
-    = (struct captured_read_memory_integer_arguments*) data;
-  CORE_ADDR memaddr = args->memaddr;
-  int len = args->len;
-  enum bfd_endian byte_order = args->byte_order;
-
-  args->result = read_memory_integer (memaddr, len, byte_order);
-
-  return 1;
-}
-
 /* Read memory at MEMADDR of length LEN and put the contents in
    RETURN_VALUE.  Return 0 if MEMADDR couldn't be read and non-zero
    if successful.  */
@@ -333,19 +299,13 @@ safe_read_memory_integer (CORE_ADDR memaddr, int len,
 			  enum bfd_endian byte_order,
 			  LONGEST *return_value)
 {
-  int status;
-  struct captured_read_memory_integer_arguments args;
-
-  args.memaddr = memaddr;
-  args.len = len;
-  args.byte_order = byte_order;
+  gdb_byte buf[sizeof (LONGEST)];
 
-  status = catch_errors (do_captured_read_memory_integer, &args,
-			 "", RETURN_MASK_ALL);
-  if (status)
-    *return_value = args.result;
+  if (target_read_memory (memaddr, buf, len))
+    return 0;
 
-  return status;
+  *return_value = extract_signed_integer (buf, len, byte_order);
+  return 1;
 }
 
 LONGEST


-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

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

* Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-17 12:41   ` Ulrich Weigand
@ 2014-09-17 13:03     ` Pedro Alves
  2014-09-17 15:34       ` [PUSHED][PR gdb/17384] " Ulrich Weigand
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2014-09-17 13:03 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Edjunior Barbosa Machado, gdb-patches

On 09/17/2014 01:41 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
> 
>> See https://sourceware.org/bugzilla/show_bug.cgi?id=17384 .
>>
>> When safe_read_memory_integer call fails, GDB prints a
>> surprising/confusing error message, more so in case the unwinder
>> is triggered for some reason other than the "bt" command, like
>> with "step"/"next".  I take you're now seeing the same errors
>> with this patch.
>>
>> IMO, printing the error is not something a low-level helper function
>> like  safe_read_memory_integer should be doing, as GDB uses it when
>> probing with heuristics because it can't sure its guesses make sense
>> (whether there's a frame at all, etc.)  safe_frame_unwind_memory, which is
>> used in rs6000_in_function_epilogue_p doesn't print the error either.
> 
> Agreed, it doesn't make sense for safe_read_memory_integer to ever
> print an error.  In fact, it doesn't make sense for it to start
> using a routine that raises exceptions and then attempt to catch it.
> The following patch simplifies the whole logic by just using
> target_read_memory directly.   Does this look reasonable?

Definitely reasonable.  Looks great to me.  Thanks for doing this.

> 
> [ B.t.w. the naming of safe_frame_unwind_memory is a bit weird.  This
> should either be "safe_read_memory" in corefile.c, or else something
> like safe_get_frame_memory in analogy to get_frame_memory.  ]

Agreed.  It seems like that and get_frame_memory were added
in order to make sure frame code consistently used
target_read_memory_nobpt to mask out breakpoints:

  https://sourceware.org/ml/gdb-patches/2004-04/msg00067.html

Seems like all that wrapping is unnecessary nowadays, as we have to
go out of way to bypass breakpoint masking.

Thanks,
Pedro Alves

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

* [PUSHED][PR gdb/17384] Re: [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid
  2014-09-17 13:03     ` Pedro Alves
@ 2014-09-17 15:34       ` Ulrich Weigand
  0 siblings, 0 replies; 20+ messages in thread
From: Ulrich Weigand @ 2014-09-17 15:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Edjunior Barbosa Machado, gdb-patches

Pedro Alves wrote:
> On 09/17/2014 01:41 PM, Ulrich Weigand wrote:
> > Agreed, it doesn't make sense for safe_read_memory_integer to ever
> > print an error.  In fact, it doesn't make sense for it to start
> > using a routine that raises exceptions and then attempt to catch it.
> > The following patch simplifies the whole logic by just using
> > target_read_memory directly.   Does this look reasonable?
> 
> Definitely reasonable.  Looks great to me.  Thanks for doing this.

OK, I've committed that patch now.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

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

end of thread, other threads:[~2014-09-17 15:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11 23:03 [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Edjunior Barbosa Machado
2014-09-11 23:21 ` Sergio Durigan Junior
2014-09-12  2:47   ` Edjunior Barbosa Machado
2014-09-12  3:20     ` Sergio Durigan Junior
2014-09-12  8:39       ` Ulrich Weigand
2014-09-12  9:59     ` Pedro Alves
2014-09-12 12:31       ` Edjunior Barbosa Machado
2014-09-12 13:00       ` Joel Brobecker
2014-09-12 13:38         ` Pedro Alves
2014-09-12 13:50           ` Joel Brobecker
2014-09-12 14:21             ` Pedro Alves
2014-09-12 15:27               ` [PATCH] after gdb_run_cmd, gdb_expect -> gdb_test_multiple/gdb_test Pedro Alves
2014-09-12 17:10                 ` Joel Brobecker
2014-09-12 21:44                   ` Pedro Alves
2014-09-12 22:45                     ` Pedro Alves
2014-09-15 13:55                     ` Joel Brobecker
2014-09-17 10:07 ` [PATCH] [PR tdep/17379] Fix internal-error when stack pointer is invalid Pedro Alves
2014-09-17 12:41   ` Ulrich Weigand
2014-09-17 13:03     ` Pedro Alves
2014-09-17 15:34       ` [PUSHED][PR gdb/17384] " Ulrich Weigand

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).