Re: RFA: check executable file's timestamp before running

[Fernando Nasser <fnasser at redhat dot com>]

The new test that detects the bug is approved, of course.

I can't approve the fix for infcmd.c, but it does look reasonable
to me.

Fernando

Jim Blandy wrote:
> 
> It turns out that the logic for rereading debugging info when an
> executable file has changed is completely separate from the logic for
> re-opening `exec_bfd', the BFD the `exec' target layer uses for memory
> reads.
> 
> This patch adds a test that detects the bug, and a fix.  The fix seems
> reasonable, but I'm not sure it's right; there's too much stray state
> in that portion of GDB for me to really understand.  But it does fix
> the bug.
> 
> gdb/ChangeLog:
> 2001-12-07  Jim Blandy  <jimb@redhat.com>
> 
>         * infcmd.c (run_command): Check that the `exec' target layer's BFD
>         is up-to-date before running the program, not just when a program
>         exits.
> 
> gdb/testsuite/ChangeLog:
> 2001-12-07  Jim Blandy  <jimb@redhat.com>
> 
>         * gdb.base/reread.exp: Check that GDB properly re-reads the
>         executable file when it changes while no inferior is running.
> 
> Index: gdb/infcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infcmd.c,v
> retrieving revision 1.35
> diff -c -r1.35 infcmd.c
> *** gdb/infcmd.c        2001/11/27 03:09:44     1.35
> --- gdb/infcmd.c        2001/12/07 18:26:14
> ***************
> *** 391,406 ****
> 
>     clear_breakpoint_hit_counts ();
> 
> -   exec_file = (char *) get_exec_file (0);
> -
>     /* Purge old solib objfiles. */
>     objfile_purge_solibs ();
> 
>     do_run_cleanups (NULL);
> 
> !   /* The exec file is re-read every time we do a generic_mourn_inferior, so
> !      we just have to worry about the symbol file.  */
>     reread_symbols ();
> 
>     /* We keep symbols from add-symbol-file, on the grounds that the
>        user might want to add some symbols before running the program
> --- 391,412 ----
> 
>     clear_breakpoint_hit_counts ();
> 
>     /* Purge old solib objfiles. */
>     objfile_purge_solibs ();
> 
>     do_run_cleanups (NULL);
> 
> !   /* The comment here used to read, "The exec file is re-read every
> !      time we do a generic_mourn_inferior, so we just have to worry
> !      about the symbol file."  The `generic_mourn_inferior' function
> !      gets called whenever the program exits.  However, suppose the
> !      program exits, and *then* the executable file changes?  We need
> !      to check again here.  Since reopen_exec_file doesn't do anything
> !      if the timestamp hasn't changed, I don't see the harm.  */
> !   reopen_exec_file ();
>     reread_symbols ();
> +
> +   exec_file = (char *) get_exec_file (0);
> 
>     /* We keep symbols from add-symbol-file, on the grounds that the
>        user might want to add some symbols before running the program
> Index: gdb/testsuite/gdb.base/reread.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/reread.exp,v
> retrieving revision 1.4
> diff -c -r1.4 reread.exp
> *** gdb/testsuite/gdb.base/reread.exp   2001/03/06 08:21:51     1.4
> --- gdb/testsuite/gdb.base/reread.exp   2001/12/07 18:26:15
> ***************
> *** 87,93 ****
> 
>   # Restore first executable to its original name, and move
>   # second executable into its place.  Ensure that the new
> ! # executable is at least a second older than the old.
> 
>   gdb_test "shell mv ${binfile} ${binfile1}" "" ""
>   gdb_test "shell mv ${binfile2} ${binfile}" "" ""
> --- 87,93 ----
> 
>   # Restore first executable to its original name, and move
>   # second executable into its place.  Ensure that the new
> ! # executable is at least a second newer than the old.
> 
>   gdb_test "shell mv ${binfile} ${binfile1}" "" ""
>   gdb_test "shell mv ${binfile2} ${binfile}" "" ""
> ***************
> *** 98,103 ****
> --- 98,105 ----
>   # and reset the breakpoints correctly.
>   # Should see "Breakpoint 1, foo () at reread2.c:9"
> 
> + set prms_id 0
> +
>   if ![isnative] {
>       unsupported "run to foo() second time ";
>   } else {
> ***************
> *** 113,118 ****
> --- 115,182 ----
>         }
>         timeout {
>             fail "run to foo() second time (timeout)" ;
> +           gdb_suppress_tests
> +       }
> +     }
> + }
> +
> +
> + ### Second pass: verify that GDB checks the executable file's
> + ### timestamp when the program is *restarted*, not just when it exits.
> +
> + if ![isnative] {
> +     unsupported "second pass: GDB should check for changes before running"
> + } else {
> +
> +     # Put the older executable back in place.
> +     gdb_test "shell mv ${binfile} ${binfile2}" "" ""
> +     gdb_test "shell mv ${binfile1} ${binfile}" "" ""
> +
> +     # Restart GDB entirely.
> +     gdb_start
> +     gdb_reinitialize_dir $srcdir/$subdir
> +     gdb_load ${binfile}
> +
> +     # Set a breakpoint on foo and run to it.
> +     gdb_test "break foo" \
> +             "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
> +             "second pass: breakpoint foo in first file"
> +     gdb_run_cmd
> +     gdb_expect {
> +         -re ".*Breakpoint.* foo .* at .*$srcfile1:14.*$gdb_prompt $"  {
> +             pass "second pass: run to foo()";
> +         }
> +         -re ".*$gdb_prompt $" {
> +             fail "second pass: run to foo()";
> +             gdb_suppress_tests;
> +         }
> +         timeout {
> +             fail "second pass: run to foo() (timeout)"
> +             gdb_suppress_tests
> +         }
> +     }
> +
> +     # This time, let the program run to completion.  If GDB checks the
> +     # executable file's timestamp now, it won't notice any change.
> +     gdb_test "continue" ".*Program exited.*" \
> +             "second pass: continue to completion"
> +
> +     # Now move the newer executable into place, and re-run.  GDB
> +     # should still notice that the executable file has changed,
> +     # and still re-set the breakpoint appropriately.
> +     gdb_test "shell mv ${binfile} ${binfile1}" "" ""
> +     gdb_test "shell mv ${binfile2} ${binfile}" "" ""
> +     gdb_run_cmd
> +     gdb_expect {
> +       -re ".*Breakpoint.* foo .* at .*:9.*$gdb_prompt $" {
> +           pass "second pass: run to foo() second time ";
> +       }
> +       -re ".*$gdb_prompt $" {
> +           fail "second pass: run to foo() second time";
> +           gdb_suppress_tests;
> +       }
> +       timeout {
> +           fail "second pass: run to foo() second time (timeout)" ;
>             gdb_suppress_tests
>         }
>       }

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9