Skip to content

Commit

Permalink
Annotate methods that get MethodBase from a stackwalk as RUC (#59851)
Browse files Browse the repository at this point in the history
Fixes #53242. Saddened this didn't make .NET 6.0.

Stack traces in trimmed apps look ugly without this fix:

```
Unhandled exception. System.ArgumentNullException: Value cannot be null. (Parameter 'id')
   at System.TimeZoneInfo.ValidateTimeZoneInfo(String , TimeSpan , AdjustmentRule[] , Boolean& )
   at System.TimeZoneInfo..ctor(String , TimeSpan , String , String , String , AdjustmentRule[] , Boolean , Boolean )
   at System.TimeZoneInfo.CreateCustomTimeZone(String , TimeSpan , String , String )
   at Program.<Main>$(String[] args) in C:\stacktrace\Program.cs:line 3
```
  • Loading branch information
MichalStrehovsky committed Oct 4, 2021
1 parent e118d3f commit b72b8a2
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -69,6 +70,7 @@ private IDictionary CreateDataContainer()

public MethodBase? TargetSite
{
[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
get
{
if (_exceptionMethod != null)
Expand All @@ -85,13 +87,17 @@ private IDictionary CreateDataContainer()
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "The API will return null if the metadata for current method cannot be established.")]
private string? CreateSourceName()
{
StackTrace st = new StackTrace(this, fNeedFileInfo: false);
if (st.FrameCount > 0)
{
StackFrame sf = st.GetFrame(0)!;
MethodBase method = sf.GetMethod()!;
MethodBase? method = sf.GetMethod();
if (method == null)
return null;

Module module = method.Module;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.InteropServices;
using System.Threading;
Expand Down Expand Up @@ -36,6 +37,7 @@ public abstract partial class MethodBase : MemberInfo
return RuntimeType.GetMethodBase(declaringType.GetRuntimeType(), handle.GetMethodInfo());
}

[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
[System.Security.DynamicSecurityMethod] // Methods containing StackCrawlMark local var has to be marked DynamicSecurityMethod
public static MethodBase? GetCurrentMethod()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public partial class StackFrame
public virtual int GetFileLineNumber() { throw null; }
public virtual string? GetFileName() { throw null; }
public virtual int GetILOffset() { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Metadata for the method might be incomplete or removed")]
public virtual System.Reflection.MethodBase? GetMethod() { throw null; }
public virtual int GetNativeOffset() { throw null; }
public override string ToString() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,9 @@ public static bool Exists<T>(IEnumerable<T> collection, Predicate<T> predicate)
/// This method is used internally to trigger a failure indicating to the "programmer" that they are using the interface incorrectly.
/// It is NEVER used to indicate failure of actual contracts at runtime.
/// </summary>
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "StackFrame.GetMethod is only used to help diagnosing incorrect use of contracts. " +
"It handles missing or incomplete metadata.")]
private static void AssertMustUseRewriter(ContractFailureKind kind, string contractKind)
{
// For better diagnostics, report which assembly is at fault. Walk up stack and
Expand Down
Original file line number Diff line number Diff line change
@@ -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.Diagnostics.CodeAnalysis;
using System.Text;
using System.Reflection;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -136,6 +137,7 @@ public StackFrame(string? fileName, int lineNumber, int colNumber)
/// <summary>
/// Returns the method the frame is executing
/// </summary>
[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
public virtual MethodBase? GetMethod()
{
return _method;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand All @@ -12,6 +13,8 @@ public static bool HasNativeImage(this StackFrame stackFrame)
return stackFrame.GetNativeImageBase() != IntPtr.Zero;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "StackFrame.GetMethod is used to establish if method is available.")]
public static bool HasMethod(this StackFrame stackFrame)
{
return stackFrame.GetMethod() != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ internal string ToString(TraceFormat traceFormat)
}

#if !CORERT
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "ToString is best effort when it comes to available information.")]
internal void ToString(TraceFormat traceFormat, StringBuilder sb)
{
// Passing a default string for "at" in case SR.UsingResourceKeys() is true
Expand Down Expand Up @@ -301,8 +303,12 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb)
if (pi[j].ParameterType != null)
typeName = pi[j].ParameterType.Name;
sb.Append(typeName);
sb.Append(' ');
sb.Append(pi[j].Name);
string? parameterName = pi[j].Name;
if (parameterName != null)
{
sb.Append(' ');
sb.Append(parameterName);
}
}
sb.Append(')');
}
Expand Down
3 changes: 2 additions & 1 deletion src/libraries/System.Runtime/ref/System.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2839,7 +2839,7 @@ public partial class Exception : System.Runtime.Serialization.ISerializable
public virtual string Message { get { throw null; } }
public virtual string? Source { get { throw null; } set { } }
public virtual string? StackTrace { get { throw null; } }
public System.Reflection.MethodBase? TargetSite { get { throw null; } }
public System.Reflection.MethodBase? TargetSite { [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Metadata for the method might be incomplete or removed")] get { throw null; } }
[System.ObsoleteAttribute("BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.", DiagnosticId = "SYSLIB0011", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")]
protected event System.EventHandler<System.Runtime.Serialization.SafeSerializationEventArgs>? SerializeObjectState { add { } remove { } }
public virtual System.Exception GetBaseException() { throw null; }
Expand Down Expand Up @@ -11873,6 +11873,7 @@ public abstract partial class MethodBase : System.Reflection.MemberInfo
public abstract System.RuntimeMethodHandle MethodHandle { get; }
public virtual System.Reflection.MethodImplAttributes MethodImplementationFlags { get { throw null; } }
public override bool Equals(object? obj) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Metadata for the method might be incomplete or removed")]
public static System.Reflection.MethodBase? GetCurrentMethod() { throw null; }
public virtual System.Type[] GetGenericArguments() { throw null; }
public override int GetHashCode() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -35,6 +36,8 @@ public partial class StackTrace
[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal static extern MonoStackFrame[] get_trace(Exception e, int skipFrames, bool needFileInfo);

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "StackFrame.GetMethod is getting compared to null but nothing else on it is touched.")]
[MethodImplAttribute(MethodImplOptions.NoInlining)]
private void InitializeForCurrentThread(int skipFrames, bool needFileInfo)
{
Expand Down
8 changes: 7 additions & 1 deletion src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Diagnostics.Tracing;
Expand Down Expand Up @@ -50,6 +51,7 @@ public DispatchState(MonoStackFrame[]? stackFrames)

public MethodBase? TargetSite
{
[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
get
{
StackTrace st = new StackTrace(this, true);
Expand Down Expand Up @@ -107,15 +109,19 @@ private bool CanSetRemoteStackTrace()
return true; // mono runtime doesn't have immutable agile exceptions, always return true
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "The API will return null if the metadata for current method cannot be established.")]
private string? CreateSourceName()
{
var st = new StackTrace(this, fNeedFileInfo: false);
if (st.FrameCount > 0)
{
StackFrame sf = st.GetFrame(0)!;
MethodBase? method = sf.GetMethod();
if (method == null)
return null;

Module? module = method?.Module;
Module module = method.Module;
RuntimeModule? rtModule = module as RuntimeModule;

if (rtModule == null)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System.Reflection
Expand Down Expand Up @@ -34,6 +35,7 @@ public partial class MethodBase
return m;
}

[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern MethodBase? GetCurrentMethod();

Expand Down

0 comments on commit b72b8a2

Please sign in to comment.