diff options
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/icode/GenICode.scala | 75 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/icode/Members.scala | 17 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala | 377 | ||||
-rw-r--r-- | test/files/jvm/duration-tck.scala | 5 | ||||
-rw-r--r-- | test/files/jvm/t7006/Foo_1.flags | 1 | ||||
-rw-r--r-- | test/files/jvm/t7006/Foo_1.scala | 10 | ||||
-rw-r--r-- | test/files/jvm/t7006/Test.scala | 19 | ||||
-rw-r--r-- | test/files/jvm/t7181/Foo_1.scala | 26 | ||||
-rw-r--r-- | test/files/jvm/t7181/Test.scala | 24 | ||||
-rw-r--r-- | test/files/run/inline-ex-handlers.check | 150 | ||||
-rw-r--r-- | test/files/run/t6102.flags | 2 | ||||
-rw-r--r-- | test/files/run/t7181.check | 23 | ||||
-rw-r--r-- | test/files/run/t7181.scala | 78 |
13 files changed, 505 insertions, 302 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala index 122972039b..5438fd8590 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala @@ -1921,15 +1921,46 @@ abstract class GenICode extends SubComponent { * }), (AnotherExceptionClass, * ctx => {... * } ))` + * + * The resulting structure will look something like + * + * outer: + * // this 'useless' jump will be removed later, + * // for now it separates the try body's blocks from previous + * // code since the try body needs its own exception handlers + * JUMP body + * + * body: + * [ try body ] + * JUMP normalExit + * + * catch[i]: + * [ handler[i] body ] + * JUMP normalExit + * + * catchAll: + * STORE exception + * [ finally body ] + * THROW exception + * + * normalExit: + * [ finally body ] + * + * each catch[i] will cover body. catchAll will cover both body and each catch[i] + * Additional finally copies are created on the emission of every RETURN in the try body and exception handlers. + * + * This could result in unreachable code which has to be cleaned up later, e.g. if the try and all the exception + * handlers always end in RETURN then there will be no "normal" flow out of the try/catch/finally. + * Later reachability analysis will remove unreacahble code. */ def Try(body: Context => Context, handlers: List[(Symbol, TypeKind, Context => Context)], finalizer: Tree, tree: Tree) = { - val outerCtx = this.dup // context for generating exception handlers, covered by finalizer + val outerCtx = this.dup // context for generating exception handlers, covered by the catch-all finalizer val finalizerCtx = this.dup // context for generating finalizer handler - val afterCtx = outerCtx.newBlock() + val normalExitCtx = outerCtx.newBlock() // context where flow will go on a "normal" (non-return, non-throw) exit from a try or catch handler var tmp: Local = null val kind = toTypeKind(tree.tpe) val guardResult = kind != UNIT && mayCleanStack(finalizer) @@ -1956,30 +1987,31 @@ abstract class GenICode extends SubComponent { } else ctx + // Generate the catch-all exception handler that deals with uncaught exceptions coming + // from the try or exception handlers. It catches the exception, runs the finally code, then rethrows + // the exception if (finalizer != EmptyTree) { val exh = outerCtx.newExceptionHandler(NoSymbol, finalizer.pos) // finalizer covers exception handlers this.addActiveHandler(exh) // .. and body aswell - val ctx = finalizerCtx.enterExceptionHandler(exh) - val exception = ctx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc") - loadException(ctx, exh, finalizer.pos) - ctx.bb.emit(STORE_LOCAL(exception)) - val ctx1 = genLoad(finalizer, ctx, UNIT) - ctx1.bb.emit(LOAD_LOCAL(exception)) - ctx1.bb.emit(THROW(ThrowableClass)) - ctx1.bb.enterIgnoreMode() - ctx1.bb.close() + val exhStartCtx = finalizerCtx.enterExceptionHandler(exh) + val exception = exhStartCtx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc") + loadException(exhStartCtx, exh, finalizer.pos) + exhStartCtx.bb.emit(STORE_LOCAL(exception)) + val exhEndCtx = genLoad(finalizer, exhStartCtx, UNIT) + exhEndCtx.bb.emit(LOAD_LOCAL(exception)) + exhEndCtx.bb.closeWith(THROW(ThrowableClass)) + exhEndCtx.bb.enterIgnoreMode() finalizerCtx.endHandler() } + // Generate each exception handler for ((sym, kind, handler) <- handlers) { val exh = this.newExceptionHandler(sym, tree.pos) - var ctx1 = outerCtx.enterExceptionHandler(exh) - ctx1.addFinalizer(finalizer, finalizerCtx) - loadException(ctx1, exh, tree.pos) - ctx1 = handler(ctx1) - // emit finalizer - val ctx2 = emitFinalizer(ctx1) - ctx2.bb.closeWith(JUMP(afterCtx.bb)) + val exhStartCtx = outerCtx.enterExceptionHandler(exh) + exhStartCtx.addFinalizer(finalizer, finalizerCtx) + loadException(exhStartCtx, exh, tree.pos) + val exhEndCtx = handler(exhStartCtx) + exhEndCtx.bb.closeWith(JUMP(normalExitCtx.bb)) outerCtx.endHandler() } @@ -1987,14 +2019,13 @@ abstract class GenICode extends SubComponent { if (finalizer != EmptyTree) bodyCtx.addFinalizer(finalizer, finalizerCtx) - var finalCtx = body(bodyCtx) - finalCtx = emitFinalizer(finalCtx) + val bodyEndCtx = body(bodyCtx) outerCtx.bb.closeWith(JUMP(bodyCtx.bb)) - finalCtx.bb.closeWith(JUMP(afterCtx.bb)) + bodyEndCtx.bb.closeWith(JUMP(normalExitCtx.bb)) - afterCtx + emitFinalizer(normalExitCtx) } } } diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala index 5c90fbf366..e471f4256b 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala @@ -62,17 +62,18 @@ trait Members { def removeBlock(b: BasicBlock) { if (settings.debug.value) { - assert(blocks forall (p => !(p.successors contains b)), - "Removing block that is still referenced in method code " + b + "preds: " + b.predecessors - ) - assert(b != startBlock || b.successors.length == 1, - "Removing start block with more than one successor." - ) + // only do this sanity check when debug is turned on because it's moderately expensive + val referers = blocks filter (_.successors contains b) + assert(referers.isEmpty, s"Trying to removing block $b (with preds ${b.predecessors.mkString}) but it is still refered to from block(s) ${referers.mkString}") } - if (b == startBlock) + if (b == startBlock) { + assert(b.successors.length == 1, + s"Removing start block ${b} with ${b.successors.length} successors (${b.successors.mkString})." + ) startBlock = b.successors.head - + } + blocks -= b assert(!blocks.contains(b)) method.exh filter (_ covers b) foreach (_.covered -= b) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 75a8dfff90..91cb1857ac 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -11,6 +11,7 @@ import scala.reflect.internal.pickling.{ PickleFormat, PickleBuffer } import scala.tools.nsc.symtab._ import scala.tools.asm import asm.Label +import scala.annotation.tailrec /** * @author Iulian Dragos (version 1.0, FJBG-based implementation) @@ -2515,21 +2516,13 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { jcode.emitSWITCH(flatKeys, flatBranches, defaultLabel, MIN_SWITCH_DENSITY) case JUMP(whereto) => - if (nextBlock != whereto) { + if (nextBlock != whereto) jcode goTo labels(whereto) - } else if (m.exh.exists(eh => eh.covers(b))) { // SI-6102: Determine whether eliding this JUMP results in an empty range being covered by some EH. - // If so, emit a NOP in place of the elided JUMP, to avoid "java.lang.ClassFormatError: Illegal exception table range" - val isSthgLeft = b.toList.exists { - case _: LOAD_EXCEPTION => false - case _: SCOPE_ENTER => false - case _: SCOPE_EXIT => false - case _: JUMP => false - case _ => true - } - if (!isSthgLeft) { - emit(asm.Opcodes.NOP) - } + // If so, emit a NOP in place of the elided JUMP, to avoid "java.lang.ClassFormatError: Illegal exception table range" + else if (newNormal.isJumpOnly(b) && m.exh.exists(eh => eh.covers(b))) { + debugwarn("Had a jump only block that wasn't collapsed") + emit(asm.Opcodes.NOP) } case CJUMP(success, failure, cond, kind) => @@ -3084,111 +3077,50 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { * TODO Eventually, these utilities should be moved to IMethod and reused from normalize() (there's nothing JVM-specific about them). */ object newNormal { - - def startsWithJump(b: BasicBlock): Boolean = { assert(b.nonEmpty, "empty block"); b.firstInstruction.isInstanceOf[JUMP] } - - /** Prune from an exception handler those covered blocks which are jump-only. */ - private def coverWhatCountsOnly(m: IMethod): Boolean = { - assert(m.hasCode, "code-less method") - - var wasReduced = false - for(h <- m.exh) { - val shouldntCover = (h.covered filter startsWithJump) - if(shouldntCover.nonEmpty) { - wasReduced = true - h.covered --= shouldntCover // not removing any block on purpose. - } - } - - wasReduced + /** + * True if a block is "jump only" which is defined + * as being a block that consists only of 0 or more instructions that + * won't make it to the JVM followed by a JUMP. + */ + def isJumpOnly(b: BasicBlock): Boolean = { + val nonICode = firstNonIcodeOnlyInstructions(b) + // by definition a block has to have a jump, conditional jump, return, or throw + assert(nonICode.hasNext, "empty block") + nonICode.next.isInstanceOf[JUMP] } - - /** An exception handler is pruned provided any of the following holds: - * (1) it covers nothing (for example, this may result after removing unreachable blocks) - * (2) each block it covers is of the form: JUMP(_) - * Return true iff one or more ExceptionHandlers were removed. - * - * A caveat: removing an exception handler, for whatever reason, means that its handler code (even if unreachable) - * won't be able to cause a class-loading-exception. As a result, behavior can be different. + + /** + * Returns the list of instructions in a block that follow all ICode only instructions, + * where an ICode only instruction is one that won't make it to the JVM */ - private def elimNonCoveringExh(m: IMethod): Boolean = { - assert(m.hasCode, "code-less method") - - def isRedundant(eh: ExceptionHandler): Boolean = { - (eh.cls != NoSymbol) && ( // TODO `eh.isFinallyBlock` more readable than `eh.cls != NoSymbol` - eh.covered.isEmpty - || (eh.covered forall startsWithJump) - ) - } - - var wasReduced = false - val toPrune = (m.exh.toSet filter isRedundant) - if(toPrune.nonEmpty) { - wasReduced = true - for(h <- toPrune; r <- h.blocks) { m.code.removeBlock(r) } // TODO m.code.removeExh(h) - m.exh = (m.exh filterNot toPrune) - } - - wasReduced + private def firstNonIcodeOnlyInstructions(b: BasicBlock): Iterator[Instruction] = { + def isICodeOnlyInstruction(i: Instruction) = i match { + case LOAD_EXCEPTION(_) | SCOPE_ENTER(_) | SCOPE_EXIT(_) => true + case _ => false + } + b.iterator dropWhile isICodeOnlyInstruction } - private def isJumpOnly(b: BasicBlock): Option[BasicBlock] = { - b.toList match { - case JUMP(whereto) :: rest => - assert(rest.isEmpty, "A block contains instructions after JUMP (looks like enterIgnoreMode() was itself ignored.)") + /** + * Returns the target of a block that is "jump only" which is defined + * as being a block that consists only of 0 or more instructions that + * won't make it to the JVM followed by a JUMP. + * + * @param b The basic block to examine + * @return Some(target) if b is a "jump only" block or None if it's not + */ + private def getJumpOnlyTarget(b: BasicBlock): Option[BasicBlock] = { + val nonICode = firstNonIcodeOnlyInstructions(b) + // by definition a block has to have a jump, conditional jump, return, or throw + assert(nonICode.nonEmpty, "empty block") + nonICode.next match { + case JUMP(whereto) => + assert(!nonICode.hasNext, "A block contains instructions after JUMP (looks like enterIgnoreMode() was itself ignored.)") Some(whereto) case _ => None } } - private def directSuccStar(b: BasicBlock): List[BasicBlock] = { directSuccStar(List(b)) } - - /** Transitive closure of successors potentially reachable due to normal (non-exceptional) control flow. - Those BBs in the argument are also included in the result */ - private def directSuccStar(starters: Traversable[BasicBlock]): List[BasicBlock] = { - val result = new mutable.ListBuffer[BasicBlock] - var toVisit: List[BasicBlock] = starters.toList.distinct - while(toVisit.nonEmpty) { - val h = toVisit.head - toVisit = toVisit.tail - result += h - for(p <- h.directSuccessors; if !result.contains(p) && !toVisit.contains(p)) { toVisit = p :: toVisit } - } - result.toList - } - - /** Returns: - * for single-block self-loops, the pair (start, Nil) - * for other cycles, the pair (backedge-target, basic-blocks-in-the-cycle-except-backedge-target) - * otherwise a pair consisting of: - * (a) the endpoint of a (single or multi-hop) chain of JUMPs - * (such endpoint does not start with a JUMP and therefore is not part of the chain); and - * (b) the chain (ie blocks to be removed when collapsing the chain of jumps). - * Precondition: the BasicBlock given as argument starts with an unconditional JUMP. - */ - private def finalDestination(start: BasicBlock): (BasicBlock, List[BasicBlock]) = { - assert(startsWithJump(start), "not the start of a (single or multi-hop) chain of JUMPs.") - var hops: List[BasicBlock] = Nil - var prev = start - var done = false - do { - done = isJumpOnly(prev) match { - case Some(dest) => - if (dest == start) { return (start, hops) } // leave infinite-loops in place - hops ::= prev - if (hops.contains(dest)) { - // leave infinite-loops in place - return (dest, hops filterNot (dest eq _)) - } - prev = dest - false - case None => true - } - } while(!done) - - (prev, hops) - } - /** * Collapse a chain of "jump-only" blocks such as: * @@ -3204,7 +3136,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { * In more detail: * Starting at each of the entry points (m.startBlock, the start block of each exception handler) * rephrase those control-flow instructions targeting a jump-only block (which jumps to a final destination D) to target D. - * The blocks thus skipped are also removed from IMethod.blocks. + * The blocks thus skipped become eligible to removed by the reachability analyzer * * Rationale for this normalization: * test/files/run/private-inline.scala after -optimize is chock full of @@ -3215,106 +3147,163 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { * and thus ranges with identical (start, end) (i.e, identical after GenJVM omitted the JUMPs in question) * could be weeded out to avoid "java.lang.ClassFormatError: Illegal exception table range" * Now that visitTryCatchBlock() must be called before Labels are resolved, - * this method gets rid of the BasicBlocks described above (to recap, consisting of just a JUMP). + * renders the BasicBlocks described above (to recap, consisting of just a JUMP) unreachable. */ - private def collapseJumpOnlyBlocks(m: IMethod): Boolean = { + private def collapseJumpOnlyBlocks(m: IMethod) { assert(m.hasCode, "code-less method") - /* "start" is relative in a cycle, but we call this helper with the "first" entry-point we found. */ - def realTarget(jumpStart: BasicBlock): Map[BasicBlock, BasicBlock] = { - assert(startsWithJump(jumpStart), "not part of a jump-chain") - val Pair(dest, redundants) = finalDestination(jumpStart) - (for(skipOver <- redundants) yield Pair(skipOver, dest)).toMap - } - - def rephraseGotos(detour: Map[BasicBlock, BasicBlock]) { - for(Pair(oldTarget, newTarget) <- detour.iterator) { - if(m.startBlock == oldTarget) { - m.code.startBlock = newTarget - } - for(eh <- m.exh; if eh.startBlock == oldTarget) { - eh.setStartBlock(newTarget) - } - for(b <- m.blocks; if !detour.isDefinedAt(b)) { - val idxLast = (b.size - 1) - b.lastInstruction match { - case JUMP(whereto) => - if (whereto == oldTarget) { - b.replaceInstruction(idxLast, JUMP(newTarget)) - } - case CJUMP(succ, fail, cond, kind) => - if ((succ == oldTarget) || (fail == oldTarget)) { - b.replaceInstruction(idxLast, CJUMP(detour.getOrElse(succ, succ), - detour.getOrElse(fail, fail), - cond, kind)) - } - case CZJUMP(succ, fail, cond, kind) => - if ((succ == oldTarget) || (fail == oldTarget)) { - b.replaceInstruction(idxLast, CZJUMP(detour.getOrElse(succ, succ), - detour.getOrElse(fail, fail), - cond, kind)) - } - case SWITCH(tags, labels) => - if(labels exists (detour.isDefinedAt(_))) { - val newLabels = (labels map { lab => detour.getOrElse(lab, lab) }) - b.replaceInstruction(idxLast, SWITCH(tags, newLabels)) - } - case _ => () - } - } + def rephraseGotos(detour: mutable.Map[BasicBlock, BasicBlock]) { + def lookup(b: BasicBlock) = detour.getOrElse(b, b) + + m.code.startBlock = lookup(m.code.startBlock) + + for(eh <- m.exh) + eh.setStartBlock(lookup(eh.startBlock)) + + for (b <- m.blocks) { + def replaceLastInstruction(i: Instruction) = { + if (b.lastInstruction != i) { + val idxLast = b.size - 1 + debuglog(s"In block $b, replacing last instruction ${b.lastInstruction} with ${i}") + b.replaceInstruction(idxLast, i) } } - - /* remove from all containers that may contain a reference to */ - def elide(redu: BasicBlock) { - assert(m.startBlock != redu, "startBlock should have been re-wired by now") - m.code.removeBlock(redu) + + b.lastInstruction match { + case JUMP(whereto) => + replaceLastInstruction(JUMP(lookup(whereto))) + case CJUMP(succ, fail, cond, kind) => + replaceLastInstruction(CJUMP(lookup(succ), lookup(fail), cond, kind)) + case CZJUMP(succ, fail, cond, kind) => + replaceLastInstruction(CZJUMP(lookup(succ), lookup(fail), cond, kind)) + case SWITCH(tags, labels) => + val newLabels = (labels map lookup) + replaceLastInstruction(SWITCH(tags, newLabels)) + case _ => () } + } + } - var wasReduced = false - val entryPoints: List[BasicBlock] = m.startBlock :: (m.exh map (_.startBlock)) - - val elided = mutable.Set.empty[BasicBlock] // debug - val newTargets = mutable.Set.empty[BasicBlock] // debug - - for (ep <- entryPoints) { - var reachable = directSuccStar(ep) // this list may contain blocks belonging to jump-chains that we'll skip over - while(reachable.nonEmpty) { - val h = reachable.head - reachable = reachable.tail - if(startsWithJump(h)) { - val detour = realTarget(h) - if(detour.nonEmpty) { - wasReduced = true - reachable = (reachable filterNot (detour.keySet.contains(_))) - rephraseGotos(detour) - detour.keySet foreach elide - elided ++= detour.keySet - newTargets ++= detour.values - } + /** + * Computes a mapping from jump only block to its + * final destination which is either a non-jump-only + * block or, if it's in a jump-only block cycle, is + * itself + */ + def computeDetour: mutable.Map[BasicBlock, BasicBlock] = { + // fetch the jump only blocks and their immediate destinations + val pairs = for { + block <- m.blocks.toIterator + target <- getJumpOnlyTarget(block) + } yield(block, target) + + // mapping from a jump-only block to our current knowledge of its + // final destination. Initially it's just jump block to immediate jump + // target + val detour = mutable.Map[BasicBlock, BasicBlock](pairs.toSeq:_*) + + // for each jump-only block find its final destination + // taking advantage of the destinations we found for previous + // blocks + for (key <- detour.keySet) { + // we use the Robert Floyd's classic Tortoise and Hare algorithm + @tailrec + def findDestination(tortoise: BasicBlock, hare: BasicBlock): BasicBlock = { + if (tortoise == hare) + // cycle detected, map key to key + key + else if (detour contains hare) { + // advance hare once + val hare1 = detour(hare) + // make sure we can advance hare a second time + if (detour contains hare1) + // advance tortoise once and hare a second time + findDestination(detour(tortoise), detour(hare1)) + else + // hare1 is not in the map so it's not a jump-only block, it's the destination + hare1 + } else + // hare is not in the map so it's not a jump-only block, it's the destination + hare } + // update the mapping for key based on its final destination + detour(key) = findDestination(key, detour(key)) + } + detour + } + + val detour = computeDetour + rephraseGotos(detour) + + if (settings.debug.value) { + val (remappings, cycles) = detour partition {case (source, target) => source != target} + for ((source, target) <- remappings) { + debuglog(s"Will elide jump only block $source because it can be jumped around to get to $target.") + if (m.startBlock == source) debugwarn("startBlock should have been re-wired by now") + } + val sources = remappings.keySet + val targets = remappings.values.toSet + val intersection = sources intersect targets + + if (intersection.nonEmpty) debugwarn(s"contradiction: we seem to have some source and target overlap in blocks ${intersection.mkString}. Map was ${detour.mkString}") + + for ((source, _) <- cycles) { + debuglog(s"Block $source is in a do-nothing infinite loop. Did the user write 'while(true){}'?") } } - assert(newTargets.intersect(elided).isEmpty, "contradiction: we just elided the final destionation of a jump-chain") + } + + /** + * Removes all blocks that are unreachable in a method using a standard reachability analysis. + */ + def elimUnreachableBlocks(m: IMethod) { + assert(m.hasCode, "code-less method") + + // assume nothing is reachable until we prove it can be reached + val reachable = mutable.Set[BasicBlock]() + + // the set of blocks that we know are reachable but have + // yet to be marked reachable, initially only the start block + val worklist = mutable.Set(m.startBlock) + + while (worklist.nonEmpty) { + val block = worklist.head + worklist remove block + // we know that one is reachable + reachable add block + // so are its successors, so go back around and add the ones we still + // think are unreachable + worklist ++= (block.successors filterNot reachable) + } + + // exception handlers need to be told not to cover unreachable blocks + // and exception handlers that no longer cover any blocks need to be + // removed entirely + val unusedExceptionHandlers = mutable.Set[ExceptionHandler]() + for (exh <- m.exh) { + exh.covered = exh.covered filter reachable + if (exh.covered.isEmpty) { + unusedExceptionHandlers += exh + } + } + + // remove the unusued exception handler references + if (settings.debug.value) + for (exh <- unusedExceptionHandlers) debuglog(s"eliding exception handler $exh because it does not cover any reachable blocks") + m.exh = m.exh filterNot unusedExceptionHandlers - wasReduced + // everything not in the reachable set is unreachable, unused, and unloved. buh bye + for (b <- m.blocks filterNot reachable) { + debuglog(s"eliding block $b because it is unreachable") + m.code removeBlock b + } } def normalize(m: IMethod) { if(!m.hasCode) { return } collapseJumpOnlyBlocks(m) - var wasReduced = false - do { - wasReduced = false - // Prune from an exception handler those covered blocks which are jump-only. - wasReduced |= coverWhatCountsOnly(m); icodes.checkValid(m) // TODO should be unnecessary now that collapseJumpOnlyBlocks(m) is in place - // Prune exception handlers covering nothing. - wasReduced |= elimNonCoveringExh(m); icodes.checkValid(m) - - // TODO see note in genExceptionHandlers about an ExceptionHandler.covered containing dead blocks (newNormal should remove them, but, where do those blocks come from?) - } while (wasReduced) - - // TODO this would be a good time to remove synthetic local vars seeing no use, don't forget to call computeLocalVarsIndex() afterwards. + elimUnreachableBlocks(m) + icodes checkValid m } } diff --git a/test/files/jvm/duration-tck.scala b/test/files/jvm/duration-tck.scala index d0f13816a6..b2573448c7 100644 --- a/test/files/jvm/duration-tck.scala +++ b/test/files/jvm/duration-tck.scala @@ -176,8 +176,9 @@ object Test extends App { Thread.sleep(1.second.toMillis) - { val l = dead.timeLeft; assert(l <= 1.second, s"$l > 1.second") } - { val l = dead2.timeLeft; assert(l <= 1.second, s"$l > 1.second") } + // unfortunately it can happen that the sleep() returns early without throwing + { val l = dead.timeLeft; assert(l <= 1100.millis, s"$l > 1100.millis") } + { val l = dead2.timeLeft; assert(l <= 1100.millis, s"$l > 1100.millis") } // test integer mul/div diff --git a/test/files/jvm/t7006/Foo_1.flags b/test/files/jvm/t7006/Foo_1.flags new file mode 100644 index 0000000000..72fe7b1aa0 --- /dev/null +++ b/test/files/jvm/t7006/Foo_1.flags @@ -0,0 +1 @@ + -Ydead-code -Ydebug -Xfatal-warnings diff --git a/test/files/jvm/t7006/Foo_1.scala b/test/files/jvm/t7006/Foo_1.scala new file mode 100644 index 0000000000..995619ce6b --- /dev/null +++ b/test/files/jvm/t7006/Foo_1.scala @@ -0,0 +1,10 @@ +class Foo_1 { + def foo { + try { + val x = 3 // this will be optimized away, leaving a useless jump only block + } finally { + print("hello") + } + while(true){} // ensure infinite loop doesn't break the algoirthm + } +} diff --git a/test/files/jvm/t7006/Test.scala b/test/files/jvm/t7006/Test.scala new file mode 100644 index 0000000000..065a23510e --- /dev/null +++ b/test/files/jvm/t7006/Test.scala @@ -0,0 +1,19 @@ +import scala.tools.partest.BytecodeTest +import scala.tools.asm +import asm.tree.InsnList +import scala.collection.JavaConverters._ + +object Test extends BytecodeTest { + def show: Unit = { + val classNode = loadClassNode("Foo_1") + val methodNode = getMethod(classNode, "foo") + assert(count(methodNode.instructions, asm.Opcodes.NOP) == 0) + assert(count(methodNode.instructions, asm.Opcodes.GOTO) == 1) + } + + def count(insnList: InsnList, opcode: Int): Int = { + def isNop(node: asm.tree.AbstractInsnNode): Boolean = + (node.getOpcode == opcode) + insnList.iterator.asScala.count(isNop) + } +} diff --git a/test/files/jvm/t7181/Foo_1.scala b/test/files/jvm/t7181/Foo_1.scala new file mode 100644 index 0000000000..f9dfdd4442 --- /dev/null +++ b/test/files/jvm/t7181/Foo_1.scala @@ -0,0 +1,26 @@ +class Exception1 extends RuntimeException +class Exception2 extends RuntimeException + +class Foo_1 { + def foo(baz: Baz) { + try { + baz.bar + } catch { + case _: Exception1 => println("exception 1") + case _: Exception2 => println("exception 2") + } finally { + // this should be the only copy of the magic constant 3 + // making it easy to detect copies of this finally block + println(s"finally ${3}") + } + println(s"normal flow") + } +} + +trait Baz { + // does it throw? who knows? This way + // I can ensure that no optimization that honors + // separate compilation could ever + // change the exception handling structure + def bar: Unit +} diff --git a/test/files/jvm/t7181/Test.scala b/test/files/jvm/t7181/Test.scala new file mode 100644 index 0000000000..35dba436c1 --- /dev/null +++ b/test/files/jvm/t7181/Test.scala @@ -0,0 +1,24 @@ +import scala.tools.partest.BytecodeTest +import scala.tools.asm +import asm.tree.InsnList +import scala.collection.JavaConverters._ + +object Test extends BytecodeTest { + def show: Unit = { + val classNode = loadClassNode("Foo_1") + val methodNode = getMethod(classNode, "foo") + // there should be 2 copies of the finally block, each with the magic constant 3 + // one for the "normal" exit + // one for the uncaught exception exit + // prior to this PR there would have been 4 since each exception handler would also get a copy + val expected = 2 + val got = countMagicThrees(methodNode.instructions) + assert(got == expected, s"expected $expected but got $got magic threes") + } + + def countMagicThrees(insnList: InsnList): Int = { + def isMagicThree(node: asm.tree.AbstractInsnNode): Boolean = + (node.getOpcode == asm.Opcodes.ICONST_3) + insnList.iterator.asScala.count(isMagicThree) + } +} diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check index f2f0b60687..0a234e2659 100644 --- a/test/files/run/inline-ex-handlers.check +++ b/test/files/run/inline-ex-handlers.check @@ -107,27 +107,27 @@ --- > catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10, 11, 12) starting at: 3 619c642 -< blocks: [1,2,3,4,5,6,7,9,10] +< blocks: [1,3,4,5,6,8,9] --- -> blocks: [1,2,3,4,5,6,7,9,10,11,12] +> blocks: [1,3,4,5,6,8,9,10,11] 643c666,667 < 78 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) -> ? JUMP 11 +> ? JUMP 10 644a669,673 -> 11: +> 10: > 81 LOAD_LOCAL(value e) > ? STORE_LOCAL(variable exc1) -> ? JUMP 12 +> ? JUMP 11 > -672c701,702 +669c698,699 < 81 THROW(Exception) --- > ? STORE_LOCAL(variable exc1) -> ? JUMP 12 -688a719,731 -> 12: +> ? JUMP 11 +685a716,728 +> 11: > 83 LOAD_MODULE object Predef > 83 CONSTANT("finally") > 83 CALL_METHOD scala.Predef.println (dynamic) @@ -140,88 +140,88 @@ > 84 LOAD_LOCAL(variable exc1) > 84 THROW(Throwable) > -694c737 -< catch (<none>) in ArrayBuffer(4, 6, 7, 9) starting at: 3 +691c734 +< catch (<none>) in ArrayBuffer(4, 5, 6, 8) starting at: 3 --- -> catch (<none>) in ArrayBuffer(4, 6, 7, 9, 11) starting at: 3 -718c761 +> catch (<none>) in ArrayBuffer(4, 5, 6, 8, 10) starting at: 3 +715c758 < locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value message, value x, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value x, value ex6, value x4, value x5, value x -720c763 -< blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25] +717c760 +< blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24] --- -> blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25,26,27,28] -744c787,794 +> blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24,25,26,27] +741c784,791 < 172 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 26 +> ? JUMP 25 > -> 26: +> 25: > 170 LOAD_LOCAL(value ex6) > 170 STORE_LOCAL(value x4) > 170 SCOPE_ENTER value x4 -> 170 JUMP 15 -787,790d836 +> 170 JUMP 14 +781,784d830 < 175 LOAD_LOCAL(value x5) < 175 CALL_METHOD MyException.message (dynamic) < 175 STORE_LOCAL(value message) < 175 SCOPE_ENTER value message -792c838,839 +786c832,833 < 176 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 176 CALL_METHOD MyException.message (dynamic) -796c843,844 +790c837,838 < 177 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 177 CALL_METHOD MyException.message (dynamic) -798c846,847 +792c840,841 < 177 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 27 -802c851,852 +> ? JUMP 26 +796c845,846 < 170 THROW(Throwable) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 27 -811a862,867 -> 27: +> ? JUMP 26 +805a856,861 +> 26: > 169 LOAD_LOCAL(value ex6) > 169 STORE_LOCAL(value x4) > 169 SCOPE_ENTER value x4 > 169 JUMP 5 > -822,825d877 +816,819d871 < 180 LOAD_LOCAL(value x5) < 180 CALL_METHOD MyException.message (dynamic) < 180 STORE_LOCAL(value message) < 180 SCOPE_ENTER value message -827c879,880 +821c873,874 < 181 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 181 CALL_METHOD MyException.message (dynamic) -831c884,885 +825c878,879 < 182 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 182 CALL_METHOD MyException.message (dynamic) -833c887,888 +827c881,882 < 182 THROW(MyException) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 28 -837c892,893 +> ? JUMP 27 +831c886,887 < 169 THROW(Throwable) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 28 -853a910,922 -> 28: +> ? JUMP 27 +847a904,916 +> 27: > 184 LOAD_MODULE object Predef > 184 CONSTANT("finally") > 184 CALL_METHOD scala.Predef.println (dynamic) @@ -234,23 +234,23 @@ > 185 LOAD_LOCAL(variable exc2) > 185 THROW(Throwable) > -859c928 -< catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24) starting at: 4 +853c922 +< catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23) starting at: 4 --- -> catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24, 26) starting at: 4 -862c931 -< catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24) starting at: 3 +> catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23, 25) starting at: 4 +856c925 +< catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23) starting at: 3 --- -> catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24, 26, 27) starting at: 3 -886c955 +> catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23, 25, 26) starting at: 3 +880c949 < locals: value args, variable result, value e, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value e, value ex6, value x4, value x5, value x -888c957 +882c951 < blocks: [1,2,3,6,7,8,11,13,14,16] --- > blocks: [1,2,3,6,7,8,11,13,14,16,17] -912c981,988 +906c975,982 < 124 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -261,29 +261,29 @@ > 122 STORE_LOCAL(value x4) > 122 SCOPE_ENTER value x4 > 122 JUMP 7 -937,940d1012 +931,934d1006 < 127 LOAD_LOCAL(value x5) < 127 CALL_METHOD MyException.message (dynamic) < 127 STORE_LOCAL(value message) < 127 SCOPE_ENTER value message -942c1014,1015 +936c1008,1009 < 127 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 127 CALL_METHOD MyException.message (dynamic) -971c1044 +965c1038 < catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16) starting at: 3 --- > catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16, 17) starting at: 3 -995c1068 +989c1062 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x, value e --- > locals: value args, variable result, value ex6, value x4, value x5, value x, value e -997c1070 +991c1064 < blocks: [1,2,3,4,5,8,12,13,14,16] --- > blocks: [1,2,3,5,8,12,13,14,16,17] -1021c1094,1103 +1015c1088,1097 < 148 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -296,25 +296,25 @@ > 154 LOAD_LOCAL(value x4) > 154 IS_INSTANCE REF(class MyException) > 154 CZJUMP (BOOL)NE ? 5 : 8 -1042,1044d1123 +1036,1038d1117 < 145 JUMP 4 < < 4: -1054,1057d1132 +1048,1051d1126 < 154 LOAD_LOCAL(value x5) < 154 CALL_METHOD MyException.message (dynamic) < 154 STORE_LOCAL(value message) < 154 SCOPE_ENTER value message -1059c1134,1135 +1053c1128,1129 < 154 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 154 CALL_METHOD MyException.message (dynamic) -1276c1352 +1270c1346 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1300c1376,1383 +1294c1370,1377 < 38 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) @@ -325,20 +325,20 @@ > 42 CONSTANT("IllegalArgumentException") > 42 CALL_METHOD scala.Predef.println (dynamic) > 42 JUMP 2 -1347c1430 +1341c1424 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, value x4, value x5, value x -1349c1432 +1343c1426 < blocks: [1,2,3,4,5,8,10,11,13,14,16] --- > blocks: [1,2,3,5,8,10,11,13,14,16,17] -1373c1456,1457 +1367c1450,1451 < 203 THROW(MyException) --- > ? STORE_LOCAL(value ex6) > ? JUMP 17 -1393c1477,1486 +1387c1471,1480 < 209 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -351,41 +351,41 @@ > 212 LOAD_LOCAL(value x4) > 212 IS_INSTANCE REF(class MyException) > 212 CZJUMP (BOOL)NE ? 5 : 8 -1406,1408d1498 +1400,1402d1492 < 200 JUMP 4 < < 4: -1418,1421d1507 +1412,1415d1501 < 212 LOAD_LOCAL(value x5) < 212 CALL_METHOD MyException.message (dynamic) < 212 STORE_LOCAL(value message) < 212 SCOPE_ENTER value message -1423c1509,1510 +1417c1503,1504 < 213 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 213 CALL_METHOD MyException.message (dynamic) -1467c1554 +1461c1548 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1491c1578,1579 +1485c1572,1573 < 58 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 8 -1492a1581,1586 +1486a1575,1580 > 8: > 62 LOAD_MODULE object Predef > 62 CONSTANT("RuntimeException") > 62 CALL_METHOD scala.Predef.println (dynamic) > 62 JUMP 2 > -1540c1634 +1534c1628 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1560c1654,1659 +1554c1648,1653 < 229 THROW(MyException) --- > ? JUMP 5 @@ -394,19 +394,19 @@ > ? LOAD_LOCAL(variable monitor1) > 228 MONITOR_EXIT > 228 THROW(Throwable) -1566c1665 +1560c1659 < ? THROW(Throwable) --- > 228 THROW(Throwable) -1594c1693 +1588c1687 < locals: value args, variable result, variable monitor2, variable monitorResult1 --- > locals: value exception$1, value args, variable result, variable monitor2, variable monitorResult1 -1596c1695 +1590c1689 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1619c1718,1726 +1613c1712,1720 < 245 THROW(MyException) --- > ? STORE_LOCAL(value exception$1) @@ -418,7 +418,7 @@ > ? LOAD_LOCAL(variable monitor2) > 244 MONITOR_EXIT > 244 THROW(Throwable) -1625c1732 +1619c1726 < ? THROW(Throwable) --- > 244 THROW(Throwable) diff --git a/test/files/run/t6102.flags b/test/files/run/t6102.flags index e35535c8ea..72fe7b1aa0 100644 --- a/test/files/run/t6102.flags +++ b/test/files/run/t6102.flags @@ -1 +1 @@ - -Ydead-code + -Ydead-code -Ydebug -Xfatal-warnings diff --git a/test/files/run/t7181.check b/test/files/run/t7181.check new file mode 100644 index 0000000000..e4b8e30dfe --- /dev/null +++ b/test/files/run/t7181.check @@ -0,0 +1,23 @@ +normal exit MainNormalExit +finally MainNormalExit +normal flow MainNormalExit + +return MainReturn +finally MainReturn + +uncaught exception MainUncaughtException +finally MainUncaughtException + +caught exception ExceptionNormalExit +normal exit ExceptionNormalExit +finally ExceptionNormalExit +normal flow ExceptionNormalExit + +caught exception ExceptionReturn +return ExceptionReturn +finally ExceptionReturn + +caught exception ExceptionUncaughtException +uncaught exception ExceptionUncaughtException +finally ExceptionUncaughtException + diff --git a/test/files/run/t7181.scala b/test/files/run/t7181.scala new file mode 100644 index 0000000000..a055e43481 --- /dev/null +++ b/test/files/run/t7181.scala @@ -0,0 +1,78 @@ +sealed abstract class Action +// exit the try body normally +case object MainNormalExit extends Action +// exit the try body with a 'return' +case object MainReturn extends Action +// exit the try body with an uncaught exception +case object MainUncaughtException extends Action +// exit the try body with a caught exception and exit the exception handler normally +case object ExceptionNormalExit extends Action +// exit the try body with a caught exception and exit the exception handler with a 'return' +case object ExceptionReturn extends Action +// exit the try body with a caught exception and exit the exception handler with an uncaught exception +case object ExceptionUncaughtException extends Action + +case class UncaughtException(action: Action) extends RuntimeException +case class CaughtException(action: Action) extends RuntimeException + +object Test extends App { + def test(action: Action, expectException: Boolean = false) { + var gotException = false + val result = try + driver(action) + catch { + case UncaughtException(a) => + gotException = true + a + } + if (gotException) assert(expectException, "Got unexpected exception") + else assert(!expectException, "Did not get expected exception") + + assert(result == action, s"Expected $action but got $result") + println() + } + + def driver(action: Action): Action = { + val result = try { + action match { + case MainNormalExit => + println(s"normal exit $action") + action + case MainReturn => + println(s"return $action") + return action + case MainUncaughtException => + println(s"uncaught exception $action") + throw UncaughtException(action) + case _ => + println(s"caught exception $action") + throw CaughtException(action) + } + } catch { + case CaughtException(action) => action match { + case ExceptionNormalExit => + println(s"normal exit $action") + action + case ExceptionReturn => + println(s"return $action") + return action + case ExceptionUncaughtException => + println(s"uncaught exception $action") + throw UncaughtException(action) + case _ => + sys.error(s"unexpected $action in exception handler") + } + } finally { + println(s"finally $action") + } + println(s"normal flow $action") + result + } + + test(MainNormalExit) + test(MainReturn) + test(MainUncaughtException, true) + test(ExceptionNormalExit) + test(ExceptionReturn) + test(ExceptionUncaughtException, true) +} |