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

Fix reporting of callee saved registers with nongnu libuwind #527

Merged
merged 2 commits into from
Mar 23, 2015
Merged

Fix reporting of callee saved registers with nongnu libuwind #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
285f3a5
Fix reporting of callee saved registers with nongnu libuwind
jkotas Mar 21, 2015
412664b
Incorporated codereview feedback
jkotas Mar 23, 2015
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
23 changes: 12 additions & 11 deletions src/vm/amd64/AsmHelpers.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,22 @@ endif
; So, if m_pReg points into the MachState, we need to update
; the register here. That's what this macro does.
;
RestoreReg macro reg
lea rax, [rcx + OFFSETOF__MachState__m_Capture&reg]
mov rdx, [rcx + OFFSETOF__MachState__m_p&reg]
RestoreReg macro reg, regnum
lea rax, [rcx + OFFSETOF__MachState__m_Capture + 8 * regnum]
mov rdx, [rcx + OFFSETOF__MachState__m_Ptrs + 8 * regnum]
cmp rax, rdx
cmove reg, [rax]
endm

RestoreReg Rdi
RestoreReg Rsi
RestoreReg Rbx
RestoreReg Rbp
RestoreReg R12
RestoreReg R13
RestoreReg R14
RestoreReg R15
; regnum has to match ENUM_CALLEE_SAVED_REGISTERS macro
RestoreReg Rdi, 0
RestoreReg Rsi, 1
RestoreReg Rbx, 2
RestoreReg Rbp, 3
RestoreReg R12, 4
RestoreReg R13, 5
RestoreReg R14, 6
RestoreReg R15, 7

xor eax, eax
ret
Expand Down
88 changes: 18 additions & 70 deletions src/vm/amd64/asmconstants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,87 +374,35 @@ ASMCONSTANTS_C_ASSERT( CORINFO_ArgumentException_ASM

// MachState offsets (AMD64\gmscpu.h)

#define OFFSETOF__MachState__m_Rip 0x00
#define OFFSETOF__MachState__m_Rip 0x00
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_Rip
== offsetof(MachState, m_Rip));

#define OFFSETOF__MachState__m_Rsp 0x08
#define OFFSETOF__MachState__m_Rsp 0x08
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_Rsp
== offsetof(MachState, m_Rsp));

#define OFFSETOF__MachState__m_CaptureRdi 0x10
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureRdi
== offsetof(MachState, m_CaptureRdi));
#define OFFSETOF__MachState__m_Capture 0x10
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_Capture
== offsetof(MachState, m_Capture));

#define OFFSETOF__MachState__m_CaptureRsi 0x18
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureRsi
== offsetof(MachState, m_CaptureRsi));

#define OFFSETOF__MachState__m_CaptureRbx 0x20
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureRbx
== offsetof(MachState, m_CaptureRbx));

#define OFFSETOF__MachState__m_CaptureRbp 0x28
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureRbp
== offsetof(MachState, m_CaptureRbp));

#define OFFSETOF__MachState__m_CaptureR12 0x30
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureR12
== offsetof(MachState, m_CaptureR12));

#define OFFSETOF__MachState__m_CaptureR13 0x38
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureR13
== offsetof(MachState, m_CaptureR13));

#define OFFSETOF__MachState__m_CaptureR14 0x40
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureR14
== offsetof(MachState, m_CaptureR14));

#define OFFSETOF__MachState__m_CaptureR15 0x48
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_CaptureR15
== offsetof(MachState, m_CaptureR15));

#define OFFSETOF__MachState__m_pRdi 0x50
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pRdi
== offsetof(MachState, m_pRdi));

#define OFFSETOF__MachState__m_pRsi 0x58
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pRsi
== offsetof(MachState, m_pRsi));

#define OFFSETOF__MachState__m_pRbx 0x60
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pRbx
== offsetof(MachState, m_pRbx));

#define OFFSETOF__MachState__m_pRbp 0x68
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pRbp
== offsetof(MachState, m_pRbp));

#define OFFSETOF__MachState__m_pR12 0x70
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pR12
== offsetof(MachState, m_pR12));

#define OFFSETOF__MachState__m_pR13 0x78
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pR13
== offsetof(MachState, m_pR13));

#define OFFSETOF__MachState__m_pR14 0x80
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pR14
== offsetof(MachState, m_pR14));

#define OFFSETOF__MachState__m_pR15 0x88
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_pR15
== offsetof(MachState, m_pR15));

#define OFFSETOF__MachState___pRetAddr 0x90
#ifdef UNIX_AMD64_ABI
#define OFFSETOF__MachState__m_Ptrs 0x40
#define OFFSETOF__MachState___pRetAddr 0x70
#define OFFSETOF__LazyMachState__m_CaptureRip 0xA8
#define OFFSETOF__LazyMachState__m_CaptureRsp 0xB0
#else
#define OFFSETOF__MachState__m_Ptrs 0x50
#define OFFSETOF__MachState___pRetAddr 0x90
#define OFFSETOF__LazyMachState__m_CaptureRip 0x98
#define OFFSETOF__LazyMachState__m_CaptureRsp 0xA0
#endif
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState__m_Ptrs
== offsetof(MachState, m_Ptrs));
ASMCONSTANTS_C_ASSERT(OFFSETOF__MachState___pRetAddr
== offsetof(MachState, _pRetAddr));

#define OFFSETOF__LazyMachState__m_CaptureRip 0x98
ASMCONSTANTS_C_ASSERT(OFFSETOF__LazyMachState__m_CaptureRip
== offsetof(LazyMachState, m_CaptureRip));

#define OFFSETOF__LazyMachState__m_CaptureRsp 0xA0
ASMCONSTANTS_C_ASSERT(OFFSETOF__LazyMachState__m_CaptureRsp
== offsetof(LazyMachState, m_CaptureRsp));

Expand Down
123 changes: 36 additions & 87 deletions src/vm/amd64/cgenamd64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,14 @@ void UpdateRegDisplayFromCalleeSavedRegisters(REGDISPLAY * pRD, CalleeSavedRegis
LIMITED_METHOD_CONTRACT;

T_CONTEXT * pContext = pRD->pCurrentContext;
pContext->Rbp = pRegs->rbp;
#ifndef UNIX_AMD64_ABI
pContext->Rsi = pRegs->rsi;
pContext->Rdi = pRegs->rdi;
#endif
pContext->R12 = pRegs->r12;
pContext->R13 = pRegs->r13;
pContext->R14 = pRegs->r14;
pContext->R15 = pRegs->r15;
#define CALLEE_SAVED_REGISTER(regname) pContext->regname = pRegs->regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

KNONVOLATILE_CONTEXT_POINTERS * pContextPointers = pRD->pCurrentContextPointers;
pContextPointers->Rbx = (PULONG64)&pRegs->rbx;
#ifndef UNIX_AMD64_ABI
pContextPointers->Rsi = (PULONG64)&pRegs->rsi;
pContextPointers->Rdi = (PULONG64)&pRegs->rdi;
#endif
pContextPointers->Rbp = (PULONG64)&pRegs->rbp;
pContextPointers->R12 = (PULONG64)&pRegs->r12;
pContextPointers->R13 = (PULONG64)&pRegs->r13;
pContextPointers->R14 = (PULONG64)&pRegs->r14;
pContextPointers->R15 = (PULONG64)&pRegs->r15;
#define CALLEE_SAVED_REGISTER(regname) pContextPointers->regname = (PULONG64)&pRegs->regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER
}

void ClearRegDisplayArgumentAndScratchRegisters(REGDISPLAY * pRD)
Expand Down Expand Up @@ -96,16 +83,10 @@ void TailCallFrame::InitFromContext(T_CONTEXT * pContext)
{
WRAPPER_NO_CONTRACT;

#ifndef UNIX_AMD64_ABI
m_calleeSavedRegisters.rdi = pContext->Rdi;
m_calleeSavedRegisters.rsi = pContext->Rsi;
#endif
m_calleeSavedRegisters.rbx = pContext->Rbx;
m_calleeSavedRegisters.rbp = pContext->Rbp;
m_calleeSavedRegisters.r12 = pContext->R12;
m_calleeSavedRegisters.r13 = pContext->R13;
m_calleeSavedRegisters.r14 = pContext->R14;
m_calleeSavedRegisters.r15 = pContext->R15;
#define CALLEE_SAVED_REGISTER(regname) m_calleeSavedRegisters.regname = pContext->regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

m_pGCLayout = 0;
m_ReturnAddress = pContext->Rip;
}
Expand Down Expand Up @@ -160,16 +141,11 @@ void InlinedCallFrame::UpdateRegDisplay(const PREGDISPLAY pRD)

ClearRegDisplayArgumentAndScratchRegisters(pRD);

