[1/3] gdb/testsuite: Fix broken regexp in gdbstub case

Message ID 1ec70ce9afe8bebbb3f0257778943d4634ea126b.1523286728.git.andrew.burgess@embecosm.com
State New
Headers show
Series
  • Small testsuite updates
Related show

Commit Message

Andrew Burgess April 9, 2018, 3:15 p.m.
When $use_gdb_stub is true then, when we start an MI target there's a
regexp to match GDB's startup pattern.  Unfortunately the pattern is
broken, and we're also missing a timeout case in the match list (which
would have helped point out that the regexp was broken).

gdb/testsuite/ChangeLog:

	* lib/mi-support.exp (mi_run_cmd_full): Fix regexp and add a
	timeout.
---
 gdb/testsuite/ChangeLog          | 5 +++++
 gdb/testsuite/lib/mi-support.exp | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.12.2

Comments

Pedro Alves April 13, 2018, 12:12 p.m. | #1
On 04/09/2018 04:15 PM, Andrew Burgess wrote:
> When $use_gdb_stub is true then, when we start an MI target there's a

> regexp to match GDB's startup pattern.  Unfortunately the pattern is

> broken, and we're also missing a timeout case in the match list (which

> would have helped point out that the regexp was broken).


That seems to have been added by:
 https://sourceware.org/ml/gdb-patches/2015-12/msg00357.html

curious how it seems to have worked back then.  I wonder whether
it was GDB's CLI-in-MI output that changed meanwhile?

I'd have appreciated a bit more detail on what is actually broken in
the pattern.  The git log above gives no clue, and the diff is not
exactly easy to read.  I trust that it's now correct though.  Thus, OK.

Thanks,
Pedro Alves
Andrew Burgess May 3, 2018, 7:41 p.m. | #2
* Pedro Alves <palves@redhat.com> [2018-04-13 13:12:30 +0100]:

> On 04/09/2018 04:15 PM, Andrew Burgess wrote:

> > When $use_gdb_stub is true then, when we start an MI target there's a

> > regexp to match GDB's startup pattern.  Unfortunately the pattern is

> > broken, and we're also missing a timeout case in the match list (which

> > would have helped point out that the regexp was broken).

> 

> That seems to have been added by:

>  https://sourceware.org/ml/gdb-patches/2015-12/msg00357.html

> 

> curious how it seems to have worked back then.  I wonder whether

> it was GDB's CLI-in-MI output that changed meanwhile?

> 

> I'd have appreciated a bit more detail on what is actually broken in

> the pattern.  The git log above gives no clue, and the diff is not

> exactly easy to read.  I trust that it's now correct though.  Thus, OK.


I aim to please, so here's a revised commit message detailing what
changed.  The actual patch content hasn't changed.  I'll wait a few
days, but if I don't hear anything I'll assume your approval above
still stands and apply the patch.

Thanks,
Andrew

---

gdb/testsuite: Fix broken regexp in gdbstub case

When $use_gdb_stub is true then, when we start an MI target there's a
regexp to match GDB's startup pattern.  Unfortunately the pattern is
broken, and we're also missing a timeout case in the match list (which
would have helped point out that the regexp was broken).

The changes to the regexp are:

  1. Remove '${run_match}' prefix, the issued command doesn't include
  '${run_prefix}' so expecting '${run_match}' is wrong.

  2. Replaced '\\n' with '\\\\n' in order to match literal '\n' in
  GDBs output (that is, match a backslash followed by 'n', not a
  newline character).

  3. Replaced a '.' (matching any character) with '\.' to match a '.'
  and moved the '\.' into the correct place in the regexp.

  4. Replaced '\r\n' with '[\r\n]+' to match the end of a line.  This
  change isn't esential, but matches the other end of line patterns
  within this regexp.

Here's an example of the output that the regexp should match taken
from a testfile log, the first line is the command sent to GDB, and
the remaining lines are the response from GDB:

  jump *_start
  &"jump *_start\n"
  ~"Continuing at 0x10074.\n"
  ^running
  *running,thread-id="all"
  (gdb)

gdb/testsuite/ChangeLog:

	* lib/mi-support.exp (mi_run_cmd_full): Fix regexp and add a
	timeout.

---
 gdb/testsuite/ChangeLog          | 5 +++++
 gdb/testsuite/lib/mi-support.exp | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 2846da74e47..851e490f4de 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -979,7 +979,11 @@ proc mi_run_cmd_full {use_mi_command args} {
 	send_gdb  "jump *$start\n"
 	warning "Using CLI jump command, expect run-to-main FAIL"
 	gdb_expect {
-	    -re "${run_match}&\"jump \\*${start}\\n\"\[\r\n\]+~\"Continuing at 0x\[0-9A-Fa-f\]+\\n.\"\[\r\n\]+\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" {}
+	    -re "&\"jump \\*${start}\\\\n\"\[\r\n\]+~\"Continuing at 0x\[0-9A-Fa-f\]+\.\\\\n\"\[\r\n\]+\\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\[\r\n\]+${mi_gdb_prompt}" {}
+	    timeout {
+		perror "Unable to start target"
+		return -1
+	    }
 	}
 	return 0
     }
-- 
2.14.3
Pedro Alves May 4, 2018, 9:18 a.m. | #3
On 05/03/2018 08:41 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2018-04-13 13:12:30 +0100]:

> 

>> On 04/09/2018 04:15 PM, Andrew Burgess wrote:

>>> When $use_gdb_stub is true then, when we start an MI target there's a

>>> regexp to match GDB's startup pattern.  Unfortunately the pattern is

>>> broken, and we're also missing a timeout case in the match list (which

>>> would have helped point out that the regexp was broken).

>>

>> That seems to have been added by:

>>  https://sourceware.org/ml/gdb-patches/2015-12/msg00357.html

>>

>> curious how it seems to have worked back then.  I wonder whether

>> it was GDB's CLI-in-MI output that changed meanwhile?

>>

>> I'd have appreciated a bit more detail on what is actually broken in

>> the pattern.  The git log above gives no clue, and the diff is not

>> exactly easy to read.  I trust that it's now correct though.  Thus, OK.

> 

> I aim to please, 


:-)

> so here's a revised commit message detailing what

> changed.  The actual patch content hasn't changed.  I'll wait a few

> days, but if I don't hear anything I'll assume your approval above

> still stands and apply the patch.


Perfect.  This is OK.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 2846da74e47..851e490f4de 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -979,7 +979,11 @@  proc mi_run_cmd_full {use_mi_command args} {
 	send_gdb  "jump *$start\n"
 	warning "Using CLI jump command, expect run-to-main FAIL"
 	gdb_expect {
-	    -re "${run_match}&\"jump \\*${start}\\n\"\[\r\n\]+~\"Continuing at 0x\[0-9A-Fa-f\]+\\n.\"\[\r\n\]+\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" {}
+	    -re "&\"jump \\*${start}\\\\n\"\[\r\n\]+~\"Continuing at 0x\[0-9A-Fa-f\]+\.\\\\n\"\[\r\n\]+\\^running\[\r\n\]+\\*running,thread-id=\"\[^\"\]+\"\[\r\n\]+${mi_gdb_prompt}" {}
+	    timeout {
+		perror "Unable to start target"
+		return -1
+	    }
 	}
 	return 0
     }