[RFC] Entry point update with "run" command

[RFC] Entry point update with "run" command

[Luis Machado]

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

Hi folks,

It seems GDB doesn't really update the entry point of an object file if
we "reload" the modified binary through the "run" command.

This can be clearly seen with the reread.exp testcase:

1 - Run reread until "foo" breakpoint
2 - Move reread2 on top of reread
3 - "Run" and expect GDB to notice that the binary has changed
4 - GDB says it will re-read the symbols
5 - The entry point of the second binary is just the same as the first
one. This causes problems with the displaced stepping since the entry
point is used to store the instructions.

The problem is that "init_entry_point_info" is never called with the
"run" command. If we do a "file", then we get the right entry point.

The attached patch does this on "reread_symbols", though it seems a
brute-force method. Is this OK?

Regards

-- 
Luis Machado


[-- Attachment #2: entry_point.diff --]
[-- Type: text/x-patch, Size: 405 bytes --]

Index: hg/gdb/symfile.c
===================================================================
--- hg.orig/gdb/symfile.c	2008-04-24 08:09:30.000000000 -0700
+++ hg/gdb/symfile.c	2008-04-24 08:45:50.000000000 -0700
@@ -2467,6 +2467,7 @@
 	      objfile->mtime = new_modtime;
 	      reread_one = 1;
               reread_separate_symbols (objfile);
+	      init_entry_point_info (objfile);
 	    }
 	}
     }

Re: [RFC] Entry point update with "run" command

[Jan Kratochvil]

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

On Thu, 24 Apr 2008 17:58:14 +0200, Luis Machado wrote:
> Hi folks,
> 
> It seems GDB doesn't really update the entry point of an object file if
> we "reload" the modified binary through the "run" command.

There is a problem whole EXEC_BFD becomes stale - using the more general
attached patch.

Still I agree it is a hack as besides whole reload-on-change is a hack also GDB
still misses some real OO framework.


Regards,
Jan

