From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32234 invoked by alias); 21 Jul 2003 16:27:54 -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 32225 invoked from network); 21 Jul 2003 16:27:52 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 21 Jul 2003 16:27:52 -0000 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 19edVu-0001oG-00; Mon, 21 Jul 2003 12:27:50 -0400 Date: Mon, 21 Jul 2003 16:27:00 -0000 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com, jimb@redhat.com, fedor@doc.com Subject: Re: RFA symtab: Fix for PR c++/1267 ("next" and shared libraries) Message-ID: <20030721162750.GA6754@nevyn.them.org> Mail-Followup-To: Andrew Cagney , gdb-patches@sources.redhat.com, ezannoni@redhat.com, jimb@redhat.com, fedor@doc.com References: <20030719181817.GA11670@nevyn.them.org> <3F1C1254.2070007@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3F1C1254.2070007@redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-07/txt/msg00374.txt.bz2 On Mon, Jul 21, 2003 at 12:18:28PM -0400, Andrew Cagney wrote: > > >By the way, I'm convinced that all is not well in step_over_function. This > >comment, > > > > /* NOTE: cagney/2003-04-06: > > > > The intent of DEPRECATED_SAVED_PC_AFTER_CALL was to: > > > > - provide a very light weight equivalent to frame_unwind_pc() > > (nee FRAME_SAVED_PC) that avoids the prologue analyzer > > > > - avoid handling the case where the PC hasn't been saved in the > > prologue analyzer > > > > Unfortunatly, not five lines further down, is a call to > > get_frame_id() and that is guarenteed to trigger the prologue > > analyzer. > > > >is either incorrect or has gotten out of sync with the code: > > Nope (it pays to look at the archives). > > > if (DEPRECATED_SAVED_PC_AFTER_CALL_P ()) > > sr_sal.pc = ADDR_BITS_REMOVE (DEPRECATED_SAVED_PC_AFTER_CALL > > (get_current_frame ())); > > else > > sr_sal.pc = ADDR_BITS_REMOVE (frame_pc_unwind (get_current_frame ())); > > sr_sal.section = find_pc_overlay (sr_sal.pc); > > > > check_for_old_step_resume_breakpoint (); > > step_resume_breakpoint = > > set_momentary_breakpoint (sr_sal, get_frame_id (get_current_frame ()), > > bp_step_resume); > > > > > >Note that get_frame_id unwinds from the NEXT frame, and > >frame_pc_unwind/DEPRECATED_SAVED_PC_AFTER_CALL unwind from THIS frame. > >This throws me a loop every time I have to work in this function. Also, I > >have the nagging feeling we're saving the wrong frame. I have an old MIPS > >patch where I needed to use get_prev_frame in step_over_function. As soon > >as I have time to revisit that patch I'll be back to clean this up some > >more. > > The complete code body is: > > if (DEPRECATED_SAVED_PC_AFTER_CALL_P ()) > sr_sal.pc = ADDR_BITS_REMOVE (DEPRECATED_SAVED_PC_AFTER_CALL > (get_current_frame ())); > else > sr_sal.pc = ADDR_BITS_REMOVE (frame_pc_unwind (get_current_frame ())); > sr_sal.section = find_pc_overlay (sr_sal.pc); > > check_for_old_step_resume_breakpoint (); > step_resume_breakpoint = > set_momentary_breakpoint (sr_sal, get_frame_id (get_current_frame ()), > bp_step_resume); > > if (frame_id_p (step_frame_id) > && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc)) > step_resume_breakpoint->frame_id = step_frame_id; > > while the original code looks like: > > struct symtab_and_line sr_sal; > > init_sal (&sr_sal); /* initialize to zeros */ > sr_sal.pc = ADDR_BITS_REMOVE (SAVED_PC_AFTER_CALL (get_current_frame > ())); > sr_sal.section = find_pc_overlay (sr_sal.pc); > > check_for_old_step_resume_breakpoint (); > step_resume_breakpoint = > set_momentary_breakpoint (sr_sal, get_current_frame (), > bp_step_resume); > > if (step_frame_address && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc)) > step_resume_breakpoint->frame = step_frame_address; > > if (breakpoints_inserted) > insert_breakpoints (); No, really, I still think that the comment is wrong. The get_frame_id call triggers the prologue analyzer to analyze the NEXT frame. I.E. this_id is called with the NEXT frame, and THIS cache. The call to frame_pc_unwind uses THIS frame and creates the PREV cache. It's one frame off on the stack. The frame it would be saving from prologue analysis is not the one we'd need to analyze for the get_frame_id call. What am I missing? Certainly you converted the code correctly, I just don't understand the comment. > It would appear that the get_frame_id() call has been wrong for a long > long time but, at a guess, was worked around by picking up step_frame_id > / step_frame_address. Maybe... -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer