From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25416 invoked by alias); 26 Sep 2002 16:35:42 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 25397 invoked from network); 26 Sep 2002 16:35:39 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 26 Sep 2002 16:35:39 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 690093CFC; Thu, 26 Sep 2002 12:35:35 -0400 (EDT) Message-ID: <3D933757.1000906@redhat.com> Date: Thu, 26 Sep 2002 09:35:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner , gdb-patches@sources.redhat.com Subject: Re: [patch/rfc/rfa*] Only compare against dummy frame tos when saved References: <3D87CEF5.3000302@ges.redhat.com> <1020919172106.ZM21484@localhost.localdomain> Content-Type: multipart/mixed; boundary="------------090203060900070207060704" X-SW-Source: 2002-09/txt/msg00634.txt.bz2 This is a multi-part message in MIME format. --------------090203060900070207060704 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 4615 > > 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: fp=20 top=30 > 2: frameless() fp=20 top=20 > 3: fp=10 top=20 > > When trying to unwind 2:frameless(), GDB calls frame_saved_pc()= 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 --------------090203060900070207060704 Content-Type: text/plain; name="diffs" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="diffs" Content-length: 2826 2002-09-25 Andrew Cagney * 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; --------------090203060900070207060704--