[ILVerify] Implement tail-ret and invalid method end verification (#5122)

* Implemented tail-ret sequence verification.

* Added test cases for tail-ret sequence.

* Fixed ILImporter not calling EndImportingInstruction when returning from switch.

* Added ReportMethodEndInsideInstruction to ILImporter.

* Changed ReportMethodEndInsideInstruction to be reported inside FindJumpTargets.

* Added additional tail-ret tests.

* Changed tail-ret verification to be done in ImportCall.

* Changed ReportMethodEndInsideInstruction to be reported for all il stream read operations.

5 files changed, 105 insertions, 7 deletions

diff --git a/src/ILVerify/src/ILImporter.Verify.cs b/src/ILVerify/src/ILImporter.Verify.cs
index 48f5b503b..9d3d128ec 100644
--- a/src/ILVerify/src/ILImporter.Verify.cs
+++ b/src/ILVerify/src/ILImporter.Verify.cs
@@ -289,6 +289,8 @@ namespace Internal.IL
         /// </summary>
         private void InitialPass()
         {
+            FatalCheck(_ilBytes.Length > 0, VerifierError.CodeSizeZero);
+
             _modifiesThisPtr = false;
             _validTargetOffsets = new bool[_ilBytes.Length];
 
@@ -458,9 +460,12 @@ again:
                         break;
                 }
 
-                var fallthrough = _basicBlocks[_currentOffset];
-                if (fallthrough != null)
-                    MarkPredecessorWithLowerOffset(0);
+                if (_currentOffset < _basicBlocks.Length)
+                {
+                    var fallthrough = _basicBlocks[_currentOffset];
+                    if (fallthrough != null)
+                        MarkPredecessorWithLowerOffset(0);
+                }
             }
         }
 
@@ -1673,6 +1678,9 @@ again:
 
                 // for tailcall, stack must be empty
                 Check(_stackTop == 0, VerifierError.TailStackEmpty);
+
+                // The instruction following a tail.call shall be a ret
+                Check(_currentOffset < _ilBytes.Length && (ILOpcode)_ilBytes[_currentOffset] == ILOpcode.ret, VerifierError.TailRet);
             }
 
             // now push on the result
@@ -2652,6 +2660,12 @@ again:
             VerificationError(VerifierError.MethodFallthrough);
         }
 
+        void ReportMethodEndInsideInstruction()
+        {
+            VerificationError(VerifierError.MethodEnd);
+            AbortMethodVerification();
+        }
+
         void ReportInvalidInstruction(ILOpcode opcode)
         {
             VerificationError(VerifierError.UnknownOpcode);
diff --git a/src/ILVerify/src/Resources/Strings.resx b/src/ILVerify/src/Resources/Strings.resx
index 53eadc3ee..63ab4cb01 100644
--- a/src/ILVerify/src/Resources/Strings.resx
+++ b/src/ILVerify/src/Resources/Strings.resx
@@ -171,6 +171,9 @@
   <data name="CatchByRef" xml:space="preserve">
     <value>ByRef not allowed as catch type.</value>
   </data>
+  <data name="CodeSizeZero" xml:space="preserve">
+    <value>Code size is zero.</value>
+  </data>
   <data name="Constrained" xml:space="preserve">
     <value>Missing callvirt following constrained prefix.</value>
   </data>
@@ -285,6 +288,9 @@
   <data name="MethodAccess" xml:space="preserve">
     <value>Method is not visible.</value>
   </data>
+  <data name="MethodEnd" xml:space="preserve">
+    <value>Method ends in the middle of an instruction.</value>
+  </data>
   <data name="MethodFallthrough" xml:space="preserve">
     <value>Fall through end of the method without returning.</value>
   </data>
@@ -366,6 +372,9 @@
   <data name="TailCallInsideER" xml:space="preserve">
     <value>The tail.call (or calli or callvirt) instruction cannot be used to transfer control out of a try, filter, catch, or finally block.</value>
   </data>
+  <data name="TailRet" xml:space="preserve">
+    <value>tail.call may only be followed by ret.</value>
+  </data>
   <data name="TailRetType" xml:space="preserve">
     <value>Tail call return type not compatible.</value>
   </data>
diff --git a/src/ILVerify/src/VerifierError.cs b/src/ILVerify/src/VerifierError.cs
index 728545a2c..8b2db4831 100644
--- a/src/ILVerify/src/VerifierError.cs
+++ b/src/ILVerify/src/VerifierError.cs
@@ -122,14 +122,14 @@ namespace ILVerify
         //E_SIG_ARRAY                   "Cannot resolve Array type."
         ArrayByRef,                     // Array of ELEMENT_TYPE_BYREF or ELEMENT_TYPE_TYPEDBYREF.
         ByrefOfByref,                   // ByRef of ByRef.
-        //E_CODE_SIZE_ZERO              "Code size is zero."
+        CodeSizeZero,                   // Code size is zero.
         TailCall,                       // Missing call/callvirt/calli.
         TailByRef,                      // Cannot pass ByRef to a tail call.
-        //E_TAIL_RET                    "Missing ret."
+        TailRet,                        // tail.call may only be followed by ret.
         TailRetVoid,                    // Void ret type expected for tail call.
         TailRetType,                    // Tail call return type not compatible.
         TailStackEmpty,                 // Stack not empty after tail call.
-        //E_METHOD_END                  "Method ends in the middle of an instruction."
+        MethodEnd,                      // Method ends in the middle of an instruction.
         BadBranch,                      // Branch out of the method.
         //E_LEXICAL_NESTING             "Lexical nesting."
         Volatile,                       // Missing ldsfld, stsfld, ldind, stind, ldfld, stfld, ldobj, stobj, initblk, or cpblk.
diff --git a/src/ILVerify/tests/ILTests/PrefixTests.il b/src/ILVerify/tests/ILTests/PrefixTests.il
index ec1fbe004..e5ba9244d 100644
--- a/src/ILVerify/tests/ILTests/PrefixTests.il
+++ b/src/ILVerify/tests/ILTests/PrefixTests.il
@@ -95,7 +95,7 @@
         ret
     }
 
-    .method static public hidebysig void Tail.FromTry_Invalid_TailCallInsideER() cil managed 
+    .method static public hidebysig void Tail.FromTry_Invalid_TailCallInsideER.TailRet() cil managed 
     {
         .try
         {
@@ -112,4 +112,74 @@
     MethodEnd:
         ret
     }
+
+    .method static public hidebysig void Tail.FollowedByRet_Valid() cil managed 
+    {
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+        ret
+    }
+
+    .method static public hidebysig void Tail.NoRet_Invalid_TailRet.MethodFallthrough() cil managed 
+    {
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+    }
+
+    .method static public hidebysig void Tail.DoubleCall_Invalid_TailRet() cil managed 
+    {
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+        call        void PrefixTestsType::StaticMethod()
+        ret
+    }
+
+    .method static public hidebysig void Tail.BranchToRet_Valid() cil managed 
+    {
+        ldc.i4.0
+        brfalse     MethodEnd
+
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+
+    MethodEnd:
+        ret
+    }
+
+    .method static public hidebysig void Tail.ComplexBranchToRet_Valid() cil managed 
+    {
+        br          AfterTail
+
+    TailCall:
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+
+    MethodEnd:
+        ret
+
+    AfterTail:
+        ldc.i4.0
+        brfalse     TailCall
+
+        br          MethodEnd
+    }
+
+    .method static public hidebysig void Tail.BranchToNopRet_Invalid_TailRet() cil managed 
+    {
+        ldc.i4.0
+        brfalse     MethodEnd
+
+        tail.
+        call        void PrefixTestsType::StaticMethod()
+
+    MethodEnd:
+        nop
+        ret
+    }
+
+    .method static public hidebysig void Tail.Ret_Invalid_TailCall() cil managed 
+    {
+        tail.
+        ret
+    }
 }
diff --git a/src/ILVerify/tests/ILTests/ReturnTests.il b/src/ILVerify/tests/ILTests/ReturnTests.il
index 487dd3218..4cc0b5081 100644
--- a/src/ILVerify/tests/ILTests/ReturnTests.il
+++ b/src/ILVerify/tests/ILTests/ReturnTests.il
@@ -149,6 +149,11 @@
         ret
     }
 
+    .method public hidebysig instance void MissingReturn_Invalid_MethodFallthrough() cil managed
+    {
+        ldarg.0
+    }
+
     .method public hidebysig instance void 'special.ReturnAfterBaseCtor..ctor_Valid'() cil managed { ret }
     .method public hidebysig specialname rtspecialname instance void .ctor() cil managed 
     {