Re: RFA symtab: Fix for PR c++/1267 ("next" and shared libraries)

[Adam Fedor <fedor@doc.com>]

I have Solaris with Sun CC. Does that count? What do I do, just run the 
testsuite?

On Thursday, July 24, 2003, at 02:58 PM, Daniel Jacobowitz wrote:

> 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 <drow@mvista.com> 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 <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;
>>>>
>>>
>>> -- 
>>> Daniel Jacobowitz
>>> MontaVista Software                         Debian GNU/Linux 
>>> Developer
>>
>
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer