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.

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

"Go to disassembly" in VS aborts debugging session #18602

Closed
jkotas opened this issue Jun 22, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@jkotas
Copy link
Member

commented Jun 22, 2018

Repro:

  • Attach VS to CoreCLR process
  • Find frame in System.Private.Corelib
  • Select "Go to disassembly"

Result:

msvsmon.exe crashes

@jkotas

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2018

On checked CoreCLR build, it hits "IsNeutered" assert at this callstack - it is where things begin to go wrong:

mscordbi_7fff9fcd0000!DbgAssertDialog+0x150 [d:\coreclr\src\utilcode\debug.cpp @ 726] 
mscordbi_7fff9fcd0000!CordbCode::~CordbCode+0x4d [d:\coreclr\src\debug\di\module.cpp @ 2829] 
mscordbi_7fff9fcd0000!_CallSettingFrame+0x20 [f:\dd\vctools\crt\vcruntime\src\eh\amd64\handlers.asm @ 50] 
mscordbi_7fff9fcd0000!__FrameHandler3::FrameUnwindToState+0x201 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 1210] 
mscordbi_7fff9fcd0000!__FrameHandler3::FrameUnwindToEmptyState+0x8e [f:\dd\vctools\crt\vcruntime\src\eh\risctrnsctrl.cpp @ 246] 
mscordbi_7fff9fcd0000!__InternalCxxFrameHandler<__FrameHandler3>+0x273 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 306] 
mscordbi_7fff9fcd0000!__CxxFrameHandler3+0xa7 [f:\dd\vctools\crt\vcruntime\src\eh\risctrnsctrl.cpp @ 270] 
mscordbi_7fff9fcd0000!__GSHandlerCheck_EH+0x90 [f:\dd\vctools\crt\vcstartup\src\gs\amd64\gshandlereh.cpp @ 168] 
ntdll!RtlpExecuteHandlerForUnwind+0xd [minkernel\ntos\rtl\amd64\xcptmisc.asm @ 252] 
ntdll!RtlUnwindEx+0x3a0 [minkernel\ntos\rtl\amd64\exdsptch.c @ 1069] 
mscordbi_7fff9fcd0000!__FrameHandler3::UnwindNestedFrames+0x155 [f:\dd\vctools\crt\vcruntime\src\eh\risctrnsctrl.cpp @ 744] 
mscordbi_7fff9fcd0000!CatchIt<__FrameHandler3>+0x104 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 1367] 
mscordbi_7fff9fcd0000!FindHandler<__FrameHandler3>+0x539 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 642] 
mscordbi_7fff9fcd0000!__InternalCxxFrameHandler<__FrameHandler3>+0x471 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 365] 
mscordbi_7fff9fcd0000!__CxxFrameHandler3+0xa7 [f:\dd\vctools\crt\vcruntime\src\eh\risctrnsctrl.cpp @ 270] 
ntdll!RtlpExecuteHandlerForException+0xd [minkernel\ntos\rtl\amd64\xcptmisc.asm @ 131] 
ntdll!RtlDispatchException+0x3c6 [minkernel\ntos\rtl\amd64\exdsptch.c @ 569] 
ntdll!KiUserExceptionDispatch+0x2e [minkernel\ntos\rtl\amd64\trampoln.asm @ 745] 
KERNELBASE!RaiseException+0x68 [minkernel\kernelbase\xcpt.c @ 922] 
mscordaccore!_CxxThrowException+0x12d [f:\dd\vctools\crt\vcruntime\src\eh\throw.cpp @ 133] 
mscordaccore!DacError+0xc0 [d:\coreclr\src\debug\daccess\dacfn.cpp @ 118] 
mscordaccore!DacReadAll+0x9d [d:\coreclr\src\debug\daccess\dacfn.cpp @ 179] 
mscordaccore!DacInstantiateClassByVTable+0xbd [d:\coreclr\src\debug\daccess\dacfn.cpp @ 576] 
mscordaccore!__VPtr<Module>::operator->+0x12 [d:\coreclr\src\inc\daccess.h @ 1336] 
mscordaccore!ILCodeVersion::GetIL+0x5a [d:\coreclr\src\vm\codeversion.cpp @ 800] 
mscordaccore!DacDbiInterfaceImpl::GetILCodeVersionNodeData+0x91 [d:\coreclr\src\debug\daccess\dacdbiimpl.cpp @ 7254] 
mscordbi_7fff9fcd0000!CordbReJitILCode::CordbReJitILCode+0x147 [d:\coreclr\src\debug\di\module.cpp @ 3437] 
mscordbi_7fff9fcd0000!CordbFunction::LookupOrCreateReJitILCode+0xdf [d:\coreclr\src\debug\di\rsfunction.cpp @ 1216] 
mscordbi_7fff9fcd0000!CordbFunction::GetActiveReJitRequestILCode+0x24b [d:\coreclr\src\debug\di\rsfunction.cpp @ 569] 
vsdebugeng_impl!ManagedDM::InstructionAddress::GetRejitVersionFromCorIlMap+0x117 [f:\dd\src\debugger\concord\impl\manageddm\common\instructionaddress.cpp @ 968] 
@jkotas

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2018

