fixed bugs in .NET bytecode generation (branchi...

fixed bugs in .NET bytecode generation (branching out of try / catch /
finally blocks is not allowed). predef.dll now almost passes PEVerify.
no review

3 files changed, 150 insertions, 50 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 7f351293c5..da695e45d7 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -301,6 +301,15 @@ abstract class GenICode extends SubComponent  {
     private def genSynchronized(tree: Apply, ctx: Context, expectedType: TypeKind): (Context, TypeKind) = {
       val Apply(fun, args) = tree
       val monitor = ctx.makeLocal(tree.pos, ObjectClass.tpe, "monitor")
+      var monitorResult: Local = null
+
+      // if the synchronized block returns a result, store it in a local variable. just leaving
+      // it on the stack is not valid in MSIL (stack is cleaned when leaving try-blocks)
+      val argTpe = args.head.tpe
+      val hasResult = expectedType != UNIT
+      if (hasResult)
+        monitorResult = ctx.makeLocal(tree.pos, argTpe, "monitorResult")
+
       var ctx1 = genLoadQualifier(fun, ctx)
       ctx1.bb.emit(Seq(
         DUP(ANY_REF_CLASS),
@@ -313,6 +322,8 @@ abstract class GenICode extends SubComponent  {
       ctx1 = ctx1.Try(
         bodyCtx => {
           val ctx2 = genLoad(args.head, bodyCtx, expectedType /* toTypeKind(tree.tpe.resultType) */)
+          if (hasResult)
+            ctx2.bb.emit(STORE_LOCAL(monitorResult))
           ctx2.bb.emit(Seq(
             LOAD_LOCAL(monitor),
             MONITOR_EXIT() setPos tree.pos
@@ -332,7 +343,8 @@ abstract class GenICode extends SubComponent  {
 
       debugLog("synchronized block end with block %s closed=%s".format(ctx1.bb, ctx1.bb.closed))
       ctx1.exitSynchronized(monitor)
-
+      if (hasResult)
+        ctx1.bb.emit(LOAD_LOCAL(monitorResult))
       (ctx1, expectedType)
     }
 
@@ -1477,10 +1489,10 @@ abstract class GenICode extends SubComponent  {
 
       def prune0(block: BasicBlock): Unit = {
         val optCont = block.lastInstruction match {
-          case JUMP(b) if (b != block) => Some(b);
+          case JUMP(b) if (b != block) => Some(b)
           case _ => None
         }
-        if (block.size == 1 && optCont != None) {
+        if (block.size == 1 && optCont.isDefined) {
           val Some(cont) = optCont;
           val pred = block.predecessors;
           log("Preds: " + pred + " of " + block + " (" + optCont + ")");
@@ -1954,12 +1966,16 @@ abstract class GenICode extends SubComponent  {
        *
        *  A
        *    try { .. } catch { .. } finally { .. }
-       *  blocks is de-sugared into
+       *  block is de-sugared into
        *    try { try { ..} catch { .. } } finally { .. }
        *
-       *  A `finally` block is represented exactly the same as an exception handler, but
-       *  with `NoSymbol` as the exception class. The covered blocks are all blocks of
+       *  In ICode `finally` block is represented exactly the same as an exception handler,
+       *  but with `NoSymbol` as the exception class. The covered blocks are all blocks of
        *  the `try { .. } catch { .. }`.
+       *
+       *  Also, TryMsil does not enter any Finalizers into the `cleanups', because the
+       *  CLI takes care of running the finalizer when seeing a `leave' statement inside
+       *  a try / catch.
        */
       def TryMsil(body: Context => Context,
                   handlers: List[(Symbol, TypeKind, (Context => Context))],
@@ -1978,6 +1994,7 @@ abstract class GenICode extends SubComponent  {
           val ctx = finalizerCtx.enterHandler(exh)
           if (settings.Xdce.value) ctx.bb.emit(LOAD_EXCEPTION())
           val ctx1 = genLoad(finalizer, ctx, UNIT)
+          // need jump for the ICode to be valid. MSIL backend will emit `Endfinally` instead.
           ctx1.bb.emit(JUMP(afterCtx.bb))
           ctx1.bb.close
           finalizerCtx.endHandler()
@@ -1988,6 +2005,7 @@ abstract class GenICode extends SubComponent  {
           var ctx1 = outerCtx.enterHandler(exh)
           if (settings.Xdce.value) ctx1.bb.emit(LOAD_EXCEPTION())
           ctx1 = handler._3(ctx1)
+          // msil backend will emit `Leave` to jump out of a handler
           ctx1.bb.emit(JUMP(afterCtx.bb))
           ctx1.bb.close
           outerCtx.endHandler()
@@ -2000,6 +2018,7 @@ abstract class GenICode extends SubComponent  {
         outerCtx.bb.emit(JUMP(bodyCtx.bb))
         outerCtx.bb.close
 
+        // msil backend will emit `Leave` to jump out of a try-block
         finalCtx.bb.emit(JUMP(afterCtx.bb))
         finalCtx.bb.close
 
diff --git a/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala b/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
index 127c695f43..d9261a3006 100644
--- a/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
+++ b/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
@@ -601,10 +601,18 @@ abstract class GenMSIL extends SubComponent {
 
       genBlocks(linearization)
 
+      // RETURN inside exception blocks are replaced by Leave. The target of the
+      // levae is a `Ret` outside any exception block (generated here).
+      if (handlerReturnMethod == m) {
+        mcode.MarkLabel(handlerReturnLabel)
+        if (handlerReturnKind != UNIT)
+          mcode.Emit(OpCodes.Ldloc, handlerReturnLocal)
+        mcode.Emit(OpCodes.Ret)
+      }
+
       beginExBlock.clear()
       beginCatchBlock.clear()
       endExBlock.clear()
-      omitJumpBlocks.clear()
     }
 
     def genBlocks(blocks: List[BasicBlock], previous: BasicBlock = null) {
@@ -622,15 +630,36 @@ abstract class GenMSIL extends SubComponent {
     val beginCatchBlock = new HashMap[BasicBlock, ExceptionHandler]()
     val endExBlock = new HashMap[BasicBlock, List[ExceptionHandler]]()
 
-    // at the end of a try or catch block, the jumps must not be emitted.
-    // the automatically generated leave will do the job.
-    val omitJumpBlocks: HashSet[BasicBlock] = new HashSet()
+    // when emitting the code (genBlock), the number of currently active try / catch
+    // blocks. When seeing a `RETURN' inside a try / catch, we need to replace
+    // it with Leave
+    var currentHandlers = 0
+    // The IMethod the Local/Label/Kind below belong to
+    var handlerReturnMethod: IMethod = _
+    // Stores the result when returning inside an exception block
+    var handlerReturnLocal: LocalBuilder = _
+    // Label for a return instruction outside any exception block
+    var handlerReturnLabel: Label = _
+    // The result kind.
+    var handlerReturnKind: TypeKind = _
+    def returnFromHandler(kind: TypeKind): (LocalBuilder, Label) = {
+      if (handlerReturnMethod != method) {
+        handlerReturnMethod = method
+        if (kind != UNIT) {
+          handlerReturnLocal = mcode.DeclareLocal(msilType(kind))
+          handlerReturnLocal.SetLocalSymInfo("$handlerReturn")
+        }
+        handlerReturnLabel = mcode.DefineLabel()
+        handlerReturnKind = kind
+      }
+      (handlerReturnLocal, handlerReturnLabel)
+    }
 
     /** Computes which blocks are the beginning / end of a try or catch block */
     private def computeExceptionMaps(blocks: List[BasicBlock], m: IMethod): List[BasicBlock] = {
       val visitedBlocks = new HashSet[BasicBlock]()
 
-      // handlers which have not been intruduced so far
+      // handlers which have not been introduced so far
       var openHandlers = m.exh
 
 
@@ -661,8 +690,6 @@ abstract class GenMSIL extends SubComponent {
       // tail is all following catch blocks. Example *2*: Stack(List(h3), List(h4, h5))
       val currentCatchHandlers = new Stack[List[ExceptionHandler]]()
 
-      var prev: BasicBlock = null
-
       for (b <- blocks) {
 
         // are we past the current catch blocks?
@@ -676,12 +703,12 @@ abstract class GenMSIL extends SubComponent {
                      "Bad linearization of basic blocks inside catch. Found block not part of the handler\n"+
                      b.fullString +"\nwhile in catch-part of\n"+ handler)
 
-              omitJumpBlocks += prev
-
               val rest = currentCatchHandlers.pop.tail
               if (rest.isEmpty) {
+                // all catch blocks of that exception handler are covered
                 res = handler :: endHandlers()
               } else {
+                // there are more catch blocks for that try (handlers covering the same)
                 currentCatchHandlers.push(rest)
                 beginCatchBlock(b) = rest.head
               }
@@ -708,7 +735,6 @@ abstract class GenMSIL extends SubComponent {
             val handlers = currentTryHandlers.pop
             currentCatchHandlers.push(handlers)
             beginCatchBlock(b) = handler
-            omitJumpBlocks += prev
           }
         }
 
@@ -745,7 +771,6 @@ abstract class GenMSIL extends SubComponent {
         }
         beginExBlock(b) = beginHandlers.toList
         visitedBlocks += b
-        prev = b
       }
 
       // if there handlers left (i.e. handlers covering nothing, or a
@@ -769,7 +794,6 @@ abstract class GenMSIL extends SubComponent {
       if (rest.isEmpty) {
         liveBlocks
       } else {
-        omitJumpBlocks += prev
         val b = m.code.newBlock
         b.emit(Seq(
           NEW(REFERENCE(definitions.ThrowableClass)),
@@ -802,14 +826,18 @@ abstract class GenMSIL extends SubComponent {
 
       var lastLineNr: Int = 0
 
-      mcode.MarkLabel(labels(block))
+      // EndExceptionBlock must happen before MarkLabel because it adds the
+      // Leave instruction. Otherwise, labels(block) points to the Leave
+      // (inside the catch) instead of the instruction afterwards.
+      for (handlers <- endExBlock.get(block); exh <- handlers) {
+        currentHandlers -= 1
+        mcode.EndExceptionBlock()
+      }
 
+      mcode.MarkLabel(labels(block))
       if (settings.debug.value)
         log("Generating code for block: " + block)
 
-      for (handlers <- endExBlock.get(block); exh <- handlers) {
-        mcode.EndExceptionBlock()
-      }
       for (handler <- beginCatchBlock.get(block)) {
         if (handler.cls == NoSymbol) {
           // `finally` blocks are represented the same as `catch`, but with no catch-type
@@ -820,6 +848,7 @@ abstract class GenMSIL extends SubComponent {
         }
       }
       for (handlers <- beginExBlock.get(block); exh <- handlers) {
+        currentHandlers += 1
         mcode.BeginExceptionBlock()
       }
 
@@ -1131,7 +1160,7 @@ abstract class GenMSIL extends SubComponent {
             //    if the int on stack is 4, and 4 is in the second list => jump
             //    to second label
             // branches is List[BasicBlock]
-            //    the labels to jump to (the last one ist the default one)
+            //    the labels to jump to (the last one is the default one)
 
             val switchLocal = mcode.DeclareLocal(MINT)
             // several switch variables will appear with the same name in the
@@ -1150,12 +1179,17 @@ abstract class GenMSIL extends SubComponent {
               i += 1
             }
             val defaultTarget = labels(branches(i))
-            if (next != defaultTarget && !omitJumpBlocks.contains(block))
+            if (next != defaultTarget)
               mcode.Emit(OpCodes.Br, defaultTarget)
 
-
           case JUMP(whereto) =>
-            if (next != whereto && !omitJumpBlocks.contains(block))
+            val (leaveHandler, leaveFinally) = leavesHandler(block, whereto, method.exh)
+            if (leaveHandler) {
+              if (leaveFinally)
+                mcode.Emit(OpCodes.Endfinally)
+              else
+                mcode.Emit(OpCodes.Leave, labels(whereto))
+            } else if (next != whereto)
               mcode.Emit(OpCodes.Br, labels(whereto))
 
           case CJUMP(success, failure, cond, kind) =>
@@ -1163,30 +1197,21 @@ abstract class GenMSIL extends SubComponent {
             // values EQ, NE, LT, GE LE, GT
             // kind is TypeKind
             val isFloat = kind == FLOAT || kind == DOUBLE
-            if (next == success || omitJumpBlocks.contains(block)) {
-              emitBr(cond.negate, labels(failure), isFloat)
-            } else {
-              emitBr(cond, labels(success), isFloat)
-              if (next != failure && !omitJumpBlocks.contains(block)) {
-                mcode.Emit(OpCodes.Br, labels(failure))
-              }
-            }
+            val emit = (c: TestOp, l: Label) => emitBr(c, l, isFloat)
+            emitCondBr(block, cond, success, failure, next, method.exh, emit)
 
           case CZJUMP(success, failure, cond, kind) =>
-            (kind: @unchecked) match {
-              case BOOL | REFERENCE(_) =>
-                if (next == success || omitJumpBlocks.contains(block)) {
-                  emitBrBool(cond.negate, labels(failure))
-                } else {
-                  emitBrBool(cond, labels(success))
-                  if (next != failure && !omitJumpBlocks.contains(block)) {
-                    mcode.Emit(OpCodes.Br, labels(failure))
-                  }
-                }
-            }
+            emitCondBr(block, cond, success, failure, next, method.exh, emitBrBool(_, _))
 
           case RETURN(kind) =>
-            mcode.Emit(OpCodes.Ret)
+            if (currentHandlers == 0)
+              mcode.Emit(OpCodes.Ret)
+            else {
+              val (local, label) = returnFromHandler(kind)
+              if (kind != UNIT)
+                mcode.Emit(OpCodes.Stloc, local)
+              mcode.Emit(OpCodes.Leave, label)
+            }
 
           case THROW() =>
             mcode.Emit(OpCodes.Throw)
@@ -1335,8 +1360,63 @@ abstract class GenMSIL extends SubComponent {
           code.Emit(OpCodes.Ldloc, localBuilders(local))
     }
 
-    ////////////////////// labels ///////////////////////
+    ////////////////////// branches ///////////////////////
+
+    def leavesHandler(from: BasicBlock, to: BasicBlock, handlers: List[ExceptionHandler]) =
+      if (currentHandlers == 0) (false, false)
+      else {
+        val h = handlers.find(e => {
+          e.covers(from) != e.covers(to) ||
+          e.blocks.contains(from) != e.blocks.contains(to)
+        })
+        if (h.isDefined) {
+          val leavesFinalizerHandler =
+            h.get.cls == NoSymbol && h.get.blocks.contains(from) != h.get.blocks.contains(to)
+          (true, leavesFinalizerHandler)
+        } else (false, false)
+      }
+
+    def emitCondBr(block: BasicBlock, cond: TestOp, success: BasicBlock, failure: BasicBlock,
+                   next: BasicBlock, handlers: List[ExceptionHandler], emitBrFun: (TestOp, Label) => Unit) {
+      val (sLeaveHandler, sLeaveFinally) = leavesHandler(block, success, handlers)
+      val (fLeaveHandler, fLeaveFinally) = leavesHandler(block, failure, handlers)
+
+      if (sLeaveHandler || fLeaveHandler) {
+        val sLabelOpt = if (sLeaveHandler) {
+          val leaveSLabel = mcode.DefineLabel()
+          emitBrFun(cond, leaveSLabel)
+          Some(leaveSLabel)
+        } else {
+          emitBrFun(cond, labels(success))
+          None
+        }
 
+        if (fLeaveHandler) {
+          if (fLeaveFinally)
+            mcode.Emit(OpCodes.Endfinally)
+          else
+            mcode.Emit(OpCodes.Leave, labels(failure))
+        } else
+          mcode.Emit(OpCodes.Br, labels(failure))
+
+        sLabelOpt.map(l => {
+          mcode.MarkLabel(l)
+          if (sLeaveFinally)
+            mcode.Emit(OpCodes.Endfinally)
+          else
+            mcode.Emit(OpCodes.Leave, labels(success))
+        })
+      } else {
+        if (next == success) {
+          emitBrFun(cond.negate, labels(failure))
+        } else {
+          emitBrFun(cond, labels(success))
+          if (next != failure) {
+            mcode.Emit(OpCodes.Br, labels(failure))
+          }
+        }
+      }
+    }
 
     def emitBr(condition: TestOp, dest: Label, isFloat: Boolean) {
       condition match {
diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index 0371526b40..a1ef4a07ae 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -834,9 +834,10 @@ trait Definitions {
 
         // additional methods of Object
         newMethod(ObjectClass, "clone", List(), AnyRefClass.typeConstructor)
-        newMethod(ObjectClass, "wait", List(), unitType)
-        newMethod(ObjectClass, "wait", List(longType), unitType)
-        newMethod(ObjectClass, "wait", List(longType, intType), unitType)
+        // wait in Java returns void, on .NET Wait returns boolean. by putting
+        //  `booltype` the compiler adds a `drop` after calling wait.
+        newMethod(ObjectClass, "wait", List(), booltype)
+        newMethod(ObjectClass, "wait", List(longType), booltype)
         newMethod(ObjectClass, "notify", List(), unitType)
         newMethod(ObjectClass, "notifyAll", List(), unitType)