[commit] Don't call deprecated_inside_entry_file from ...id_unwind()

[commit] Don't call deprecated_inside_entry_file from ...id_unwind()

[Andrew Cagney]

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

Hello,

Sometime ago, while developing the frame unwind code using the d10v, I 
added an innocent looking hack that stopped the d10v falling off the end 
of its stack:

-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame.  */
-  if (func <= IMEM_START || inside_entry_file (func))
-    return;

That logic being lifed from even older frame chain code.

I then later fixed the underlying problem (inside_main_func was broken), 
but forgot to remove that hack.

Oops!

That innocent looking code as quitely spread to at least 4 other 
architectures (there was no comment saying "hey you don't need this").

Anyway, the attached patch removes all occurance of the hack.

committed,
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 4478 bytes --]

2003-10-31  Andrew Cagney  <cagney@redhat.com>

	* avr-tdep.c (avr_frame_this_id): Do not call
	deprecated_inside_entry_file.
	* m68hc11-tdep.c (m68hc11_frame_this_id): Ditto.
	* m32r-tdep.c (m32r_frame_this_id): Ditto.
	* d10v-tdep.c (d10v_frame_this_id): Ditto.
	* arm-tdep.c (arm_prologue_this_id): Ditto.
	* alpha-tdep.c (alpha_heuristic_frame_this_id): Ditto.

Index: alpha-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.c,v
retrieving revision 1.122
diff -u -r1.122 alpha-tdep.c
--- alpha-tdep.c	14 Sep 2003 16:32:12 -0000	1.122
+++ alpha-tdep.c	31 Oct 2003 23:46:17 -0000
@@ -1136,11 +1136,6 @@
   struct alpha_heuristic_unwind_cache *info
     = alpha_heuristic_frame_unwind_cache (next_frame, this_prologue_cache, 0);
 
-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame. */
-  if (deprecated_inside_entry_file (info->start_pc))
-    return;
-
   *this_id = frame_id_build (info->vfp, info->start_pc);
 }
 
Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.153
diff -u -r1.153 arm-tdep.c
--- arm-tdep.c	2 Oct 2003 20:28:28 -0000	1.153
+++ arm-tdep.c	31 Oct 2003 23:46:17 -0000
@@ -994,7 +994,7 @@
 
   /* This is meant to halt the backtrace at "_start".  Make sure we
      don't halt it at a generic dummy frame. */
-  if (func <= LOWEST_PC || deprecated_inside_entry_file (func))
+  if (func <= LOWEST_PC)
     return;
 
   /* If we've hit a wall, stop.  */
Index: avr-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/avr-tdep.c,v
retrieving revision 1.72
diff -u -r1.72 avr-tdep.c
--- avr-tdep.c	17 Oct 2003 18:24:49 -0000	1.72
+++ avr-tdep.c	31 Oct 2003 23:46:18 -0000
@@ -970,11 +970,6 @@
   /* The FUNC is easy.  */
   func = frame_func_unwind (next_frame);
 
-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame. */
-  if (deprecated_inside_entry_file (func))
-    return;
-
   /* Hopefully the prologue analysis either correctly determined the
      frame's base (which is the SP from the previous frame), or set
      that base to "NULL".  */
Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.135
diff -u -r1.135 d10v-tdep.c
--- d10v-tdep.c	10 Oct 2003 21:59:05 -0000	1.135
+++ d10v-tdep.c	31 Oct 2003 23:46:18 -0000
@@ -1405,11 +1405,6 @@
   /* The FUNC is easy.  */
   func = frame_func_unwind (next_frame);
 
-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame.  */
-  if (func <= IMEM_START || deprecated_inside_entry_file (func))
-    return;
-
   /* Hopefully the prologue analysis either correctly determined the
      frame's base (which is the SP from the previous frame), or set
      that base to "NULL".  */
Index: m32r-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m32r-tdep.c,v
retrieving revision 1.19
diff -u -r1.19 m32r-tdep.c
--- m32r-tdep.c	8 Oct 2003 02:41:49 -0000	1.19
+++ m32r-tdep.c	31 Oct 2003 23:46:18 -0000
@@ -831,11 +831,6 @@
   /* The FUNC is easy.  */
   func = frame_func_unwind (next_frame);
 
-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame. */
-  if (deprecated_inside_entry_file (func))
-    return;
-
   /* Check if the stack is empty.  */
   msym_stack = lookup_minimal_symbol ("_stack", NULL, NULL);
   if (msym_stack && info->base == SYMBOL_VALUE_ADDRESS (msym_stack))