[-- Attachment #2: gdb-6.7-reread-exec_bfd.patch --]
[-- Type: text/plain, Size: 1799 bytes --]

2007-10-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile.c (reread_symbols): Reread also EXEC_BFD if changed.

2008-04-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile.c (reread_symbols): Reload whole EXEC_BFD by the EXEC module
	as its in-place patching did cause regressions.

Testcase: Regressed by the gdb-6.7 version of `gdb-6.3-pie-20050110.patch':
 Running ../../../gdb/testsuite/gdb.base/reread.exp ...
 PASS: gdb.base/reread.exp: breakpoint foo in first file (PRMS 13484)
 PASS: gdb.base/reread.exp: run to foo() (PRMS 13484)
-PASS: gdb.base/reread.exp: run to foo() second time
+FAIL: gdb.base/reread.exp: run to foo() second time
 PASS: gdb.base/reread.exp: second pass: breakpoint foo in first file
-PASS: gdb.base/reread.exp: second pass: run to foo()
-PASS: gdb.base/reread.exp: second pass: continue to completion
-PASS: gdb.base/reread.exp: second pass: run to foo() second time
+FAIL: gdb.base/reread.exp: second pass: run to foo()
+FAIL: gdb.base/reread.exp: second pass: continue to completion
+FAIL: gdb.base/reread.exp: second pass: run to foo() second time

--- gdb-6.7-orig/gdb/symfile.c	2007-10-29 01:04:35.000000000 +0100
+++ gdb-6.7-patched/gdb/symfile.c	2007-10-29 01:03:13.000000000 +0100
@@ -2810,6 +2810,12 @@ reread_symbols (void)
 	      /* We need to do this whenever any symbols go away.  */
 	      make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
 
+	      if (exec_bfd != NULL && strcmp (bfd_get_filename (objfile->obfd),
+					      bfd_get_filename (exec_bfd)) == 0)
+		{
+		  exec_ops.to_open (bfd_get_filename (objfile->obfd), 0);
+		}
+
 	      /* Clean up any state BFD has sitting around.  We don't need
 	         to close the descriptor but BFD lacks a way of closing the
 	         BFD without closing the descriptor.  */

Re: [RFC] Entry point update with "run" command

[Luis Machado]

The attached patch doesn't fix things in case displaced stepping is
being used since the entry point is not updated.

Regards,
Luis

On Sat, 2008-04-26 at 15:22 +0200, Jan Kratochvil wrote:
> +             if (exec_bfd != NULL && strcmp (bfd_get_filename
> (objfile->obfd),
> +                                             bfd_get_filename
> (exec_bfd)) == 0)
> +               {
> +                 exec_ops.to_open (bfd_get_filename (objfile->obfd),
> 0);
> +               }
> +

Re: [RFC] Entry point update with "run" command

[Daniel Jacobowitz]

On Thu, Apr 24, 2008 at 12:58:14PM -0300, Luis Machado wrote:
> The attached patch does this on "reread_symbols", though it seems a
> brute-force method. Is this OK?

This is OK.  Wow, this function is a wreck.  Despite the comments it
really does need to be updated to share code with initial reading of
the symfile at some point.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Entry point update with "run" command

[Daniel Jacobowitz]

On Sat, Apr 26, 2008 at 03:22:25PM +0200, Jan Kratochvil wrote:
> There is a problem whole EXEC_BFD becomes stale - using the more general
> attached patch.

Does this work if you just call exec_file_attach instead of
exec_ops.to_open?  The difference is target_preopen which I do not
believe we want here.

If that works, it's OK to commit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Entry point update with "run" command

[Jan Kratochvil]

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

On Fri, 02 May 2008 17:44:47 +0200, Daniel Jacobowitz wrote:
> On Sat, Apr 26, 2008 at 03:22:25PM +0200, Jan Kratochvil wrote:
> > There is a problem whole EXEC_BFD becomes stale - using the more general
> > attached patch.
> 
> Does this work if you just call exec_file_attach instead of
> exec_ops.to_open?  The difference is target_preopen which I do not
> believe we want here.

Thanks, it works (I was tryint to use exec_file_command instead before).


> If that works, it's OK to commit.

Committed, no regressions on native x86_64-unknown-linux-gnu.


Regards,
Jan

[-- Attachment #2: bfd-reread.patch --]
[-- Type: text/plain, Size: 992 bytes --]

2008-05-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile.c (reread_symbols): Reload EXEC_BFD on its change.

===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.201
retrieving revision 1.202
diff -u -r1.201 -r1.202
--- src/gdb/symfile.c	2008/05/04 03:45:42	1.201
+++ src/gdb/symfile.c	2008/05/04 14:34:06	1.202
@@ -2331,6 +2331,14 @@
 	      /* We need to do this whenever any symbols go away.  */
 	      make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
 
+	      if (exec_bfd != NULL && strcmp (bfd_get_filename (objfile->obfd),
+					      bfd_get_filename (exec_bfd)) == 0)
+		{
+		  /* Reload EXEC_BFD without asking anything.  */
+
+		  exec_file_attach (bfd_get_filename (objfile->obfd), 0);
+		}
+
 	      /* Clean up any state BFD has sitting around.  We don't need
 	         to close the descriptor but BFD lacks a way of closing the
 	         BFD without closing the descriptor.  */

Re: [RFC] Entry point update with "run" command

[Luis Machado]

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

I've checked this in now. Thanks!

On Fri, 2008-05-02 at 11:43 -0400, Daniel Jacobowitz wrote:
> On Thu, Apr 24, 2008 at 12:58:14PM -0300, Luis Machado wrote:
> > The attached patch does this on "reread_symbols", though it seems a
> > brute-force method. Is this OK?
> 
> This is OK.  Wow, this function is a wreck.  Despite the comments it
> really does need to be updated to share code with initial reading of
> the symfile at some point.
> 

Luis


[-- Attachment #2: entry_point.diff --]
[-- Type: text/x-patch, Size: 521 bytes --]

2008-05-05  Luis Machado  <luisgpm@br.ibm.com>

	* symfile.c (reread_symbols): Update objfile's entry point.

Index: HEAD/gdb/symfile.c
===================================================================
--- HEAD.orig/gdb/symfile.c	2008-05-05 05:27:16.000000000 -0700
+++ HEAD/gdb/symfile.c	2008-05-05 05:31:54.000000000 -0700
@@ -2474,6 +2474,7 @@
 	      objfile->mtime = new_modtime;
 	      reread_one = 1;
               reread_separate_symbols (objfile);
+	      init_entry_point_info (objfile);
 	    }
 	}
     }