🍒 [lldb] Patches implementing MultiBreakpoint #12930
Open
felipepiovezan wants to merge 16 commits intoswiftlang:stable/21.xfrom
Open
🍒 [lldb] Patches implementing MultiBreakpoint #12930felipepiovezan wants to merge 16 commits intoswiftlang:stable/21.xfrom
felipepiovezan wants to merge 16 commits intoswiftlang:stable/21.xfrom
Conversation
… error (llvm#190189) (cherry picked from commit 27a762c)
…ss (llvm#190762) This allows the callsite to be simplified. This also exposes a bug where the variable `ShouldShowError` is guarding more than the error printing. (cherry picked from commit 30a99ce)
…vm#191136) Re-using this code will be important in an upcoming patch. This commit also greatly simplifies the comments in the function. (cherry picked from commit 2f2bd5e)
This describes the packet discussed in the RFC: https://discourse.llvm.org/t/rfc-a-new-packet-to-set-remove-multiple-breakpoints/90623 The following PRs are related: * [[lldb] Propose MultiBreakpoint extension to GDB Remote](llvm#192910) * [[debugserver] Implement MultiBreakpoint](llvm#192914) * [[lldb-server][NFC] Factor out code handling breakpoint packets](llvm#192915) * [[lldb-server] Implement support for MultiBreakpoint packet](llvm#192919) * [[lldb][GDBRemote] Parse MultiBreakpoint+ capability](llvm#192962) * [[lldb][NFC] Move BreakpointSite::IsEnabled/SetEnabled into Process](llvm#192964) * [[lldb] Implement delayed breakpoints](llvm#192971) * [[lldb] Override UpdateBreakpointSites in ProcessGDBRemote to use MultiBreakpoint](llvm#192988) (cherry picked from commit 5c73c7a)
This is causing bot failures. (cherry picked from commit 74781cf)
The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988 (cherry picked from commit 1ef7d35)
…lvm#192964) The Process class is the one responsible for managing the state of a BreakpointSite inside the process. As such, it should be the one answering questions about the state of the site. Future patches will make this even more important by introducing a "logical" is enabled, by delaying the moment in which breakpoints are actually updated in the process. The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988 (cherry picked from commit 3e8fbb2)
This patch changes the Process class so that it delays *physically* enabling/disabling breakpoints until the process is about to resume/detach/be destroyed, potentially reducing the packets transmitted by batching all breakpoints together. Most classes only need to know whether a breakpoint is "logically" enabled, as opposed to "physically" enabled (i.e. the remote server has actually enabled the breakpoint). However, lower level classes like derived Process classes, or StopInfo may actually need to know whether the breakpoint was physically enabled. As such, this commit also adds a "IsPhysicallyEnabled" API. The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988 (cherry picked from commit 776ee6f)
Tests started failing on a mysterious way there, potentially related to: llvm#191222 (cherry picked from commit f5eb7a9)
This removes some duplicated code, and also helps future tests. It's also surprisingly not easy to write these, as there are a few footguns. (cherry picked from commit 3cad5df)
…5815) See the discussion in llvm#192971 When LLDB makes the decision to eagerly send a breakpoint packet, it should first ensure the delayed breakpoints are flushed, as they may interfere with the eager breakpoint that is about to be changed. Implementation note: we could have included the eager breakpoint in the batch that is about to be flushed. However, it's important to get information about the error status of this eager breakpoint, and the current APIs dont make it easy to distinguish which breakpoint caused an error. (cherry picked from commit 11d15c3)
…92915) This commit extracts the code handling breakpoint packets into a helper function that can be used by a future implementation of the MultiBreakpointPacket. It is meant to be purely NFC. There are two functions handling breakpoint packets (`handle_Z` and `handle_z`) with a lot of repeated code. This commit did not attempt to merge the two, as that would make the diff much larger due to subtle differences in the error message produced by the two. The only deduplication done is in the code processing a GDBStoppointType, where a helper struct (`BreakpointKind`) and function (`std::optional<BreakpointKind> getBreakpointKind(GDBStoppointType stoppoint_type)`) was created. The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988 (cherry picked from commit c2ab7f2)
Some old compilers complained about the `static_assert(false)` pattern. Fixes https://lab.llvm.org/buildbot/#/builders/163/builds/39139 (cherry picked from commit 4974ae5)
This is fairly straightforward, thanks to the helper functions created in the previous commit. The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988 (cherry picked from commit e33bac0)
…iBreakpoint (llvm#192988) This concludes the implementation of MultiBreakpoint by actually using the new packet to batch breakpoint requests. The following PRs are related to the MultiBreakpoint feature: * llvm#192910 * llvm#192914 * llvm#192915 * llvm#192919 * llvm#192962 * llvm#192964 * llvm#192971 * llvm#192988
Author
|
@swift-ci test |
felipepiovezan
force-pushed
the
felipe/cherry_pick_multibreak
branch
from
58265b0 to
59dbe33
Compare
Author
|
@swift-ci test |
Author
|
Author
|
@swift-ci test macos platform |
felipepiovezan
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These all cherry-picks from upstream