[RFC] Adjust SID-specific version of gdb_load to honor empty argument

[RFC] Adjust SID-specific version of gdb_load to honor empty argument

[Kevin Buettner]

I recently ran into problems with several tests in gdb.base/break.exp
during my testing of a mep-elf toolchain.  My patch below also
affects several other tests (giving an improvement in test results),
but I'll focus here on the behavior of gdb.base/break.exp.

When I test a mep-elf toolchain (using SID) against break.exp, I see
78 passes and no fails when using the patch below.  When not using the
patch below, I see 72 passes and 6 fails.  So my patch does improve the
results.

Here is my analysis of what's going on:

The proc `test_next_with_recursion' (in gdb.base/break.exp) places a
breakpoint on factorial() prior to invoking gdb_run_cmd.  gdb_run_cmd
will, using the mep configuration, cause gdb_reload to be called,
which in turn calls gdb_load with an empty argument.  The empty
argument is supposed to indicate that load should simply reuse the
current file.  (See the comment in gdb_reload.)

The pre-patch version of the SID-specifc gdb_load doesn't check to see
if it's getting an empty argument or not.  The prepatch version will
unconditionally invoke gdb_file_cmd with the same empty argument.  At
this point a "file " command is sent to GDB and GDB will discard all
of the symbols.  GDB will then complain that it can't reset the
breakpoint corresponding to factorial() due to the symbols being
discarded.  Things go rapidly downhill from there.

The relevant part of the log file looks like this:

(gdb) break factorial
Breakpoint 32 at 0x800d10: file /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c, line 115.
(gdb) PASS: gdb.base/break.exp: break at factorial
jump *_start
The program is not being run.
(gdb) file
No executable file now.
Discard symbol table from `/ironwood1/toshmep-070403-branch/mep-elf/bld/gdb/testsuite/gdb.base/break'? (y or n) y
Error in re-setting breakpoint 32:
No symbol table is loaded.  Use the "file" command.
No symbol file now.
(gdb) kill
The program is not being run.
(gdb) file 
No executable file now.
Error in re-setting breakpoint 32:
No symbol table is loaded.  Use the "file" command.
No symbol file now.
(gdb) ERROR: couldn't load  into /ironwood1/toshmep-070403-branch/mep-elf/bld/gdb/testsuite/../../gdb/gdb.
UNRESOLVED: gdb.base/break.exp: run to factorial(6) (timeout)
continue
The program is not being run.
(gdb) FAIL: gdb.base/break.exp: continue to factorial(5) (the program is no longer running)
backtrace
No stack.
(gdb) FAIL: gdb.base/break.exp: backtrace from factorial(5)
...

My patch below adjusts the SID version of gdb_load to honor the convention
of using the current file when an empty argument is passed to it.  It
also adjusts the return value so as to be compatible with the invocation
of gdb_reload in gdb_run_cmd.

The same portion of the log looks like this with my patch in place:

(gdb) break factorial
Breakpoint 32 at 0x800d10: file /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c, line 115.
(gdb) PASS: gdb.base/break.exp: break at factorial
jump *_start
The program is not being run.
(gdb) target remote localhost:3430
Remote debugging using localhost:3430
0x00000000 in _reset ()
(gdb) load
Loading section .vec, size 0xb0 lma 0x0
Loading section .hwinit, size 0x44 lma 0xb8
Loading section .data, size 0x87c lma 0x8000b8
Loading section .eh_frame, size 0x54 lma 0x800934
Loading section .ctors, size 0x8 lma 0x800988
Loading section .dtors, size 0x8 lma 0x800990
Loading section .jcr, size 0x4 lma 0x800998
Loading section .init, size 0x16 lma 0x8009c0
Loading section .text, size 0x9568 lma 0x8009d8
Loading section .fini, size 0x12 lma 0x809f40
Loading section .rodata, size 0x7a8 lma 0x809f54
Loading section .rostacktab, size 0xc lma 0x80a6fc
Start address 0x0, load size 42780
Transfer rate: 8556000 bits/sec, 1043 bytes/write.
(gdb) jump *_start
Continuing at 0x8009d8.

Breakpoint 32, factorial (value=6) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:115
115	  if (value > 1) {  /* set breakpoint 7 here */
(gdb) continue
Continuing.

Breakpoint 32, factorial (value=5) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:115
115	  if (value > 1) {  /* set breakpoint 7 here */
(gdb) PASS: gdb.base/break.exp: continue to factorial(5)
backtrace
#0  factorial (value=5) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:115
#1  0x00800d22 in factorial (value=6) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:116
#2  0x00800c9a in main (argc=0, argv=0x0, envp=0x0) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:94
(gdb) PASS: gdb.base/break.exp: backtrace from factorial(5)
next
116		value *= factorial (value - 1);
(gdb) PASS: gdb.base/break.exp: next to recursive call
delete breakpoints
Delete all breakpoints? (y or n) y
(gdb) info breakpoints
No breakpoints or watchpoints.
(gdb) next
118	    return (value); /* set breakpoint 19 here */
(gdb) PASS: gdb.base/break.exp: next over recursive call
backtrace
#0  factorial (value=120) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:118
#1  0x00800d22 in factorial (value=6) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:116
#2  0x00800c9a in main (argc=0, argv=0x0, envp=0x0) at /ironwood1/toshmep-070403-branch/mep-elf/bld/../../devo/gdb/testsuite/gdb.base/break.c:94
(gdb) PASS: gdb.base/break.exp: backtrace from factorial(5.1)

I apologize for the complicated explanation above.  I found the code
involved to also be complicated and it took me a while to understand
it and arrive at this solution. 

Comments?

In testsuite/ChangeLog:

	* config/sid.exp (gdb_load): Add test for empty argument.  Adjust
	return value to be compatible with gdb_run_cmd in lib/gdb.exp.

Index: ./testsuite/config/sid.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/config/sid.exp,v
retrieving revision 1.9
diff -u -p -r1.9 sid.exp
--- ./testsuite/config/sid.exp	9 Jan 2007 17:59:08 -0000
1.9 +++ ./testsuite/config/sid.exp	13 Jul 2007 22:45:32 -0000
@@ -175,8 +175,11 @@ proc gdb_load { arg } {
     global gdb_prompt
     global retval
 
-    gdb_unload
-    if [gdb_file_cmd $arg] then { return -1 }
+    if { $arg != "" } {
+	gdb_unload
+	if [gdb_file_cmd $arg] then { return -1 }
+    }
+
     gdb_target_sid
 
     send_gdb "load\n"
@@ -195,7 +198,7 @@ proc gdb_load { arg } {
 	    if $verbose>1 then {
 		send_user "Loaded $arg into $GDB\n"
 	    }
-	    set retval 1;
+	    set retval 0;
 	}
 	-re "$gdb_prompt $"     {
 	    if $verbose>1 then {

Re: [RFC] Adjust SID-specific version of gdb_load to honor empty argument

[Daniel Jacobowitz]

On Fri, Jul 13, 2007 at 04:32:26PM -0700, Kevin Buettner wrote:
> Comments?
> 
> In testsuite/ChangeLog:
> 
> 	* config/sid.exp (gdb_load): Add test for empty argument.  Adjust
> 	return value to be compatible with gdb_run_cmd in lib/gdb.exp.

Yes, this looks right to me.  There's a lot of sid-specific bits
sprinkled throughout the test harness; if you're feeling motivated
(:-) the stuff I've done for gdbserver really should be flexible
enough to work for sid too, but if it ain't broke...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Adjust SID-specific version of gdb_load to honor empty argument

[Kevin Buettner]

Below is the patch that I ended up committing.   This version differs
from the patch that I originally submitted in that this version removes
the call to gdb_unload.  This change was suggested by Daniel in the
`Add another "Are you sure..."...' thread.  I've tested it and it works
fine.

	* config/sid.exp (gdb_load): Add test for empty argument.  Adjust
	return value to be compatible with gdb_run_cmd in lib/gdb.exp.
	Remove call to gdb_unload.

Index: testsuite/config/sid.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/config/sid.exp,v
retrieving revision 1.9
diff -u -p -r1.9 sid.exp
--- testsuite/config/sid.exp	9 Jan 2007 17:59:08 -0000	1.9
+++ testsuite/config/sid.exp	19 Jul 2007 00:40:33 -0000
@@ -175,8 +175,10 @@ proc gdb_load { arg } {
     global gdb_prompt
     global retval
 
-    gdb_unload
-    if [gdb_file_cmd $arg] then { return -1 }
+    if { $arg != "" } {
+	if [gdb_file_cmd $arg] then { return -1 }
+    }
+
     gdb_target_sid
 
     send_gdb "load\n"
@@ -195,7 +197,7 @@ proc gdb_load { arg } {
 	    if $verbose>1 then {
 		send_user "Loaded $arg into $GDB\n"
 	    }
-	    set retval 1;
+	    set retval 0;
 	}
 	-re "$gdb_prompt $"     {
 	    if $verbose>1 then {