Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Encode Reverse PInvoke frame stack slot in GCInfo#2484

Merged
jkotas merged 1 commit intomasterfrom
Encode-Reverse-PInvoke-frame-stack-slot-in-GCInfo
Jan 14, 2017
Merged

Encode Reverse PInvoke frame stack slot in GCInfo#2484
jkotas merged 1 commit intomasterfrom
Encode-Reverse-PInvoke-frame-stack-slot-in-GCInfo

Conversation

@sandreenko
Copy link

Fix #2115

INT32 slot = decoder.GetReversePInvokeFrameStackSlot();
assert(slot != NO_REVERSE_PINVOKE_FRAME);

*ppPreviousTransitionFrame = (PTR_VOID)-1;
Copy link
Member

@jkotas jkotas Jan 11, 2017

Choose a reason for hiding this comment

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

Could you also set *ppPreviousTransitionFrame to the address of the slot?

If I understand the JIT logic for the slot offset, it should call GetFramePointer and use that as a base if there is one; otherwise use GetSP() as the base.

Once you do that, could you please also verify under debugger that it computes the right address for both frame pointer and non-framepointer cases?

@jkotas
Copy link
Member

jkotas commented Jan 13, 2017

I have stepped through this in debugger. I see two problems:

  1. m_ReversePInvokeFrameStackSlot = (INT32)m_Reader.DecodeVarLengthSigned(REVERSE_PINVOKE_FRAME_ENCBASE);
    is missing call to DENORMALIZE_STACK_SLOT similar to all other slot properties. Once you fix this problem, the slot should be right for the "GetSP()" codepath.

  2. The implementation of CodeManager::GetFramePointer does not agree with where the JIT decides to use FP as the base. Here is small repro:

To fix this problem, the base should be picked based on gcInfoDecoder.GetStackBaseRegister() instead.

@sandreenko sandreenko force-pushed the Encode-Reverse-PInvoke-frame-stack-slot-in-GCInfo branch from 12e3563 to d1ebc54 Compare January 14, 2017 00:53
Fix #2115. Set ppPreviousTransitionFrame.
@sandreenko sandreenko force-pushed the Encode-Reverse-PInvoke-frame-stack-slot-in-GCInfo branch from d1ebc54 to 8a3390a Compare January 14, 2017 00:55
@sandreenko
Copy link
Author

It works significantly better after last fixes, but the test program finishes with 'ASSERT_UNCONDITIONALLY', when trying to "Reset nesting levels for exceptions on this thread that might not be currently in flight". It looks like there is an another issue, but this part works right.

@jkotas
Copy link
Member

jkotas commented Jan 14, 2017

the test program finishes with 'ASSERT_UNCONDITIONALLY', when trying to "Reset nesting levels for exceptions on this thread that might not be currently in flight

I do not see this problem. I agree that it is a different issue. Could you please open an issue with repro step details?

@jkotas jkotas merged commit cfe34d0 into master Jan 14, 2017
@jkotas jkotas deleted the Encode-Reverse-PInvoke-frame-stack-slot-in-GCInfo branch January 17, 2017 18:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants