Simplified the thread stack unwind; removed some (probably) unnecessary fallback code

Author: Mike McLaughlin

documentation/clrma.md

@@ -54,8 +54,7 @@ ISOSDacInterface::GetMethodDescName()
 ISOSDacInterface::GetModuleData()
 ISOSDacInterface::GetPEFileBase()
 ISOSDacInterface::GetPEFileName()
-ISOSDacInterface::GetMethodDescPtrFromIP
-ISOSDacInterface::GetMethodDescPtrFromFrame()
+ISOSDacInterface::GetMethodDescPtrFromIP()
 
 // Module name fallback if debugger and GetPEFileName() can't get the name.
 ISOSDacInterface::GetModule()
@@ -67,14 +66,6 @@ IXCLRDataTask::CreateStackWalk()
 IXCLRDataStackWalk::Request(DACSTACKPRIV_REQUEST_FRAME_DATA, ...)
 IXCLRDataStackWalk::GetContext()
 IXCLRDataStackWalk::Next()
-
-// Fallback if GetMethodDescPtrFromFrame() fails when getting the stack frame method name. Not sure how important this fallback path is.
-IXCLRDataStackWalk::GetFrame()
-IXCLRDataFrame::GetMethodInstance()
-IXCLRDataMethodInstance::GetRepresentativeEntryAddress()
-IXCLRDataMethodInstance::GetTokenAndScope()
-IXCLRDataMethodInstance::GetName()
-IXCLRDataModule::GetFileName 
 ```
 
 ### References

src/SOS/Strike/clrma/thread.cpp

@@ -199,50 +199,28 @@ ClrmaThread::get_FrameCount(
                     frame.Frame = index;
                     if (FAILED(hr = GetFrameLocation(pStackWalk, &frame.IP, &frame.SP)))
                     {
+                        TraceError("Unwind: GetFrameLocation() FAILED %08x\n", hr);
                         break;
                     }
+                    // Only include normal frames, skipping any special frames
                     DacpFrameData frameData;
                     if (SUCCEEDED(hr = frameData.Request(pStackWalk)) && frameData.frameAddr != 0)
                     {
-                        frame.SP = frameData.frameAddr;
-
-                        // Get special stack frame name
-                        CLRDATA_ADDRESS methodDesc = 0;
-                        if (SUCCEEDED(hr = m_managedAnalysis->SosDacInterface()->GetMethodDescPtrFromFrame(frame.SP, &methodDesc)))
-                        {
-                            if (FAILED(hr = m_managedAnalysis->GetMethodDescInfo(methodDesc, frame, /* stripFunctionParameters */ false)))
-                            {
-                                continue;
-                            }
-                        }
-                        else
-                        {
-                            if (FAILED(hr = GetFrameFromStackWalk(pStackWalk, frame)))
-                            {
-                                continue;
-                            }
-                        }
+                        TraceInformation("Unwind: skipping special frame SP %016llx IP %016llx\n", frame.SP, frame.IP);
+                        continue;
                     }
-                    else
+                    CLRDATA_ADDRESS methodDesc = 0;
+                    if (FAILED(hr = m_managedAnalysis->SosDacInterface()->GetMethodDescPtrFromIP(frame.IP, &methodDesc)))
                     {
-                        // Get normal module and method names like MethodNameFromIP() does for !clrstack
-                        CLRDATA_ADDRESS methodDesc = 0;
-                        if (SUCCEEDED(hr = m_managedAnalysis->SosDacInterface()->GetMethodDescPtrFromIP(frame.IP, &methodDesc)))
-                        {
-                            if (FAILED(hr = m_managedAnalysis->GetMethodDescInfo(methodDesc, frame, /* stripFunctionParameters */ false)))
-                            {
-                                continue;
-                            }
-                        }
-                        else
-                        {
-                            if (FAILED(hr = GetFrameFromStackWalk(pStackWalk, frame)))
-                            {
-                                continue;
-                            }
-                        }
+                        TraceInformation("Unwind: skipping frame GetMethodDescPtrFromIP(%016llx) FAILED %08x\n", frame.IP, hr);
+                        continue;
+                    }
+                    // Get normal module and method names like MethodNameFromIP() does for !clrstack
+                    if (FAILED(hr = m_managedAnalysis->GetMethodDescInfo(methodDesc, frame, /* stripFunctionParameters */ false)))
+                    {
+                        TraceInformation("Unwind: skipping frame GetMethodDescInfo(%016llx) FAILED %08x\n", methodDesc, hr);
+                        continue;
                     }
-
                     m_stackFrames.push_back(frame);
                     index++;
 
@@ -534,81 +512,3 @@ ClrmaThread::GetFrameLocation(
     }
     return S_OK;
 }
-
-HRESULT
-ClrmaThread::GetFrameFromStackWalk(
-    IXCLRDataStackWalk* pStackWalk,
-    StackFrame& frame
-)
-{
-    HRESULT hr;
-    ReleaseHolder<IXCLRDataFrame> clrDataFrame;
-    if (SUCCEEDED(hr = pStackWalk->GetFrame((IXCLRDataFrame**)&clrDataFrame)))
-    {
-        ReleaseHolder<IXCLRDataMethodInstance> methodInstance;
-        if (SUCCEEDED(hr = clrDataFrame->GetMethodInstance((IXCLRDataMethodInstance**)&methodInstance)))
-        {
-            // Don't compute the method displacement if IP is 0
-            if (frame.IP > 0)
-            {
-                CLRDATA_ADDRESS startAddr;
-                if (SUCCEEDED(hr = methodInstance->GetRepresentativeEntryAddress(&startAddr)))
-                {
-                    frame.Displacement = (frame.IP - startAddr);
-                }
-                else
-                {
-                    TraceError("GetFrameFromStackWalk: IXCLRDataMethodInstance::GetRepresentativeEntryAddress FAILED %08x\n", hr);
-                }
-            }
-
-            ArrayHolder<WCHAR> wszNameBuffer = new WCHAR[MAX_LONGPATH + 1];
-            if (SUCCEEDED(hr = methodInstance->GetName(0, MAX_LONGPATH, nullptr, wszNameBuffer)))
-            {
-                frame.Function = wszNameBuffer;
-            }
-            else
-            {
-                TraceError("GetFrameFromStackWalk: IXCLRDataMethodInstance::GetName FAILED %08x\n", hr);
-            }
-
-            mdMethodDef token;
-            IXCLRDataModule* pModule;
-            if (SUCCEEDED(hr = methodInstance->GetTokenAndScope(&token, &pModule)))
-            {
-                wszNameBuffer[0] = L'\0';
-
-                ULONG32 nameLen = 0;
-                if (SUCCEEDED(hr = pModule->GetFileName(MAX_LONGPATH, &nameLen, wszNameBuffer)))
-                {
-                    frame.Module = wszNameBuffer;
-                }
-                else
-                {
-                    TraceError("GetFrameFromStackWalk: IXCLRDataModule::GetFileName FAILED %08x\n", hr);
-                }
-            }
-            else
-            {
-                TraceError("GetFrameFromStackWalk: IXCLRDataMethodInstance::GetTokenAndScope FAILED %08x\n", hr);
-            }
-        }
-        else
-        {
-            TraceError("GetFrameFromStackWalk: IXCLRDataFrame::GetMethodInstance FAILED %08x\n", hr);
-        }
-    }
-    else
-    {
-        TraceError("GetFrameFromStackWalk: IXCLRDataStackWalk::GetFrame FAILED %08x\n", hr);
-    }
-    if (frame.Module.empty())
-    {
-        frame.Module = L"UNKNOWN";
-    }
-    if (frame.Function.empty())
-    {
-        frame.Function = L"UNKNOWN";
-    }
-    return hr;
-}

src/SOS/Strike/clrma/thread.h

@@ -49,5 +49,4 @@ class ClrmaThread : public ICLRMAClrThread
     bool m_nestedExceptionsInitialized;
 
     HRESULT GetFrameLocation(IXCLRDataStackWalk* pStackWalk, CLRDATA_ADDRESS* ip, CLRDATA_ADDRESS* sp);
-    HRESULT GetFrameFromStackWalk(IXCLRDataStackWalk* pStackWalk, StackFrame& frame);
 };

src/SOS/Strike/strike.cpp

@@ -12656,7 +12656,6 @@ HRESULT ImplementEFNGetManagedExcepStack(
 DECLARE_API(VerifyStackTrace)
 {
     INIT_API();
-    ONLY_SUPPORTED_ON_WINDOWS_TARGET();
 
     BOOL bVerifyManagedExcepStack = FALSE;
     CMDOption option[] =