-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix #1977: always create RBP chains on Unix #4019
Conversation
Does this mean we don't need libunwind anymore on unix platforms? If RBP is always available can the GC and other stack walking needs be fulfilled by them? |
@@ -817,6 +817,15 @@ typedef enum _UNWIND_OP_CODES { | |||
UWOP_SAVE_XMM128, | |||
UWOP_SAVE_XMM128_FAR, | |||
UWOP_PUSH_MACHFRAME | |||
|
|||
#ifdef PLATFORM_UNIX | |||
, UWOP_SET_FPREG_LARGE // UWOP_SET_FPREG has a 240 byte SP->FP offset range; this has a 32-bit range scaled by 16, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit - this should work fine too - looks better:
UWOP_PUSH_MACHFRAME,
#ifdef PLATFORM_UNIX
UWOP_SET_FPREG_LARGE,
LGTM modulo comments. |
UNWIND_CODE* code = (UNWIND_CODE*)&func->unwindCodes[func->unwindCodeSlot -= sizeof(UNWIND_CODE)]; | ||
code->CodeOffset = (BYTE)cbProlog; | ||
code->OpInfo = 0; | ||
code->UnwindOp = UWOP_SET_FPREG_LARGE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add UWOP_SET_FPREG_LARGE
to DumpUnwindInfo later in the file?
@@ -831,6 +840,10 @@ static const UCHAR UnwindOpExtraSlotTable[] = { | |||
1, // UWOP_SAVE_XMM128 | |||
2, // UWOP_SAVE_XMM128_FAR | |||
0 // UWOP_PUSH_MACHFRAME | |||
|
|||
#ifdef PLATFORM_UNIX | |||
, 2 // UWOP_SET_FPREG_LARGE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same nit as above re: comma placement
@CppStars No, it doesn't mean that. First, this unwinder is used for the JITted code only and the libunwind is used for native code only. Moreover, RBP chain is not enough for stack unwinding used by the GC, since GC also needs to get values of other registers than RBP, RIP and RSP from the unwinder. |
@dotnet/jit-contrib @jkotas I updated the PR changes for the fix for #1977 to respond to code review feedback, improve the clarity of the comments, and fix a couple of bugs. |
👍 |
@dotnet-bot test this please |
Looks fine to me. |
The JIT will now always create RBP chains on Unix platforms. This includes in functions the use localloc. To do this, the VM is extended with a new Unix-only AMD64 unwind code: UWOP_SET_FPREG_LARGE. The existing unwind code which is used to establish a frame pointer, UWOP_SET_FPREG, requires that the frame pointer, when established, be no more than 240 bytes offset from the stack pointer. This doesn't work well for frames that use localloc. (Large frames without localloc are ok, because we don't report the frame pointer in the unwind info except for in functions with localloc or EnC.) The new unwind code has a 32-bit range. If used, UNWIND_INFO.FrameRegister must be set to the frame pointer register, and UNWIND_INFO.FrameOffset must be set to 15 (its maximum value). This code is followed by two UNWIND_CODEs that are combined to form its 32-bit offset. The high 4 bits must be zero. This offset is then scaled by 16. This result is used as the FP register offset from SP at the time the frame pointer is established.
Fix dotnet/coreclr#1977: always create RBP chains on Unix Commit migrated from dotnet/coreclr@b54f7b5
The JIT will now always create RBP chains on Unix platforms.
To do this, the VM is extended with a new Unix-only AMD64 unwind code: UWOP_SET_FPREG_LARGE.
The existing unwind code which is used to establish a frame pointer, UWOP_SET_FPREG, requires
that the frame pointer, when established, be no more than 240 bytes offset from the stack pointer.
This doesn't work well for frames that use localloc. (Large frames are ok, because we don't
report the frame pointer in the unwind info except for in functions with localloc or EnC.)
The new code has a 32-bit range, scaled by 16. If used, UNWIND_INFO.FrameRegister
must be set to the frame pointer register, and UNWIND_INFO.FrameOffset must be set to 15
(its maximum value). This code is followed by two UNWIND_CODEs that are combined to form
its 32-bit offset. This offset is then scaled by 16. This result is used as the FP register
offset from SP at the time the frame pointer is established.
Fixes #1977