Re: [patch] Fix a crash when displaying variables from shared library.

[Paul Pluzhnikov <ppluzhnikov@google.com>]

On Fri, Mar 20, 2009 at 12:54 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> 2009-03-11  Paul Pluzhnikov  <ppluzhnikov@google.com>
>>
>>           * breakpoint.c (disable_breakpoints_in_shlibs): Use
>>           solib_contains_address_p instead of searching.
>
> Unfortunately, we can't apply this patch just yet, because of:
>
>> -#ifdef PC_SOLIB
>> -     char *so_name = PC_SOLIB (loc->address);
>> -#else

But I think we can. After the patch, we don't care about so_name, except
to print the warning, and we have it readily available in solib->so_name.

> Also, I have a couple of questions:
>
>> -     && !loc->shlib_disabled
>> +     && !loc->shlib_disabled
>
> I can't figure out what the change was in this case. The lines
> look completely identical. I suspect a change in white-spaces,
> but I couldn't see any.

There is a white-space diff, and a bogus one at that:
"cat -T gdb-rename-solib-20090309.txt" shows:

-^I&& !loc->shlib_disabled
+ ^I&& !loc->shlib_disabled

Sorry about that.

>> -     && !loc->shlib_disabled)
>> +     && !loc->shlib_disabled
>> +     && (b->type == bp_breakpoint || b->type == bp_hardware_breakpoint)
>> +     && solib_contains_address_p (solib, loc->address))
>
> I am wondering why you are checking the breakpoint type in addition
> to the location type.

Because if I don't, then a warning is issued when b->type == bp_shlib_event
and ld-linux-x86-64.so.2 is unloaded.

> In particular, I'm trying to figure out whether
> it's possible to have a b->type that's not a breakpoint if loc->type
> is a breakpoint.

Yes, for b->type == bp_shlib_event.

> Also, we weren't making that check before, so what
> did you see that made it you do it now?

That is exactly what I am trying to say: the current code works (as in,
does not issue a warning) "by accident": we remove ld-linux... from the
so_list_head, *then* run observer notification. Observer searches the list
for "so" containing the bp_shlib_event, and doesn't find it (we just removed
it from there!) and so remains silent. Reverse these lines in clear_solib:

     observer_notify_solib_unloaded (so);
     so_list_head = so->next;

and current code will start issuing the warning as well.

-- 
Paul Pluzhnikov