Index: m68hc11-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68hc11-tdep.c,v
retrieving revision 1.91
diff -u -r1.91 m68hc11-tdep.c
--- m68hc11-tdep.c	28 Sep 2003 22:32:19 -0000	1.91
+++ m68hc11-tdep.c	31 Oct 2003 23:46:18 -0000
@@ -894,11 +894,6 @@
   /* The FUNC is easy.  */
   func = frame_func_unwind (next_frame);
 
-  /* This is meant to halt the backtrace at "_start".  Make sure we
-     don't halt it at a generic dummy frame. */
-  if (deprecated_inside_entry_file (func))
-    return;
-
   /* Hopefully the prologue analysis either correctly determined the
      frame's base (which is the SP from the previous frame), or set
      that base to "NULL".  */

Re: [commit] Don't call deprecated_inside_entry_file from ...id_unwind()

[Daniel Jacobowitz]

On Fri, Oct 31, 2003 at 07:00:09PM -0500, Andrew Cagney wrote:
> Hello,
> 
> Sometime ago, while developing the frame unwind code using the d10v, I 
> added an innocent looking hack that stopped the d10v falling off the end 
> of its stack:
> 
> -  /* This is meant to halt the backtrace at "_start".  Make sure we
> -     don't halt it at a generic dummy frame.  */
> -  if (func <= IMEM_START || inside_entry_file (func))
> -    return;
> 
> That logic being lifed from even older frame chain code.
> 
> I then later fixed the underlying problem (inside_main_func was broken), 
> but forgot to remove that hack.
> 
> Oops!
> 
> That innocent looking code as quitely spread to at least 4 other 
> architectures (there was no comment saying "hey you don't need this").
> 
> Anyway, the attached patch removes all occurance of the hack.

Are you certain that none of those other architectures needed the hack
anyway?

Also, snce GDB does support backtracing when main isn't even in the
equation, I don't think we should break that unless the check is
actually harmful.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [commit] Don't call deprecated_inside_entry_file from ...id_unwind()

[Andrew Cagney]

> Are you certain that none of those other architectures needed the hack
> anyway?

To sound like a scratched record, until there is hard evidence that this 
test is needed it should _not_ be enabled.  So far the only evidence is 
that it is anything but needed:

   /* NOTE: vinschen/2003-04-01: Disabled.  It turns out that the call
      to deprecated_inside_entry_file destroys a meaningful backtrace
      under some conditions.  E. g. the backtrace tests in the
      asm-source testcase are broken for some targets.  In this test
      the functions are all implemented as part of one file and the
      testcase is not necessarily linked with a start file (depending
      on the target).  What happens is, that the first frame is printed
      normaly and following frames are treated as being inside the
      enttry file then.  This way, only the #0 frame is printed in the
      backtrace output.  */

Also, as I noted:

 > That innocent looking code as quitely spread to at least 4 other
 > architectures (there was no comment saying "hey you don't need this").

the mere presence of that code in those up-to-date targets was a great 
big foobar.

> Also, snce GDB does support backtracing when main isn't even in the
> equation, I don't think we should break that unless the check is
> actually harmful.

The only testcase that comes close to having a problem is asm-source and 
that breaks with the test present.

You'll note that I've not touched legacy targets.

Andrew

Re: [commit] Don't call deprecated_inside_entry_file from ...id_unwind()

[Daniel Jacobowitz]

On Fri, Oct 31, 2003 at 09:02:04PM -0500, Andrew Cagney wrote:
> 
> >Are you certain that none of those other architectures needed the hack
> >anyway?
> 
> To sound like a scratched record, until there is hard evidence that this 
> test is needed it should _not_ be enabled.  So far the only evidence is 
> that it is anything but needed:
> 
>   /* NOTE: vinschen/2003-04-01: Disabled.  It turns out that the call
>      to deprecated_inside_entry_file destroys a meaningful backtrace
>      under some conditions.  E. g. the backtrace tests in the
>      asm-source testcase are broken for some targets.  In this test
>      the functions are all implemented as part of one file and the
>      testcase is not necessarily linked with a start file (depending
>      on the target).  What happens is, that the first frame is printed
>      normaly and following frames are treated as being inside the
>      enttry file then.  This way, only the #0 frame is printed in the
>      backtrace output.  */
> 
> Also, as I noted:

I was confusing inside_entry_file and inside_entry_func again.  My
apologies.

I'm still pretty confident that the test was necessary when I added it
to the ARM target, i.e. something broke without it - I remember not
copying and pasting it, but actually going looking for it.  I'll retest
when I get the time.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer