[PATCH] [gdb/python] Handle bp creation and deletion in Breakpoint.stop

[Tom de Vries]

On 12/13/24 14:08, Tom de Vries wrote:
> On 12/10/24 09:33, Andrew Burgess wrote:
>> Tom de Vries <tdevries@suse.de> writes:
>>
>>> In the documentation of gdb.Breakpoint.stop [1] we read:
>>> ...
>>> You should not alter the execution state of the inferior (i.e., step, 
>>> next,
>>> etc.), alter the current frame context (i.e., change the current active
>>> frame), or alter, add or delete any breakpoint.
>>> ...
>>>
>>> PR python/32423 reports that the failure mode of attempting to delete a
>>> breakpoint in gdb.Breakpoint.stop is:
>>> - actually deleting the breakpoint, followed by
>>> - a segfault.
>>>
>>> Improve the failure mode, by instead:
>>> - not deleting the breakpoint, and
>>> - throwing an error.
>>>
>>> Likewise for attempting to create a breakpoint.  Doing so reveals that
>>> gdb.python/py-finish-breakpoint.exp contains test-cases that add a 
>>> breakpoint
>>> in Breakpoint.stop.  Drop these incorrect tests.
>>
>> Do these tests actually cause issues other than violating the docs?
> 
> Hi Andrew,
> 
> thanks for the review.
> 
> To answer your question, I don't known.
> 
> The commit introducing the text is commit 99f5279d98e, discussed here 
> ( https://inbox.sourceware.org/gdb-patches/m3pqia8jys.fsf@redhat.com/ ), 
> fixing two PRs.
> 
> The discussion seems to focus more on the (self) deletion of 
> breakpoints, which is considered, paraphrasing, currently not supported, 
> but desirable.
> 
>> It's not clear if your "Likewise" here means, likewise creating
>> breakpoints causes problems like use after free, or just likewise doing
>> this violates the docs, so lets remove that functionality.
> 
> Sorry for being not clear, I meant the latter.
> 
>> If possible I'd much rather we update the docs to cover the allowable
>> functionality, than restrict GDB to meet the docs.
> 
> I'm not against that.  I just don't known whether the functionality is 
> actually supported.
> 
> If you tell me that creating breakpoints is indeed supported, I'll be 
> happy to update the docs and update this patch.
> 
> Before I read the docs and discovered it was not allowed, I tried to fix 
> the problem reported in the PR, so I have an enabling fix for that 
> ( https://sourceware.org/bugzilla/show_bug.cgi?id=32423#c2 ).  So, 
> perhaps we should go that way then?  If so, I should check whether it 
> fixes PR12802.
> 
> Another way to look at this is if we really need this, given that we 
> have breakpoint commands, where we are allowed to do this sort of thing.
> 
> WDYT?
> 

I've attempted to move this forward by submitting a patch ( 
https://sourceware.org/pipermail/gdb-patches/2025-January/214483.html ) 
that:
- only prevents bp deletion in Breakpoint.stop, and
- removes the bit in the documentation that says bp creation in
   Breakpoint.stop is not allowed.

Thanks,
- Tom