[3/3] gdb/testsuite: Handle targets with lots of registers

Message ID 20180504120124.GK3375@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess May 4, 2018, 12:01 p.m. UTC
  Thanks for the review feedback.

I've updated the patch below inline with Pedro's feedback, and
included a header check as requested by Maciej.  I tested this on
x86-64 and against RiscV which is the target that was originally
causing me problems.

Thanks,
Andrew

---

[PATCH 1/2] gdb/testsuite: Handle targets with lots of registers

In gdb.base/maint.exp a test calls 'maint print registers'.  If the
target has lots of registers this may overflow expect's buffers,
causing the test to fail.

After this commit we process the output line at a time until we get back
to the GDB prompt, this should prevent buffer overrun while still
testing that the command works as required.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Process output from 'maint print registers'
	line at a time.
---
 gdb/testsuite/ChangeLog          |  5 +++++
 gdb/testsuite/gdb.base/maint.exp | 25 +++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)
  

Comments

Pedro Alves May 4, 2018, 12:47 p.m. UTC | #1
On 05/04/2018 01:01 PM, Andrew Burgess wrote:
> Thanks for the review feedback.
> 
> I've updated the patch below inline with Pedro's feedback, and
> included a header check as requested by Maciej.  I tested this on
> x86-64 and against RiscV which is the target that was originally
> causing me problems.

Thanks.

>  # Test for a regression where this command would internal-error if the
> -# program wasn't running.
> -gdb_test "maint print registers" "Name.*Nr.*Rel.*Offset.*Size.*Type.*"
> +# program wasn't running.  If there's a lot of registers then this
> +# might overflow expect's buffers, so process the output line at a
> +# time.
> +set saw_registers 0
> +set saw_headers 0
> +set test "maint print registers"
> +gdb_test_multiple $test $test {
> +    -re "\[^\r\n\]+Name\[^\r\n\]+Nr\[^\r\n\]+Rel\[^\r\n\]+Offset\[^\r\n\]+Size\[^\r\n\]+Type\[^\r\n\]+\[\r\n\]+" {
> +	set saw_headers 1
> +	exp_continue
> +    }
> +    -re "^\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[\r\n\]+" {
> +        set saw_registers 1
> +        exp_continue
> +    }
> +    -re "^\\*\[0-9\]+\[^\r\n\]+\[\r\n\]+" {
> +        exp_continue
> +    }
> +    -re "$gdb_prompt $" {
> +        gdb_assert $saw_registers "$test: saw some registers"
> +        gdb_assert $saw_headers "$test: saw header line"

We try to avoid the potential for different FAIL / PASS
names/messages.  I.e., if the test times out, or gdb crashes,
you'll get "FAIL: $test" only , while if it reaches the prompt,
you'll get two tests recorded in gdb.sum.  The idea of matching
FAIL/PASS is to make it easier for scripts to distinguish
regressions/progressions vs new failures/passes.  (Text in
trailing ()s, like " (timeout)" is considered informational, and
can/should be stripped for test-matching purposes).  If you want to
record that the register and headers were seen, better put it in gdb.log,
with send_log or verbose -log, and do:

        gdb_assert {$saw_registers && $saw_headers} $test

OK with that change.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 93221085653..1fe36405df3 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -62,8 +62,29 @@  gdb_test_no_output "set height 0"
 gdb_file_cmd ${binfile}
 
 # Test for a regression where this command would internal-error if the
-# program wasn't running.
-gdb_test "maint print registers" "Name.*Nr.*Rel.*Offset.*Size.*Type.*"
+# program wasn't running.  If there's a lot of registers then this
+# might overflow expect's buffers, so process the output line at a
+# time.
+set saw_registers 0
+set saw_headers 0
+set test "maint print registers"
+gdb_test_multiple $test $test {
+    -re "\[^\r\n\]+Name\[^\r\n\]+Nr\[^\r\n\]+Rel\[^\r\n\]+Offset\[^\r\n\]+Size\[^\r\n\]+Type\[^\r\n\]+\[\r\n\]+" {
+	set saw_headers 1
+	exp_continue
+    }
+    -re "^\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[\r\n\]+" {
+        set saw_registers 1
+        exp_continue
+    }
+    -re "^\\*\[0-9\]+\[^\r\n\]+\[\r\n\]+" {
+        exp_continue
+    }
+    -re "$gdb_prompt $" {
+        gdb_assert $saw_registers "$test: saw some registers"
+        gdb_assert $saw_headers "$test: saw header line"
+    }
+}
 
 # Test "mt expand-symtabs" here as it's easier to verify before we
 # run the program.