felipepiovezan wrote: Thanks for investigating this, everyone. Let me try to address some of the points mentioned here.
First, the efficiency point, which is not related to the main bug discussed. > The matching logic is site-based, so I expect this will not catch the cases > where a breakpoint deleted and a new one gets created in the same place This one is intentional because of a detail about how the BreakpointSite class is designed. If we disable a BreakpointSite which had exactly one BreakpointLocation associated with it, the BreakpointSite is removed from the site list of the Process. This has always been the case. Now, if we re-enable a breakpoint on the same address, a new site will be created, because the code will look for a BreakpointSite in the site list with a matching address and found none. This has also always been the case. The interaction with this patch is that now there will be two breakpoint operations queued for the same address (the disable, and then the enable). This is not problematic, but: 1. It's not efficient, as Pavel points out. 2. It makes it incorrect for us to "retry" operations. The follow-up PR had a fallback to retry individual components of MultiBreakpoint using "single" packets in case any of them failed. I am going to rip that code out, as it is both incorrect and we have to trust the stub to implement packets correctly. You may point out that we could, in theory, search both the Process site list and the delayed breakpoint queue for a BreakpointSite with a matching address. If there was a BreakpointSite there already enqueued for disabling, simply move it back to the Process site list. This works, except for this case: 1. BreakpointSite A is enabled 2. Remove the BreakpointLocation associated with A. 3. Create a BreakpointLocation at the same address, but make it a hardware breakpoint. In this scenario, it would simply be incorrect to re-use the site and do nothing. A new site must be created. The first one must be disabled first. Also worth pointing out that I believe LLDB, before this patch, doesn't handle well the case there you have multiple (non-hardware) breakpoints on the same address, and you try to create a new hardware breakpoint on the same address. https://github.com/llvm/llvm-project/pull/192971 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
