Fix DiagnosticMethodInfo.Create(Delegate) for valuetype methods (do…

…tnet#110782) Fixes dotnet#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.
bernd5 · Dec 17, 2024 · 78ede32 · 78ede32
1 parent 9a02107
commit 78ede32
Show file tree

Hide file tree

Showing 2 changed files with 175 additions and 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
@@ -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
@@ -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<int>;
+                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<object>;
+                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<int> 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<int> s;
+                Action del = s.InstanceGenericMethod<int>;
+                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<int> s;
+                Action del = s.InstanceGenericMethod<object>;
+                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<object> 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<object> s;
+                Action del = s.InstanceGenericMethod<int>;
+                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<object> s;
+                Action del = s.InstanceGenericMethod<object>;
+                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<T>() { }
+        }
+
+        struct DelStruct<T>
+        {
+            public void InstanceMethod() { }
+            public void InstanceGenericMethod<U>() { }
+        }
+    }
+
 }