Skip to content

Commit

Permalink
Respect StackTraceHiddenAttribute (#91572)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@MichalStrehovsky
MichalStrehovsky authored Sep 6, 2023
1 parent e33f7e6 commit ebae468
Show file tree
Hide file tree
Showing 16 changed files with 242 additions and 49 deletions.
9 changes: 5 additions & 4 deletions .../nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion ...coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion ...veaot/System.Private.CoreLib/src/Internal/Runtime/Augments/StackTraceMetadataCallbacks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/System.Private.CoreLib/src/System/CrashInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
16 changes: 10 additions & 6 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackFrame.NativeAot.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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)
{
Expand Down
69 changes: 62 additions & 7 deletions ...t/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Reflection.Runtime.General;

using Internal.Metadata.NativeFormat;
using Internal.NativeFormat;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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; }
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/tools/Common/Internal/Runtime/StackTraceData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
6 changes: 6 additions & 0 deletions .../tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/StackTraceMethodMappingNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
48 changes: 33 additions & 15 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/GeneratingMetadataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
using ILCompiler.Metadata;
using ILCompiler.DependencyAnalysis;

using Debug = System.Diagnostics.Debug;

namespace ILCompiler
{
/// <summary>
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
}
}

Expand Down
18 changes: 10 additions & 8 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down
Loading

0 comments on commit ebae468

Please sign in to comment.