From 423f48f3e9592fda4d8a58ddf7fc18d1e6c50aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 17 Dec 2024 14:11:46 +0100 Subject: [PATCH] Fix `DiagnosticMethodInfo.Create(Delegate)` for valuetype methods Fixes #108688. We actually have test coverage for this here: https://github.com/dotnet/runtime/blob/ce9dd2ad46a4842f5cf0f03c4a30b4d29bd0b4cc/src/libraries/System.Diagnostics.StackTrace/tests/DiagnosticMethodInfoTests.cs#L137-L147 But hitting the bug requires not considering the method to be target of reflection. We root libraries tests due to xUnit. Reflection can deal with both boxed and unboxed entrypoints, stack trace metadata can't. --- .../src/System/Delegate.cs | 6 +- .../StackTraceMetadata/StackTraceMetadata.cs | 170 ++++++++++++++++++ 2 files changed, 175 insertions(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index 2ad2166546121a..6c404dbe686a29 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -313,7 +313,11 @@ internal DiagnosticMethodInfo GetDiagnosticMethodInfo() } else { - functionPointer = ldftnResult; + nint unboxedPointer = RuntimeAugments.GetCodeTarget(ldftnResult); + if (unboxedPointer == ldftnResult) + unboxedPointer = RuntimeAugments.GetTargetOfUnboxingAndInstantiatingStub(ldftnResult); + + functionPointer = unboxedPointer != 0 ? unboxedPointer : ldftnResult; } return RuntimeAugments.StackTraceCallbacksIfAvailable?.TryGetDiagnosticMethodInfoFromStartAddress(functionPointer); } diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs index 7c6017c8988790..392502dca0c50a 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs @@ -3,6 +3,8 @@ using System; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; using System.Runtime.CompilerServices; class Program @@ -11,6 +13,7 @@ static int Main() { BodyFoldingTest.Run(); DiagnosticMethodInfoTests.Run(); + Test108688Regression.Run(); string stackTrace = Environment.StackTrace; @@ -83,4 +86,171 @@ public class Nested } } } + + class Test108688Regression + { + public static void Run() + { + { + DelStruct s; + Action del = s.InstanceMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + + { + DelStruct s; + Action del = s.InstanceGenericMethod; + var dmi = DiagnosticMethodInfo.Create(del); +#if STRIPPED + if (dmi != null) + throw new Exception(); +#else + if (dmi.Name != nameof(DelStruct.InstanceGenericMethod)) + throw new Exception(); + // Need to make sure it was stack trace metadata and not reflection metadata that provided this + if (GetMethodSecretly(del.Target.GetType(), dmi.Name) != null) + throw new Exception(); +#endif + } + } + + [UnconditionalSuppressMessage("", "IL2070")] + private static MethodInfo GetMethodSecretly(Type t, string name) + => t.GetMethod(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance); + + struct DelStruct + { + public void InstanceMethod() { } + public void InstanceGenericMethod() { } + } + + struct DelStruct + { + public void InstanceMethod() { } + public void InstanceGenericMethod() { } + } + } + }