From 196093171b3281c53b541038c1e3b2d4de29e1f2 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Thu, 8 May 2025 11:11:00 -0400 Subject: [PATCH] fix issues found from SOS tests --- eng/pipelines/runtime-diagnostics.yml | 2 +- .../Contracts/ILoader.cs | 23 +++++++-- .../Contracts/Loader_1.cs | 50 +++++++++++++++++-- .../StackWalk/FrameHandling/FrameIterator.cs | 17 +++++++ .../Legacy/ClrDataModule.cs | 3 +- .../Legacy/ISOSDacInterface.cs | 2 +- .../Legacy/SOSDacImpl.cs | 15 ++++-- 7 files changed, 98 insertions(+), 14 deletions(-) diff --git a/eng/pipelines/runtime-diagnostics.yml b/eng/pipelines/runtime-diagnostics.yml index acbd6f309b6ee6..f704a135249bc5 100644 --- a/eng/pipelines/runtime-diagnostics.yml +++ b/eng/pipelines/runtime-diagnostics.yml @@ -41,7 +41,7 @@ extends: platforms: - windows_x64 jobParameters: - buildArgs: -s clr+libs+tools.cdac+host+packs -c $(_BuildConfig) + buildArgs: -s clr+libs+tools.cdac+host+packs -c Debug -rc $(_BuildConfig) -lc $(_BuildConfig) nameSuffix: AllSubsets_CoreCLR isOfficialBuild: ${{ variables.isOfficialBuild }} timeoutInMinutes: 360 diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs index 02e405c2b60076..cda96efcddd73e 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs @@ -19,9 +19,26 @@ public ModuleHandle(TargetPointer address) [Flags] public enum ModuleFlags { - Tenured = 0x00000001, // Set once we know for sure the Module will not be freed until the appdomain itself exits - EditAndContinue = 0x00000008, // Edit and Continue is enabled for this module - ReflectionEmit = 0x00000040, // Reflection.Emit was used to create this module + Tenured = 0x1, // Set once we know for sure the Module will not be freed until the appdomain itself exits + ClassFreed = 0x4, + EditAndContinue = 0x8, // Edit and Continue is enabled for this module + + ProfilerNotified = 0x10, + EtwNotified = 0x20, + + ReflectionEmit = 0x40, // Reflection.Emit was used to create this module + ProfilerDisableOptimizations = 0x80, + ProfilerDisableInlining = 0x100, + + DebuggerUserOverridePriv = 0x400, + DebuggerAllowJitOptsPriv = 0x800, + DebuggerTrackJitInfoPriv = 0x1000, + DebuggerEnCEnabledPriv = 0x2000, + DebuggerPDBsCopied = 0x4000, + DebuggerIgnorePDbs = 0x8000, + + IJWFixedUp = 0x80000, + BeingUnloaded = 0x100000, } public record struct ModuleLookupTables( diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 48e2f80a280dca..1f51583b9e69fb 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -14,9 +14,26 @@ namespace Microsoft.Diagnostics.DataContractReader.Contracts; private enum ModuleFlags_1 : uint { - Tenured = 0x00000001, // Set once we know for sure the Module will not be freed until the appdomain itself exits - EditAndContinue = 0x00000008, // Edit and Continue is enabled for this module - ReflectionEmit = 0x00000040, // Reflection.Emit was used to create this module + Tenured = 0x1, // Set once we know for sure the Module will not be freed until the appdomain itself exits + ClassFreed = 0x4, + EditAndContinue = 0x8, // Edit and Continue is enabled for this module + + ProfilerNotified = 0x10, + EtwNotified = 0x20, + + ReflectionEmit = 0x40, // Reflection.Emit was used to create this module + ProfilerDisableOptimizations = 0x80, + ProfilerDisableInlining = 0x100, + + DebuggerUserOverridePriv = 0x400, + DebuggerAllowJitOptsPriv = 0x800, + DebuggerTrackJitInfoPriv = 0x1000, + DebuggerEnCEnabledPriv = 0x2000, + DebuggerPDBsCopied = 0x4000, + DebuggerIgnorePDbs = 0x8000, + + IJWFixedUp = 0x80000, + BeingUnloaded = 0x100000, } private readonly Target _target; @@ -196,10 +213,37 @@ private static ModuleFlags GetFlags(Data.Module module) ModuleFlags flags = default; if (runtimeFlags.HasFlag(ModuleFlags_1.Tenured)) flags |= ModuleFlags.Tenured; + if (runtimeFlags.HasFlag(ModuleFlags_1.ClassFreed)) + flags |= ModuleFlags.ClassFreed; if (runtimeFlags.HasFlag(ModuleFlags_1.EditAndContinue)) flags |= ModuleFlags.EditAndContinue; + if (runtimeFlags.HasFlag(ModuleFlags_1.ProfilerNotified)) + flags |= ModuleFlags.ProfilerNotified; + if (runtimeFlags.HasFlag(ModuleFlags_1.EtwNotified)) + flags |= ModuleFlags.EtwNotified; if (runtimeFlags.HasFlag(ModuleFlags_1.ReflectionEmit)) flags |= ModuleFlags.ReflectionEmit; + if (runtimeFlags.HasFlag(ModuleFlags_1.ProfilerDisableOptimizations)) + flags |= ModuleFlags.ProfilerDisableOptimizations; + if (runtimeFlags.HasFlag(ModuleFlags_1.ProfilerDisableInlining)) + flags |= ModuleFlags.ProfilerDisableInlining; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerUserOverridePriv)) + flags |= ModuleFlags.DebuggerUserOverridePriv; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerAllowJitOptsPriv)) + flags |= ModuleFlags.DebuggerAllowJitOptsPriv; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerTrackJitInfoPriv)) + flags |= ModuleFlags.DebuggerTrackJitInfoPriv; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerEnCEnabledPriv)) + flags |= ModuleFlags.DebuggerEnCEnabledPriv; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerPDBsCopied)) + flags |= ModuleFlags.DebuggerPDBsCopied; + if (runtimeFlags.HasFlag(ModuleFlags_1.DebuggerIgnorePDbs)) + flags |= ModuleFlags.DebuggerIgnorePDbs; + if (runtimeFlags.HasFlag(ModuleFlags_1.IJWFixedUp)) + flags |= ModuleFlags.IJWFixedUp; + if (runtimeFlags.HasFlag(ModuleFlags_1.BeingUnloaded)) + flags |= ModuleFlags.BeingUnloaded; + return flags; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs index 0fd8bb8a50ba47..56fdae3c78c623 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs @@ -33,6 +33,23 @@ private enum FrameType FaultingExceptionFrame, HijackFrame, + + /* Other Frame Types not handled by the iterator */ + HelperMethodFrame, + HelperMethodFrame_1OBJ, + HelperMethodFrame_2OBJ, + HelperMethodFrame_3OBJ, + HelperMethodFrame_PROTECTOBJ, + UnmanagedToManagedFrame, + ComMethodFrame, + ComPrestubMethodFrame, + TailCallFrame, + ProtectByRefsFrame, + ProtectValueClassFrame, + DebuggerClassInitMarkFrame, + DebuggerExitFrame, + DebuggerU2MCatchHandlerFrame, + ExceptionFilterFrame, } private readonly Target target; diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataModule.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataModule.cs index 7dba1c96f68549..06a28ad8fa39a5 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataModule.cs @@ -183,14 +183,13 @@ int IXCLRDataModule.GetFlags(uint* flags) Contracts.ModuleHandle handle = contract.GetModuleHandle(_address); ModuleFlags moduleFlags = contract.GetFlags(handle); - if ((moduleFlags & ModuleFlags.EditAndContinue) != 0) + if ((moduleFlags & ModuleFlags.ReflectionEmit) != 0) { *flags |= 0x1; // CLRDATA_MODULE_IS_DYNAMIC } if (contract.GetAssembly(handle) == contract.GetRootAssembly()) { - *flags |= 0x4; // CLRDATA_MODULE_FLAGS_ROOT_ASSEMBLY } } diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs index 4ff1d08ede1293..8dc935f8151f6c 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs @@ -202,7 +202,7 @@ internal unsafe partial interface ISOSDacInterface // Assemblies [PreserveSig] - int GetAssemblyList(ulong appDomain, int count, [In, Out, MarshalUsing(CountElementName = nameof(count))] ulong[] values, int* pNeeded); + int GetAssemblyList(ulong appDomain, int count, [In, Out, MarshalUsing(CountElementName = nameof(count))] ulong[]? values, int* pNeeded); [PreserveSig] int GetAssemblyData(ulong baseDomainPtr, ulong assembly, /*struct DacpAssemblyData*/ void* data); [PreserveSig] diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index 3066f597f3104d..a6360422ce5837 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -132,7 +132,7 @@ int ISOSDacInterface.GetApplicationBase(ulong appDomain, int count, char* appBas } int ISOSDacInterface.GetAssemblyData(ulong baseDomainPtr, ulong assembly, void* data) => _legacyImpl is not null ? _legacyImpl.GetAssemblyData(baseDomainPtr, assembly, data) : HResults.E_NOTIMPL; - int ISOSDacInterface.GetAssemblyList(ulong appDomain, int count, [In, MarshalUsing(CountElementName = "count"), Out] ulong[] values, int* pNeeded) + int ISOSDacInterface.GetAssemblyList(ulong appDomain, int count, [In, MarshalUsing(CountElementName = "count"), Out] ulong[]? values, int* pNeeded) { if (appDomain == 0) { @@ -173,7 +173,7 @@ int ISOSDacInterface.GetAssemblyList(ulong appDomain, int count, [In, MarshalUsi } else { - for (int i = 0; i < modules.Count && n < count; i++) + for (int i = 0; i < modules.Count; i++) { Contracts.ModuleHandle module = modules[i]; if (loader.IsAssemblyLoaded(module)) @@ -197,7 +197,7 @@ int ISOSDacInterface.GetAssemblyList(ulong appDomain, int count, [In, MarshalUsi #if DEBUG if (_legacyImpl is not null) { - ulong[] valuesLocal = new ulong[count]; + ulong[]? valuesLocal = values != null ? new ulong[count] : null; int neededLocal; int hrLocal = _legacyImpl.GetAssemblyList(appDomain, count, valuesLocal, &neededLocal); Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); @@ -210,7 +210,7 @@ int ISOSDacInterface.GetAssemblyList(ulong appDomain, int count, [In, MarshalUsi // easiest for consumers and verification if the DAC and cDAC return the same order for (int i = 0; i < neededLocal; i++) { - Debug.Assert(values[i] == valuesLocal[i], $"cDAC: {values[i]:x}, DAC: {valuesLocal[i]:x}"); + Debug.Assert(values[i] == valuesLocal![i], $"cDAC: {values[i]:x}, DAC: {valuesLocal[i]:x}"); } } } @@ -315,6 +315,13 @@ int ISOSDacInterface.GetFrameName(ulong vtable, uint count, char* frameName, uin else { OutputBufferHelpers.CopyStringToBuffer(frameName, count, pNeeded, name); + + if (frameName is not null && pNeeded is not null) + { + // the DAC version of this API does not count the trailing null terminator + // if a buffer is provided + (*pNeeded)--; + } } } catch (System.Exception ex)