Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RISC-V] Implement SOS related Debugger API code. #94454

Merged
merged 3 commits into from
Nov 9, 2023
Merged

[RISC-V] Implement SOS related Debugger API code. #94454

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c39db9f
[RISC-V] Implement SOS related Debugger API code.
viewizard Nov 7, 2023
bdb6876
[RISC-V] Fix R0 copy.
viewizard Nov 7, 2023
ab84b8c
[RISC-V] Remove R0 from DebuggerREGDISPLAY.
viewizard Nov 9, 2023
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
120 changes: 120 additions & 0 deletions src/coreclr/debug/di/rsthread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6327,6 +6327,126 @@ UINT_PTR * CordbNativeFrame::GetAddressOfRegister(CorDebugRegister regNum) const
case REGISTER_ARM64_PC:
ret = (UINT_PTR*)&m_rd.PC;
break;
#elif defined(TARGET_RISCV64)
case REGISTER_RISCV64_PC:
ret = (UINT_PTR*)&m_rd.PC;
break;

case REGISTER_RISCV64_RA:
ret = (UINT_PTR*)&m_rd.RA;
break;

case REGISTER_RISCV64_GP:
ret = (UINT_PTR*)&m_rd.GP;
break;

case REGISTER_RISCV64_TP:
ret = (UINT_PTR*)&m_rd.TP;
break;

case REGISTER_RISCV64_T0:
ret = (UINT_PTR*)&m_rd.T0;
break;

case REGISTER_RISCV64_T1:
ret = (UINT_PTR*)&m_rd.T1;
break;

case REGISTER_RISCV64_T2:
ret = (UINT_PTR*)&m_rd.T2;
break;

case REGISTER_RISCV64_S1:
ret = (UINT_PTR*)&m_rd.S1;
break;

case REGISTER_RISCV64_A0:
ret = (UINT_PTR*)&m_rd.A0;
break;

case REGISTER_RISCV64_A1:
ret = (UINT_PTR*)&m_rd.A1;
break;

case REGISTER_RISCV64_A2:
ret = (UINT_PTR*)&m_rd.A2;
break;

case REGISTER_RISCV64_A3:
ret = (UINT_PTR*)&m_rd.A3;
break;

case REGISTER_RISCV64_A4:
ret = (UINT_PTR*)&m_rd.A4;
break;

case REGISTER_RISCV64_A5:
ret = (UINT_PTR*)&m_rd.A5;
break;

case REGISTER_RISCV64_A6:
ret = (UINT_PTR*)&m_rd.A6;
break;

case REGISTER_RISCV64_A7:
ret = (UINT_PTR*)&m_rd.A7;
break;

case REGISTER_RISCV64_S2:
ret = (UINT_PTR*)&m_rd.S2;
break;

case REGISTER_RISCV64_S3:
ret = (UINT_PTR*)&m_rd.S3;
break;

case REGISTER_RISCV64_S4:
ret = (UINT_PTR*)&m_rd.S4;
break;

case REGISTER_RISCV64_S5:
ret = (UINT_PTR*)&m_rd.S5;
break;

case REGISTER_RISCV64_S6:
ret = (UINT_PTR*)&m_rd.S6;
break;

case REGISTER_RISCV64_S7:
ret = (UINT_PTR*)&m_rd.S7;
break;

case REGISTER_RISCV64_S8:
ret = (UINT_PTR*)&m_rd.S8;
break;

case REGISTER_RISCV64_S9:
ret = (UINT_PTR*)&m_rd.S9;
break;

case REGISTER_RISCV64_S10:
ret = (UINT_PTR*)&m_rd.S10;
break;

case REGISTER_RISCV64_S11:
ret = (UINT_PTR*)&m_rd.S11;
break;

case REGISTER_RISCV64_T3:
ret = (UINT_PTR*)&m_rd.T3;
break;

case REGISTER_RISCV64_T4:
ret = (UINT_PTR*)&m_rd.T4;
break;

case REGISTER_RISCV64_T5:
ret = (UINT_PTR*)&m_rd.T5;
break;

case REGISTER_RISCV64_T6:
ret = (UINT_PTR*)&m_rd.T6;
break;
#endif

default:
Expand Down
35 changes: 35 additions & 0 deletions src/coreclr/debug/inc/dbgipcevents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,41 @@ struct MSLAYOUT DebuggerREGDISPLAY
SIZE_T S7;
SIZE_T S8;
SIZE_T PC;
#elif defined(TARGET_RISCV64)
#define DebuggerIPCE_FloatCount 32
SIZE_T R0;
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of having R0 here? It is not a real register - it should be always zero.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I miss this one, when copy registers from register context. Sure, no reason store R0 for frames at stack trace.

SIZE_T RA;
SIZE_T SP;
SIZE_T GP;
SIZE_T TP;
SIZE_T T0;
SIZE_T T1;
SIZE_T T2;
SIZE_T FP;
SIZE_T S1;
SIZE_T A0;
SIZE_T A1;
SIZE_T A2;
SIZE_T A3;
SIZE_T A4;
SIZE_T A5;
SIZE_T A6;
SIZE_T A7;
SIZE_T S2;
SIZE_T S3;
SIZE_T S4;
SIZE_T S5;
SIZE_T S6;
SIZE_T S7;
SIZE_T S8;
SIZE_T S9;
SIZE_T S10;
SIZE_T S11;
SIZE_T T3;
SIZE_T T4;
SIZE_T T5;
SIZE_T T6;
SIZE_T PC;
#else
#define DebuggerIPCE_FloatCount 1

Expand Down
75 changes: 73 additions & 2 deletions src/coreclr/debug/shared/riscv64/primitives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,83 @@
//
void CORDbgCopyThreadContext(DT_CONTEXT* pDst, const DT_CONTEXT* pSrc)
{
_ASSERTE(!"RISCV64:NYI");
DWORD dstFlags = pDst->ContextFlags;
DWORD srcFlags = pSrc->ContextFlags;
LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: pDst=0x%08x dstFlags=0x%x, pSrc=0x%08x srcFlags=0x%x\n",
pDst, dstFlags, pSrc, srcFlags));

if ((dstFlags & srcFlags & DT_CONTEXT_CONTROL) == DT_CONTEXT_CONTROL)
{
LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: RA: pDst=0x%lx, pSrc=0x%lx, Flags=0x%x\n",
pDst->Ra, pSrc->Ra, DT_CONTEXT_CONTROL));
pDst->Ra = pSrc->Ra;

LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: SP: pDst=0x%lx, pSrc=0x%lx, Flags=0x%x\n",
pDst->Sp, pSrc->Sp, DT_CONTEXT_CONTROL));
pDst->Sp = pSrc->Sp;

LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: FP: pDst=0x%lx, pSrc=0x%lx, Flags=0x%x\n",
pDst->Fp, pSrc->Fp, DT_CONTEXT_CONTROL));
pDst->Fp = pSrc->Fp;

LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: PC: pDst=0x%lx, pSrc=0x%lx, Flags=0x%x\n",
pDst->Pc, pSrc->Pc, DT_CONTEXT_CONTROL));
pDst->Pc = pSrc->Pc;
}

if ((dstFlags & srcFlags & DT_CONTEXT_INTEGER) == DT_CONTEXT_INTEGER)
{
CopyContextChunk(&pDst->Gp, &pSrc->Gp, &pDst->Fp,
DT_CONTEXT_INTEGER);
CopyContextChunk(&pDst->S1, &pSrc->S1, &pDst->Pc,
DT_CONTEXT_INTEGER);
LOG((LF_CORDB, LL_INFO1000000,
"CP::CTC: T0: pDst=0x%lx, pSrc=0x%lx, Flags=0x%x\n",
pDst->R0, pSrc->R0, DT_CONTEXT_INTEGER));
pDst->R0 = pSrc->R0;
}

if ((dstFlags & srcFlags & DT_CONTEXT_FLOATING_POINT) == DT_CONTEXT_FLOATING_POINT)
{
CopyContextChunk(&pDst->F[0], &pSrc->F[0], &pDst->F[32],
DT_CONTEXT_FLOATING_POINT);
pDst->Fcsr = pSrc->Fcsr;
}
}

#if defined(ALLOW_VMPTR_ACCESS) || !defined(RIGHT_SIDE_COMPILE)
void SetDebuggerREGDISPLAYFromREGDISPLAY(DebuggerREGDISPLAY* pDRD, REGDISPLAY* pRD)
{
_ASSERTE(!"RISCV64:NYI");
SUPPORTS_DAC_HOST_ONLY;

DT_CONTEXT* pContext = reinterpret_cast<DT_CONTEXT*>(pRD->pCurrentContext);

// We must pay attention to the context flags so that we only use valid portions
// of the context.
DWORD flags = pContext->ContextFlags;
if ((flags & DT_CONTEXT_CONTROL) == DT_CONTEXT_CONTROL)
{
pDRD->FP = (SIZE_T)CORDbgGetFP(pContext);
pDRD->PC = (SIZE_T)pContext->Pc;
pDRD->RA = (SIZE_T)pContext->Ra;
}

if ((flags & DT_CONTEXT_INTEGER) == DT_CONTEXT_INTEGER)
{
memcpy(&pDRD->GP, &pContext->Gp, sizeof(pDRD->GP) * 5);
memcpy(&pDRD->S1, &pContext->S1, sizeof(pDRD->S1) * 23);
pDRD->R0 = (SIZE_T)pContext->R0;
}

pDRD->SP = pRD->SP;

LOG( (LF_CORDB, LL_INFO1000, "DT::TASSC:Registers:"
"SP = %x",
pDRD->SP) );
}
#endif // ALLOW_VMPTR_ACCESS || !RIGHT_SIDE_COMPILE
4 changes: 2 additions & 2 deletions src/coreclr/inc/cordebug.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3988,14 +3988,14 @@ interface ICorDebugRegisterSet : IUnknown
REGISTER_LOONGARCH64_F31,

REGISTER_RISCV64_PC = 0,
REGISTER_RISCV64_RA,
REGISTER_RISCV64_SP,
REGISTER_RISCV64_FP,
jkotas marked this conversation as resolved.
Show resolved Hide resolved
REGISTER_RISCV64_RA,
REGISTER_RISCV64_GP,
Copy link
Member

Choose a reason for hiding this comment

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

There is REGISTER_RISCV64_X0 with TODO at the end of this list that does not appear to be used anywhere. Delete it while you are on it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am still not sure about REGISTER_RISCV64_X0, plus, I see it used here for some purpose:

runtime/src/coreclr/debug/inc/riscv64/primitives.h

Line 52 in 23695f9

REGISTER_RISCV64_X0, // TODO-RISCV64-CQ: Add X0 to access DbgReg in correct order

Note, current implementation aimed and tested only with diagnostics SOS command with simple tests, that is extremely limited and I belive not cover all cases of this code usage. So, I leave it as is for now.

Copy link
Member

Choose a reason for hiding this comment

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

I think REGISTER_RISCV64_X0 tries to be the R0 register that is not a real register, so it does not need to be in the enum.

Copy link
Member Author

Choose a reason for hiding this comment

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

@clamp03 could you please clarify, is X0 here used in the R0 meaning?

Copy link
Member Author

@viewizard viewizard Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

ICorDebugInfo::RegNum have REGNUM_R0

runtime/src/coreclr/inc/cordebuginfo.h

Line 183 in 0ad3723

REGNUM_R0,

and (looks like) REGISTER_RISCV64_X0 used just as "padding" for proper mapping with g_JITToCorDbgReg

runtime/src/coreclr/debug/inc/riscv64/primitives.h

Lines 155 to 165 in 23695f9

//
// Mapping from ICorDebugInfo register numbers to CorDebugRegister
// numbers. Note: this must match the order in corinfo.h.
//
inline CorDebugRegister ConvertRegNumToCorDebugRegister(ICorDebugInfo::RegNum reg)
{
LIMITED_METHOD_CONTRACT;
_ASSERTE(reg >= 0);
_ASSERTE(static_cast<size_t>(reg) < ARRAY_SIZE(g_JITToCorDbgReg));
return g_JITToCorDbgReg[reg];
}

I am not really strong in this code, do we really could remove R0 from ICorDebugInfo::RegNum ? Is not ICorDebugInfo::RegNum used as indexes for "normal" (not debug related) register context?

Copy link
Member

Choose a reason for hiding this comment

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

This padding is an internal implementation detail. It should not leak through the public ICorDebug surface.

Arm64 has similar wzr register and we do not have a constant for it in the ICorDebug register enum either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for explanation, I got the idea. Will prepare appropriate changes.

REGISTER_RISCV64_TP,
REGISTER_RISCV64_T0,
REGISTER_RISCV64_T1,
REGISTER_RISCV64_T2,
REGISTER_RISCV64_FP,
REGISTER_RISCV64_S1,
REGISTER_RISCV64_A0,
REGISTER_RISCV64_A1,
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/inc/gcinfodecoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ inline TADDR GetSP(T_CONTEXT* context)
return (TADDR)context->Sp;
#elif defined(TARGET_LOONGARCH64)
return (TADDR)context->Sp;
#elif defined(TARGET_RISCV64)
return (TADDR)context->Sp;
#else
_ASSERTE(!"nyi for platform");
#endif
Expand All @@ -102,6 +104,8 @@ inline PCODE GetIP(T_CONTEXT* context)
return (PCODE)context->Pc;
#elif defined(TARGET_LOONGARCH64)
return (PCODE)context->Pc;
#elif defined(TARGET_RISCV64)
return (PCODE)context->Pc;
#else
_ASSERTE(!"nyi for platform");
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/regdisp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ inline TADDR GetRegdisplayStackMark(REGDISPLAY *display)
_ASSERTE(GetRegdisplaySP(display) == GetSP(display->pCurrentContext));
return GetRegdisplaySP(display);

#elif defined(TARGET_ARM64)
#elif defined(TARGET_ARM64) || defined(TARGET_RISCV64)

_ASSERTE(display->IsCallerContextValid);
return GetSP(display->pCallerContext);
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/pal/prebuilt/inc/cordebug.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9707,15 +9707,15 @@ enum CorDebugRegister
REGISTER_LOONGARCH64_F30 = ( REGISTER_LOONGARCH64_F29 + 1 ) ,
REGISTER_LOONGARCH64_F31 = ( REGISTER_LOONGARCH64_F30 + 1 ) ,
REGISTER_RISCV64_PC = 0,
REGISTER_RISCV64_RA = ( REGISTER_RISCV64_PC + 1 ) ,
REGISTER_RISCV64_SP = ( REGISTER_RISCV64_RA + 1 ) ,
REGISTER_RISCV64_GP = ( REGISTER_RISCV64_SP + 1 ) ,
REGISTER_RISCV64_SP = ( REGISTER_RISCV64_PC + 1 ) ,
REGISTER_RISCV64_FP = ( REGISTER_RISCV64_SP + 1 ) ,
REGISTER_RISCV64_RA = ( REGISTER_RISCV64_FP + 1 ) ,
REGISTER_RISCV64_GP = ( REGISTER_RISCV64_RA + 1 ) ,
REGISTER_RISCV64_TP = ( REGISTER_RISCV64_GP + 1 ) ,
REGISTER_RISCV64_T0 = ( REGISTER_RISCV64_TP + 1 ) ,
REGISTER_RISCV64_T1 = ( REGISTER_RISCV64_T0 + 1 ) ,
REGISTER_RISCV64_T2 = ( REGISTER_RISCV64_T1 + 1 ) ,
REGISTER_RISCV64_FP = ( REGISTER_RISCV64_T2 + 1 ) ,
REGISTER_RISCV64_S1 = ( REGISTER_RISCV64_FP + 1 ) ,
REGISTER_RISCV64_S1 = ( REGISTER_RISCV64_T2 + 1 ) ,
REGISTER_RISCV64_A0 = ( REGISTER_RISCV64_S1 + 1 ) ,
REGISTER_RISCV64_A1 = ( REGISTER_RISCV64_A0 + 1 ) ,
REGISTER_RISCV64_A2 = ( REGISTER_RISCV64_A1 + 1 ) ,
Expand Down
Loading