Skip to content
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

Fix unwind from signal handler #25

Merged
merged 1 commit into from
May 15, 2024

Conversation

azat
Copy link

@azat azat commented May 15, 2024

In case of this is frame of signal handler, the IP should be incremented, because the IP saved in the signal handler points to first non-executed instruction, while FDE/CIE expects IP to be after the first non-executed instruction.

PR in upstream: llvm/llvm-project#92291

In case of this is frame of signal handler, the IP should be
incremented, because the IP saved in the signal handler points to first
non-executed instruction, while FDE/CIE expects IP to be after the
first non-executed instruction.
// incremented, because the IP saved in the signal handler points to
// first non-executed instruction, while FDE/CIE expects IP to be after
// the first non-executed instruction.
newRegisters.setIP(returnAddress + cieInfo.isSignalFrame);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can point in the middle between two instructions. Is this ok?

Copy link
Author

@azat azat May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only needed to execute proper CFI here -

ParseInfo parseInfoArray[] = {
{cieInfo.cieInstructions, cieInfo.cieStart + cieInfo.cieLength,
(pint_t)(-1)},
{fdeInfo.fdeInstructions, fdeInfo.fdeStart + fdeInfo.fdeLength,
upToPC - fdeInfo.pcStart}};

So in other words, everything should be OK

Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give it a try.

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.

2 participants