Fix missing metadata problems (#370)
authorMike McLaughlin <mikem@microsoft.com>
Tue, 2 Jul 2019 22:59:05 +0000 (15:59 -0700)
committerGitHub <noreply@github.com>
Tue, 2 Jul 2019 22:59:05 +0000 (15:59 -0700)
Fix missing metadata problems

The previous fix forced the metadata to always go through
the metadata locator callback interface. This fix the problem
when the metadata in the core dump isn't complete, but broke
getting metadata for user/app assemblies that only exist in
the core dump.

Needed to "fallback" to reading the core dump memory for the
metadata when the assembly can't be found or downloaded.

Cleaned up building the module list.

Add PublishPackages variable

.vsts-dotnet.yml
src/SOS/SOS.UnitTests/Scripts/StackAndOtherTests.script
src/SOS/SOS.UnitTests/Scripts/StackTests.script
src/SOS/Strike/datatarget.cpp
src/SOS/Strike/disasm.cpp
src/SOS/Strike/hostcoreclr.cpp
src/SOS/Strike/hostcoreclr.h
src/SOS/Strike/strike.cpp
src/SOS/Strike/util.cpp
src/SOS/Strike/util.h

index d8b4a9736d1672d088bc01f02895b7b4733eea88..39589a28ec16c119f7e6a823c3ab2fbb24692de4 100644 (file)
@@ -460,7 +460,7 @@ phases:
         /p:DotNetSignType=$(SignType) 
         /p:DotNetPublishBlobFeedKey=$(dotnetfeed-storage-access-key-1) 
         /p:DotNetPublishBlobFeedUrl=$(_PublishBlobFeedUrl)
-        /p:DotNetPublishToBlobFeed=true
+        /p:DotNetPublishToBlobFeed=$(PublishPackages)
         /p:DotNetSymbolServerTokenMsdl=$(microsoft-symbol-server-pat)
         /p:DotNetSymbolServerTokenSymWeb=$(symweb-symbol-server-pat)
         /p:OfficialBuildId=$(BUILD.BUILDNUMBER)
index c3559bbaf314078ddc5a4f63677a0fa079923d8f..ac2ec47fedfb33cc115c8b699e6c0ce912f0527c 100644 (file)
@@ -64,9 +64,6 @@ VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+SymbolTestApp\.Program\.Foo1\(.*\)\s+\[(?i:.*[
 VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+SymbolTestApp\.Program\.Main\(.*\)\s+\[(?i:.*[\\|/]SymbolTestApp\.cs) @ 19\]\s*
 ENDIF:PROJECTK
 
-# Issue #https://github.com/dotnet/diagnostics/issues/266
-IFDEF:NEVER
-
 # Verify that ClrStack with the ICorDebug options works
 IFDEF:PROJECTK
 SOSCOMMAND:ClrStack -i
@@ -80,6 +77,8 @@ VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+\[DEFAULT\] Void SymbolTestApp\.Program\.Main\
 VERIFY:.*\s+Stack walk complete.\s+
 ENDIF:PROJECTK
 
+!IFDEF:ALPINE
+
 # Verify that ClrStack with the ICorDebug options and all option (locals/params) works
 IFDEF:PROJECTK
 SOSCOMMAND:ClrStack -i -a
@@ -97,10 +96,6 @@ VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+\[DEFAULT\] Void SymbolTestApp\.Program\.Main\
 VERIFY:.*\s+Stack walk complete.\s+
 ENDIF:PROJECTK
 
-ENDIF:NEVER
-
-!IFDEF:ALPINE
-
 # Verify DumpStackObjects works
 IFDEF:PROJECTK
 SOSCOMMAND:DumpStackObjects
index 05ad9453648b2ef5d50a78cf7c1cba0d1a339d3e..115509c26d3ee82730b4a20e0ecde666f95de4bb 100644 (file)
@@ -66,9 +66,6 @@ VERIFY:\s+[r|e]ax=<HEXVAL>\s+[r|e]bx=<HEXVAL>\s+[r|e]cx=<HEXVAL>\s+
 ENDIF:64BIT
 ENDIF:PROJECTK
 
-# Issue #https://github.com/dotnet/diagnostics/issues/266
-IFDEF:NEVER
-
 # 5) Verifying that ClrStack with the ICorDebug options works
 SOSCOMMAND:ClrStack -i
 VERIFY:.*\s+Dumping managed stack and managed variables using ICorDebug.\s+
@@ -79,6 +76,7 @@ VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+\[DEFAULT\] Void NestedExceptionTest\.Program\
 VERIFY:.*\s+Stack walk complete.\s+
 
 # 6) Verifying that ClrStack with the ICorDebug options and all option (locals/params) works
+!IFDEF:ALPINE
 IFDEF:PROJECTK
 SOSCOMMAND:ClrStack -i -a
 VERIFY:.*\s+Dumping managed stack and managed variables using ICorDebug.\s+
@@ -92,8 +90,7 @@ VERIFY:\s+\+ System.FormatException ex @ 0x<HEXVAL>\s+
 VERIFY:.*\s+<HEXVAL>\s+<HEXVAL>\s+\[DEFAULT\] Void NestedExceptionTest\.Program\.Main\(.*\)\s+\(.*\)\s+
 VERIFY:.*\s+Stack walk complete.\s+
 ENDIF:PROJECTK
-
-ENDIF:NEVER
+ENDIF:ALPINE
 
 # 7) Verify DumpStackObjects works
 IFDEF:PROJECTK
index 71970e6f3c8bfc7f50f0c0f598a39e9da02ac37a..e9bcf395dd747fab4806b6b2f7df93d2a0d47bea 100644 (file)
@@ -138,13 +138,16 @@ DataTarget::ReadVirtual(
 #ifdef FEATURE_PAL
     if (g_sos != nullptr)
     {
-        // LLDB synthesizes memory (returns 0's) for missing pages (in this case the missing metadata 
-        // pages) in core dumps. This functions creates a list of the metadata regions and returns true
-        // if the read would be in the metadata of a loaded assembly. This allows an error to be returned 
-        // instead of 0's so the DAC will call the GetMetadataLocator datatarget callback.
-        if (IsMetadataMemory(address, request))
-        {
-            return E_ACCESSDENIED;
+        // LLDB synthesizes memory (returns 0's) for missing pages (in this case the missing metadata  pages) 
+        // in core dumps. This functions creates a list of the metadata regions and caches the metadata if 
+        // available from the local or downloaded assembly. If the read would be in the metadata of a loaded 
+        // assembly, the metadata from the this cache will be returned.
+        HRESULT hr = GetMetadataMemory(address, request, buffer);
+        if (SUCCEEDED(hr)) {
+            if (done != nullptr) {
+                *done = request;
+            }
+            return hr;
         }
     }
 #endif
@@ -303,14 +306,7 @@ DataTarget::GetMetadata(
     BYTE* buffer,
     /* [out] */ ULONG32* dataSize)
 {
-    HRESULT hr = InitializeHosting();
-    if (FAILED(hr))
-    {
-        return hr;
-    }
-    InitializeSymbolStore();
-    _ASSERTE(g_SOSNetCoreCallbacks.GetMetadataLocatorDelegate != nullptr);
-    return g_SOSNetCoreCallbacks.GetMetadataLocatorDelegate(imagePath, imageTimestamp, imageSize, mvid, mdRva, flags, bufferSize, buffer, dataSize);
+    return ::GetMetadataLocator(imagePath, imageTimestamp, imageSize, mvid, mdRva, flags, bufferSize, buffer, dataSize);
 }
 
 
index ceb0fed30de8b74f14fca75dc08eb6842198a0f5..0d8356ea6a6bbf5085b1588d846ae6815675e951 100644 (file)
@@ -26,6 +26,7 @@ namespace X86GCDump
 #define DAC_ARG(x)
 #define CONTRACTL_END
 #define LIMITED_METHOD_CONTRACT
+#undef NOTHROW
 #define NOTHROW
 #define GC_NOTRIGGER
 #define SUPPORTS_DAC
index f0e23b2bf8194eaa4bf063746c073f1e4bcd5726..1447718a5eea7cc1f6bd9273f2d6b5c96fa3fdae 100644 (file)
@@ -924,6 +924,29 @@ void DisableSymbolStore()
     }
 }
 
+/**********************************************************************\
+ * Returns the metadata from a local or downloaded assembly
+\**********************************************************************/
+HRESULT GetMetadataLocator(
+    LPCWSTR imagePath,
+    ULONG32 imageTimestamp,
+    ULONG32 imageSize,
+    GUID* mvid,
+    ULONG32 mdRva,
+    ULONG32 flags,
+    ULONG32 bufferSize,
+    BYTE* buffer,
+    ULONG32* dataSize)
+{
+    HRESULT hr = InitializeHosting();
+    if (FAILED(hr)) {
+        return hr;
+    }
+    InitializeSymbolStore();
+    _ASSERTE(g_SOSNetCoreCallbacks.GetMetadataLocatorDelegate != nullptr);
+    return g_SOSNetCoreCallbacks.GetMetadataLocatorDelegate(imagePath, imageTimestamp, imageSize, mvid, mdRva, flags, bufferSize, buffer, dataSize);
+}
+
 /**********************************************************************\
  * Load symbols for an ICorDebugModule. Used by "clrstack -i".
 \**********************************************************************/
@@ -1193,6 +1216,9 @@ HRESULT SymbolReader::GetLineByILOffset(___in mdMethodDef methodToken, ___in ULO
     return E_FAIL;
 }
 
+/**********************************************************************\
+ * Returns the name of the local variable from a PDB. 
+\**********************************************************************/
 HRESULT SymbolReader::GetNamedLocalVariable(___in ISymUnmanagedScope * pScope, ___in ICorDebugILFrame * pILFrame, ___in mdMethodDef methodToken, 
     ___in ULONG localIndex, __out_ecount(paramNameLen) WCHAR* paramName, ___in ULONG paramNameLen, ICorDebugValue** ppValue)
 {
@@ -1410,4 +1436,4 @@ HRESULT SymbolReader::ResolveSequencePoint(__in_z WCHAR* pFilename, ___in ULONG3
 #endif // FEATURE_PAL
 
     return E_FAIL;
-}
+}
\ No newline at end of file
index f6c17d9c997d316950b9766b05a026f1d41f4ac8..3e92486b438d8cc838b20a99dae2dd702c547462 100644 (file)
@@ -73,6 +73,17 @@ extern HRESULT LoadNativeSymbols(bool runtimeOnly = false);
 extern void DisplaySymbolStore();
 extern void DisableSymbolStore();
 
+extern HRESULT GetMetadataLocator(
+    LPCWSTR imagePath,
+    ULONG32 imageTimestamp,
+    ULONG32 imageSize,
+    GUID* mvid,
+    ULONG32 mdRva,
+    ULONG32 flags,
+    ULONG32 bufferSize,
+    BYTE* buffer,
+    ULONG32* dataSize);
+
 class SymbolReader
 {
 private:
index 4461be4eace02f22a6e3634e11fae25bca53e55a..a9e31bb5f5d0976f29caf485482a8f7fbfbcb819 100644 (file)
@@ -197,7 +197,6 @@ HMODULE g_hInstance = NULL;
 
 #ifdef FEATURE_PAL
 
-#define NOTHROW
 #define MINIDUMP_NOT_SUPPORTED()
 
 #else // !FEATURE_PAL
@@ -210,8 +209,6 @@ HMODULE g_hInstance = NULL;
         return Status;         \
     }
 
-#define NOTHROW (std::nothrow)
-
 #include "safemath.h"
 
 DECLARE_API (MinidumpMode)
index 6bc5f23250e522722754d458dfd925f14761c8a1..8bc368dea0246f8048c518e8d9eb82d804350318 100644 (file)
@@ -2469,8 +2469,7 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule)
         return NULL;
     }
 
-    WCHAR StringData[MAX_LONGPATH];
-    char fileName[sizeof(StringData)/2];
+    ArrayHolder<char> fileName = new char[MAX_LONGPATH];
     
     // Search all domains to find a module
     for (int n = 0; n < adsData.DomainCount+numSpecialDomains; n++)
@@ -2550,13 +2549,14 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule)
                         goto Failure;
                     }
 
