[x86/Linux] Fix framepointer while unwinding#9678
Conversation
|
Fixes issue #9626 |
|
This patch also fixes unit test
|
| { | ||
| _ASSERTE(stackDepth == 0); | ||
| #if defined(_TARGET_X86_) && defined(FEATURE_PAL) | ||
| taArgBase = GetCallerSp(pContext) - 2 * sizeof(TADDR); |
There was a problem hiding this comment.
This code is already in _TARGET_X86_ region, and thus _TARGET_X86_ is unnecessary.
I think that it would be better to use WIN64EXCEPTIONS instead of FEATURE_PAL.
| #if defined(_TARGET_X86_) && defined(FEATURE_PAL) | ||
| taArgBase = GetCallerSp(pContext) - 2 * sizeof(TADDR); | ||
| #else | ||
| taArgBase = *pContext->GetEbpLocation(); |
There was a problem hiding this comment.
Now this region is not shared between Windows and Linux, and thus it would be better to use *pContext->pEbp instead of *pContext->GetEbpLocation().
There was a problem hiding this comment.
It was originally *pContext->GetEbpLocation(). Should I change it?
There was a problem hiding this comment.
Below line (5180 ?) has direct access to pContext->SP
taArgBase = pContext->SP + stackDepth;
Would it be better to use like this?
taArgBase = pContext->SP - 2 * sizeof(TADDR);
There was a problem hiding this comment.
@seanshpark It was *pContext->pEbp;, but revised as the current form while x86/Linux bring up (#9121).
The code around 5180 line takes care of esp-based frames (not ebp-based frames), and thus it would be better to leave it as the current form.
There was a problem hiding this comment.
@seanshpark If you intend to use pContext->SP instead of GetCallerSp(pContext) in 5173 line, I'm not sure about that.
As I understand, pContext->SP stores current stack pointer, and thus it looks strange for me to infer current frame pointer using pContext->SP.
| #if defined(WIN64EXCEPTIONS) | ||
| taArgBase = GetCallerSp(pContext) - 2 * sizeof(TADDR); | ||
| #else | ||
| taArgBase = *pContext->Ebp; |
There was a problem hiding this comment.
We need to use pEbp instead of Ebp here (please refer to inc/regdisp.h for details).
There was a problem hiding this comment.
Or, it seems reasonable to use GetRegdisplayFP(pContext) here (similarly as below).
There was a problem hiding this comment.
Thansk, I'll go with pEbp for now.
Use pCallerContext when getting frame pointer
|
@janvorli , PTAL |
|
Thank you! |
Use pCallerContext when getting frame pointer
Use pCallerContext when getting frame pointer Commit migrated from dotnet/coreclr@2d7eedb
4 participants
Use pCallerContext when getting frame pointer