From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7325 invoked by alias); 24 Jul 2003 20:58:41 -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 7318 invoked from network); 24 Jul 2003 20:58:41 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 24 Jul 2003 20:58:41 -0000 Received: from drow by nevyn.them.org with local (Exim 4.20 #1 (Debian)) id 19fnAe-0000W9-In; Thu, 24 Jul 2003 16:58:40 -0400 Date: Thu, 24 Jul 2003 20:58:00 -0000 From: Daniel Jacobowitz To: Jim Blandy Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com, fedor@doc.com Subject: Re: RFA symtab: Fix for PR c++/1267 ("next" and shared libraries) Message-ID: <20030724205839.GA1919@nevyn.them.org> Mail-Followup-To: Jim Blandy , gdb-patches@sources.redhat.com, ezannoni@redhat.com, fedor@doc.com References: <20030719181817.GA11670@nevyn.them.org> <20030721125337.GA23055@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2003-07/txt/msg00442.txt.bz2 On Thu, Jul 24, 2003 at 02:59:27PM -0500, Jim Blandy wrote: > > For the trunk, put it in, definitely. > > For 6.0, could you test it on a COFF toolchain, and on some non-GNU > toolchain? It would be nice to have those three PR's closed in 6.0. I no longer have access to any non-GNU toolchains. I'll try to build a COFF simulator target and give it a whirl, though... > > Daniel Jacobowitz writes: > > Only the testsuite on i386-linux. What would you recommend? > > > > On Mon, Jul 21, 2003 at 02:13:55AM -0500, Jim Blandy wrote: > > > > > > I think this is a great idea. How widely have you tested it? > > > > > > Daniel Jacobowitz writes: > > > > > > > This patch fixes c++/1267, a bug where stepping over a function call that > > > > went through the PLT (as happens when a -fPIC function makes a call to a > > > > globally visible symbol) would lose control of the inferior. I'll spare you > > > > the complete debugging session, as it really doesn't make much sense. But > > > > here's the root of the problem: > > > > > > > > When we called frame_pc_unwind on the sentinel frame, we got an address in > > > > the PLT. But when we called frame_func_unwind, we got "_init", in ".init", > > > > which is generally located right before the PLT. Then, we'd run the > > > > new-and-improved prologue unwinder on _init, and get some completely bogus > > > > information, since things weren't actually saved on the stack where it > > > > thought they were. This led to the unwound stack pointer being wrong for > > > > the step_resume breakpoint, so when we hit the step_resume breakpoint we > > > > kept going. > > > > > > > > I fixed this by changing lookup_minimal_symbol_pc_section to be paranoid > > > > about returning a minsym in the same section as the PC. Technically, at > > > > least on ELF targets, that doesn't _have_ to be true. I've never > > > > encountered an exception or a good reason for one, though. Does anyone see > > > > any pitfalls for this change? Symtab maintainers, is this patch OK? > > > > > > > > I believe this patch should also fix shlibs/1237, and may also fix > > > > shlibs/1280. Adam, could you check those? > > > > > > > > > > > > > > > > > > > > > > > > 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: > > > > > > > > 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. > > > > > > > > -- > > > > Daniel Jacobowitz > > > > MontaVista Software Debian GNU/Linux Developer > > > > > > > > 2003-07-19 Daniel Jacobowitz > > > > > > > > PR c++/1267 > > > > * minsyms.c (lookup_minimal_symbol_by_pc_section): If SECTION is > > > > NULL, default to the section containing PC. > > > > > > > > Index: minsyms.c > > > > =================================================================== > > > > RCS file: /cvs/src/src/gdb/minsyms.c,v > > > > retrieving revision 1.31 > > > > diff -u -p -r1.31 minsyms.c > > > > --- minsyms.c 15 May 2003 22:23:24 -0000 1.31 > > > > +++ minsyms.c 19 Jul 2003 18:03:08 -0000 > > > > @@ -403,12 +403,22 @@ lookup_minimal_symbol_by_pc_section (COR > > > > struct objfile *objfile; > > > > struct minimal_symbol *msymbol; > > > > struct minimal_symbol *best_symbol = NULL; > > > > + struct obj_section *pc_section; > > > > > > > > /* pc has to be in a known section. This ensures that anything beyond > > > > the end of the last segment doesn't appear to be part of the last > > > > function in the last segment. */ > > > > - if (find_pc_section (pc) == NULL) > > > > + pc_section = find_pc_section (pc); > > > > + if (pc_section == NULL) > > > > return NULL; > > > > + > > > > + /* If no section was specified, then just make sure that the PC is in > > > > + the same section as the minimal symbol we find. */ > > > > + if (section == NULL) > > > > + section = pc_section->the_bfd_section; > > > > + > > > > + /* FIXME drow/2003-07-19: Should we also check that PC is in SECTION > > > > + if we were passed a non-NULL SECTION argument? */ > > > > > > > > for (objfile = object_files; > > > > objfile != NULL; > > > > > > > -- > > Daniel Jacobowitz > > MontaVista Software Debian GNU/Linux Developer > -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer