fix load command bug [was : Re: Make remote-sim target always have a thread]

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 4070 bytes --]

A Thursday 03 July 2008 19:04:20, Pedro Alves wrote:

> Managed to get myself an arm-elf toolchain and tested with a
> x86_64-unknow-linux-gnu x arm-elf, --target_board=arm-sim,
> without regressions.  Does it look OK?
>
> Default results look much better than I was expecting without
> tweaking the boardfile.
>

> FAIL: gdb.base/chng-syms.exp: running with invalidated bpt condition after
> executable changes

Actually, looking back, this one was hiding in the middle.
It's a real bug.

1708    static void
1709    load_command (char *arg, int from_tty)
1710    {
1711      if (arg == NULL)
1712        {
1713          char *parg;
1714          int count = 0;
1715
1716          parg = arg = get_exec_file (1);
1717
1718          /* Count how many \ " ' tab space there are in the name.  */
1719          while ((parg = strpbrk (parg, "\\\"'\t ")))
1720            {
1721              parg++;
1722              count++;
1723            }
1724
1725          if (count)
1726            {
1727              /* We need to quote this string so buildargv can pull it apart.  */
1728              char *temp = xmalloc (strlen (arg) + count + 1 );
1729              char *ptemp = temp;
1730              char *prev;
1731
1732              make_cleanup (xfree, temp);
1733
1734              prev = parg = arg;
1735              while ((parg = strpbrk (parg, "\\\"'\t ")))
1736                {
1737                  strncpy (ptemp, prev, parg - prev);
1738                  ptemp += parg - prev;
1739                  prev = parg++;
1740                  *ptemp++ = '\\';
1741                }
1742              strcpy (ptemp, prev);
1743
1744              arg = temp;
1745            }
1746        }
1747
1748      /* The user might be reloading because the binary has changed.  Take
1749         this opportunity to check.  */
1750      reopen_exec_file ();
1751      reread_symbols ();
1752
1753      target_load (arg, from_tty);
1754
1755      /* After re-loading the executable, we don't really know which
1756         overlays are mapped any more.  */
1757      overlay_cache_invalid = 1;
1758    }


 Executing on host: arm-elf-gcc ../../../src/gdb/testsuite/gdb.base/chng-syms.c 
gdb_tg.o  -DVARIABLE=var2      -g  -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm   -o /home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms   
(timeout = 300)
 target sim
 Connected to the simulator.
 (gdb) load
 `/home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms' has changed; re-reading symbols.
 warning: failed to reevaluate condition for breakpoint 1: No symbol "var1" in current context.
 gdbsim: can't open "": No such file or directory
 unable to load program
 (gdb) run
 Starting program: /home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms
 warning: No program loaded.

 *** EXIT code 0

 Program exited normally.
 (gdb) FAIL: gdb.base/chng-syms.exp: running with invalidated bpt condition after executable changes

Notice the 'can't open ""' part.

Sometimes I'd get "#33sfads" kind of garbage instead of "".
Surprisingly, I can't seem to reproduce it that easilly now that I
fixed the issue.  The same heap address is probably being reused,
and the file name didn't change, so it goes unnoticed many times.

The issue is that reopen_exec_file destroys the previous exec_file
memory, which invalidates the arg that is passed into target_load.

The fix it to move the reopening of the exec file to the top of load_command.

The other problem is that on the sym target, the 
warning: failed to reevaluate condition for breakpoint 1: No symbol "var1" in current context.

notice is printed on load, but not after a run, as after a run is issued,
the inferior doesn't stop until it exits.

So, I've included a tweak to the test file to allow this.  If the inferior runs
to completion, it means the breakpoint wasn't sucessfully reset, and that's
what is expected in this test to happen.

Test on arm-elf, and x86_64-unknown-linux-gnu

OK?

-- 
Pedro Alves

[-- Attachment #2: fix_load_reload_syms.diff --]
[-- Type: text/x-diff, Size: 2131 bytes --]

gdb/
2008-07-04  Pedro Alves  <pedro@codesourcery.com>

	* symfile.c (load_command): Reopen the exec file and reread
	symbols before anything else.

gdb/testsuite/
2008-07-04  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/chng-syms.exp: Don't expect "No symbol ...".

---
 gdb/symfile.c                        |   10 +++++-----
 gdb/testsuite/gdb.base/chng-syms.exp |    4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

Index: src/gdb/symfile.c
===================================================================
--- src.orig/gdb/symfile.c	2008-07-04 15:53:26.000000000 +0100
+++ src/gdb/symfile.c	2008-07-04 15:53:48.000000000 +0100
@@ -1708,6 +1708,11 @@ find_sym_fns (bfd *abfd)
 static void
 load_command (char *arg, int from_tty)
 {
+  /* The user might be reloading because the binary has changed.  Take
+     this opportunity to check.  */
+  reopen_exec_file ();
+  reread_symbols ();
+
   if (arg == NULL)
     {
       char *parg;
@@ -1745,11 +1750,6 @@ load_command (char *arg, int from_tty)
 	}
     }
 
-  /* The user might be reloading because the binary has changed.  Take
-     this opportunity to check.  */
-  reopen_exec_file ();
-  reread_symbols ();
-
   target_load (arg, from_tty);
 
   /* After re-loading the executable, we don't really know which
Index: src/gdb/testsuite/gdb.base/chng-syms.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/chng-syms.exp	2008-07-04 15:53:26.000000000 +0100
+++ src/gdb/testsuite/gdb.base/chng-syms.exp	2008-07-04 15:53:48.000000000 +0100
@@ -105,10 +105,10 @@ if  { [gdb_compile "${srcdir}/${subdir}/
 
     gdb_run_cmd
     gdb_expect {
-	-re ".*No symbol .var1..*Program exited normally.*$gdb_prompt $" {
+	-re ".*Program exited normally.*$gdb_prompt $" {
 	    pass "running with invalidated bpt condition after executable changes" 
 	}
-	-re "No symbol .var1..*Breakpoint .*,( 0x.* in)? (\[^ \]*)exit .*$gdb_prompt $" {
+	-re ".*Breakpoint .*,( 0x.* in)? (\[^ \]*)exit .*$gdb_prompt $" {
 	    pass "running with invalidated bpt condition after executable changes" 
 	}
 	-re "$gdb_prompt $" {