@noahfalk Could you please take a look? Looks related to #18322

@noahfalk

This comment has been minimized.

Copy link
Member

commented Jun 22, 2018

@jkotas thanks for finding this! Interestingly this appears to be a pre-existing bug in 2.1.0 RTM but #18322 increased its exposure. In my attempt to repro it did not crash msvsmon, but in both 2.1.0 and current daily builds the debugger is effectively doing an invalid type cast that treats the block of memory for a Module* as if it was an ILCodeVersionNode*. Later use of this invalid pointer in the code path that #18322 changed presumably gave you the crash.

The bug occurs at: https://github.com/dotnet/coreclr/blob/master/src/debug/daccess/dacdbiimpl.cpp#L7178

Not all ILCodeVersions are backed by an explicit ILCodeVersionNode. The code would be correct if ILCodeVersion::AsNode had C# style semantics where non-Node versions return NULL, but that isn't what ILCodeVersion::AsNode does (it is, confusingly, what NativeCodeVersion::AsNode does)

The other usage of AsNode() does it safely:
https://github.com/dotnet/coreclr/blob/master/src/debug/daccess/dacdbiimpl.cpp#L7231

Because the bug is dereferencing the wrong memory there is some limited non-determinism at play, but in the example I had under the debugger for 2.1.0 the failure proceeds like this:

  1. https://github.com/dotnet/coreclr/blob/master/src/debug/daccess/dacdbiimpl.cpp#L7178 creates a bad VMPTR_ILCodeVersionNode pointer that should have been NULL
  2. https://github.com/dotnet/coreclr/blob/master/src/debug/di/rsfunction.cpp#L566 incorrectly goes down the ReJIT IL path for an IL code body that was never modified.
  3. https://github.com/dotnet/coreclr/blob/master/src/debug/daccess/dacdbiimpl.cpp#L7248 DacDbiInterfaceImpl::GetILCodeVersionNodeData reads garbage into DacSharedReJitInfo* pData
  4. https://github.com/dotnet/coreclr/blob/master/src/debug/di/module.cpp#L3499
    CordbReJitILCode::Init parses the bad data and rejects it
  5. https://github.com/dotnet/coreclr/blob/master/src/debug/di/module.cpp#L3438
    CordbReJitILCode::CordbReJitILCode converts the failing HRESULT to a C++ exception
0:007> knL
 # Child-SP          RetAddr           Call Site
