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 from
Jan 18, 2017
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/vm/eetwain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,8 @@ FrameType GetHandlerFrameInfo(hdrInfo * info,
// The slots grow towards lower address on the stack and is terminted by a NULL entry.
// 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

@janvorli janvorli Jan 18, 2017

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

@jkotas jkotas Jan 18, 2017

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

@janvorli janvorli Jan 18, 2017

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

@jkotas jkotas Jan 18, 2017

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.

#ifndef WIN64EXCEPTIONS
PTR_TADDR pSlot;
for(lvl = 0, pSlot = pFirstBaseSPslot;
*pSlot && lvl < unwindLevel;
Expand Down Expand Up @@ -625,6 +626,7 @@ FrameType GetHandlerFrameInfo(hdrInfo * info,
baseSP = curSlotVal;
}
}
#endif // WIN64EXCEPTIONS

if (unwindESP != (TADDR) IGNORE_VAL)
{
Expand Down