This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFA symtab: Fix for PR c++/1267 ("next" and shared libraries)
- From: Jim Blandy <jimb at redhat dot com>
- To: Daniel Jacobowitz <drow at mvista dot com>
- Cc: gdb-patches at sources dot redhat dot com, ezannoni at redhat dot com,fedor at doc dot com
- Date: 21 Jul 2003 02:13:55 -0500
- Subject: Re: RFA symtab: Fix for PR c++/1267 ("next" and shared libraries)
- References: <20030719181817.GA11670@nevyn.them.org>
I think this is a great idea. How widely have you tested it?
Daniel Jacobowitz <drow@mvista.com> 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 <drow@mvista.com>
>
> 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;