diff options
author | Samuel Arzt <arzt.samuel@live.de> | 2017-09-29 15:20:09 +0300 |
---|---|---|
committer | Jan Kotas <jkotas@microsoft.com> | 2017-09-29 15:20:09 +0300 |
commit | 55c9db39ce33698a5aec98fd5bea2a0b9c835ab8 (patch) | |
tree | 7d7faaf6fbebf921073a57bc84ff486de0c90e55 | |
parent | 57fbe8d91575d596a38f1b9c2ea52a4669572e2f (diff) |
[ILVerify] Fix invalid branch/fallthrough into catch/finally causing assert (#4636)
* Fixed fallthrough into filter/catch/finally causing assert.
* Fixed catch block of TryIntoFilter test not leaving correctly.
-rw-r--r-- | src/ILVerify/src/ILImporter.Verify.cs | 84 | ||||
-rw-r--r-- | src/ILVerify/tests/ILTests/BranchingTests.il | 27 |
2 files changed, 90 insertions, 21 deletions
diff --git a/src/ILVerify/src/ILImporter.Verify.cs b/src/ILVerify/src/ILImporter.Verify.cs index 3b94fd25c..fd8d8e3e1 100644 --- a/src/ILVerify/src/ILImporter.Verify.cs +++ b/src/ILVerify/src/ILImporter.Verify.cs @@ -393,18 +393,28 @@ namespace Internal.IL VerificationError(VerifierError.StackUnexpected, src, dst); } - void CheckIsValidBranchTarget(BasicBlock src, BasicBlock target) + bool IsValidBranchTarget(BasicBlock src, BasicBlock target, bool reportErrors = true) { + bool isValid = true; + if (src.TryIndex != target.TryIndex) { if (src.TryIndex == null) { // Branching to first instruction of try-block is valid if (target.StartOffset != _exceptionRegions[(int)target.TryIndex].ILRegion.TryOffset || !IsDirectChildRegion(src, target)) - VerificationError(VerifierError.BranchIntoTry); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoTry); + isValid = false; + } } else if (target.TryIndex == null) - VerificationError(VerifierError.BranchOutOfTry); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfTry); + isValid = false; + } else { ref var srcRegion = ref _exceptionRegions[(int)src.TryIndex].ILRegion; @@ -415,46 +425,88 @@ namespace Internal.IL { // Only branching to first instruction of try-block is valid if (target.StartOffset != targetRegion.TryOffset || !IsDirectChildRegion(src, target)) - VerificationError(VerifierError.BranchIntoTry); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoTry); + isValid = false; + } } else - VerificationError(VerifierError.BranchOutOfTry); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfTry); + isValid = false; + } } } if (src.FilterIndex != target.FilterIndex) { if (src.FilterIndex == null) - VerificationError(VerifierError.BranchIntoFilter); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoFilter); + isValid = false; + } else if (target.HandlerIndex == null) - VerificationError(VerifierError.BranchOutOfFilter); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfFilter); + isValid = false; + } else { ref var srcRegion = ref _exceptionRegions[(int)src.FilterIndex].ILRegion; ref var targetRegion = ref _exceptionRegions[(int)target.FilterIndex].ILRegion; if (srcRegion.FilterOffset <= targetRegion.FilterOffset) - VerificationError(VerifierError.BranchIntoFilter); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoFilter); + isValid = false; + } else - VerificationError(VerifierError.BranchOutOfFilter); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfFilter); + isValid = false; + } } } if (src.HandlerIndex != target.HandlerIndex) { if (src.HandlerIndex == null) - VerificationError(VerifierError.BranchIntoHandler); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoHandler); + isValid = false; + } else if (target.HandlerIndex == null) - VerificationError(VerifierError.BranchOutOfHandler); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfHandler); + isValid = false; + } else { ref var srcRegion = ref _exceptionRegions[(int)src.HandlerIndex].ILRegion; ref var targetRegion = ref _exceptionRegions[(int)target.HandlerIndex].ILRegion; if (srcRegion.HandlerOffset <= targetRegion.HandlerOffset) - VerificationError(VerifierError.BranchIntoHandler); + { + if (reportErrors) + VerificationError(VerifierError.BranchIntoHandler); + isValid = false; + } else - VerificationError(VerifierError.BranchOutOfHandler); + { + if (reportErrors) + VerificationError(VerifierError.BranchOutOfHandler); + isValid = false; + } } } + + return isValid; } /// <summary> @@ -1210,6 +1262,9 @@ namespace Internal.IL void ImportFallthrough(BasicBlock next) { + if (!IsValidBranchTarget(_currentBasicBlock, next)) + return; + StackValue[] entryStack = next.EntryStack; if (entryStack != null) @@ -1262,7 +1317,6 @@ namespace Internal.IL for (int i = 0; i < jmpDelta.Length; i++) { BasicBlock target = _basicBlocks[jmpBase + jmpDelta[i]]; - CheckIsValidBranchTarget(_currentBasicBlock, target); ImportFallthrough(target); } @@ -1272,8 +1326,6 @@ namespace Internal.IL void ImportBranch(ILOpcode opcode, BasicBlock target, BasicBlock fallthrough) { - CheckIsValidBranchTarget(_currentBasicBlock, target); - switch (opcode) { case ILOpcode.br: diff --git a/src/ILVerify/tests/ILTests/BranchingTests.il b/src/ILVerify/tests/ILTests/BranchingTests.il index 199ffae88..abd4b7ba7 100644 --- a/src/ILVerify/tests/ILTests/BranchingTests.il +++ b/src/ILVerify/tests/ILTests/BranchingTests.il @@ -571,8 +571,7 @@ lbl_ret: ret } - /* These tests currently yield an assert (see issue #4537), due to a non-null entry stack to a catch/finally block; uncomment once fixed - .method static public hidebysig void Branching.FromTryIntoCatch_Invalid_BranchOutOfTry.BranchIntoFilter() cil managed + .method static public hidebysig void Branching.FromTryIntoCatch_Invalid_BranchOutOfTry.BranchIntoHandler() cil managed { .maxstack 2 @@ -594,8 +593,27 @@ lbl_ret: ret } - */ - /* + + .method static public hidebysig void Branching.FromTryIntoFilter_Invalid_BranchOutOfTry.BranchIntoFilter() cil managed + { + .try + { + br.s lbl_filter + } + filter + { + lbl_filter: pop + ldc.i4.0 + endfilter + } + { + pop + leave.s lbl_ret + } + + lbl_ret: ret + } + .method static public hidebysig void Branching.FromTryIntoFinally_Invalid_BranchOutOfTry.BranchIntoHandler() cil managed { .maxstack 2 @@ -617,7 +635,6 @@ lbl_ret: ret } - */ .method static public hidebysig void Branching.FromTryIntoOtherTryStart_Invalid_BranchOutOfTry() cil managed { |