From ebae4682bc5b075f670e19e0ef5983cdbc516647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 6 Sep 2023 14:18:39 +0900 Subject: [PATCH] Respect StackTraceHiddenAttribute (#91572) Fixes #91309. NativeAOT sources stack trace data from two places: reflection metadata (if the method was reflection-visible), or specialized stack trace metadata. Finding out if a method should be hidden from stack traces in the former case is easy: just look for the attribute. The latter case requires compiler work. In this PR, I'm extending the specialized stack trace metadata format to contain a bit that says if the frame should be hidden or not. I'm doing it in a way that we can also do this for compiler-generated methods (that otherwise don't show up in stack trace metadata). The runtime behavior is similar to CoreCLR - the methods show up in individual stack frames, but if we stringify the stack trace, they'll be skipped. I'm adding a specialized test that tests the two sources of metadata. I'm also marking the methods involved in the startup path as stack trace hidden. There is one thing I skipped - CoreCLR has logic to force generate the stack trace metadata for the first frame. If we do that, the compiler-generated startup code starts showing up in stack traces again. Marking `Main` as stack trace hidden might lead to an empty stack trace. --- .../DeveloperExperience.cs | 9 ++- .../Runtime/Augments/RuntimeAugments.cs | 2 +- .../Augments/StackTraceMetadataCallbacks.cs | 3 +- .../src/System/CrashInfo.cs | 2 +- .../Diagnostics/StackFrame.NativeAot.cs | 16 ++-- .../StackTraceMetadata/StackTraceMetadata.cs | 69 ++++++++++++++-- .../Common/Internal/Runtime/StackTraceData.cs | 2 + .../StackTraceMethodMappingNode.cs | 6 ++ .../Compiler/GeneratingMetadataManager.cs | 48 +++++++---- .../Compiler/MetadataManager.cs | 18 +++-- .../Compiler/StackTraceEmissionPolicy.cs | 28 +++++-- .../StartupCode/StartupCodeMainMethod.cs | 6 ++ .../StackTraceHiddenAttributeTests.cs | 1 - .../nativeaot/SmokeTests/UnitTests/Main.cs | 1 + .../SmokeTests/UnitTests/StackTraces.cs | 79 +++++++++++++++++++ .../SmokeTests/UnitTests/UnitTests.csproj | 1 + 16 files changed, 242 insertions(+), 49 deletions(-) create mode 100644 src/tests/nativeaot/SmokeTests/UnitTests/StackTraces.cs diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs index 231761bb6a7d3..c772cd2b0f0c4 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs @@ -32,9 +32,9 @@ private static bool IsMetadataStackTraceResolutionDisabled() return disableMetadata; } - public virtual string CreateStackTraceString(IntPtr ip, bool includeFileInfo) + public virtual string CreateStackTraceString(IntPtr ip, bool includeFileInfo, out bool isStackTraceHidden) { - string methodName = GetMethodName(ip, out IntPtr methodStart); + string methodName = GetMethodName(ip, out IntPtr methodStart, out isStackTraceHidden); if (methodName != null) { if (ip != methodStart) @@ -58,7 +58,7 @@ public virtual string CreateStackTraceString(IntPtr ip, bool includeFileInfo) return $"{fileNameWithoutExtension}!+0x{rva:x}"; } - internal static string GetMethodName(IntPtr ip, out IntPtr methodStart) + internal static string GetMethodName(IntPtr ip, out IntPtr methodStart, out bool isStackTraceHidden) { methodStart = IntPtr.Zero; if (!IsMetadataStackTraceResolutionDisabled()) @@ -69,10 +69,11 @@ internal static string GetMethodName(IntPtr ip, out IntPtr methodStart) methodStart = RuntimeImports.RhFindMethodStartAddress(ip); if (methodStart != IntPtr.Zero) { - return stackTraceCallbacks.TryGetMethodNameFromStartAddress(methodStart); + return stackTraceCallbacks.TryGetMethodNameFromStartAddress(methodStart, out isStackTraceHidden); } } } + isStackTraceHidden = false; return null; } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs index 43b23737f503e..22f00000c7e66 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs @@ -811,7 +811,7 @@ public static string TryGetMethodDisplayStringFromIp(IntPtr ip) if (ip == IntPtr.Zero) return null; - return callbacks.TryGetMethodNameFromStartAddress(ip); + return callbacks.TryGetMethodNameFromStartAddress(ip, out _); } private static volatile ReflectionExecutionDomainCallbacks s_reflectionExecutionDomainCallbacks; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/StackTraceMetadataCallbacks.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/StackTraceMetadataCallbacks.cs index 86fa2082186d7..fbfb2172541fb 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/StackTraceMetadataCallbacks.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/StackTraceMetadataCallbacks.cs @@ -22,7 +22,8 @@ public abstract class StackTraceMetadataCallbacks /// Return null if stack trace information is not available. /// /// Memory address representing the start of a method + /// Returns a value indicating whether the method should be hidden in stack traces /// Formatted method name or null if metadata for the method is not available - public abstract string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress); + public abstract string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress, out bool isStackTraceHidden); } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/CrashInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/CrashInfo.cs index b2598eec37b7e..0998720acaf40 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/CrashInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/CrashInfo.cs @@ -250,7 +250,7 @@ private bool WriteStackFrame(StackFrame frame, int maxNameSize) if (!WriteHexValue("offset"u8, frame.GetNativeOffset())) return false; - string method = DeveloperExperience.GetMethodName(ip, out IntPtr _); + string method = DeveloperExperience.GetMethodName(ip, out _, out _); if (method != null) { if (!WriteStringValue("name"u8, method, maxNameSize)) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs index e920b34259fed..24a8a13afd3e9 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs @@ -142,7 +142,7 @@ internal bool HasMethod() /// private bool AppendStackFrameWithoutMethodBase(StringBuilder builder) { - builder.Append(DeveloperExperience.Default.CreateStackTraceString(_ipAddress, includeFileInfo: false)); + builder.Append(DeveloperExperience.Default.CreateStackTraceString(_ipAddress, includeFileInfo: false, out _)); return true; } @@ -161,11 +161,15 @@ internal void AppendToStackTrace(StringBuilder builder) { if (_ipAddress != Exception.EdiSeparator) { - // Passing a default string for "at" in case SR.UsingResourceKeys() is true - // as this is a special case and we don't want to have "Word_At" on stack traces. - string word_At = SR.UsingResourceKeys() ? "at" : SR.Word_At; - builder.Append(" ").Append(word_At).Append(' '); - builder.AppendLine(DeveloperExperience.Default.CreateStackTraceString(_ipAddress, _needFileInfo)); + string s = DeveloperExperience.Default.CreateStackTraceString(_ipAddress, _needFileInfo, out bool isStackTraceHidden); + if (!isStackTraceHidden) + { + // Passing a default string for "at" in case SR.UsingResourceKeys() is true + // as this is a special case and we don't want to have "Word_At" on stack traces. + string word_At = SR.UsingResourceKeys() ? "at" : SR.Word_At; + builder.Append(" ").Append(word_At).Append(' '); + builder.AppendLine(s); + } } if (_isLastFrameFromForeignExceptionStackTrace) { diff --git a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs index a203b7acbf94a..ab9ae2967f2bb 100644 --- a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs +++ b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Reflection.Runtime.General; using Internal.Metadata.NativeFormat; using Internal.NativeFormat; @@ -42,7 +43,7 @@ internal static void Initialize() /// /// Locate the containing module for a method and try to resolve its name based on start address. /// - public static unsafe string GetMethodNameFromStartAddressIfAvailable(IntPtr methodStartAddress) + public static unsafe string GetMethodNameFromStartAddressIfAvailable(IntPtr methodStartAddress, out bool isStackTraceHidden) { IntPtr moduleStartAddress = RuntimeAugments.GetOSModuleFromPointer(methodStartAddress); int rva = (int)((byte*)methodStartAddress - (byte*)moduleStartAddress); @@ -50,18 +51,38 @@ public static unsafe string GetMethodNameFromStartAddressIfAvailable(IntPtr meth { if (moduleInfo.Handle.OsModuleBase == moduleStartAddress) { - string name = _perModuleMethodNameResolverHashtable.GetOrCreateValue(moduleInfo.Handle.GetIntPtrUNSAFE()).GetMethodNameFromRvaIfAvailable(rva); - if (name != null) + string name = _perModuleMethodNameResolverHashtable.GetOrCreateValue(moduleInfo.Handle.GetIntPtrUNSAFE()).GetMethodNameFromRvaIfAvailable(rva, out isStackTraceHidden); + if (name != null || isStackTraceHidden) return name; } } + isStackTraceHidden = false; + // We haven't found information in the stack trace metadata tables, but maybe reflection will have this if (IsReflectionExecutionAvailable() && ReflectionExecution.TryGetMethodMetadataFromStartAddress(methodStartAddress, out MetadataReader reader, out TypeDefinitionHandle typeHandle, out MethodHandle methodHandle)) { + foreach (CustomAttributeHandle cah in reader.GetTypeDefinition(typeHandle).CustomAttributes) + { + if (cah.IsCustomAttributeOfType(reader, "System.Diagnostics", "StackTraceHiddenAttribute")) + { + isStackTraceHidden = true; + break; + } + } + + foreach (CustomAttributeHandle cah in reader.GetMethod(methodHandle).CustomAttributes) + { + if (cah.IsCustomAttributeOfType(reader, "System.Diagnostics", "StackTraceHiddenAttribute")) + { + isStackTraceHidden = true; + break; + } + } + return MethodNameFormatter.FormatMethodName(reader, typeHandle, methodHandle); } @@ -127,9 +148,9 @@ protected override PerModuleMethodNameResolver CreateValueFromKey(IntPtr key) /// private sealed class StackTraceMetadataCallbacksImpl : StackTraceMetadataCallbacks { - public override string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress) + public override string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress, out bool isStackTraceHidden) { - return GetMethodNameFromStartAddressIfAvailable(methodStartAddress); + return GetMethodNameFromStartAddressIfAvailable(methodStartAddress, out isStackTraceHidden); } } @@ -256,6 +277,7 @@ private unsafe void PopulateRvaToTokenMap(TypeManagerHandle handle, byte* pMap, _stacktraceDatas[current++] = new StackTraceData { Rva = methodRva, + IsHidden = (command & StackTraceDataCommand.IsStackTraceHidden) != 0, OwningType = currentOwningType, Name = currentName, Signature = currentSignature, @@ -274,8 +296,10 @@ private unsafe void PopulateRvaToTokenMap(TypeManagerHandle handle, byte* pMap, /// /// Try to resolve method name based on its address using the stack trace metadata /// - public string GetMethodNameFromRvaIfAvailable(int rva) + public string GetMethodNameFromRvaIfAvailable(int rva, out bool isStackTraceHidden) { + isStackTraceHidden = false; + if (_stacktraceDatas == null) { // No stack trace metadata for this module @@ -290,12 +314,43 @@ public string GetMethodNameFromRvaIfAvailable(int rva) } StackTraceData data = _stacktraceDatas[index]; + + isStackTraceHidden = data.IsHidden; + + if (data.OwningType.IsNull(_metadataReader)) + { + Debug.Assert(data.Name.IsNull(_metadataReader) && data.Signature.IsNull(_metadataReader)); + Debug.Assert(isStackTraceHidden); + return null; + } + return MethodNameFormatter.FormatMethodName(_metadataReader, data.OwningType, data.Name, data.Signature, data.GenericArguments); } private struct StackTraceData : IComparable { - public int Rva { get; init; } + private const int IsHiddenFlag = 0x2; + + private readonly int _rvaAndIsHiddenBit; + + public int Rva + { + get => _rvaAndIsHiddenBit & ~IsHiddenFlag; + init + { + Debug.Assert((value & IsHiddenFlag) == 0); + _rvaAndIsHiddenBit = value | (_rvaAndIsHiddenBit & IsHiddenFlag); + } + } + public bool IsHidden + { + get => (_rvaAndIsHiddenBit & IsHiddenFlag) != 0; + init + { + if (value) + _rvaAndIsHiddenBit |= IsHiddenFlag; + } + } public Handle OwningType { get; init; } public ConstantStringValueHandle Name { get; init; } public MethodSignatureHandle Signature { get; init; } diff --git a/src/coreclr/tools/Common/Internal/Runtime/StackTraceData.cs b/src/coreclr/tools/Common/Internal/Runtime/StackTraceData.cs index 2db0523ae2843..46fb51450a9d8 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/StackTraceData.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/StackTraceData.cs @@ -9,5 +9,7 @@ internal static class StackTraceDataCommand public const byte UpdateName = 0x02; public const byte UpdateSignature = 0x04; public const byte UpdateGenericSignature = 0x08; // Just a shortcut - sig metadata has the info + + public const byte IsStackTraceHidden = 0x10; } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/StackTraceMethodMappingNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/StackTraceMethodMappingNode.cs index 7845ac28a1b89..1c5fc8658e8be 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/StackTraceMethodMappingNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/StackTraceMethodMappingNode.cs @@ -121,6 +121,12 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) command |= StackTraceDataCommand.UpdateSignature; } } + + if (entry.IsHidden) + { + command |= StackTraceDataCommand.IsStackTraceHidden; + } + objData.EmitByte(commandReservation, command); objData.EmitReloc(factory.MethodEntrypoint(entry.Method), RelocType.IMAGE_REL_BASED_RELPTR32); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/GeneratingMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/GeneratingMetadataManager.cs index ee984bb51fc68..1e7442fec7486 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/GeneratingMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/GeneratingMetadataManager.cs @@ -11,6 +11,8 @@ using ILCompiler.Metadata; using ILCompiler.DependencyAnalysis; +using Debug = System.Diagnostics.Debug; + namespace ILCompiler { /// @@ -73,17 +75,24 @@ protected void ComputeMetadata( (GetMetadataCategory(method) & MetadataCategory.RuntimeMapping) != 0) continue; - if (!_stackTraceEmissionPolicy.ShouldIncludeMethod(method)) - continue; + MethodStackTraceVisibilityFlags stackVisibility = _stackTraceEmissionPolicy.GetMethodVisibility(method); + bool isHidden = (stackVisibility & MethodStackTraceVisibilityFlags.IsHidden) != 0; - StackTraceRecordData record = CreateStackTraceRecord(transform, method); + if ((stackVisibility & MethodStackTraceVisibilityFlags.HasMetadata) != 0) + { + StackTraceRecordData record = CreateStackTraceRecord(transform, method, isHidden); - stackTraceRecords.Add(record); + stackTraceRecords.Add(record); - writer.AdditionalRootRecords.Add(record.OwningType); - writer.AdditionalRootRecords.Add(record.MethodName); - writer.AdditionalRootRecords.Add(record.MethodSignature); - writer.AdditionalRootRecords.Add(record.MethodInstantiationArgumentCollection); + writer.AdditionalRootRecords.Add(record.OwningType); + writer.AdditionalRootRecords.Add(record.MethodName); + writer.AdditionalRootRecords.Add(record.MethodSignature); + writer.AdditionalRootRecords.Add(record.MethodInstantiationArgumentCollection); + } + else if (isHidden) + { + stackTraceRecords.Add(new StackTraceRecordData(method, null, null, null, null, isHidden)); + } } var ms = new MemoryStream(); @@ -178,13 +187,22 @@ record ??= transformed.GetTransformedTypeReference(definition); // Generate stack trace metadata mapping foreach (var stackTraceRecord in stackTraceRecords) { - StackTraceMapping mapping = new StackTraceMapping( - stackTraceRecord.Method, - writer.GetRecordHandle(stackTraceRecord.OwningType), - writer.GetRecordHandle(stackTraceRecord.MethodSignature), - writer.GetRecordHandle(stackTraceRecord.MethodName), - stackTraceRecord.MethodInstantiationArgumentCollection != null ? writer.GetRecordHandle(stackTraceRecord.MethodInstantiationArgumentCollection) : 0); - stackTraceMapping.Add(mapping); + if (stackTraceRecord.OwningType != null) + { + StackTraceMapping mapping = new StackTraceMapping( + stackTraceRecord.Method, + writer.GetRecordHandle(stackTraceRecord.OwningType), + writer.GetRecordHandle(stackTraceRecord.MethodSignature), + writer.GetRecordHandle(stackTraceRecord.MethodName), + stackTraceRecord.MethodInstantiationArgumentCollection != null ? writer.GetRecordHandle(stackTraceRecord.MethodInstantiationArgumentCollection) : 0, + stackTraceRecord.IsHidden); + stackTraceMapping.Add(mapping); + } + else + { + Debug.Assert(stackTraceRecord.IsHidden); + stackTraceMapping.Add(new StackTraceMapping(stackTraceRecord.Method, 0, 0, 0, 0, stackTraceRecord.IsHidden)); + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index 8658068c1ebe4..73133b10de83a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -619,7 +619,7 @@ protected abstract void ComputeMetadata(NodeFactory factory, out List> fieldMappings, out List stackTraceMapping); - protected StackTraceRecordData CreateStackTraceRecord(Metadata.MetadataTransform transform, MethodDesc method) + protected StackTraceRecordData CreateStackTraceRecord(Metadata.MetadataTransform transform, MethodDesc method, bool isHidden) { // In the metadata, we only represent the generic definition MethodDesc methodToGenerateMetadataFor = method.GetTypicalMethodDefinition(); @@ -668,7 +668,7 @@ protected StackTraceRecordData CreateStackTraceRecord(Metadata.MetadataTransform methodInst = null; } - return new StackTraceRecordData(method, owningType, signature, name, methodInst); + return new StackTraceRecordData(method, owningType, signature, name, methodInst, isHidden); } /// @@ -950,10 +950,11 @@ public readonly struct StackTraceMapping public readonly int MethodSignatureHandle; public readonly int MethodNameHandle; public readonly int MethodInstantiationArgumentCollectionHandle; + public readonly bool IsHidden; - public StackTraceMapping(MethodDesc method, int owningTypeHandle, int methodSignatureHandle, int methodNameHandle, int methodInstantiationArgumentCollectionHandle) - => (Method, OwningTypeHandle, MethodSignatureHandle, MethodNameHandle, MethodInstantiationArgumentCollectionHandle) - = (method, owningTypeHandle, methodSignatureHandle, methodNameHandle, methodInstantiationArgumentCollectionHandle); + public StackTraceMapping(MethodDesc method, int owningTypeHandle, int methodSignatureHandle, int methodNameHandle, int methodInstantiationArgumentCollectionHandle, bool isHidden) + => (Method, OwningTypeHandle, MethodSignatureHandle, MethodNameHandle, MethodInstantiationArgumentCollectionHandle, IsHidden) + = (method, owningTypeHandle, methodSignatureHandle, methodNameHandle, methodInstantiationArgumentCollectionHandle, isHidden); } public readonly struct StackTraceRecordData @@ -963,10 +964,11 @@ public readonly struct StackTraceRecordData public readonly MetadataRecord MethodSignature; public readonly MetadataRecord MethodName; public readonly MetadataRecord MethodInstantiationArgumentCollection; + public readonly bool IsHidden; - public StackTraceRecordData(MethodDesc method, MetadataRecord owningType, MetadataRecord methodSignature, MetadataRecord methodName, MetadataRecord methodInstantiationArgumentCollection) - => (Method, OwningType, MethodSignature, MethodName, MethodInstantiationArgumentCollection) - = (method, owningType, methodSignature, methodName, methodInstantiationArgumentCollection); + public StackTraceRecordData(MethodDesc method, MetadataRecord owningType, MetadataRecord methodSignature, MetadataRecord methodName, MetadataRecord methodInstantiationArgumentCollection, bool isHidden) + => (Method, OwningType, MethodSignature, MethodName, MethodInstantiationArgumentCollection, IsHidden) + = (method, owningType, methodSignature, methodName, methodInstantiationArgumentCollection, isHidden); } [Flags] diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/StackTraceEmissionPolicy.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/StackTraceEmissionPolicy.cs index 08c891f95d535..ea04f63db0baf 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/StackTraceEmissionPolicy.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/StackTraceEmissionPolicy.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using Internal.TypeSystem; namespace ILCompiler @@ -10,14 +11,14 @@ namespace ILCompiler /// public abstract class StackTraceEmissionPolicy { - public abstract bool ShouldIncludeMethod(MethodDesc method); + public abstract MethodStackTraceVisibilityFlags GetMethodVisibility(MethodDesc method); } public class NoStackTraceEmissionPolicy : StackTraceEmissionPolicy { - public override bool ShouldIncludeMethod(MethodDesc method) + public override MethodStackTraceVisibilityFlags GetMethodVisibility(MethodDesc method) { - return false; + return default; } } @@ -27,9 +28,26 @@ public override bool ShouldIncludeMethod(MethodDesc method) /// public class EcmaMethodStackTraceEmissionPolicy : StackTraceEmissionPolicy { - public override bool ShouldIncludeMethod(MethodDesc method) + public override MethodStackTraceVisibilityFlags GetMethodVisibility(MethodDesc method) { - return method.GetTypicalMethodDefinition() is Internal.TypeSystem.Ecma.EcmaMethod; + MethodStackTraceVisibilityFlags result = 0; + + if (method.HasCustomAttribute("System.Diagnostics", "StackTraceHiddenAttribute") + || (method.OwningType is MetadataType mdType && mdType.HasCustomAttribute("System.Diagnostics", "StackTraceHiddenAttribute"))) + { + result |= MethodStackTraceVisibilityFlags.IsHidden; + } + + return method.GetTypicalMethodDefinition() is Internal.TypeSystem.Ecma.EcmaMethod + ? result | MethodStackTraceVisibilityFlags.HasMetadata + : result; } } + + [Flags] + public enum MethodStackTraceVisibilityFlags + { + HasMetadata = 0x1, + IsHidden = 0x2, + } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/Stubs/StartupCode/StartupCodeMainMethod.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/Stubs/StartupCode/StartupCodeMainMethod.cs index 2b88c59e1c0cc..c86852973e527 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/Stubs/StartupCode/StartupCodeMainMethod.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/Stubs/StartupCode/StartupCodeMainMethod.cs @@ -193,6 +193,9 @@ public override bool IsUnmanagedCallersOnly } } + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) + => attributeNamespace == "System.Diagnostics" && attributeName == "StackTraceHiddenAttribute"; + /// /// Wraps the main method in a layer of indirection. This is necessary to protect the startup code /// infrastructure from situations when the owning type of the main method cannot be loaded, and codegen @@ -271,6 +274,9 @@ public override MethodIL EmitIL() return emit.Link(this); } + + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) + => attributeNamespace == "System.Diagnostics" && attributeName == "StackTraceHiddenAttribute"; } } } diff --git a/src/libraries/System.Runtime/tests/System/Diagnostics/StackTraceHiddenAttributeTests.cs b/src/libraries/System.Runtime/tests/System/Diagnostics/StackTraceHiddenAttributeTests.cs index 68dce35a2198f..90371c32bd245 100644 --- a/src/libraries/System.Runtime/tests/System/Diagnostics/StackTraceHiddenAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System/Diagnostics/StackTraceHiddenAttributeTests.cs @@ -8,7 +8,6 @@ namespace System.Tests { [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/91309", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] public class StackTraceHiddenAttributeTests { [Fact] diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/Main.cs b/src/tests/nativeaot/SmokeTests/UnitTests/Main.cs index e71884a69110e..e91a219dc4f52 100644 --- a/src/tests/nativeaot/SmokeTests/UnitTests/Main.cs +++ b/src/tests/nativeaot/SmokeTests/UnitTests/Main.cs @@ -10,6 +10,7 @@ success &= RunTest(Interfaces.Run); success &= RunTest(Threading.Run); success &= RunTest(Devirtualization.Run); +success &= RunTest(StackTraces.Run); return success ? 100 : 1; diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/StackTraces.cs b/src/tests/nativeaot/SmokeTests/UnitTests/StackTraces.cs new file mode 100644 index 0000000000000..06f211c113af7 --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/UnitTests/StackTraces.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Text; + +class StackTraces +{ + internal static int Run() + { + TestStackTraceHidden.Run(); + + return 100; + } + + class TestStackTraceHidden + { + [DynamicDependency(nameof(HiddenMethodWithMetadata))] + internal static void Run() + { + string s = null; + StackTrace t = null; + try + { + HiddenMethod(ref t); + } + catch (Exception ex) + { + s = ex.StackTrace; + } + + Verify(s, false); + Verify(t.ToString(), false); + + StringBuilder sb = new StringBuilder(); + foreach (StackFrame f in t.GetFrames()) + sb.AppendLine(f.ToString()); + + Verify(sb.ToString(), true); + + if (GetSecretMethod(typeof(TestStackTraceHidden), nameof(HiddenMethodWithMetadata)) == null) + throw new Exception(); + + if (GetSecretMethod(typeof(TestStackTraceHidden), nameof(HiddenMethod)) != null) + throw new Exception(); + + static MethodInfo GetSecretMethod(Type t, string name) => t.GetMethod(name); + + static void Verify(string s, bool expected) + { + if (s.Contains(nameof(HiddenMethod)) != expected + || s.Contains(nameof(HiddenMethodWithMetadata)) != expected) + throw new Exception(s); + if (!s.Contains(nameof(Collector))) + throw new Exception(s); + } + } + + [StackTraceHidden] + [MethodImpl(MethodImplOptions.NoInlining)] + public static void HiddenMethod(ref StackTrace t) => HiddenMethodWithMetadata(ref t); + + [StackTraceHidden] + [MethodImpl(MethodImplOptions.NoInlining)] + public static void HiddenMethodWithMetadata(ref StackTrace t) => Collector(ref t); + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Collector(ref StackTrace t) + { + t = new StackTrace(); + throw new Exception(); + } + } + +} diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/UnitTests.csproj b/src/tests/nativeaot/SmokeTests/UnitTests/UnitTests.csproj index 5b0a03685b6e8..d61a6236b1f07 100644 --- a/src/tests/nativeaot/SmokeTests/UnitTests/UnitTests.csproj +++ b/src/tests/nativeaot/SmokeTests/UnitTests/UnitTests.csproj @@ -16,6 +16,7 @@ +