Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[x86/Linux] Intreface-based x86 unwinder (Work in progress)#9366

Closed
parjong wants to merge 2 commits into
dotnet:masterfrom
parjong:fix/x86_unwinder_interface
Closed

[x86/Linux] Intreface-based x86 unwinder (Work in progress)#9366
parjong wants to merge 2 commits into
dotnet:masterfrom
parjong:fix/x86_unwinder_interface

Conversation

@parjong
Copy link
Copy Markdown

@parjong parjong commented Feb 6, 2017

This commit revises x86 unwinder based on two interfaces: IUnwindFrameReader and IUnwindFrameListener.

To be specific, this commit revises x86 unwinder to read initial values from IUnwindFrameReader and notify changes during unwinding via IUnwindFrameListener, which allows flexible and uniform implementation for Linux and Windows.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 6, 2017

I do not think that introducing interface here is a good idea.

@janvorli
Copy link
Copy Markdown
Member

janvorli commented Feb 6, 2017

I agree with @jkotas.

@parjong
Copy link
Copy Markdown
Author

parjong commented Feb 6, 2017

@jkotas @janvorli

This PR is an attempt to resolve #9272.

While investigating #9272, I discovered that JIT.Regression.CLR-x86-JIT.V2.0-Beta2.b415164 test also has a similar issue, but the callee is a JITed method unlike #9272.

It currently works well as managed unwinder takes care of callee-poped stack elements.
Unfortunately, we can't leave managed unwinder as its current form as we need to update Esp as SP before call instruction according to the discussion in #9235.

These findings indicate that we need to maintain SP just before call instruction for unwind (as discussed in #9235), but we also need to maintain SP just after call instruction for resume (to resolve #9272).

It may be possible to achieve this via modifying unwinder and REGDISPLAY, but I'm afraid side-effect on Windows as x86 unwinder is shared between Windows and Linux.

This PR (interface-based unwinder) is an attempt to isolate that side effect.

Please let me know your opinion. It would be very helpful if we could find a better approach.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 7, 2017

we need to maintain SP just before call instruction for unwind

It feels pretty unnatural for unwinder to maintain SP before call instruction. I do not think that the fix for the problem you are hitting should be to redo how unwinding works. The SP before call instruction is not even well defined in the presence of tailcalls with callee-pop calling convention.

Instead, the places that do not work with SP after the call should be fixed to work with it instead.

@parjong
Copy link
Copy Markdown
Author

parjong commented Feb 7, 2017

@jkotas CLR ABI document states that Caller SP as a stack pointer just before call instruction is executed. I'm not sure, but this indicates that the unwinder should maintain SP before call instruction. Do I understand correctly?

It seems that all the architectures except x86 uses caller-pop calling convention, and thus SP before call instruction is same as SP after call instruction.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 7, 2017

Caller-SP is problematic on platforms with caller-pop calling convention.

Can we use Initial-SP for establisher frame like on amd64?

@parjong
Copy link
Copy Markdown
Author

parjong commented Feb 8, 2017

@jkotas Sorry for late response. I'm not sure about Initial-SP as wrote in #9384. It depends on #9300.

@parjong parjong closed this Feb 8, 2017
@parjong parjong deleted the fix/x86_unwinder_interface branch March 8, 2017 11:54
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants