Skip to content

Fix debugger interaction in the interpreter #118699

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

Closed
wants to merge 4 commits into from
Closed

Fix debugger interaction in the interpreter #118699

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
067fb99
Fix the debugger data to be more reasonable for the interpreter
davidwrighton Aug 13, 2025
9e0075f
Rename GetNativeCode_CurrentDefault back to GetNativeCode
davidwrighton Aug 13, 2025
5be5f2b
Remove spurious comment
davidwrighton Aug 13, 2025
771066a
Fix build issue
davidwrighton Aug 13, 2025
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
3 changes: 2 additions & 1 deletion src/coreclr/debug/daccess/dacdbiimpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,8 @@ void DacDbiInterfaceImpl::GetMethodRegionInfo(MethodDesc * pMethodDe
codeInfo.GetMethodRegionInfo(&methodRegionInfo);

// now get the rest of the region information
pCodeInfo->m_rgCodeRegions[kHot].cbSize = (ULONG)methodRegionInfo.hotSize;
pCodeInfo->m_rgCodeRegions[kHot].Init(PCODEToPINSTR(methodRegionInfo.hotStartAddress),
(ULONG)methodRegionInfo.hotSize);
pCodeInfo->m_rgCodeRegions[kCold].Init(PCODEToPINSTR(methodRegionInfo.coldStartAddress),
(ULONG)methodRegionInfo.coldSize);
_ASSERTE(pCodeInfo->IsValid());
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/debug/daccess/task.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4341,9 +4341,10 @@ ClrDataMethodInstance::GetRepresentativeEntryAddress(

EX_TRY
{
if (m_methodDesc->HasNativeCode())
PCODE codeAddr = m_methodDesc->GetNativeCode();
if (codeAddr != NULL)
{
*addr = TO_CDADDR(m_methodDesc->GetNativeCode());
*addr = TO_CDADDR(codeAddr);
status = S_OK;
}
else
Expand Down
14 changes: 0 additions & 14 deletions src/coreclr/vm/eventtrace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4415,20 +4415,6 @@ TADDR MethodAndStartAddressToEECodeInfoPointer(MethodDesc *pMethodDesc, PCODE pN
return 0;
}

#ifdef FEATURE_INTERPRETER
RangeSection * pRS = ExecutionManager::FindCodeRange(PINSTRToPCODE(start), ExecutionManager::GetScanFlags());
if (pRS != NULL && pRS->_flags & RangeSection::RANGE_SECTION_RANGELIST)
{
if (pRS->_pRangeList->GetCodeBlockKind() == STUB_CODE_BLOCK_STUBPRECODE)
{
if (((StubPrecode*)start)->GetType() == PRECODE_INTERPRETER)
{
start = ((InterpreterPrecode*)start)->GetData()->ByteCodeAddr;
}
}
}
#endif // FEATURE_INTERPRETER

return start;
}

Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15019,6 +15019,23 @@ void EECodeInfo::Init(PCODE codeAddress, ExecutionManager::ScanFlag scanFlag)
if (pRS == NULL)
goto Invalid;

#ifdef FEATURE_INTERPRETER
if (pRS != NULL && pRS->_flags & RangeSection::RANGE_SECTION_RANGELIST)
{
if (pRS->_pRangeList->GetCodeBlockKind() == STUB_CODE_BLOCK_STUBPRECODE)
{
PTR_StubPrecode pStubPrecode = dac_cast<PTR_StubPrecode>(PCODEToPINSTR(codeAddress));
if (pStubPrecode->GetType() == PRECODE_INTERPRETER)
{
codeAddress = dac_cast<PTR_InterpreterPrecode>(pStubPrecode)->GetData()->ByteCodeAddr;
Copy link
Member

@jkotas jkotas Aug 15, 2025

Choose a reason for hiding this comment

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

This will end up creating codeinfo for a different IP than the one passed into Init. I can imagine that it is fixing a problem somewhere, but it is likely creating different problems somewhere else. Is this going to break places like

runtime/src/coreclr/vm/threadsuspend.cpp

Lines 5787 to 5789 in 2a95831

EECodeInfo codeInfo(ip);
if (!codeInfo.IsValid())
return;
where we expect the valid EECodeInfo to have real code with GCInfo, etc. behind it?

pRS = ExecutionManager::FindCodeRange(codeAddress, scanFlag);
if (pRS == NULL)
goto Invalid;
}
}
}
#endif // FEATURE_INTERPRETER

if (!pRS->_pjit->JitCodeToMethodInfo(pRS, codeAddress, &m_pMD, this))
goto Invalid;

Expand Down
16 changes: 14 additions & 2 deletions src/coreclr/vm/method.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2207,7 +2207,8 @@ MethodDesc* NonVirtualEntry2MethodDesc(PCODE entryPoint)
}
CONTRACTL_END

RangeSection* pRS = ExecutionManager::FindCodeRange(entryPoint, ExecutionManager::GetScanFlags());
ExecutionManager::ScanFlag scanFlag = ExecutionManager::GetScanFlags();
RangeSection* pRS = ExecutionManager::FindCodeRange(entryPoint, scanFlag);
if (pRS == NULL)
{
return NULL;
Expand All @@ -2222,7 +2223,18 @@ MethodDesc* NonVirtualEntry2MethodDesc(PCODE entryPoint)
}
if (pRS->_pRangeList->GetCodeBlockKind() == STUB_CODE_BLOCK_STUBPRECODE)
{
return (MethodDesc*)((StubPrecode*)PCODEToPINSTR(entryPoint))->GetMethodDesc();
StubPrecode stubPrecode = (StubPrecode*)PCODEToPINSTR(entryPoint);
#ifdef FEATURE_INTERPRETER
if (stubPrecode->GetType() == PRECODE_INTERPRETER))
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

There is an extra closing parenthesis that will cause a compilation error.

Suggested change
if (stubPrecode->GetType() == PRECODE_INTERPRETER))
if (stubPrecode->GetType() == PRECODE_INTERPRETER)

Copilot uses AI. Check for mistakes.
{
entryPoint = dac_cast<PTR_InterpreterPrecode>(pStubPrecode)->GetData()->ByteCodeAddr;
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

The variable 'pStubPrecode' is used but not defined. It should be 'stubPrecode' to match the variable declared on line 2226.

Suggested change
entryPoint = dac_cast<PTR_InterpreterPrecode>(pStubPrecode)->GetData()->ByteCodeAddr;
entryPoint = dac_cast<PTR_InterpreterPrecode>(stubPrecode)->GetData()->ByteCodeAddr;

Copilot uses AI. Check for mistakes.
pRS = ExecutionManager::FindCodeRange(entryPoint, scanFlag);
}
else
#endif // FEATURE_INTERPRETER
{
return (MethodDesc*)(stubPrecode)->GetMethodDesc();
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

The parentheses around 'stubPrecode' are unnecessary and the cast should be removed. It should be 'return stubPrecode->GetMethodDesc();'

Suggested change
return (MethodDesc*)(stubPrecode)->GetMethodDesc();
return stubPrecode->GetMethodDesc();

Copilot uses AI. Check for mistakes.
Copy link
Member

@kg kg Aug 14, 2025

Choose a reason for hiding this comment

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

I think it wants (MethodDesc*)(stubPrecode->GetMethodDesc()), which feels better to me also

}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/multicorejitplayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ void MulticoreJitProfilePlayer::CompileMethodInfoRecord(Module *pModule, MethodD
}
}

if (pMethod->GetNativeCode() == (PCODE)NULL && !GetAppDomain()->GetMulticoreJitManager().GetMulticoreJitCodeStorage().LookupMethodCode(pMethod))
if (!pMethod->HasNativeCode() && !GetAppDomain()->GetMulticoreJitManager().GetMulticoreJitCodeStorage().LookupMethodCode(pMethod))
{
if (CompileMethodDesc(pModule, pMethod))
{
Expand Down
Loading