[x86/Linux] Port SWCB_GetExecutionState#9436
Conversation
|
\CC @seanshpark @wateret |
|
@dotnet-bot test Ubuntu x64 Checked Build and Test please |
| _ASSERTE( pRD->IsCallerContextValid ); | ||
| } | ||
|
|
||
| size_t EECodeManager::GetCallerSpUnsafe( PREGDISPLAY pRD ) |
There was a problem hiding this comment.
I don't think it is worth extracting this simple expression into a function, especially when it is obvious what it does and it is used at two places only. Could you please revert this part of the change? Otherwise LGTM.
| pES->m_ppvRetAddrPtr = (void **) ((size_t)GetSP(pRDT->pCallerContext) - sizeof(void*)); | ||
| #endif | ||
| #elif defined(_TARGET_X86_) || defined(_TARGET_AMD64_) | ||
| pES->m_ppvRetAddrPtr = (void **) (EECodeManager::GetCallerSp(pRDT) - sizeof(void*)); |
There was a problem hiding this comment.
@parjong I meant just use the original form without calling the EECodeManager method. Is there a problem with it?
There was a problem hiding this comment.
As we discussed in #9384, it is possible that caller's esp is not same as Caller-SP. I would like to express that we need Caller-SP here.
There was a problem hiding this comment.
@parjong I am sorry, I have not realized that it was the reason for introducing the unsafe method before. But that means that this change depends on the CallerSP change and so it should not be merged before the other one, right?
There was a problem hiding this comment.
@janvorli This change requires Caller SP change to be correct, but it seems that this change could be merged first as the current implementation (based on PCTAddr) already has an issue.
Commit migrated from dotnet/coreclr@2ecadf5
4 participants
SWCB_GetExecutionState currently updates m_ppvRetAddrPtr using PCTAddr.
Unfortunately, however, there is no guarantee that PCTAddr is in actual stack range (for x86/Linux), which results in #9435.
This commit use Caller SP based routine (which is used for other architectures) for x86/Linux in order to fix #9435.