From 61efb48cd6f3935731d4ffa842ba1d9b76dacb7e Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Sun, 17 Feb 2019 12:04:37 -0800 Subject: [PATCH] Strike.cpp/util.cpp clean and fixes. Cleanup GetCoreClrDirectory service function. No need to have separate GetModuleDirectory method. DacpGetModuleData.Request fails on some modules. Skip them in GetModuleFromAddress. Cleanup some spurious error messages in stack trace commands. --- src/SOS/Strike/strike.cpp | 134 ++++++++++++++++++-------------- src/SOS/Strike/util.cpp | 7 ++ src/SOS/lldbplugin/services.cpp | 46 ++++------- src/SOS/lldbplugin/services.h | 2 - src/pal/src/loader/module.cpp | 5 +- 5 files changed, 100 insertions(+), 94 deletions(-) diff --git a/src/SOS/Strike/strike.cpp b/src/SOS/Strike/strike.cpp index e71b5a371..7f8a56ee0 100644 --- a/src/SOS/Strike/strike.cpp +++ b/src/SOS/Strike/strike.cpp @@ -5972,6 +5972,8 @@ DECLARE_API(DumpModule) DMLOut("Assembly: %s\n", DMLAssembly(module.Assembly)); + ExtOut("PEFile: %p\n", SOS_PTR(module.File)); + ExtOut("ModuleId: %p\n", SOS_PTR(module.dwModuleID)); ExtOut("LoaderHeap: %p\n", SOS_PTR(module.pLookupTableHeap)); ExtOut("TypeDefToMethodTableMap: %p\n", SOS_PTR(module.TypeDefToMethodTableMap)); ExtOut("TypeRefToMethodTableMap: %p\n", SOS_PTR(module.TypeRefToMethodTableMap)); @@ -6261,7 +6263,7 @@ HRESULT PrintThreadsFromThreadStore(BOOL bMiniDump, BOOL bPrintLiveThreadsOnly) table.SetColAlignment(4, AlignRight); table.WriteColumn(8, "Lock"); - table.WriteRow("", "ID", "OSID", "ThreadOBJ", "State", "GC Mode", "GC Alloc Context", "Domain", "Count", "Apt"); + table.WriteRow("DBG", "ID", "OSID", "ThreadOBJ", "State", "GC Mode", "GC Alloc Context", "Domain", "Count", "Apt"); if (hosted) table.WriteColumn("Fiber"); @@ -12694,7 +12696,10 @@ public: &cbContextActual, (BYTE *)context.GetPtr())) != S_OK) { - ExtOut("GetFrameContext failed: %lx\n",Status); + if (FAILED(Status)) + { + ExtOut("GetFrameContext failed: %lx\n", Status); + } break; } @@ -12946,64 +12951,66 @@ public: } CLRDATA_ADDRESS ip = 0, sp = 0; hr = GetFrameLocation(pStackWalk, &ip, &sp); - - DacpFrameData FrameData; - HRESULT frameDataResult = FrameData.Request(pStackWalk); - if (SUCCEEDED(frameDataResult) && FrameData.frameAddr) - sp = FrameData.frameAddr; + if (SUCCEEDED(hr)) + { + DacpFrameData FrameData; + HRESULT frameDataResult = FrameData.Request(pStackWalk); + if (SUCCEEDED(frameDataResult) && FrameData.frameAddr) + sp = FrameData.frameAddr; #ifdef DEBUG_STACK_CONTEXT - while ((numNativeFrames > 0) && (currentNativeFrame->StackOffset <= sp)) - { - if (currentNativeFrame->StackOffset != sp) + while ((numNativeFrames > 0) && (currentNativeFrame->StackOffset <= sp)) { - PrintNativeStackFrame(out, currentNativeFrame, bSuppressLines); + if (currentNativeFrame->StackOffset != sp) + { + PrintNativeStackFrame(out, currentNativeFrame, bSuppressLines); + } + currentNativeFrame++; + numNativeFrames--; } - currentNativeFrame++; - numNativeFrames--; - } #endif // DEBUG_STACK_CONTEXT - // Print the stack pointer. - out.WriteColumn(0, sp); + // Print the stack pointer. + out.WriteColumn(0, sp); - // Print the method/Frame info - if (SUCCEEDED(frameDataResult) && FrameData.frameAddr) - { - // Skip the instruction pointer because it doesn't really mean anything for method frames - out.WriteColumn(1, bFull ? String("") : NativePtr(ip)); - - // This is a clr!Frame. - out.WriteColumn(2, GetFrameFromAddress(TO_TADDR(FrameData.frameAddr), pStackWalk, bFull)); - - // Print out gc references for the Frame. - for (unsigned int i = 0; i < refCount; ++i) - if (pRefs[i].Source == sp) - PrintRef(pRefs[i], out); - - // Print out an error message if we got one. - for (unsigned int i = 0; i < errCount; ++i) - if (pErrs[i].Source == sp) - out.WriteColumn(2, "Failed to enumerate GC references."); - } - else - { - out.WriteColumn(1, InstructionPtr(ip)); - out.WriteColumn(2, MethodNameFromIP(ip, bSuppressLines, bFull, bFull)); - - // Print out gc references. refCount will be zero if bGC is false (or if we - // failed to fetch gc reference information). - for (unsigned int i = 0; i < refCount; ++i) - if (pRefs[i].Source == ip && pRefs[i].StackPointer == sp) - PrintRef(pRefs[i], out); + // Print the method/Frame info + if (SUCCEEDED(frameDataResult) && FrameData.frameAddr) + { + // Skip the instruction pointer because it doesn't really mean anything for method frames + out.WriteColumn(1, bFull ? String("") : NativePtr(ip)); + + // This is a clr!Frame. + out.WriteColumn(2, GetFrameFromAddress(TO_TADDR(FrameData.frameAddr), pStackWalk, bFull)); - // Print out an error message if we got one. - for (unsigned int i = 0; i < errCount; ++i) - if (pErrs[i].Source == sp) - out.WriteColumn(2, "Failed to enumerate GC references."); + // Print out gc references for the Frame. + for (unsigned int i = 0; i < refCount; ++i) + if (pRefs[i].Source == sp) + PrintRef(pRefs[i], out); - if (bParams || bLocals) - PrintArgsAndLocals(pStackWalk, bParams, bLocals); + // Print out an error message if we got one. + for (unsigned int i = 0; i < errCount; ++i) + if (pErrs[i].Source == sp) + out.WriteColumn(2, "Failed to enumerate GC references."); + } + else + { + out.WriteColumn(1, InstructionPtr(ip)); + out.WriteColumn(2, MethodNameFromIP(ip, bSuppressLines, bFull, bFull)); + + // Print out gc references. refCount will be zero if bGC is false (or if we + // failed to fetch gc reference information). + for (unsigned int i = 0; i < refCount; ++i) + if (pRefs[i].Source == ip && pRefs[i].StackPointer == sp) + PrintRef(pRefs[i], out); + + // Print out an error message if we got one. + for (unsigned int i = 0; i < errCount; ++i) + if (pErrs[i].Source == sp) + out.WriteColumn(2, "Failed to enumerate GC references."); + + if (bParams || bLocals) + PrintArgsAndLocals(pStackWalk, bParams, bLocals); + } } if (bDisplayRegVals) @@ -13025,13 +13032,16 @@ public: { CROSS_PLATFORM_CONTEXT context; HRESULT hr = pStackWalk->GetContext(DT_CONTEXT_FULL, g_targetMachine->GetContextSize(), NULL, (BYTE *)&context); - if (FAILED(hr) || hr == S_FALSE) + if (FAILED(hr)) { - // GetFrameContext returns S_FALSE if the frame iterator is invalid. That's basically an error for us. ExtOut("GetFrameContext failed: %lx\n", hr); + return hr; + } + if (hr == S_FALSE) + { + // GetFrameContext returns S_FALSE if the frame iterator is invalid. That's basically an error for us. return E_FAIL; } - #if defined(SOS_TARGET_AMD64) String outputFormat3 = " %3s=%016x %3s=%016x %3s=%016x\n"; String outputFormat2 = " %3s=%016x %3s=%016x\n"; @@ -13082,13 +13092,16 @@ public: { CROSS_PLATFORM_CONTEXT context; HRESULT hr = pStackWalk->GetContext(DT_CONTEXT_FULL, g_targetMachine->GetContextSize(), NULL, (BYTE *)&context); - if (FAILED(hr) || hr == S_FALSE) + if (FAILED(hr)) { - // GetFrameContext returns S_FALSE if the frame iterator is invalid. That's basically an error for us. ExtOut("GetFrameContext failed: %lx\n", hr); + return hr; + } + if (hr == S_FALSE) + { + // GetFrameContext returns S_FALSE if the frame iterator is invalid. That's basically an error for us. return E_FAIL; } - // First find the info for the Frame object, if the current frame has an associated clr!Frame. *ip = GetIP(context); *sp = GetSP(context); @@ -13179,8 +13192,11 @@ public: ExtErr("Failed to request thread at %p\n", CurThread); return; } - ExtOut("OS Thread Id: 0x%x\n", Thread.osThreadId); - PrintThread(Thread.osThreadId, bParams, bLocals, bSuppressLines, bGC, bNative, bDisplayRegVals); + if (Thread.osThreadId != 0) + { + ExtOut("OS Thread Id: 0x%x\n", Thread.osThreadId); + PrintThread(Thread.osThreadId, bParams, bLocals, bSuppressLines, bGC, bNative, bDisplayRegVals); + } CurThread = Thread.nextThread; } } diff --git a/src/SOS/Strike/util.cpp b/src/SOS/Strike/util.cpp index 74feb7903..13aacac11 100644 --- a/src/SOS/Strike/util.cpp +++ b/src/SOS/Strike/util.cpp @@ -2582,6 +2582,13 @@ HRESULT GetModuleFromAddress(___in CLRDATA_ADDRESS peAddress, ___out IXCLRDataMo if (FAILED(hr)) { break; } + ULONG32 flags; + if ((hr = module->GetFlags(&flags)) != S_OK) { + continue; + } + if (flags != CLRDATA_MODULE_DEFAULT) { + continue; + } DacpGetModuleData moduleData; HRESULT hr = moduleData.Request(module); if (FAILED(hr)) { diff --git a/src/SOS/lldbplugin/services.cpp b/src/SOS/lldbplugin/services.cpp index 2fe56dcdd..11b92b195 100644 --- a/src/SOS/lldbplugin/services.cpp +++ b/src/SOS/lldbplugin/services.cpp @@ -88,17 +88,22 @@ LLDBServices::GetCoreClrDirectory() { if (g_coreclrDirectory == nullptr) { - const char *coreclrModule = MAKEDLLNAME_A("coreclr"); - const char *directory = GetModuleDirectory(coreclrModule); - if (directory != nullptr) - { - std::string path(directory); - path.append("/"); - g_coreclrDirectory = strdup(path.c_str()); - } - else + lldb::SBTarget target = m_debugger.GetSelectedTarget(); + if (target.IsValid()) { - Output(DEBUG_OUTPUT_WARNING, "The %s module is not loaded yet in the target process\n", coreclrModule); + const char *coreclrModule = MAKEDLLNAME_A("coreclr"); + lldb::SBFileSpec fileSpec; + fileSpec.SetFilename(coreclrModule); + + lldb::SBModule module = target.FindModule(fileSpec); + if (module.IsValid()) + { + const char *directory = module.GetFileSpec().GetDirectory(); + std::string path(directory); + path.append("/"); + + g_coreclrDirectory = strdup(path.c_str()); + } } } return g_coreclrDirectory; @@ -1304,27 +1309,6 @@ LLDBServices::FindSourceFile( } // Internal functions -PCSTR -LLDBServices::GetModuleDirectory( - PCSTR name) -{ - lldb::SBTarget target = m_debugger.GetSelectedTarget(); - if (!target.IsValid()) - { - return NULL; - } - - lldb::SBFileSpec fileSpec; - fileSpec.SetFilename(name); - - lldb::SBModule module = target.FindModule(fileSpec); - if (!module.IsValid()) - { - return NULL; - } - - return module.GetFileSpec().GetDirectory(); -} ULONG64 LLDBServices::GetModuleBase( diff --git a/src/SOS/lldbplugin/services.h b/src/SOS/lldbplugin/services.h index a02b2ba0d..8eb79f6a1 100644 --- a/src/SOS/lldbplugin/services.h +++ b/src/SOS/lldbplugin/services.h @@ -281,8 +281,6 @@ public: // LLDBServices (internal) //---------------------------------------------------------------------------- - PCSTR GetModuleDirectory( - PCSTR name); void LoadNativeSymbols( lldb::SBTarget target, lldb::SBModule module, diff --git a/src/pal/src/loader/module.cpp b/src/pal/src/loader/module.cpp index 4755fb9fb..04a9ba1bf 100644 --- a/src/pal/src/loader/module.cpp +++ b/src/pal/src/loader/module.cpp @@ -366,8 +366,9 @@ GetProcAddress( } else { - TRACE("Symbol %s not found in module %p (named %S)\n", - lpProcName, module, MODNAME(module)); + TRACE("Symbol %s not found in module %p (named %S) %s\n", + lpProcName, module, MODNAME(module), dlerror()); + SetLastError(ERROR_PROC_NOT_FOUND); } done: -- 2.34.1