This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/rfc/rfa*] Only compare against dummy frame tos when saved



I think this change will be okay for ia64 so long as the following
FIXME comment in ia64-tdep.c is fixed:

/* FIXME: This doesn't belong here! Instead, SAVE_DUMMY_FRAME_TOS needs
to be defined to call generic_save_dummy_frame_tos(). But at the
time of this writing, SAVE_DUMMY_FRAME_TOS wasn't gdbarch'd, so
I chose to put this call here instead of using the old mechanisms. Once SAVE_DUMMY_FRAME_TOS is gdbarch'd, all we need to do is add the
line

set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);

to ia64_gdbarch_init() and remove the line below. */

At the moment, ia64 depends upon ->top being set, but your change will
disable the appropriate test because ia64-tdep.c doesn't (correctly) set
the SAVE_DUMMY_FRAME_TOS method.

I'll try to find a moment or two to give it a try...
No need. I skipped ``plan b'' and went straight onto ``plan c''.

The attached revised patch modifies generic_find_dummy_frame() so that, as before, it only tests TOP when it was set. However, this time it uses the value of TOP instead of find_dummy_frame_p(). I think this is more robust since, as with the ia64, it allows an architecture to explicitly set TOS (rather than rely on hand_function_call() to set it).

I should note that, per the rs6000, this change is likely to flush out some latent architecture bugs and, as a consequence, cause regressions :-( Sigh!

There is going to be a trade off here .... :-/

Andrew



The original description was:

The generic code for finding a previously saved dummy frame, given the PC and ``frame'', looks like:

static struct regcache *
generic_find_dummy_frame (CORE_ADDR pc, CORE_ADDR fp)
{
struct dummy_frame *dummyframe;

for (dummyframe = dummy_frame_stack; dummyframe != NULL;
dummyframe = dummyframe->next)
if ((pc >= dummyframe->call_lo && pc < dummyframe->call_hi)
&& (fp == dummyframe->fp
|| fp == dummyframe->sp
|| fp == dummyframe->top))
/* The frame in question lies between the saved fp and sp, inclusive */
return dummyframe->regcache;

return 0;
}

I think it is trying to handle two cases:

1. Where the FP passed in is the dummy frames true base and should match SP of the previous frame or FP of that frame.

2. Where the FP passed in is the frame base from the next inner most frame (returned by frame_chain()) it should match the saved top-of-stack value (using SAVE_DUMMY_FRAME_TOS()).

If a target is somehow capable of unwinding through the dummy frame back to its base then #1 is used. If, however, it isn't possible to unwind back through a [generic] dummy frame, then targets use #2. Case #2 is easy to spot because the frame_chain code starts with:

if (PC_IN_CALL_DUMMY(frame_saved_pc(frame)...)
return frame->frame;

Unfortunatly, the test is too generous and a problem occures when using #2 while trying to unwind through frameless functions :-(

0: innermost() fp=30 top=40
1: <dummy frame> fp=20 top=30
2: frameless() fp=20 top=20
3: <dummy frame> fp=10 top=20

When trying to unwind 2:frameless(), GDB calls frame_saved_pc()=<dummy frame> and frame_chain()=20. These two values are then used to find saved the dummy frame on the dummy_frame_stack. The above test is so generous, though, that it incorrectly matches the first dummy frame (#1, 20 == 1:fp) instead of (#3 20 == 3:top). This will send GDB into a backtrace loop #0, #1, #2, #1, #2, #1 #2 ....

The attached patch fixes this problem by being more careful about which of the above tests is used and when. the ``fp == dummyframe->top'' test is used IFF top was saved. The other tests IFF top wasn't saved.

This change does, however, have the potential to break break every target that sets save_dummy_frame_tos(). Going through the list of targets (and treating any with a frame_chain() like the above as ok):

cris-tdep.c: ok
ia64-tdep.c: ok
mcore-tdep.c: *** I suspect it doesn't but should
mn10300-tdep.c: *** I suspect it doesn't but should
rs6000-tdep.c: ok
s390-tdep.c: ok
xstormy16-tdep.c: ok (I think).

I think, for the two *** cases, the code can be tweaked to have frame_chain() return frame->frame.

Thoughts? Ok for the targets with maintainers?

I'll need this or something similar to get MIPS generic dummy frames working.

Oh, and I have a plan B: Add an extra gdbarch flag that, when set, enables the more strict tests.

Andrew

2002-09-25  Andrew Cagney  <ac131313@redhat.com>

	* blockframe.c (generic_find_dummy_frame): Rewrite.  Only test
	against TOP when TOP was explictly set.
	(generic_push_dummy_frame): Set TOP to zero.

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.40
diff -u -r1.40 blockframe.c
--- blockframe.c	17 Sep 2002 20:42:01 -0000	1.40
+++ blockframe.c	26 Sep 2002 16:19:30 -0000
@@ -1151,8 +1151,8 @@
 
 /* Function: find_dummy_frame(pc, fp, sp)
 
-   Search the stack of dummy frames for one matching the given PC, FP
-   and SP.  Unlike PC_IN_CALL_DUMMY, this function doesn't need to
+   Search the stack of dummy frames for one matching the given PC and
+   FP/SP.  Unlike PC_IN_CALL_DUMMY, this function doesn't need to
    adjust for DECR_PC_AFTER_BREAK.  This is because it is only legal
    to call this function after the PC has been adjusted.  */
 
@@ -1163,12 +1163,37 @@
 
   for (dummyframe = dummy_frame_stack; dummyframe != NULL;
        dummyframe = dummyframe->next)
-    if ((pc >= dummyframe->call_lo && pc < dummyframe->call_hi)
-	&& (fp == dummyframe->fp
-	    || fp == dummyframe->sp
-	    || fp == dummyframe->top))
-      /* The frame in question lies between the saved fp and sp, inclusive */
+    {
+      /* Does the PC fall within the dummy frame's breakpoint
+         instruction.  If not, discard this one.  */
+      if (!(pc >= dummyframe->call_lo && pc < dummyframe->call_hi))
+	continue;
+      /* Does the FP match?  */
+      if (dummyframe->top != 0)
+	{
+	  /* If the target architecture explicitly saved the
+	     top-of-stack before the inferior function call, assume
+	     that that same architecture will always pass in an FP
+	     (frame base) value that eactly matches that saved TOS.
+	     Don't check the saved SP and SP as they can lead to false
+	     hits.  */
+	  if (fp != dummyframe->top)
+	    continue;
+	}
+      else
+	{
+	  /* An older target that hasn't explicitly or implicitly
+             saved the dummy frame's top-of-stack.  Try matching the
+             FP against the saved SP and FP.  NOTE: If you're trying
+             to fix a problem with GDB not correctly finding a dummy
+             frame, check the comments that go with FRAME_ALIGN() and
+             SAVE_DUMMY_FRAME_TOS().  */
+	  if (fp != dummyframe->fp && fp != dummyframe->sp)
+	    continue;
+	}
+      /* The FP matches this dummy frame.  */
       return dummyframe->regcache;
+    }
 
   return 0;
 }
@@ -1265,7 +1290,7 @@
 
   dummy_frame->pc = read_pc ();
   dummy_frame->sp = read_sp ();
-  dummy_frame->top = dummy_frame->sp;
+  dummy_frame->top = 0;
   dummy_frame->fp = fp;
   regcache_cpy (dummy_frame->regcache, current_regcache);
   dummy_frame->next = dummy_frame_stack;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]