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

[x86/Linux] Fix EnclosingClauseCallerSP #9607

Closed
wateret wants to merge 1 commit into from
Closed

[x86/Linux] Fix EnclosingClauseCallerSP #9607

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
6 changes: 3 additions & 3 deletions src/vm/exceptionhandling.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1930,12 +1930,12 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification(
// DispatcherContext->EstablisherFrame's value
// represents the CallerSP of the current frame.
UINT_PTR EnclosingClauseCallerSP = (UINT_PTR)pDispatcherContext->EstablisherFrame;
#elif defined(_TARGET_AMD64_)
// Extract the CallerSP from RegDisplay on AMD64
#elif defined(_TARGET_AMD64_) || defined(_TARGET_X86_)
Copy link
Copy Markdown
Member

@janvorli janvorli Feb 15, 2017

Choose a reason for hiding this comment

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

I am not sure if we want to use the ARM or AMD64 way for x86 - did we already make a choice in this matter? I
If we follow the amd64 way, then we also need to fix the code here:
https://github.com/dotnet/coreclr/blob/master/src/vm/exceptionhandling.cpp#L2843-L2849
and here:
https://github.com/dotnet/coreclr/blob/master/src/vm/exceptionhandling.cpp#L3109-L3115
(and if not, we should modify the comments at those places to mention x86).

// Extract the CallerSP from RegDisplay on AMD64 and x86
REGDISPLAY *pRD = cfThisFrame.GetRegisterSet();
_ASSERTE(pRD->IsCallerContextValid || pRD->IsCallerSPValid);
UINT_PTR EnclosingClauseCallerSP = (UINT_PTR)GetSP(pRD->pCallerContext);
#else // !_ARM_ && !_AMD64_ && !_ARM64_
#else // !_ARM_ && !_AMD64_ && !_ARM64_ && !_X86_
Copy link
Copy Markdown
Member

@janvorli janvorli Feb 15, 2017

Choose a reason for hiding this comment

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

A nit - can you please fix the comment to reflect the real define names (_TARGET_XXX_)?

PORTABILITY_ASSERT("ExceptionTracker::ProcessOSExceptionNotification");
UINT_PTR EnclosingClauseCallerSP = NULL;
#endif // defined(_TARGET_ARM_) || defined(_TARGET_ARM64_)
Expand Down