Skip to content

[Linux][Backtracing] Fix a couple of backtracer issues. #67430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 21, 2023
Merged

[Linux][Backtracing] Fix a couple of backtracer issues. #67430

merged 2 commits into from
Jul 21, 2023

Conversation

al45tair
Copy link
Contributor

Fix the FramePointerUnwinder so it always returns nil after the it first stops iteration.

Fix line number processing so that we don't erroneously match regions between the end of one sequence and the start of the next.

rdar://112534548, rdar://112595022

al45tair added 2 commits July 20, 2023 15:39
@al45tair
[Linux][Backtracing] Fix unwinder so it stops properly.
11d77b5 
On Linux specifically, it was possible for it to return `nil` to signal
the end of the unwind, then subsequently return non-`nil` again.  That
violates the expectations of the code layered above it, and can cause
an "Index out of range" error to occur.

rdar://112534548
@al45tair
[Linux][Backtracing] Fix incorrect line number generation.
e5ab62f 
The code that looks up line numbers wasn't respecting the end-sequence
flag, so it could "find" a line number between the end of one sequence
and the beginning of another.  That was wrong.

rdar://112595022
@al45tair al45tair requested a review from a team as a code owner July 20, 2023 14:40
@al45tair
Copy link
Contributor Author

al45tair commented Jul 20, 2023 •
edited
Loading

Explanation: On Linux, the mechanism FramePointerUnwinder uses to detect async frames means that it's possible for the FramePointerUnwinder to return nil and then on the next call to next(), to return a frame again. This violates expectations in the code that uses it and can lead to an attempt to access outside the bounds of an array (this is not a security issue, because Swift is memory safe). Additionally, and also only on Linux, the code that handles the DWARF line number processing wasn't respecting the end-sequence flag properly, which was leading to incorrect line numbers in some situations.
Risk: Low. Only affects Linux, the changes are very small and self-contained as well as being entirely in Swift code.
Original PR: #67429
Reviewed by: @mikeash, @ktoso, @tomerd
Resolves: rdar://112534548, rdar://112595022
Tests: These are hard to trigger, but one of @ktoso's tests (see #67392) was crashing and triggering both of them. After the fix, it no longer does (and instead, we get a backtrace, albeit a single frame because of the lack of frame pointers).

@al45tair al45tair requested review from ktoso and mikeash July 20, 2023 14:46
@al45tair
Copy link
Contributor Author

@swift-ci Please test

@ktoso ktoso added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.9 labels Jul 21, 2023
@al45tair al45tair mentioned this pull request Jul 21, 2023
Merged
@al45tair al45tair merged commit 4cef53f into swiftlang:release/5.9 Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktoso ktoso ktoso approved these changes

@tomerd tomerd tomerd approved these changes

@mikeash mikeash mikeash approved these changes

Assignees
No one assigned
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@al45tair @ktoso @tomerd @mikeash