00 00000079`f76fefc0 00007ffc`93f81b32 KERNELBASE!RaiseException+0x68
01 00000079`f76ff0a0 00007ffc`93f1b161 mscordbi_7ffc93eb0000!_CxxThrowException+0xc2
02 00000079`f76ff120 00007ffc`93ee8489 mscordbi_7ffc93eb0000!ThrowHR+0x111
03 (Inline Function) --------`-------- mscordbi_7ffc93eb0000!IfFailThrow+0x3a
04 00000079`f76ff180 00007ffc`93f0f992 mscordbi_7ffc93eb0000!CordbReJitILCode::CordbReJitILCode+0x169
05 00000079`f76ff220 00007ffc`93f0e903 mscordbi_7ffc93eb0000!CordbFunction::LookupOrCreateReJitILCode+0x62
06 00000079`f76ff270 00007ffc`7abac3d7 mscordbi_7ffc93eb0000!CordbFunction::GetActiveReJitRequestILCode+0x103
07 00000079`f76ff350 00007ffc`7abab56f vsdebugeng_impl!ReleaseForeground+0x81907
08 00000079`f76ff3a0 00007ffc`7ab7bbf1 vsdebugeng_impl!ReleaseForeground+0x80a9f
09 00000079`f76ff450 00007ffc`7ab7d8a2 vsdebugeng_impl!ReleaseForeground+0x51121
0a 00000079`f76ff520 00007ffc`7d9c53c6 vsdebugeng_impl!ReleaseForeground+0x52dd2
0b 00000079`f76ff570 00007ffc`7da42798 VSDebugEng!ProcF47641A9B4F4EF84594FF51457BD43AD+0x92
0c 00000079`f76ff610 00007ffc`7da0f0dd VSDebugEng!ProcDkmWorkListSetDescription+0x2e888
0d 00000079`f76ff6a0 00007ffc`7da60404 VSDebugEng!DkmCreateManagedInterface+0x57b9
0e 00000079`f76ff720 00007ffc`7da069cf VSDebugEng!ProcDkmWorkListSetDescription+0x4c4f4
0f 00000079`f76ff750 00007ffc`7d974020 VSDebugEng!ProcDkmMainVsThread0+0x147f
10 00000079`f76ff790 00007ffc`c1bc1fe4 VSDebugEng!DkmDllUninitialize+0x1718
11 00000079`f76ff8d0 00007ffc`c3a6cb31 KERNEL32!BaseThreadInitThunk+0x14
12 00000079`f76ff900 00000000`00000000 ntdll!RtlUserThreadStart+0x21

  1. The exception is caught in CordbFunction::GetActiveReJitRequestILCode and converted back to a failing HRESULT, then returned to VS:
    https://github.com/dotnet/coreclr/blob/master/src/debug/di/rsfunction.cpp#L573

After the change in #18322 this failure throws exceptions at step (3) due to different usage of the bad VMPTR_ILCodeVersionNode pointer.

noahfalk added a commit to noahfalk/coreclr that referenced this issue Jun 22, 2018

Fix incorrect usage of ILCodeVersion::AsNode (issue dotnet#18602)
When the debugger is querying the active rejit IL for an IL method that has not been rejitted it incorrectly creates a VMPTR_ILCodeVersionNode for a code version that shouldn't have one.

@noahfalk noahfalk self-assigned this Jun 23, 2018

noahfalk added a commit that referenced this issue Jun 23, 2018

Fix incorrect usage of ILCodeVersion::AsNode (issue #18602) (#18606)
When the debugger is querying the active rejit IL for an IL method that has not been rejitted it incorrectly creates a VMPTR_ILCodeVersionNode for a code version that shouldn't have one.

noahfalk added a commit to noahfalk/coreclr that referenced this issue Jun 26, 2018

Fix incorrect usage of ILCodeVersion::AsNode (issue dotnet#18602)
When the debugger is querying the active rejit IL for an IL method that has not been rejitted it incorrectly creates a VMPTR_ILCodeVersionNode for a code version that shouldn't have one.

noahfalk added a commit that referenced this issue Jun 27, 2018

Allow ILCodeVersion to fallback to default IL (#18502)
* Allow ILCodeVersion to fallback to default IL

For compat with profilers that used our APIs in unexpected ways we can allow
the ILCodeVersion to fallback to the default IL code when no IL was explicitly
given.

* Fix incorrect usage of ILCodeVersion::AsNode (issue #18602)

When the debugger is querying the active rejit IL for an IL method that has not been rejitted it incorrectly creates a VMPTR_ILCodeVersionNode for a code version that shouldn't have one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.