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

[x86/Linux] Do NOT use Shadow SP#8913

Merged
janvorli merged 1 commit into
dotnet:masterfrom
parjong:fix/revise_GetHandlerFrameInfo
Jan 18, 2017
Merged

[x86/Linux] Do NOT use Shadow SP#8913
janvorli merged 1 commit into
dotnet:masterfrom
parjong:fix/revise_GetHandlerFrameInfo

Conversation

@parjong
Copy link
Copy Markdown

@parjong parjong commented Jan 12, 2017

JIT with FEATURE_EH_FUNCLETS seems not to generate shadow SP slots, which results in assert failures inside GetHandlerFrameInfo.

This is another step to resolve #8887.

@parjong
Copy link
Copy Markdown
Author

parjong commented Jan 12, 2017

\CC @seanshpark

@parjong
Copy link
Copy Markdown
Author

parjong commented Jan 16, 2017

@dotnet-bot test OSX x64 Checked Build and Test please

@parjong
Copy link
Copy Markdown
Author

parjong commented Jan 18, 2017

@janvorli Please take a look.

Comment thread src/vm/eetwain.cpp
// Since each subsequent slot contains the SP of a more nested EH clause, the contents of the slots are
// expected to be in decreasing order.
size_t lvl;
size_t lvl = 0;
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.

@parjong looking at the code that calls this function, I wonder if it should be called at all for x86 with WIN64EXCEPTIONS. Most of the callers do nothing if the TARGET_X86 is not defined and it seems like these ifdefs should be augmented for TARGET_X86 && !WIN64EXCEPTIONS.
I am not 100% sure though.
@jkotas do you know if I am right here or will I need to look through the code to get full understanding?

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.

Both ifdefing the implementation or ifdefing all callers sounds plausible to me. I am not able to tell what will turn out better.

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.

@jkotas I guess I was not clear. My question was if this code is really meant just for the Windows x86 style EH.

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.

Yes, the shadow SP slots that this change is ifdefing out should not be needed for WIN64EXCEPTIONS.

Copy link
Copy Markdown
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@janvorli janvorli merged commit e312e9a into dotnet:master Jan 18, 2017
@parjong parjong deleted the fix/revise_GetHandlerFrameInfo branch January 18, 2017 23:36
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
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.

[x86/Linux] Support Simple Exception Catch

5 participants