Tentative: add -location option to clear command

Tentative: add -location option to clear command

[Giles Atkinson]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

Greetings.

This patch adds a variation of the "clear" command, "clear -location",
so that (some) watchpoints can be removed by expression.  The idea is
to support setting watchpoints in dynamically-allocated memory,
removing them automatically when the memory is released.
Something like this:

# Break on malloc()d memory initialisation.
break new_stuff:init
commands
  watch -location new->foo[0]
  commands
    printf "Item %d next %p\n", it->count, it->next
    bt
    continue
  end
  continue
end
#
# Break just before free().
break free_stuff:release
commands
  clear -location old->foo[0]
  continue
end

The delete command will not work here without an external mechanism to
associate the watchpoint number with the memory address.

Some background to this is in the recent thread
"Dynamic watchpoints in dynamic memory" on the gdb list.

The change works by pulling out bits of the watch and clear commands
into new functions, and re-using them.  No real understanding of gdb
internals was needed, so there may be some subtle errors.
In making parse_expression_for_watchpoint(), I removed what looked like
redundant stripping of whitespace from the expression.

In its current form, the change appears to pass the tests as well as
the original.  (I get some variation with repeated runs of "make check".)
The results for changes named "watch*" and "break*" seem identical.

If the change can be accepted in principle, I will add tests, and
documention changes.  There is also a potential extension to other watchpoints
(set without -location).  The clear command would only remove watchpoints
after failing to match any breakpoints, and would probably select watchpoints
to be removed by matching the frame and parsed expression.

Diff is relative to 7.10.1.

Thanks,

Giles

[-- Attachment #2: patch.gz --]
[-- Type: application/x-gzip, Size: 4879 bytes --]

Re: Tentative: add -location option to clear command

[Giles Atkinson]

I did not see any responses to the message below.
If there is something more I should do for this patch to be considered,
can someone please tell me what it is.

Thanks,

Giles

On 20 Feb 2016, at 19:17, Giles Atkinson wrote:

> Greetings.
> 
> This patch adds a variation of the "clear" command, "clear -location",
> so that (some) watchpoints can be removed by expression.  The idea is
> to support setting watchpoints in dynamically-allocated memory,
> removing them automatically when the memory is released.
> Something like this:
> 
> # Break on malloc()d memory initialisation.
> break new_stuff:init
> commands
>  watch -location new->foo[0]
>  commands
>    printf "Item %d next %p\n", it->count, it->next
>    bt
>    continue
>  end
>  continue
> end
> #
> # Break just before free().
> break free_stuff:release
> commands
>  clear -location old->foo[0]
>  continue
> end
> 
> The delete command will not work here without an external mechanism to
> associate the watchpoint number with the memory address.
> 
> Some background to this is in the recent thread
> "Dynamic watchpoints in dynamic memory" on the gdb list.
> 
> The change works by pulling out bits of the watch and clear commands
> into new functions, and re-using them.  No real understanding of gdb
> internals was needed, so there may be some subtle errors.
> In making parse_expression_for_watchpoint(), I removed what looked like
> redundant stripping of whitespace from the expression.
> 
> In its current form, the change appears to pass the tests as well as
> the original.  (I get some variation with repeated runs of "make check".)
> The results for changes named "watch*" and "break*" seem identical.
> 
> If the change can be accepted in principle, I will add tests, and
> documention changes.  There is also a potential extension to other watchpoints
> (set without -location).  The clear command would only remove watchpoints
> after failing to match any breakpoints, and would probably select watchpoints
> to be removed by matching the frame and parsed expression.
> 
> Diff is relative to 7.10.1.
> 
> Thanks,
> 
> Giles
> <patch.gz>

Re: Tentative: add -location option to clear command

[Luis Machado]

Hi,

On 03/29/2016 09:50 AM, Giles Atkinson wrote:
> I did not see any responses to the message below.
> If there is something more I should do for this patch to be considered,
> can someone please tell me what it is.

Yes, there is. I'd start with either sending the patch itself inline or 
a diff, not a gzipped diff. It makes it much easier to handle the patch 
and to review it.

Another thing is that the patch should follow the GNU C coding 
standards. There are formatting issues with your patch that need to be 
corrected. So you may want to check the standard and update your patch 
or at least send the patch as suggested above so we can at least point 
out the problems.

The feature itself sounds useful, but we need to make sure it is shaped 
in a way that fits what we currently have in GDB.

Luis

>
> Thanks,
>
> Giles
>
> On 20 Feb 2016, at 19:17, Giles Atkinson wrote:
>
>> Greetings.
>>
>> This patch adds a variation of the "clear" command, "clear -location",
>> so that (some) watchpoints can be removed by expression.  The idea is
>> to support setting watchpoints in dynamically-allocated memory,
>> removing them automatically when the memory is released.
>> Something like this:
>>
>> # Break on malloc()d memory initialisation.
>> break new_stuff:init
>> commands
>>   watch -location new->foo[0]
>>   commands
>>     printf "Item %d next %p\n", it->count, it->next
>>     bt
>>     continue
>>   end
>>   continue
>> end
>> #
>> # Break just before free().
>> break free_stuff:release
>> commands
>>   clear -location old->foo[0]
>>   continue
>> end
>>
>> The delete command will not work here without an external mechanism to
>> associate the watchpoint number with the memory address.
>>
>> Some background to this is in the recent thread
>> "Dynamic watchpoints in dynamic memory" on the gdb list.
>>
>> The change works by pulling out bits of the watch and clear commands
>> into new functions, and re-using them.  No real understanding of gdb
>> internals was needed, so there may be some subtle errors.
>> In making parse_expression_for_watchpoint(), I removed what looked like
>> redundant stripping of whitespace from the expression.
>>
>> In its current form, the change appears to pass the tests as well as
>> the original.  (I get some variation with repeated runs of "make check".)
>> The results for changes named "watch*" and "break*" seem identical.
>>
>> If the change can be accepted in principle, I will add tests, and
>> documention changes.  There is also a potential extension to other watchpoints
>> (set without -location).  The clear command would only remove watchpoints
>> after failing to match any breakpoints, and would probably select watchpoints
>> to be removed by matching the frame and parsed expression.
>>
>> Diff is relative to 7.10.1.
>>
>> Thanks,
>>
>> Giles
>> <patch.gz>
>
>