pRD->pCurrentContextPointers->Rbx = NULL;
#ifndef UNIX_AMD64_ABI
pRD->pCurrentContextPointers->Rsi = NULL;
pRD->pCurrentContextPointers->Rdi = NULL;
#endif
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = NULL;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

pRD->pCurrentContextPointers->Rbp = (DWORD64 *)&m_pCalleeSavedFP;
pRD->pCurrentContextPointers->R12 = NULL;
pRD->pCurrentContextPointers->R13 = NULL;
pRD->pCurrentContextPointers->R14 = NULL;
pRD->pCurrentContextPointers->R15 = NULL;

SyncRegDisplayToCurrentContext(pRD);

Expand Down Expand Up @@ -210,62 +186,35 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD)
pRD->pCurrentContext->Rip = pRD->ControlPC = pUnwoundState->m_Rip;
pRD->pCurrentContext->Rsp = pRD->SP = pUnwoundState->m_Rsp;

#ifndef UNIX_AMD64_ABI
pRD->pCurrentContext->Rdi = pUnwoundState->m_CaptureRdi;
pRD->pCurrentContext->Rsi = pUnwoundState->m_CaptureRsi;
#endif
pRD->pCurrentContext->Rbx = pUnwoundState->m_CaptureRbx;
pRD->pCurrentContext->Rbp = pUnwoundState->m_CaptureRbp;
pRD->pCurrentContext->R12 = pUnwoundState->m_CaptureR12;
pRD->pCurrentContext->R13 = pUnwoundState->m_CaptureR13;
pRD->pCurrentContext->R14 = pUnwoundState->m_CaptureR14;
pRD->pCurrentContext->R15 = pUnwoundState->m_CaptureR15;
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContext->regname = pUnwoundState->m_Capture.regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

return;
}
#endif // DACCESS_COMPILE

pRD->pCurrentContext->Rip = pRD->ControlPC = m_MachState.m_Rip;
pRD->pCurrentContext->Rsp = pRD->SP = m_MachState.m_Rsp;
pRD->pCurrentContext->Rsp = pRD->SP = m_MachState.m_Rsp;

#ifndef UNIX_AMD64_ABI
pRD->pCurrentContext->Rdi = *m_MachState.m_pRdi;
pRD->pCurrentContext->Rsi = *m_MachState.m_pRsi;
#endif
if (m_MachState.m_pRbx != NULL)
{
pRD->pCurrentContext->Rbx = *m_MachState.m_pRbx;
}
if (m_MachState.m_pRbp != NULL)
{
pRD->pCurrentContext->Rbp = *m_MachState.m_pRbp;
}
if (m_MachState.m_pR12 != NULL)
{
pRD->pCurrentContext->R12 = *m_MachState.m_pR12;
}
if (m_MachState.m_pR13 != NULL)
{
pRD->pCurrentContext->R13 = *m_MachState.m_pR13;
}
if (m_MachState.m_pR14 != NULL)
{
pRD->pCurrentContext->R14 = *m_MachState.m_pR14;
}
if (m_MachState.m_pR15 != NULL)
{
pRD->pCurrentContext->R15 = *m_MachState.m_pR15;
}
#ifndef UNIX_AMD64_ABI
pRD->pCurrentContextPointers->Rdi = m_MachState.m_pRdi;
pRD->pCurrentContextPointers->Rsi = m_MachState.m_pRsi;
#endif
pRD->pCurrentContextPointers->Rbx = m_MachState.m_pRbx;
pRD->pCurrentContextPointers->Rbp = m_MachState.m_pRbp;
pRD->pCurrentContextPointers->R12 = m_MachState.m_pR12;
pRD->pCurrentContextPointers->R13 = m_MachState.m_pR13;
pRD->pCurrentContextPointers->R14 = m_MachState.m_pR14;
pRD->pCurrentContextPointers->R15 = m_MachState.m_pR15;
#ifdef FEATURE_PAL

#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContext->regname = (m_MachState.m_Ptrs.p##regname != NULL) ? \
*m_MachState.m_Ptrs.p##regname : m_MachState.m_Unwound.regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

#else // FEATURE_PAL

#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContext->regname = *m_MachState.m_Ptrs.p##regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

#endif // FEATURE_PAL

#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = m_MachState.m_Ptrs.p##regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

//
// Clear all knowledge of scratch registers. We're skipping to any
Expand Down
62 changes: 42 additions & 20 deletions src/vm/amd64/cgencpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,28 +173,8 @@ typedef INT64 StackElemType;
// This represents some of the TransitionFrame fields that are
// stored at negative offsets.
//--------------------------------------------------------------------
typedef DPTR(struct CalleeSavedRegisters) PTR_CalleeSavedRegisters;
struct CalleeSavedRegisters {
#ifndef UNIX_AMD64_ABI
INT_PTR rdi;
INT_PTR rsi;
INT_PTR rbx;
INT_PTR rbp;
#endif
INT_PTR r12;
INT_PTR r13;
INT_PTR r14;
INT_PTR r15;
#ifdef UNIX_AMD64_ABI
INT_PTR rbx;
INT_PTR rbp;
#endif
};

struct REGDISPLAY;

void UpdateRegDisplayFromCalleeSavedRegisters(REGDISPLAY * pRD, CalleeSavedRegisters * pRegs);

//--------------------------------------------------------------------
// This represents the arguments that are stored in volatile registers.
// This should not overlap the CalleeSavedRegisters since those are already
Expand All @@ -215,6 +195,18 @@ void UpdateRegDisplayFromCalleeSavedRegisters(REGDISPLAY * pRD, CalleeSavedRegis

#define NUM_ARGUMENT_REGISTERS 6

// The order of registers in this macro is hardcoded in assembly code
// at number of places
#define ENUM_CALLEE_SAVED_REGISTERS() \
Copy link

Choose a reason for hiding this comment

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

Could you please add a comment saying that the order of registers here must match the index of the register that is passed into RestoreReg macro?

CALLEE_SAVED_REGISTER(R12) \
CALLEE_SAVED_REGISTER(R13) \
CALLEE_SAVED_REGISTER(R14) \
CALLEE_SAVED_REGISTER(R15) \
CALLEE_SAVED_REGISTER(Rbx) \
CALLEE_SAVED_REGISTER(Rbp)

#define NUM_CALLEE_SAVED_REGISTERS 6

#else // UNIX_AMD64_ABI

#define ENUM_ARGUMENT_REGISTERS() \
Expand All @@ -225,6 +217,20 @@ void UpdateRegDisplayFromCalleeSavedRegisters(REGDISPLAY * pRD, CalleeSavedRegis

#define NUM_ARGUMENT_REGISTERS 4

// The order of registers in this macro is hardcoded in assembly code
// at number of places
#define ENUM_CALLEE_SAVED_REGISTERS() \
CALLEE_SAVED_REGISTER(Rdi) \
CALLEE_SAVED_REGISTER(Rsi) \
CALLEE_SAVED_REGISTER(Rbx) \
Copy link

Choose a reason for hiding this comment

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

Is there a reason why the order of registers is different between Windows (..., rbx, rbp, r12, r13, r14, r15) and Unix (r12, r13, r14, r15, rbx, rbp)?

Copy link
Member Author

Choose a reason for hiding this comment

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

This order is hardcoded in assembly code at number of places, not just the places I am changing. I have added comment about it.

The order on Windows is what it used to be for a long time - not worth it to rock the boat to "fix it".

We need RBP to be last on Unix to erect proper RBP frames.

Copy link

Choose a reason for hiding this comment

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

Thank you for the explanation! I agree, it is not worth changing it.

CALLEE_SAVED_REGISTER(Rbp) \
CALLEE_SAVED_REGISTER(R12) \
CALLEE_SAVED_REGISTER(R13) \
CALLEE_SAVED_REGISTER(R14) \
CALLEE_SAVED_REGISTER(R15)

#define NUM_CALLEE_SAVED_REGISTERS 8

#endif // UNIX_AMD64_ABI

typedef DPTR(struct ArgumentRegisters) PTR_ArgumentRegisters;
Expand All @@ -234,6 +240,19 @@ struct ArgumentRegisters {
#undef ARGUMENT_REGISTER
};

typedef DPTR(struct CalleeSavedRegisters) PTR_CalleeSavedRegisters;
struct CalleeSavedRegisters {
#define CALLEE_SAVED_REGISTER(regname) INT_PTR regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER
};

struct CalleeSavedRegistersPointers {
#define CALLEE_SAVED_REGISTER(regname) PTR_TADDR p##regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER
};

#define SCRATCH_REGISTER_X86REG kRAX

#ifdef UNIX_AMD64_ABI
Expand All @@ -254,6 +273,9 @@ struct FloatArgumentRegisters {
#endif


void UpdateRegDisplayFromCalleeSavedRegisters(REGDISPLAY * pRD, CalleeSavedRegisters * pRegs);


// Sufficient context for Try/Catch restoration.
struct EHContext {
// Not used
Expand Down
Loading