[rfc] Increase match_max size for GDB testsuite?

[rfc] Increase match_max size for GDB testsuite?

[Ulrich Weigand]

Hello,

I'm seeing (somewhat random) failures in callfuncs.exp on spu, which 
appear to be caused by the length of the "info all-registers" output
on the platform.

I'm no dejagnu/expect expert, but seems to me there is a buffer size
that is being set to 20000 in lib/gdb.exp, and if any response from
the inferior exceeds this size, it's not completly predictable what
happens.

Now, since we have 128 registers on the SPU, and each is printed using
a union of 7 different types, the output of "info all-registers" is
quite long, in the range of 50000 bytes.  This exceeds the above limit,
and sometimes causes the test case to fail with
"ERROR: internal buffer is full."

Note that I've seen the same problem in the past with checkpoint.exp
on s390x-ibm-linux (when retrieving the list of 600 checkpoints),
so it doesn't appear to be a spu-only issue.

By simply increasing the "match_max" argument in default_gdb_init,
those failures go away.  I didn't notice any adverse effects either,
so I'd like to make that change -- however, if anybody has a better
solution to the problem, please let me know ...

Bye,
Ulrich



Index: gdb/testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.78
diff -c -p -r1.78 gdb.exp
*** gdb/testsuite/lib/gdb.exp	23 Jan 2007 17:11:55 -0000	1.78
--- gdb/testsuite/lib/gdb.exp	8 Mar 2007 18:12:31 -0000
*************** proc default_gdb_init { args } {
*** 1952,1958 ****
      
      # Uh, this is lame. Really, really, really lame. But there's this *one*
      # testcase that will fail in random places if we don't increase this.
!     match_max -d 20000
  
      # We want to add the name of the TCL testcase to the PASS/FAIL messages.
      if { [llength $args] > 0 } {
--- 1952,1958 ----
      
      # Uh, this is lame. Really, really, really lame. But there's this *one*
      # testcase that will fail in random places if we don't increase this.
!     match_max -d 100000
  
      # We want to add the name of the TCL testcase to the PASS/FAIL messages.
      if { [llength $args] > 0 } {
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Increase match_max size for GDB testsuite?

[Daniel Jacobowitz]

On Thu, Mar 08, 2007 at 07:39:00PM +0100, Ulrich Weigand wrote:
> I'm no dejagnu/expect expert, but seems to me there is a buffer size
> that is being set to 20000 in lib/gdb.exp, and if any response from
> the inferior exceeds this size, it's not completly predictable what
> happens.

I think that's accurate.

> Note that I've seen the same problem in the past with checkpoint.exp
> on s390x-ibm-linux (when retrieving the list of 600 checkpoints),
> so it doesn't appear to be a spu-only issue.
> 
> By simply increasing the "match_max" argument in default_gdb_init,
> those failures go away.  I didn't notice any adverse effects either,
> so I'd like to make that change -- however, if anybody has a better
> solution to the problem, please let me know ...

The checkpoint tests have a similar problem, but nowhere else should.
Basically, there are two ways you can approach large tests: you can
make the buffer bigger, and perform regex matching on an increasingly
huge output, or you can match one line at a time and use exp_continue.
One example of this is in auxv.exp.

I'd mildly prefer that approach - the big buffer can be very slow, and
it just means we'll be back to the problem again later if another
target has even more registers.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Increase match_max size for GDB testsuite?

[Ulrich Weigand]

Daniel Jacobowitz:

> The checkpoint tests have a similar problem, but nowhere else should.
> Basically, there are two ways you can approach large tests: you can
> make the buffer bigger, and perform regex matching on an increasingly
> huge output, or you can match one line at a time and use exp_continue.
> One example of this is in auxv.exp.

Thanks for pointing this out, I was not aware of that expect feature.
 
> I'd mildly prefer that approach - the big buffer can be very slow, and
> it just means we'll be back to the problem again later if another
> target has even more registers.

I agree.  The following patch implements a fetch_all_registers function
modeled after the fetch_auvx routine.  This also fixes the problem on
spu-elf.  Does it look OK to you?

Bye,
Ulrich


ChangeLog:

	* gdb.base/callfuncs.exp (do_get_all_registers): Remove.
	(fetch_all_registers): New function, uses gdb_test_multiple and
	exp_continue to fetch inferior output line-by-line.
	Replace all uses of do_get_all_registers by fetch_all_registers.

Index: gdb/testsuite/gdb.base/callfuncs.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/callfuncs.exp,v
retrieving revision 1.19
diff -u -p -r1.19 callfuncs.exp
--- gdb/testsuite/gdb.base/callfuncs.exp	31 Jan 2007 15:37:49 -0000	1.19
+++ gdb/testsuite/gdb.base/callfuncs.exp	8 Mar 2007 19:55:14 -0000
@@ -236,28 +236,56 @@ proc do_function_calls {} {
 }
 
 # Procedure to get current content of all registers.
-global all_registers_content
-set all_registers_content ""
-proc do_get_all_registers { } {
+proc fetch_all_registers {test} {
     global gdb_prompt
-    global expect_out
-    global all_registers_content
 
-    set all_registers_content ""
-    send_gdb "info all-registers\n"
-    gdb_expect {
-	-re "info all-registers\r\n(.*)$gdb_prompt $" {
-	    set all_registers_content $expect_out(1,string)
+    set all_registers_lines {}
+    set bad -1
+    if {[gdb_test_multiple "info all-registers" $test {
+	-re "info all-registers\[\r\n\]+" {
+	    exp_continue
+	}
+	-ex "The program has no registers now" {
+	    set bad 1
+	    exp_continue
+	}
+	-re "^bspstore\[ \t\]+\[^\r\n\]+\[\r\n\]+" {
 	    if [istarget "ia64-*-*"] {
 		# Filter out bspstore which is specially tied to bsp,
 		# giving spurious differences.
-		regsub {\nbspstore[^\n]*\n} $all_registers_content "\n" all_registers_content
+	    } else {
+		lappend all_registers_lines $expect_out(0,string)
+	    }
+	    exp_continue
+	}
+	-re "^\[^ \t\]+\[ \t\]+\[^\r\n\]+\[\r\n\]+" {
+	    lappend all_registers_lines $expect_out(0,string)
+	    exp_continue
+	}
+	-re ".*$gdb_prompt $" {
+	    incr bad
+	}
+	-re "^\[^\r\n\]+\[\r\n\]+" {
+	    if {!$bad} {
+		warning "Unrecognized output: $expect_out(0,string)"
+		set bad 1
 	    }
+	    exp_continue
 	}
-	default {}
+    }] != 0} {
+	return {}
+    }
+
+    if {$bad} {
+	fail $test
+	return {}
     }
+
+    pass $test
+    return $all_registers_lines
 }
 
+
 # Start with a fresh gdb.
 
 gdb_exit
@@ -302,19 +330,18 @@ gdb_test "next" "t_structs_c\\(struct_va
   "next to t_structs_c"
 
 # Save all register contents.
-do_get_all_registers
-set old_reg_content $all_registers_content
+set old_reg_content [fetch_all_registers "retrieve original register contents"]
 
 # Perform function calls.
 do_function_calls
 
 # Check if all registers still have the same value.
-do_get_all_registers
-set new_reg_content $all_registers_content
-if ![string compare $old_reg_content $new_reg_content] then {
+set new_reg_content [fetch_all_registers \
+		     "register contents after gdb function calls"]
+if {$old_reg_content == $new_reg_content} then {
     pass "gdb function calls preserve register contents"
 } else {
-    set old_reg_content $all_registers_content
+    set old_reg_content $new_reg_content
     fail "gdb function calls preserve register contents"
 }
 
@@ -330,9 +357,9 @@ gdb_test "continue" "Continuing.*" "cont
 if ![gdb_test "bt 2" \
 	      "#0  main.*" \
 	      "bt after continuing from call dummy breakpoint"] then {
-    do_get_all_registers
-    set new_reg_content $all_registers_content
-    if ![string compare $old_reg_content $new_reg_content] then {
+    set new_reg_content [fetch_all_registers \
+			 "register contents after stop in call dummy"]
+    if {$old_reg_content == $new_reg_content} then {
 	pass "continue after stop in call dummy preserves register contents"
     } else {
 	fail "continue after stop in call dummy preserves register contents"
@@ -349,9 +376,9 @@ gdb_test "finish" \
 if ![gdb_test "bt 2" \
 	      "#0  main.*" \
 	      "bt after finishing from call dummy breakpoint"] then {
-    do_get_all_registers
-    set new_reg_content $all_registers_content
-    if ![string compare $old_reg_content $new_reg_content] then {
+    set new_reg_content [fetch_all_registers \
+			 "register contents after finish in call dummy"]
+    if {$old_reg_content == $new_reg_content} then {
 	pass "finish after stop in call dummy preserves register contents"
     } else {
 	fail "finish after stop in call dummy preserves register contents"
@@ -367,9 +394,9 @@ if ![gdb_test "return 7" \
 	      "back at main after return from call dummy breakpoint" \
 	      "Make add return now. .y or n.*" \
 	      "y"] then {
-    do_get_all_registers
-    set new_reg_content $all_registers_content
-    if ![string compare $old_reg_content $new_reg_content] then {
+    set new_reg_content [fetch_all_registers \
+                         "register contents after return in call dummy"]
+    if {$old_reg_content == $new_reg_content} then {
 	pass "return after stop in call dummy preserves register contents"
     } else {
 	fail "return after stop in call dummy preserves register contents"
@@ -424,9 +451,9 @@ gdb_test "finish" "Value returned is .* 
 gdb_test "backtrace" "\#0  main .*" \
 	"backtrace after finish from nested call level 1"
 
-do_get_all_registers
-set new_reg_content $all_registers_content
-if ![string compare $old_reg_content $new_reg_content] then {
+set new_reg_content [fetch_all_registers \
+		     "register contents after nested call dummies"]
+if {$old_reg_content == $new_reg_content} then {
     pass "nested call dummies preserve register contents"
 } else {
     fail "nested call dummies preserve register contents"


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Increase match_max size for GDB testsuite?

[Daniel Jacobowitz]

On Thu, Mar 08, 2007 at 09:00:58PM +0100, Ulrich Weigand wrote:
> I agree.  The following patch implements a fetch_all_registers function
> modeled after the fetch_auvx routine.  This also fixes the problem on
> spu-elf.  Does it look OK to you?

Yes, it does.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Increase match_max size for GDB testsuite?

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Thu, Mar 08, 2007 at 09:00:58PM +0100, Ulrich Weigand wrote:
> > I agree.  The following patch implements a fetch_all_registers function
> > modeled after the fetch_auvx routine.  This also fixes the problem on
> > spu-elf.  Does it look OK to you?
> 
> Yes, it does.

Thanks for the review!  I've committed this patch now.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com