Skip to content

Cp/r139712783 correct fix to retrieve link register above fault handler #9645

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.

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

Conversation

jasonmolenda
Copy link

No description provided.

labath and others added 2 commits November 19, 2024 16:13
llvm#92503)

…unction (llvm#91321)"

This reapplies fd1bd53, which was
reverted due to a test failure on aarch64/windows. The failure was
caused by a combination of several factors:
- clang targeting aarch64-windows (unlike msvc, and unlike clang
targeting other aarch64 platforms) defaults to -fomit-frame-pointers
- lldb's code for looking up register values for `<same>` unwind rules
is recursive
- the test binary creates a very long chain of fp-less function frames
(it manages to fit about 22k frames before it blows its stack)

Together, these things have caused lldb to recreate the same deep
recursion when unwinding through this, and blow its own stack as well.
Since lldb frames are larger, about 4k frames like this was sufficient
to trigger the stack overflow.

This version of the patch works around this problem by increasing the
frame size of the test binary, thereby causing it to blow its stack
sooner. This doesn't fix the issue -- the same problem can occur with a
real binary -- but it's not very likely, as it requires an infinite
recursion in a simple (so it doesn't use the frame pointer) function
with a very small frame (so you can fit a lot of them on the stack).

A more principled fix would be to make lldb's lookup code non-recursive,
but I believe that's out of scope for this patch.

The original patch description follows:

A leaf function may not store the link register to stack, but we it can
still end up being a non-zero frame if it gets interrupted by a signal.
Currently, we were unable to unwind past this function because we could
not read the link register value.

To make this work, this patch:
- changes the function-entry unwind plan to include the `fp|lr = <same>`
rules. This in turn necessitated an adjustment in the generic
instruction emulation logic to ensure that `lr=[sp-X]` can override the
`<same>` rule.
- allows the `<same>` rule for pc and lr in all
`m_all_registers_available` frames (and not just frame zero).

The test verifies that we can unwind in a situation like this, and that
the backtrace matches the one we computed before getting a signal.

(cherry picked from commit bbd54e0)
…llvm#98566)"

This reverts commit fd42417.

This patch has two problems.  First, it is unnecessary, Pavel landed
a fix a week or so before mine which solves this problem in
bbd54e0 .  Second, the fix is
incorrect; for a function above a trap handler, where all registers
are available, this patch would have lldb fetch the return address
register from frame 0.  This might be 10 frames up in the stack;
the frame 0 return address register is incorrect.  The change would
have been correct a short bit later than this, but Pavel's fix is
executed earlier in the function and none of this is needed.

(cherry picked from commit d29a50f)
@JDevlieghere
Copy link

@swift-ci test

1 similar comment
@jasonmolenda
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere merged commit 370e264 into swiftlang:swift/release/6.0 Nov 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants