Re: [RFA] Don't reset watchpoint block on solib load.

[Jim Blandy <jimb at codesourcery dot com>]

Vladimir Prus <vladimir at codesourcery.com> writes:
> On Wednesday 28 November 2007 02:00:15 Jim Blandy wrote:
>> 
>> Vladimir Prus <vladimir at codesourcery.com> writes:
>> > There's code inside breakpoint_re_set_one to refresh watchpoints, 
>> > which seems suspicious to me. 
>> >
>> > First problem with that code is that it always resets watchpoint's
>> > block to NULL. So, if we have a local watchpoint, and you do 
>> > dlopen (without exiting the scope where watchpoint is valid), 
>> > then the watchpoint's block will be reset to NULL, and 
>> > watchpoint's expression will be reparsed in global block -- 
>> > which will surely break the watchpoint. 
>> 
>> Is that right?  We set innermost_block to NULL, but then we call
>> parse_expression, which should set innermost_block to the innermost
>> block containing a symbol actually used by the expression.
>
> Except that parse_expression is called while we're inside shared lib
> machinery code -- so neither original block nor original frame is
> used, and parse_expression is likely to fail.
>
>> We also call breakpoint_re_set_one when we've unloaded a shared
>> library.  At that point, b->exp_valid_block could be a dangling
>> pointer; we can't use it to re-parse the expression.
>> 
>> I think the bug is that we re-parse the expression with
>> parse_expression, which leaves the scope unspecified, defaulting to
>> the currently selected frame.  We should:
>> 
>> 1) Verify that the frame given by b->watchpoint_frame is still valid,
>>    and delete the watchpoint if it isn't.
>>    
>> 2) Call get_frame_block (b->watchpoint_frame) to see if we have a
>>    block for the frame's location, and deleting the watchpoint if we
>>    don't (saying we don't have the symbolic info available to update
>>    it), and
>> 
>> 3) Call parse_exp_1 (..., watchpoint frame's block, ...) to reparse
>>    the watchpoint's expression in the proper block.
>
> Let me try thinking out loud.
>
> 1. Suppose I have a watchpoint on a global variable. Such a watchpoint
> can reasonably survive program restart, and given that it can be set on 
> a global variable (including static variables of C++ classes) defined
> in a shared library, it makes sense recompute such watchpoint
> when a shared library is loaded, or unloaded. 

The steps I wrote don't address the case of watchpoints that aren't
frame-relative.  I wonder how we should be dealing with those.

If we have a watchpoint on a static variable local to some file or
block, then I don't honestly see how we can possibly re-set it
correctly after a symbol reload with the data we have now.  We can't
tell whether b->exp_valid_block is a dangling pointer or not, and
b->watchpoint_frame will be null on such watchpoints.

At the moment, a watchpoint has two interesting fields:

- b->exp_valid_block, which is set only when the watchpoint refers to
  frame-local variables.  Frustratingly, it's NULL if the watchpoint
  refers to block-scoped static variables, even though we have no way
  of finding those variables again if we re-parse the expression.

- b->watchpoint_frame, which establishes which instances of the local
  variables the watchpoint refers to.

The interesting thing is that exp_valid_block is almost always used as
a binary value.  The only exception is when we use it as a sanity
check on a frame we've found that matches b->watchpoint_frame.

Should we instead be saving the PC in whose scope we parsed the
expression?  That's something we could legitimately look up in
breakpoint_re_set_one after a symbol table change.  The test in
watchpoint_check could look up the PC's function and compare it to the
frame's function.

(There's also some terminology that's bugging me: the way the code
calls watchpoints "in scope" or "out of scope" is misleading: it's not
about scope, it's about what the ISO C standard calls "lifetime".  For
example, a static variable local to some block is "in scope" from the
point of its declaration to the end of its block, but the variable's
lifetime is the execution of the entire program (or until the shared
library that contains the function is dlclosed).  A watchpoint should
be deleted when the lifetime of any of the objects it refers to ends,
regardless of whether they are in scope or not.)

(I need to eat lunch, but I'll get to the rest when I get back.)