From aaf1c03a71abc97c5a6c541bf10ccc95e95847dd Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 27 Feb 2025 14:28:02 -0600 Subject: [PATCH 1/3] Harden some reflection invoke tests --- .../System/Reflection/InvokeEmitTests.cs | 51 ------------------- .../Reflection/InvokeInterpretedTests.cs | 45 ---------------- .../tests/StackFrameTests.cs | 10 ++-- .../ConstructorCommonTests.cs | 17 +++++++ .../System.Reflection.InvokeEmit.Tests.csproj | 3 +- ....Reflection.InvokeInterpreted.Tests.csproj | 1 - .../MethodCommonTests.cs | 14 +++++ 7 files changed, 38 insertions(+), 103 deletions(-) delete mode 100644 src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs delete mode 100644 src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs diff --git a/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs b/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs deleted file mode 100644 index 149be11a982597..00000000000000 --- a/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs +++ /dev/null @@ -1,51 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.CompilerServices; -using Xunit; - -namespace System.Reflection.Tests -{ - public class InvokeEmitTests - { - [ConditionalFact(typeof(InvokeEmitTests), nameof(IsEmitInvokeSupported))] - public static void VerifyInvokeIsUsingEmit_Method() - { - MethodInfo method = typeof(TestClassThatThrows).GetMethod(nameof(TestClassThatThrows.Throw))!; - TargetInvocationException ex = Assert.Throws(() => method.Invoke(null, null)); - Exception exInner = ex.InnerException; - - Assert.Contains("Here", exInner.ToString()); - Assert.Contains("InvokeStub_TestClassThatThrows", exInner.ToString()); - Assert.DoesNotContain("InterpretedInvoke_Method", exInner.ToString()); - } - - [ConditionalFact(typeof(InvokeEmitTests), nameof(IsEmitInvokeSupported))] - public static void VerifyInvokeIsUsingEmit_Constructor() - { - ConstructorInfo ctor = typeof(TestClassThatThrows).GetConstructor(Type.EmptyTypes)!; - TargetInvocationException ex = Assert.Throws(() => ctor.Invoke(null)); - Exception exInner = ex.InnerException; - - Assert.Contains("Here", exInner.ToString()); - Assert.Contains("InvokeStub_TestClassThatThrows", exInner.ToString()); - Assert.DoesNotContain("InterpretedInvoke_Constructor", exInner.ToString()); - } - - private static bool IsEmitInvokeSupported() - { - // Emit is only used for Invoke when RuntimeFeature.IsDynamicCodeSupported is true. - return RuntimeFeature.IsDynamicCodeSupported; - } - - private class TestClassThatThrows - { - public TestClassThatThrows() - { - throw new Exception("Here"); - } - - public static void Throw() => throw new Exception("Here"); - } - } -} diff --git a/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs b/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs deleted file mode 100644 index 03a876131f4787..00000000000000 --- a/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Xunit; - -namespace System.Reflection.Tests -{ - public class InvokeInterpretedTests - { - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoAOT))] - public static void VerifyInvokeIsUsingInterpreter_Method() - { - MethodInfo method = typeof(TestClassThatThrows).GetMethod(nameof(TestClassThatThrows.Throw))!; - TargetInvocationException ex = Assert.Throws(() => method.Invoke(null, null)); - Exception exInner = ex.InnerException; - - Assert.Contains("Here", exInner.ToString()); - Assert.DoesNotContain("InvokeStub_TestClassThatThrows", exInner.ToString()); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoAOT))] - public static void VerifyInvokeIsUsingInterpreter_Constructor() - { - ConstructorInfo ctor = typeof(TestClassThatThrows).GetConstructor(Type.EmptyTypes)!; - TargetInvocationException ex = Assert.Throws(() => ctor.Invoke(null)); - Exception exInner = ex.InnerException; - - Assert.Contains("Here", exInner.ToString()); - Assert.DoesNotContain("InvokeStub_TestClassThatThrows", exInner.ToString()); - } - - private class TestClassThatThrows - { - public TestClassThatThrows() - { - throw new Exception("Here"); - } - - public static void Throw() => throw new Exception("Here"); - } - } -} - diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs index b8f225f33d8cf0..d77ed76d0fbc30 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs @@ -37,10 +37,12 @@ public void Ctor_FNeedFileInfo(bool fNeedFileInfo) [Theory] [ActiveIssue("https://github.com/mono/mono/issues/15183", TestRuntimes.Mono)] - [InlineData(StackFrame.OFFSET_UNKNOWN)] - [InlineData(0)] - [InlineData(1)] - public void Ctor_SkipFrames(int skipFrames) + [InlineData(StackFrame.OFFSET_UNKNOWN, false)] + [InlineData(0, false)] + [InlineData(1, false)] + // This is highly dependent on reflection implementation and is not consistent across runtimes. + // The extra 'bool _' parameter avoids a potential reflection optimization for common signatures which may interfere with stack frame count. + public void Ctor_SkipFrames(int skipFrames, bool _) { var stackFrame = new StackFrame(skipFrames); VerifyStackFrame(stackFrame, true, skipFrames, typeof(StackFrameTests).GetMethod(nameof(Ctor_SkipFrames)), isCurrentFrame: skipFrames == 0); diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/ConstructorCommonTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/ConstructorCommonTests.cs index 5c71f79da014c1..7f2e6ea4b0a913 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/ConstructorCommonTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/ConstructorCommonTests.cs @@ -163,5 +163,22 @@ public void Invoke_Struct() Assert.Equal(1, obj.x); Assert.Equal(2, obj.y); } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoAOT))] + public static void VerifyInnerException() + { + ConstructorInfo ctor = typeof(TestClassThatThrows).GetConstructor(Type.EmptyTypes)!; + TargetInvocationException ex = Assert.Throws(() => ctor.Invoke(null)); + Assert.Contains("Here", ex.InnerException.ToString()); + } + + private class TestClassThatThrows + { + public TestClassThatThrows() + { + throw new Exception("Here"); + } + } } } diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeEmit/System.Reflection.InvokeEmit.Tests.csproj b/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeEmit/System.Reflection.InvokeEmit.Tests.csproj index 757b631140f2d6..34c01360dea3cb 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeEmit/System.Reflection.InvokeEmit.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeEmit/System.Reflection.InvokeEmit.Tests.csproj @@ -1,4 +1,4 @@ - + true true @@ -14,7 +14,6 @@ - diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeInterpreted/System.Reflection.InvokeInterpreted.Tests.csproj b/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeInterpreted/System.Reflection.InvokeInterpreted.Tests.csproj index 5d71379e2d5ca9..a4b1cb0674ecea 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeInterpreted/System.Reflection.InvokeInterpreted.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/InvokeInterpreted/System.Reflection.InvokeInterpreted.Tests.csproj @@ -14,7 +14,6 @@ - diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs index b2e8e2f84fbc27..77d78080e5ea85 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs @@ -293,5 +293,19 @@ public unsafe void TestFunctionPointerAsReturnType() Assert.IsType(ret); Assert.True((IntPtr)ret != 0); } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoAOT))] + public static void VerifyInnerException() + { + MethodInfo method = typeof(TestClassThatThrows).GetMethod(nameof(TestClassThatThrows.Throw))!; + TargetInvocationException ex = Assert.Throws(() => method.Invoke(null, null)); + Assert.Contains("Here", ex.InnerException.ToString()); + } + + private class TestClassThatThrows + { + public static void Throw() => throw new Exception("Here"); + } } } From fc41e5e9459821f256110d4fb30e8886dc5e5ace Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 28 Feb 2025 14:31:36 -0600 Subject: [PATCH 2/3] Update csproj files to remove unused files --- .../InvokeEmit/System.Runtime.ReflectionInvokeEmit.Tests.csproj | 1 - .../System.Runtime.ReflectionInvokeInterpreted.Tests.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeEmit/System.Runtime.ReflectionInvokeEmit.Tests.csproj b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeEmit/System.Runtime.ReflectionInvokeEmit.Tests.csproj index 80685872245900..4188e399626e99 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeEmit/System.Runtime.ReflectionInvokeEmit.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeEmit/System.Runtime.ReflectionInvokeEmit.Tests.csproj @@ -10,7 +10,6 @@ - diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeInterpreted/System.Runtime.ReflectionInvokeInterpreted.Tests.csproj b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeInterpreted/System.Runtime.ReflectionInvokeInterpreted.Tests.csproj index 3d7bd524b35010..4188e399626e99 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeInterpreted/System.Runtime.ReflectionInvokeInterpreted.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/InvokeInterpreted/System.Runtime.ReflectionInvokeInterpreted.Tests.csproj @@ -10,7 +10,6 @@ - From 267af4fd579fd8d85c13a22b7356c9a5d4ec8e3f Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 3 Mar 2025 12:56:42 -0600 Subject: [PATCH 3/3] Remove test that is too coupled to stack frame semantics --- .../tests/StackFrameTests.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs index d77ed76d0fbc30..72664b38685b4e 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs @@ -35,19 +35,6 @@ public void Ctor_FNeedFileInfo(bool fNeedFileInfo) VerifyStackFrame(stackFrame, fNeedFileInfo, 0, typeof(StackFrameTests).GetMethod(nameof(Ctor_FNeedFileInfo))); } - [Theory] - [ActiveIssue("https://github.com/mono/mono/issues/15183", TestRuntimes.Mono)] - [InlineData(StackFrame.OFFSET_UNKNOWN, false)] - [InlineData(0, false)] - [InlineData(1, false)] - // This is highly dependent on reflection implementation and is not consistent across runtimes. - // The extra 'bool _' parameter avoids a potential reflection optimization for common signatures which may interfere with stack frame count. - public void Ctor_SkipFrames(int skipFrames, bool _) - { - var stackFrame = new StackFrame(skipFrames); - VerifyStackFrame(stackFrame, true, skipFrames, typeof(StackFrameTests).GetMethod(nameof(Ctor_SkipFrames)), isCurrentFrame: skipFrames == 0); - } - [Theory] [ActiveIssue("https://github.com/mono/mono/issues/15187", TestRuntimes.Mono)] [InlineData(StackFrame.OFFSET_UNKNOWN, true)]