Re: [rfa:symtab] deprecate inside_entry_func

[Daniel Jacobowitz <drow@mvista.com>]

On Fri, Oct 31, 2003 at 09:37:24PM -0500, Andrew Cagney wrote:
> >On Fri, Oct 31, 2003 at 07:07:28PM -0500, Andrew Cagney wrote:
> >
> >>Hello,
> >>
> >>This patch deprecates the function inside_entry_func.  Per the new 
> >>comments:
> >>
> >>+  /* NOTE: cagney/2003-10-31: A very simple test, such as
> >>+     get_frame_func == entry_point should be sufficient for
> >>+     identifying a pc in the entry function.  Does anyone know why it
> >>+     wasn't sufficient and hence, why the very convoluted
> >>+     "deprecated_inside_entry_func" is needed.  */
> >>+  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
> >>+     and implement a clean frame termination.  Not doing that is
> >>+     really a bug in the ABI/crt0, and, hence, not a reason for
> >>+     enabling the call to deprecated_inside_entry_func.  */
> >
> >
> >If handling broken systems isn't the job of GDB, then I surely don't
> >know what is.
> >Do you have a reason for wanting to deprecate this function?
> 
> Per my first new comment:
> 
> /* NOTE: cagney/2003-10-31: A very simple test, such as
>    get_frame_func == entry_point should be sufficient for
>    identifying a pc in the entry function.  Does anyone know why it
>    wasn't sufficient and hence, why the very convoluted
>    "deprecated_inside_entry_func" is needed.  */
> 
> and the previous comment:
> 
>   /* NOTE: cagney/2003-02-25: Don't enable until someone has found
>      hard evidence that this is needed.  */
> 
> There is _still_ no evidence that this disabled hack is needed - time to 
> deprecate it.

If you think about it for a little while, and if you consider my
objection to the comment about broken ABIs above, constructing a
testcase is really quite easy.  Here's one, though it's contrived. 
Bear with me.

Take gdb.asm/asm-source (or anything else, really), for an i386 target,
and this script:

b _start
r
bt
set $ebp = 2
bt


Here's what GDB produces:

Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1". 
	[Hrm, is there a missing from_tty somewhere?  This seems
	superfluous.]
Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14.

Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
14		gdbasm_startup
Current language:  auto; currently asm
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#1  0x00000001 in ?? ()
#2  0xbffff680 in ?? ()


Apply this patch:

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.147
diff -u -p -r1.147 frame.c
--- frame.c	24 Oct 2003 17:37:03 -0000	1.147
+++ frame.c	8 Nov 2003 23:58:31 -0000
@@ -1831,7 +1831,7 @@ get_prev_frame (struct frame_info *this_
   /* NOTE: cagney/2003-07-15: Need to add a "set backtrace
      beyond-entry-func" command so that this can be selectively
      disabled.  */
-  if (0
+  if (1
 #if 0
       && backtrace_beyond_entry_func
 #endif


and GDB's output isn't changed.  That's because of the hokeyness of
figuring out the bounds of the entry function.  Apply this patch
instead, though (proof of concept only; it needs to be cleaned up and
inside_entry_func's signature changed, but it illustrates the point):

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.82
diff -u -p -r1.82 blockframe.c
--- blockframe.c	20 Oct 2003 14:38:42 -0000	1.82
+++ blockframe.c	8 Nov 2003 23:58:30 -0000
@@ -186,6 +186,9 @@ inside_entry_func (CORE_ADDR pc)
       if (DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0))
 	return 0;
     }
+  if (symfile_objfile->ei.entry_point == pc)
+    return 1;
+
   return (symfile_objfile->ei.entry_func_lowpc <= pc &&
 	  symfile_objfile->ei.entry_func_highpc > pc);
 }
@@ -615,7 +618,7 @@ legacy_frame_chain_valid (CORE_ADDR fp, 
 
   /* If we're already inside the entry function for the main objfile, then it
      isn't valid.  */
-  if (inside_entry_func (get_frame_pc (fi)))
+  if (inside_entry_func (get_frame_func (fi)))
     return 0;
 
   /* If we're inside the entry file, it isn't valid.  */
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.147
diff -u -p -r1.147 frame.c
--- frame.c	24 Oct 2003 17:37:03 -0000	1.147
+++ frame.c	8 Nov 2003 23:58:31 -0000
@@ -1831,12 +1831,12 @@ get_prev_frame (struct frame_info *this_
   /* NOTE: cagney/2003-07-15: Need to add a "set backtrace
      beyond-entry-func" command so that this can be selectively
      disabled.  */
-  if (0
+  if (1
 #if 0
       && backtrace_beyond_entry_func
 #endif
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
-      && inside_entry_func (get_frame_pc (this_frame)))
+      && inside_entry_func (get_frame_func (this_frame)))
     {
       if (frame_debug)
 	{


and we get this output:

Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14.

Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
14		gdbasm_startup
Current language:  auto; currently asm
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14


Which is, I think, an obvious improvement.  If the object file has an
entry point, by default we should not try to backtrace past it.  The
extra "set backtrace" option mentioned in your comment above is
probably called for.



Think my testcase was too contrived?  It's possible to rewrite
asm-source in a number of ways to acheive similar effect.  I believe
I've also produced this result in the ARM simulator using newlib, but I
don't have that setup around to recreate it at the moment.  You could
probably do it using static linking and -fomit-frame-pointer, except
that GDB on my host system gets lost in __libc_start_main in that case
and never gets to _start.


I believe the function should be fixed and re-enabled, not deleted.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer