Facade location cherrypick #11668
Merged
Facade location cherrypick #11668
Conversation
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
|
@swift-ci test |
|
Something didn't get merged right. Let me figure that out. |
…157577) This is something the StopInfo class manages, so it should be allowed to compute this rather than having SBThread do so. This code just moves the computation to methods in StopInfo. It is mostly NFC. The one change that I actually had to adjust the tests for was a couple of tests that were asking for the UnixSignal stop info data by asking for the data at index 1. GetStopInfoDataCount returns 1 and we don't do 1 based indexing so the test code was clearly wrong. But I don't think it makes sense to perpetuate handing out the value regardless of what index you pass us. (cherry picked from commit 2669fde)
This patch adds the notion of "Facade" locations which can be reported from a ScriptedResolver instead of the actual underlying breakpoint location for the breakpoint. Also add a "was_hit" method to the scripted resolver that allows the breakpoint to say which of these "Facade" locations was hit, and "get_location_description" to provide a description for the facade locations. I apologize in advance for the size of the patch. Almost all of what's here was necessary to (a) make the feature testable and (b) not break any of the current behavior. The motivation for this feature is given in the "Providing Facade Locations" section that I added to the python-reference.rst so I won't repeat it here. rdar://152112327 (cherry picked from commit 36bce68)
StopInfoBreakpoint keeps a BreakpointLocationCollection for all the breakpoint locations at the BreakpointSite that was hit. It is also lives through the time a given thread is stopped, so there are plenty of opportunities for one of the owning breakpoints to get deleted. But BreakpointLocations don't keep their owner Breakpoints alive, so if the BreakpointLocationCollection can live past when some code gets a chance to delete an owner breakpoint, and then you ask that location for some breakpoint information, it will access freed memory. This wasn't a problem before PR llvm#158128 because the StopInfoBreakpoint just kept the BreakpointSite that was hit, and when you asked it questions, it relooked up that list. That was not great, however, because if you hit breakpoints 5 & 6, deleted 5 and then asked which breakpoints got hit, you would just get 6. For that and other reasons that PR changed to storing a BreakpointLocationCollection of the breakpoints that were hit. That's better from a UI perspective but caused this potential problem. I fix it by adding a variant of the BreakpointLocationCollection that also holds onto a shared pointer to the Breakpoints that own the locations that were hit, thus keeping them alive till the StopInfoBreakpoint goes away. This fixed the ASAN assertion. I also added a test that works harder to cause trouble by deleting breakpoints during a stop. (cherry picked from commit c9124a1)
jimingham
force-pushed
the
facade-location-cherrypick
branch
from
351e91c to
1f0e102
Compare
|
Yes, there was one cherry-pick between rebranch and the two patches I started with that was also needed. |
|
@swift-ci please test |
|
And I have to rebuild the static bindings... |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@swift-ci test macOS Platform |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This cherry-pick adds the ability to represent a breakpoint with "facade locations" and to determine which facade location to report (if any) when the process stops at the "real" breakpoint location. It includes the original patch adding the feature and the fix for a use-after-free issue ASAN found with the patch.