Skip to content

Fix unwind from signal handler#25

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat-ch:signal-handler
May 15, 2024
Merged

Fix unwind from signal handler#25
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat-ch:signal-handler

Conversation

@azat
Copy link
Copy Markdown
Member

@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
Copy Markdown
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
Copy Markdown
Member 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
Copy Markdown
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