-                    FileNameForModule ((DWORD_PTR)ModuleAddr, StringData);
-                    int m;
-                    for (m = 0; StringData[m] != L'\0'; m++)
+                    if (mName != NULL)
                     {
-                        fileName[m] = (char)StringData[m];
+                        ArrayHolder<WCHAR> moduleName = new WCHAR[MAX_LONGPATH];
+                        FileNameForModule((DWORD_PTR)ModuleAddr, moduleName);
+
+                        int bytesWritten = WideCharToMultiByte(CP_ACP, 0, moduleName, -1, fileName, MAX_LONGPATH, NULL, NULL);
+                        _ASSERTE(bytesWritten > 0);
                     }
-                    fileName[m] = '\0';
                     
                     if ((mName == NULL) || 
                         IsSameModuleName(fileName, mName) ||
@@ -4427,6 +4427,22 @@ public:
         {
             return E_UNEXPECTED;
         }
+#ifdef FEATURE_PAL
+        if (g_sos != nullptr)
+        {
+            // LLDB synthesizes memory (returns 0's) for missing pages (in this case the missing metadata  pages) 
+            // in core dumps. This functions creates a list of the metadata regions and caches the metadata if 
+            // available from the local or downloaded assembly. If the read would be in the metadata of a loaded 
+            // assembly, the metadata from the this cache will be returned.
+            HRESULT hr = GetMetadataMemory(address, request, pBuffer);
+            if (SUCCEEDED(hr)) {
+                if (pcbRead != nullptr) {
+                    *pcbRead = request;
+                }
+                return hr;
+            }
+        }
+#endif
         return g_ExtData->ReadVirtual(address, pBuffer, request, (PULONG) pcbRead);
     }
 
