[x86/Linux] Pass proper REGDISPLAY to GetGSCookieAddr (Do not merge)

[parjong]

This commit attempts to fix segmentation fault due to incorrect GSCookie address.

The current implementation computes GSCookie address via dereferencing pEbp field in REGDISPLAY, and adding some offsets, but ExceptionTracker::ProcessOSExceptionNotification does not initialize pEbp.

This results in segmentation fault discussed in #8980.

In addition, the currenct implementation of FaultingExceptionFrame::UpdateRegDisplay does not updates pXXX fields at all, which incurs segmentation fault while checking GSCookie inside ExceptionTracker::FindNonvolatileRegisterPointers .

This commit attempts to update pEbp before GetGSCookieAddr call to fix #8980.

[parjong]

\CC @seanshpark

[parjong]

This PR attempts to isolate the effect of #8981 within x86/Linux port.

[janvorli]

It seems to me that the cfThisFrame.pRD should be initialized at this point, so we should not need to do anything extra here. My guess is that the problem is that when the cfThisFrame.pRD is initialized, we are missing updating the pEbp etc pointers in the pRD and just update the pointers in the ctxPtrsOne while we need to update both those and the pXXX ones.
It seems it is likely in the FillRegDisplay. These pXXX are updated in the code path for the windows x86, but not for Linux x86.
Actually, this dual location of the context pointers and the need to keep them consistent between those two locations seems to be a big pain and seeing all of this I now believe it is actually a bad idea to have the pXXX for Linux x86 at all. Another problem with this duality is related to the fact that the REGDISPLAY has two sets of context pointers - the ctxPtrsOne and ctxPtrsTwo. It switches between their usage as pCurrentContextPointers and pCallerContextPointers. But we don't update the pXXX registers to point to the current context when the switch happens.
It seems it would be very difficult and error prone to keep them in sync.
So, I think we should step back and see if we can get rid of the pXXX in the REGDISPLAY for x86 Linux. Amd64, ARM and ARM64 don't have anything similar either. I know we would need to modify many places to use the other context pointers, but that would be easy to find - once you remove the pXXX, the build will break at all places where we need to modify the code for X86 Linux.

[parjong]

@janvorli ExceptionTracker::InitializeCrawlFrame (called at the beginning of ExceptionTracker::ProcessOSExceptionNotification) initializes cfThisFrame.pRD via Thread::InitRegDisplay call.

Thread::InitRegDisplay, however, does not initialize pRD, and thus #8981 attempts to initialize pEbp inside Thread::InitRegDisplay.

At that point, the issue was that the context that Thread::InitRegDisplay takes is not that of the current frame, but that of the previous frame. (That is why the changes on dispatcherContext.ContextRecord is required in #8981).

I currently have no overview of x86/Windows unwider, and thus I'm not sure whether it is possible to eliminate pXXX fields without leaving any side-effect on x86/Windows.

Anyway, I'll try that approach.

I have a question related with this. EECodeManager::GetGSCookieAddr has the following code:

    if  (info->ebpFrame)
    {
        return PVOID(SIZE_T((DWORD(*pContext->pEbp) - info->gsCookieOffset)));
    }

Could you let me know the purpose of pEbp? I thought that it will point to the location where the previous frame pointer is stored, but the above code seems to assume that it will point to where the current frame pointer is stored.

[parjong]

@janvorli Could you please take a look for #8889, #8914, #8964, #8993? The changes in REGDISPLAY will make a conflict with these PRs, and thus I would like to revise REGDISPLAY after merging them.

[janvorli]

@parjong sure, let me take a look at those.
As for your question on the pEbp (and all the pXXX in the REGDISPLAY). They point to the current frame's register values (provided the REGDISPLAY itself was created for the current frame).
As you can see e.g. in the FillRegDisplay for x86 Windows, all the pXXX are initialized to point to the context members. Then when unwinding is performed, these pointers are set to the locations on stack where the register was pushed.
Let me give you an example to make it clear.

A:
push ebp
mov ebp, esp
push ebx
push esi
push edi
mov ebx, 1h
mov esi, 2h
mov edi, 3h
call B
pop edi
pop esi
pop ebx
pop ebp
ret

B: 
push ebp
mov ebp, esp
push esi
push edi
mov esi, 102h
mov edi, 103h
<<<<< This is a context where we start unwinding >>>>>
pop edi
pop esi
pop ebp
ret

When we want to unwind from the above marked location, we first capture the context. The esi member of the context is set to 102h, edi to 103h and ebp to the address of the location where the previous ebp was pushed at the entry to the function B. The ebx in the context is set to the value it has at that point and since function B has not touched it, it is set to 1.
The REGDISPLAY's pEbx is initialized to point to the ebx member of the context, pEsi to the esi member of the context, the pEdi to the edi member of the context and pEbp to the ebp member of the context.
Now we perform unwind. Since ebp, esi and edi were pushed in the function, the pEbp, pEsi and pEdi are now set to point to the addresses of the stack locations where the ebp, esi and edi were pushed. Since the pushed values were the values that these registers had in the function A, the pEbp points to the location where ebp was pushed in function A, pEsi points to value 2 and pEdi to value 3. The pEbx is not modifed and still points to the ebx in the original context, so it points to value 1. As you can see, the pXXX registers point to values that the registers had in the function A.

[parjong]

@janvorli Thanks you for explanation!

[parjong]

@dotnet-bot test Windows_NT x64 Release Priority 1 Build and Test please

[parjong]

I'll close this PR as #9235 also resolves this issue.