Check catch and throw non-Exception derived types (#43969)

Verifies:

* `X` is derived from `Exception` in `throw X`.
* `throw null` is valid.
* `X` is derived from `Exception` in `catch(X)`.

Contributes to #37390

diff --git a/src/coreclr/src/tools/ILVerification/ILImporter.Verify.cs b/src/coreclr/src/tools/ILVerification/ILImporter.Verify.cs
index 787afb5..6540aaf 100644 (file)
--- a/src/coreclr/src/tools/ILVerification/ILImporter.Verify.cs
+++ b/src/coreclr/src/tools/ILVerification/ILImporter.Verify.cs
@@ -201,11 +201,9 @@ namespace Internal.IL
             }
         }
 
-        public Action<ErrorArgument[], VerifierError> ReportVerificationError
-        {
-            set;
-            private get;
-        }
+        public Action<ErrorArgument[], VerifierError> ReportVerificationError { set; private get; }
+
+        public bool SanityChecks { set; private get; }
 
         public void Verify()
         {
@@ -509,9 +507,9 @@ again:
             if (_currentBasicBlock != null)
                 _currentBasicBlock.IncrementErrorCount();
 
-            var args = new ErrorArgument[] 
-            { 
-                new ErrorArgument("Offset", _currentInstructionOffset) 
+            var args = new ErrorArgument[]
+            {
+                new ErrorArgument("Offset", _currentInstructionOffset)
             };
             ReportVerificationError(args, error);
         }
@@ -566,7 +564,7 @@ again:
 
         void CheckIsNumeric(StackValue value)
         {
-            if (!Check(StackValueKind.Int32 <= value.Kind && value.Kind <= StackValueKind.Float, 
+            if (!Check(StackValueKind.Int32 <= value.Kind && value.Kind <= StackValueKind.Float,
                 VerifierError.ExpectedNumericType, value))
             {
                 AbortBasicBlockVerification();
@@ -601,14 +599,14 @@ again:
 
         void CheckIsArray(StackValue value)
         {
-            Check((value.Kind == StackValueKind.ObjRef) && ((value.Type == null) || value.Type.IsSzArray), 
+            Check((value.Kind == StackValueKind.ObjRef) && ((value.Type == null) || value.Type.IsSzArray),
                 VerifierError.ExpectedArray /* , value */);
         }
 
-        void CheckIsAssignable(StackValue src, StackValue dst)
+        void CheckIsAssignable(StackValue src, StackValue dst, VerifierError error = VerifierError.StackUnexpected)
         {
             if (!IsAssignable(src, dst))
-                VerificationError(VerifierError.StackUnexpected, src, dst);
+                VerificationError(error, src, dst);
         }
 
         private void CheckIsValidLeaveTarget(BasicBlock src, BasicBlock target)
@@ -647,7 +645,7 @@ again:
                     ref var targetRegion = ref _exceptionRegions[target.TryIndex.Value].ILRegion;
 
                     // Target is not enclosing source
-                    if (targetRegion.TryOffset > srcRegion.TryOffset || 
+                    if (targetRegion.TryOffset > srcRegion.TryOffset ||
                         src.StartOffset >= targetRegion.TryOffset + targetRegion.TryLength)
                     {
                         // Target is not first instruction
@@ -687,8 +685,8 @@ again:
                     ref var targetRegion = ref _exceptionRegions[target.TryIndex.Value].ILRegion;
 
                     // If target is not associated try block, and not enclosing srcRegion
-                    if (target.TryIndex != src.HandlerIndex && 
-                        (targetRegion.TryOffset > srcRegion.HandlerOffset || 
+                    if (target.TryIndex != src.HandlerIndex &&
+                        (targetRegion.TryOffset > srcRegion.HandlerOffset ||
                         targetRegion.TryOffset + targetRegion.TryLength < srcRegion.HandlerOffset))
                     {
                         // If target is not first instruction of try, or not a direct sibling
@@ -771,7 +769,7 @@ again:
                     ref var srcRegion = ref _exceptionRegions[src.TryIndex.Value].ILRegion;
                     ref var targetRegion = ref _exceptionRegions[target.TryIndex.Value].ILRegion;
                     // If target is inside source region
-                    if (srcRegion.TryOffset <= targetRegion.TryOffset && 
+                    if (srcRegion.TryOffset <= targetRegion.TryOffset &&
                         target.StartOffset < srcRegion.TryOffset + srcRegion.TryLength)
                     {
                         // Only branching to first instruction of try-block is valid
@@ -919,7 +917,7 @@ again:
         }
 
         /// <summary>
-        /// Checks whether the given enclosed try block is a direct child try-region of 
+        /// Checks whether the given enclosed try block is a direct child try-region of
         /// the given enclosing try block.
         /// </summary>
         /// <param name="enclosingBlock">The block enclosing the try block given by <paramref name="enclosedBlock"/>.</param>
@@ -1008,11 +1006,11 @@ again:
                 case TypeFlags.Char: return "Char";
                 case TypeFlags.SByte:
                 case TypeFlags.Byte: return "Byte";
-                case TypeFlags.Int16: 
+                case TypeFlags.Int16:
                 case TypeFlags.UInt16: return "Short";
                 case TypeFlags.Int32:
                 case TypeFlags.UInt32: return "Int32";
-                case TypeFlags.Int64: 
+                case TypeFlags.Int64:
                 case TypeFlags.UInt64: return "Long";
                 case TypeFlags.Single: return "Single";
                 case TypeFlags.Double: return "Double";
@@ -1302,6 +1300,12 @@ again:
                         var exceptionType = ResolveTypeToken(r.ILRegion.ClassToken);
                         Check(!exceptionType.IsByRef, VerifierError.CatchByRef);
                         basicBlock.EntryStack[0] = StackValue.CreateObjRef(exceptionType);
+
+                        if (SanityChecks && basicBlock.EntryStack[0] != StackValue.CreateObjRef(GetWellKnownType(WellKnownType.Object)))
+                        {
+                            CheckIsAssignable(basicBlock.EntryStack[0], StackValue.CreateObjRef(GetWellKnownType(WellKnownType.Exception)),
+                                VerifierError.ThrowOrCatchOnlyExceptionType);
+                        }
                     }
                 }
                 else
@@ -1606,21 +1610,21 @@ again:
                 {
                     // Rules for non-virtual call to a non-final virtual method (ECMA III.3.19: Verifiability of 'call'):
 
-                    // Define: 
+                    // Define:
                     // The "this" pointer is considered to be "possibly written" if
                     //   1. Its address have been taken (LDARGA 0) anywhere in the method.
                     //   (or)
                     //   2. It has been stored to (STARG.0) anywhere in the method.
 
                     // A non-virtual call to a non-final virtual method is only allowed if
-                    //   1. The this pointer passed to the callee is an instance of a boxed value type. 
+                    //   1. The this pointer passed to the callee is an instance of a boxed value type.
                     //   (or)
                     //   2. The this pointer passed to the callee is the current method's this pointer.
                     //      (and) The current method's this pointer is not "possibly written".
 
-                    // Thus the rule is that if you assign to this ANYWHERE you can't make "base" calls to 
-                    // virtual methods.  (Luckily this does not affect .ctors, since they are not virtual).    
-                    // This is stronger than is strictly needed, but implementing a laxer rule is significantly 
+                    // Thus the rule is that if you assign to this ANYWHERE you can't make "base" calls to
+                    // virtual methods.  (Luckily this does not affect .ctors, since they are not virtual).
+                    // This is stronger than is strictly needed, but implementing a laxer rule is significantly
                     // harder and more error prone.
                     if (method.IsVirtual && !method.IsFinal && !actualThis.IsBoxedValueType)
                     {
@@ -1636,7 +1640,7 @@ again:
                     Check(!IsByRefLike(declaredThis), VerifierError.TailByRef, declaredThis);
 
                     // Tail calls on constrained calls should be illegal too:
-                    // when instantiated at a value type, a constrained call may pass the address of a stack allocated value 
+                    // when instantiated at a value type, a constrained call may pass the address of a stack allocated value
                     Check(constrained == null, VerifierError.TailByRef);
                 }
             }
@@ -1832,7 +1836,7 @@ again:
             {
                 if (next.IsThisInitialized && !_isThisInitialized)
                 {
-                    // Next block has 'this' initialized, but current state has not 
+                    // Next block has 'this' initialized, but current state has not
                     // therefore next block must be reverified with 'this' uninitialized
                     if (next.State == BasicBlock.ImportState.WasVerified && next.ErrorCount == 0)
                         next.State = BasicBlock.ImportState.Unmarked;
@@ -2218,6 +2222,12 @@ again:
 
             CheckIsObjRef(value);
 
+            if (SanityChecks && value != StackValue.CreateObjRef(GetWellKnownType(WellKnownType.Object)))
+            {
+                CheckIsAssignable(value, StackValue.CreateFromType(GetWellKnownType(WellKnownType.Exception)),
+                    VerifierError.ThrowOrCatchOnlyExceptionType);
+            }
+
             EmptyTheStack();
         }
 
@@ -2250,7 +2260,7 @@ again:
 
             Check(!IsByRefLike(targetType), VerifierError.BoxByRef, targetType);
 
-            Check(type.IsPrimitive || targetType.Kind == StackValueKind.ObjRef || 
+            Check(type.IsPrimitive || targetType.Kind == StackValueKind.ObjRef ||
                 type.IsGenericParameter || type.IsValueType, VerifierError.ExpectedValClassObjRefVariable);
 
             Check(type.CheckConstraints(), VerifierError.UnsatisfiedBoxOperand);

diff --git a/src/coreclr/src/tools/ILVerification/Strings.resx b/src/coreclr/src/tools/ILVerification/Strings.resx
index 82f096b..726cf6d 100644 (file)
--- a/src/coreclr/src/tools/ILVerification/Strings.resx
+++ b/src/coreclr/src/tools/ILVerification/Strings.resx
@@ -171,6 +171,9 @@
   <data name="CatchByRef" xml:space="preserve">
     <value>ByRef not allowed as catch type.</value>
   </data>
+  <data name="ThrowOrCatchOnlyExceptionType" xml:space="preserve">
+    <value>The type caught or thrown must be derived from System.Exception.</value>
+  </data>
   <data name="CodeSizeZero" xml:space="preserve">
     <value>Code size is zero.</value>
   </data>

diff --git a/src/coreclr/src/tools/ILVerification/Verifier.cs b/src/coreclr/src/tools/ILVerification/Verifier.cs
index db15494..032c386 100644 (file)
--- a/src/coreclr/src/tools/ILVerification/Verifier.cs
+++ b/src/coreclr/src/tools/ILVerification/Verifier.cs
@@ -179,7 +179,10 @@ namespace ILVerify
 
             try
             {
-                var importer = new ILImporter(method, methodIL);
+                var importer = new ILImporter(method, methodIL)
+                {
+                    SanityChecks = _verifierOptions.SanityChecks
+                };
 
                 importer.ReportVerificationError = (args, code) =>
                 {
@@ -321,5 +324,6 @@ namespace ILVerify
     public class VerifierOptions
     {
         public bool IncludeMetadataTokensInErrorMessages { get; set; }
+        public bool SanityChecks { get; set; }
     }
 }

diff --git a/src/coreclr/src/tools/ILVerification/VerifierError.cs b/src/coreclr/src/tools/ILVerification/VerifierError.cs
index 331d0a8..4be09b1 100644 (file)
--- a/src/coreclr/src/tools/ILVerification/VerifierError.cs
+++ b/src/coreclr/src/tools/ILVerification/VerifierError.cs
@@ -145,6 +145,7 @@ namespace ILVerify
         DelegateCtorSigO,               // Unrecognized delegate .ctor signature; expected Object.
         //E_RA_PTR_TO_STACK             "Mkrefany on TypedReference, ArgHandle, or ArgIterator."
         CatchByRef,                     // ByRef not allowed as catch type.
+        ThrowOrCatchOnlyExceptionType,  // The type caught or thrown must be derived from System.Exception.
         LdvirtftnOnStatic,              // ldvirtftn on static.
         CallVirtOnStatic,               // callvirt on static.
         InitLocals,                     // initlocals must be set for verifiable methods with one or more local variables.
@@ -189,6 +190,6 @@ namespace ILVerify
         //IDS_E_GLOBAL         "<GlobalFunction>"
         //IDS_E_MDTOKEN        "[mdToken=0x%x]"
         InterfaceImplHasDuplicate,            // InterfaceImpl has a duplicate
-        InterfaceMethodNotImplemented         // Class implements interface but not method 
+        InterfaceMethodNotImplemented         // Class implements interface but not method
     }
 }

diff --git a/src/coreclr/src/tools/ILVerify/Program.cs b/src/coreclr/src/tools/ILVerify/Program.cs
index b9fbf35..a5b3807 100644 (file)
--- a/src/coreclr/src/tools/ILVerify/Program.cs
+++ b/src/coreclr/src/tools/ILVerify/Program.cs
@@ -46,6 +46,7 @@ namespace ILVerify
             public string[] InputFilePath { get; set; }
             public string[] Reference { get; set; }
             public string SystemModule { get; set; }
+            public bool SanityChecks { get; set; }
             public string[] Include { get; set; }
             public FileInfo IncludeFile { get; set; }
             public string[] Exclude { get; set; }
@@ -63,6 +64,7 @@ namespace ILVerify
             command.AddArgument(new Argument<string[]>("input-file-path", "Input file(s)") { Arity = new ArgumentArity(1, Int32.MaxValue) });
             command.AddOption(new Option<string[]>(new[] { "--reference", "-r" }, "Reference metadata from the specified assembly"));
             command.AddOption(new Option<string>(new[] { "--system-module", "-s" }, "System module name (default: mscorlib)"));
+            command.AddOption(new Option<bool>(new[] { "--sanity-checks", "-c" }, "Check for valid constructs that are likely mistakes"));
             command.AddOption(new Option<string[]>(new[] { "--include", "-i" }, "Use only methods/types/namespaces, which match the given regular expression(s)"));
             command.AddOption(new Option<FileInfo>(new[] { "--include-file" }, "Same as --include, but the regular expression(s) are declared line by line in the specified file.").ExistingOnly());
             command.AddOption(new Option<string[]>(new[] { "--exclude", "-e" }, "Skip methods/types/namespaces, which match the given regular expression(s)"));
@@ -144,7 +146,11 @@ namespace ILVerify
 
         private int Run()
         {
-            _verifier = new Verifier(this, GetVerifierOptions());
+            _verifier = new Verifier(this, new VerifierOptions
+            {
+                IncludeMetadataTokensInErrorMessages = _options.Tokens,
+                SanityChecks = _options.SanityChecks
+            });
             _verifier.SetSystemModuleName(new AssemblyName(_options.SystemModule ?? "mscorlib"));
 
             int numErrors = 0;
@@ -164,11 +170,6 @@ namespace ILVerify
             }
         }
 
-        private VerifierOptions GetVerifierOptions()
-        {
-            return new VerifierOptions { IncludeMetadataTokensInErrorMessages = _options.Tokens };
-        }
-
         private void PrintVerifyMethodsResult(VerificationResult result, EcmaModule module, string pathOrModuleName)
         {
             Write("[IL]: Error [");
@@ -257,7 +258,7 @@ namespace ILVerify
                     }
                 }
             }
-            catch 
+            catch
             {
                 Write("Error while getting method signature");
             }
@@ -409,7 +410,7 @@ namespace ILVerify
 
         /// <summary>
         /// This method returns the fully qualified method name by concatenating assembly, type and method name.
-        /// This method exists to avoid additional assembly resolving, which might be triggered by calling 
+        /// This method exists to avoid additional assembly resolving, which might be triggered by calling
         /// MethodDesc.ToString().
         /// </summary>
         private string GetQualifiedMethodName(MetadataReader metadataReader, MethodDefinitionHandle methodHandle)

diff --git a/src/tests/ilverify/ILMethodTester.cs b/src/tests/ilverify/ILMethodTester.cs
index b7891d6..cb5a316 100644 (file)
--- a/src/tests/ilverify/ILMethodTester.cs
+++ b/src/tests/ilverify/ILMethodTester.cs
@@ -28,7 +28,7 @@ namespace ILVerification.Tests
         void TestMethodsWithInvalidIL(InvalidILTestCase invalidIL)
         {
             IEnumerable<VerificationResult> results = null;
-            
+
             try
             {
                 results = Verify(invalidIL);
@@ -58,7 +58,12 @@ namespace ILVerification.Tests
             EcmaModule module = TestDataLoader.GetModuleForTestAssembly(testCase.ModuleName);
             var methodHandle = (MethodDefinitionHandle) MetadataTokens.EntityHandle(testCase.MetadataToken);
             var method = (EcmaMethod)module.GetMethod(methodHandle);
-            var verifier = new Verifier((ILVerifyTypeSystemContext)method.Context, new VerifierOptions() { IncludeMetadataTokensInErrorMessages = true });
+            var verifier = new Verifier((ILVerifyTypeSystemContext)method.Context, new VerifierOptions
+            {
+                IncludeMetadataTokensInErrorMessages = true,
+                SanityChecks = true
+            });
+
             return verifier.Verify(module.PEReader, methodHandle);
         }
     }

diff --git a/src/tests/ilverify/ILTests/ExceptionRegionTests.il b/src/tests/ilverify/ILTests/ExceptionRegionTests.il
index 7e066ee..874c420 100644 (file)
--- a/src/tests/ilverify/ILTests/ExceptionRegionTests.il
+++ b/src/tests/ilverify/ILTests/ExceptionRegionTests.il
@@ -12,6 +12,15 @@
 .class public sequential ansi sealed beforefieldinit ExceptionRegionTests
        extends [System.Runtime]System.Object
 {
+    .method public hidebysig specialname rtspecialname instance void .ctor () cil managed
+    {
+        .maxstack 1
+
+        IL_0000: ldarg.0
+        IL_0001: call instance void [System.Runtime]System.Object::.ctor()
+        IL_0006: ret
+    }
+
     .method public instance void ExceptionRegion.NestedTryFinally_Valid() cil managed
     {
         .try
@@ -732,5 +741,27 @@
     MethodEnd:
         ret
     }
-}
 
+    .method public hidebysig instance void Catch.NonExceptionDerivedType_Invalid_ThrowOrCatchOnlyExceptionType() cil managed
+    {
+        .maxstack 1
+
+        .try {}
+        catch ExceptionRegionTests
+        {
+            leave.s MethodEnd
+        }
+
+        MethodEnd:
+            ret
+    }
+
+    .method public hidebysig instance void Throw.NonExceptionDerivedType_Invalid_ThrowOrCatchOnlyExceptionType() cil managed
+    {
+        .maxstack 1
+
+        newobj instance void ExceptionRegionTests::.ctor()
+        throw
+        ret
+    }
+}

diff --git a/src/tests/ilverify/ILTests/ThisStateTests.il b/src/tests/ilverify/ILTests/ThisStateTests.il
index 7b73893..47b96b5 100644 (file)
--- a/src/tests/ilverify/ILTests/ThisStateTests.il
+++ b/src/tests/ilverify/ILTests/ThisStateTests.il
@@ -83,7 +83,7 @@
 .class public auto ansi beforefieldinit ThisStateTestsType
         extends [System.Runtime]System.Object
 {
-    .method public hidebysig instance void 'special.ThrowThisUninit..ctor_Invalid_UninitStack'() cil managed { ret }
+    .method public hidebysig instance void 'special.ThrowThisUninit..ctor_Invalid_UninitStack.ThrowOrCatchOnlyExceptionType'() cil managed { ret }
     .method public hidebysig specialname rtspecialname instance void .ctor() cil managed 
     {
         ldarg.0

diff --git a/src/tests/ilverify/ILTypeVerificationTester.cs b/src/tests/ilverify/ILTypeVerificationTester.cs
index 3f9959e..902e612 100644 (file)
--- a/src/tests/ilverify/ILTypeVerificationTester.cs
+++ b/src/tests/ilverify/ILTypeVerificationTester.cs
@@ -58,7 +58,11 @@ namespace ILVerification.Tests
             EcmaModule module = TestDataLoader.GetModuleForTestAssembly(testCase.ModuleName);
             var typeHandle = (TypeDefinitionHandle)MetadataTokens.EntityHandle(testCase.MetadataToken);
             var type = (EcmaType)module.GetType(typeHandle);
-            var verifier = new Verifier((ILVerifyTypeSystemContext)type.Context, new VerifierOptions() { IncludeMetadataTokensInErrorMessages = true });
+            var verifier = new Verifier((ILVerifyTypeSystemContext)type.Context, new VerifierOptions
+            {
+                IncludeMetadataTokensInErrorMessages = true,
+                SanityChecks = true
+            });
             return verifier.Verify(module.PEReader, typeHandle);
         }