@@ -6203,18 +6219,59 @@ struct MemoryRegion
 private:
     uint64_t m_startAddress;
     uint64_t m_endAddress;
+    CLRDATA_ADDRESS m_peFile;
+    BYTE* m_metadataMemory;
+    volatile LONG m_busy;
 
-public:
-    MemoryRegion(uint64_t start, uint64_t end) : 
-        m_startAddress(start),
-        m_endAddress(end)
+    HRESULT CacheMetadata()
     {
+        if (m_metadataMemory == nullptr)
+        {
+            HRESULT hr;
+            CLRDATA_ADDRESS baseAddress;
+            if (FAILED(hr = g_sos->GetPEFileBase(m_peFile, &baseAddress))) {
+                return hr;
+            }
+            ArrayHolder<WCHAR> imagePath = new WCHAR[MAX_LONGPATH];
+            if (FAILED(hr = g_sos->GetPEFileName(m_peFile, MAX_LONGPATH, imagePath.GetPtr(), NULL))) {
+                return hr;
+            }
+            IMAGE_DOS_HEADER DosHeader;
+            if (FAILED(hr = g_ExtData->ReadVirtual(baseAddress, &DosHeader, sizeof(DosHeader), NULL))) {
+                return hr;
+            }
+            IMAGE_NT_HEADERS Header;
+            if (FAILED(hr = g_ExtData->ReadVirtual(baseAddress + DosHeader.e_lfanew, &Header, sizeof(Header), NULL))) {
+                return hr;
+            }
+            // If there is no COMHeader, this can not be managed code.
+            if (Header.OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_COMHEADER].VirtualAddress == 0) {
+                return E_ACCESSDENIED;
+            }
+            ULONG32 imageSize = Header.OptionalHeader.SizeOfImage;
+            ULONG32 timeStamp = Header.FileHeader.TimeDateStamp;
+            ULONG32 bufferSize = (ULONG32)Size();
+
+            ArrayHolder<BYTE> buffer = new NOTHROW BYTE[bufferSize];
+            if (buffer == nullptr) {
+                return E_OUTOFMEMORY;
+            }
+            ULONG32 actualSize = 0;
+            if (FAILED(hr = GetMetadataLocator(imagePath, timeStamp, imageSize, nullptr, 0, 0, bufferSize, buffer, &actualSize))) {
+                return hr;
+            }
+            m_metadataMemory = buffer.Detach();
+        }
+        return S_OK;
     }
 
