Respect StackTraceHiddenAttribute (dotnet#91572)

Fixes dotnet#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.

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}!<BaseAddress>+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;
         }
 

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;

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.
         /// </summary>
         /// <param name="methodStartAddress">Memory address representing the start of a method</param>
+        /// <param name="isStackTraceHidden">Returns a value indicating whether the method should be hidden in stack traces</param>
         /// <returns>Formatted method name or null if metadata for the method is not available</returns>
-        public abstract string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress);
+        public abstract string TryGetMethodNameFromStartAddress(IntPtr methodStartAddress, out bool isStackTraceHidden);
     }
 }

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))

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs

@@ -142,7 +142,7 @@ internal bool HasMethod()
         /// </summary>
         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)
             {

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,26 +43,46 @@ internal static void Initialize()
         /// <summary>
         /// Locate the containing module for a method and try to resolve its name based on start address.
         /// </summary>
-        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);
             foreach (NativeFormatModuleInfo moduleInfo in ModuleList.EnumerateModules())
             {
                 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)
         /// </summary>
         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,
             /// <summary>
             /// Try to resolve method name based on its address using the stack trace metadata
             /// </summary>
-            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<StackTraceData>
             {
-                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; }

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;
     }
 }

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);
             }

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
 {
     /// <summary>
@@ -73,17 +75,24 @@ protected void ComputeMetadata<TPolicy>(
                     (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));
+                }
             }
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs

@@ -619,7 +619,7 @@ protected abstract void ComputeMetadata(NodeFactory factory,
                                                 out List<MetadataMapping<FieldDesc>> fieldMappings,
                                                 out List<StackTraceMapping> 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);
         }
 
         /// <summary>
@@ -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]