-    // copy constructor
-    MemoryRegion(const MemoryRegion& region) : 
-        m_startAddress(region.m_startAddress),
-        m_endAddress(region.m_endAddress)
+public:
+    MemoryRegion(uint64_t start, uint64_t end, CLRDATA_ADDRESS peFile) : 
+        m_startAddress(start),
+        m_endAddress(end),
+        m_peFile(peFile),
+        m_metadataMemory(nullptr),
+        m_busy(0)
     {
     }
 
@@ -6222,6 +6279,8 @@ public:
     uint64_t EndAddress() const { return m_endAddress; }
     uint64_t Size() const { return m_endAddress - m_startAddress; }
 
+    CLRDATA_ADDRESS const PEFile() { return m_peFile; }
+
     bool operator<(const MemoryRegion& rhs) const
     {
         return (m_startAddress < rhs.m_startAddress) && (m_endAddress <= rhs.m_startAddress);
@@ -6232,6 +6291,47 @@ public:
     {
         return (m_startAddress <= rhs.m_startAddress) && (m_endAddress >= rhs.m_endAddress);
     }
+
+    HRESULT ReadMetadata(CLRDATA_ADDRESS address, ULONG32 bufferSize, BYTE* buffer)
+    {
+        _ASSERTE((m_startAddress <= address) && (m_endAddress >= (address + bufferSize)));
+
+        HRESULT hr = E_ACCESSDENIED;
+
+        // Skip in-memory and dynamic modules or if CacheMetadata failed
+        if (m_peFile != 0)
+        {
+            if (InterlockedIncrement(&m_busy) == 1)
+            {
+                // Attempt to get the assembly metadata from local file or by downloading from a symbol server
+                hr = CacheMetadata();
+                if (FAILED(hr)) {
+                    // If we can get the metadata from the assembly, mark this region to always fail.
+                    m_peFile = 0;
+                }
+            }
+            InterlockedDecrement(&m_busy);
+        }
+
+        if (FAILED(hr)) {
+            return hr;
+        }
+
+        // Read the memory from the cached metadata blob
+        _ASSERTE(m_metadataMemory != nullptr);
+        uint64_t offset = address - m_startAddress;
+        memcpy(buffer, m_metadataMemory + offset, bufferSize);
+        return S_OK;
+    }
+
+    void Dispose()
+    {
+        if (m_metadataMemory != nullptr)
+        {
+            delete[] m_metadataMemory;
+            m_metadataMemory = nullptr;
+        }
+    }
 };
 
 std::set<MemoryRegion> g_metadataRegions;
@@ -6239,21 +6339,13 @@ bool g_metadataRegionsPopulated = false;
 
 void FlushMetadataRegions()
 {
+    for (const MemoryRegion& region : g_metadataRegions)
+    {
+        const_cast<MemoryRegion&>(region).Dispose();
+    }
     g_metadataRegionsPopulated = false;
 }
 
-//-------------------------------------------------------------------------------
-// Lifted from "..\md\inc\mdfileformat.h"
-#define STORAGE_MAGIC_SIG   0x424A5342  // BSJB
-struct STORAGESIGNATURE
-{
-    ULONG       lSignature;             // "Magic" signature.
-    USHORT      iMajorVer;              // Major file version.
-    USHORT      iMinorVer;              // Minor file version.
-    ULONG       iExtraData;             // Offset to next structure of information
-    ULONG       iVersionString;         // Length of version string
-};
-
 void PopulateMetadataRegions()
 {
     g_metadataRegions.clear();
@@ -6272,17 +6364,8 @@ void PopulateMetadataRegions()
                 {
                     if (moduleData.metadataStart != 0)
                     {
-                        MemoryRegion region(moduleData.metadataStart, moduleData.metadataStart + moduleData.metadataSize);
+                        MemoryRegion region(moduleData.metadataStart, moduleData.metadataStart + moduleData.metadataSize, moduleData.File);
                         g_metadataRegions.insert(region);
-#ifdef METADATA_REGION_LOGGING
-                        ArrayHolder<WCHAR> name = new WCHAR[MAX_LONGPATH];
-                        name[0] = '\0';
-                        if (moduleData.File != 0)
-                        {
-                            g_sos->GetPEFileName(moduleData.File, MAX_LONGPATH, name.GetPtr(), NULL);
-                        }
-                        ExtOut("%c%016x %016x %016x %S\n", add ? '*' : ' ', moduleData.metadataStart, moduleData.metadataStart + moduleData.metadataSize, moduleData.metadataSize, name.GetPtr());
-#endif // METADATA_REGION_LOGGING
                     }
                 }
             }
@@ -6290,20 +6373,21 @@ void PopulateMetadataRegions()
     }
 }
 
-bool IsMetadataMemory(CLRDATA_ADDRESS address, ULONG32 size)
+HRESULT GetMetadataMemory(CLRDATA_ADDRESS address, ULONG32 bufferSize, BYTE* buffer)
 {
+    // Populate the metadata memory region map
     if (!g_metadataRegionsPopulated)
     {
         g_metadataRegionsPopulated = true;
         PopulateMetadataRegions();
     }
-    MemoryRegion region(address, address + size);
+    // Check if the memory address is in a metadata memory region
+    MemoryRegion region(address, address + bufferSize, 0);
     const auto& found = g_metadataRegions.find(region);
-    if (found != g_metadataRegions.end())
-    {
-        return found->Contains(region);
+    if (found != g_metadataRegions.end() && found->Contains(region)) {
+        return const_cast<MemoryRegion&>(*found).ReadMetadata(address, bufferSize, buffer);
     }
-    return false;
+    return E_ACCESSDENIED;
 }
 
 #endif // FEATURE_PAL
\ No newline at end of file
index 15f7dd9395147ed11b228cfa6965f1bfc8723a71..14a205de7d32464e81fba0682de37ddafc4dae23 100644 (file)
@@ -41,6 +41,12 @@ inline void RestoreSOToleranceState() {}
 typedef LPCSTR  LPCUTF8;
 typedef LPSTR   LPUTF8;
 
+#ifdef FEATURE_PAL
+#define NOTHROW
+#else
+#define NOTHROW (std::nothrow)
+#endif 
+
 DECLARE_HANDLE(OBJECTHANDLE);
 
 #if defined(_TARGET_WIN64_)
@@ -1853,7 +1859,7 @@ BOOL TryGetMethodDescriptorForDelegate(CLRDATA_ADDRESS delegateAddr, CLRDATA_ADD
 
 #ifdef FEATURE_PAL
 void FlushMetadataRegions();
-bool IsMetadataMemory(CLRDATA_ADDRESS address, ULONG32 size);
+HRESULT GetMetadataMemory(CLRDATA_ADDRESS address, ULONG32 bufferSize, BYTE* buffer);
 #endif
 
 /* Returns a list of all modules in the process.