From 61cc8ff61c81a1276a921ad5288ee3bebea1c96e Mon Sep 17 00:00:00 2001 From: Miguel Garcia Date: Mon, 6 Aug 2012 11:01:17 +0200 Subject: SI-6188 ICodeReader notes exception handlers, Inliner takes them into account --- src/compiler/scala/tools/nsc/backend/icode/Members.scala | 1 + src/compiler/scala/tools/nsc/backend/opt/Inliners.scala | 13 ++++++++++++- .../scala/tools/nsc/symtab/classfile/ICodeReader.scala | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala index 00f4a9d262..44c4a3a6db 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala @@ -170,6 +170,7 @@ trait Members { var sourceFile: SourceFile = NoSourceFile var returnType: TypeKind = _ var recursive: Boolean = false + var bytecodeHasEHs = false // set by ICodeReader only, used by Inliner to prevent inlining (SI-6188) /** local variables and method parameters */ var locals: List[Local] = Nil diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala index cce18d436f..dd7676a371 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala @@ -405,6 +405,12 @@ abstract class Inliners extends SubComponent { val inc = new IMethodInfo(callee) val pair = new CallerCalleeInfo(caller, inc, fresh, inlinedMethodCount) + if(inc.hasHandlers && (stackLength == -1)) { + // no inlining is done, yet don't warn about it, stackLength == -1 indicates we're trying to inlineWithoutTFA. + // Shortly, a TFA will be computed and an error message reported if indeed inlining not possible. + return false + } + (pair isStampedForInlining stackLength) match { case inlInfo if inlInfo.isSafe => @@ -605,7 +611,7 @@ abstract class Inliners extends SubComponent { def isSmall = (length <= SMALL_METHOD_SIZE) && blocks(0).length < 10 def isLarge = length > MAX_INLINE_SIZE def isRecursive = m.recursive - def hasHandlers = handlers.nonEmpty + def hasHandlers = handlers.nonEmpty || m.bytecodeHasEHs def isSynchronized = sym.hasFlag(Flags.SYNCHRONIZED) def hasNonFinalizerHandler = handlers exists { @@ -941,6 +947,7 @@ abstract class Inliners extends SubComponent { if(inc.isRecursive) { rs ::= "is recursive" } if(isInlineForbidden) { rs ::= "is annotated @noinline" } if(inc.isSynchronized) { rs ::= "is synchronized method" } + if(inc.m.bytecodeHasEHs) { rs ::= "bytecode contains exception handlers / finally clause" } // SI-6188 if(rs.isEmpty) null else rs.mkString("", ", and ", "") } @@ -974,6 +981,10 @@ abstract class Inliners extends SubComponent { return DontInlineHere("too low score (heuristics)") } + if(inc.hasHandlers && (stackLength != 0)) { + // TODO pending return DontInlineHere("callee contains exception handlers / finally clause, and is invoked with non-empty operand stack") // SI-6157 + } + if(isKnownToInlineSafely) { return InlineableAtThisCaller } if(stackLength > inc.minimumStack && inc.hasNonFinalizerHandler) { diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala index bb9f9bde98..3a3be4dc78 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala @@ -594,6 +594,7 @@ abstract class ICodeReader extends ClassfileParser { while (pc < codeLength) parseInstruction val exceptionEntries = in.nextChar.toInt + code.containsEHs = (exceptionEntries != 0) var i = 0 while (i < exceptionEntries) { // skip start end PC @@ -647,6 +648,7 @@ abstract class ICodeReader extends ClassfileParser { var containsDUPX = false var containsNEW = false + var containsEHs = false def emit(i: Instruction) { instrs += ((pc, i)) @@ -664,6 +666,7 @@ abstract class ICodeReader extends ClassfileParser { val code = new Code(method) method.setCode(code) + method.bytecodeHasEHs = containsEHs var bb = code.startBlock def makeBasicBlocks: mutable.Map[Int, BasicBlock] = -- cgit v1.2.3 From 17037367049312eb3d26766a5759295ac9f8aed6 Mon Sep 17 00:00:00 2001 From: Miguel Garcia Date: Mon, 6 Aug 2012 18:31:35 +0200 Subject: SI-6157 don't inline callee with exception-handler(s) if potentially unsafe --- .../backend/icode/analysis/TypeFlowAnalysis.scala | 8 ++++++- .../scala/tools/nsc/backend/opt/Inliners.scala | 4 ++-- test/files/pos/t6157.flags | 1 + test/files/pos/t6157.scala | 25 ++++++++++++++++++++++ test/files/run/private-inline.scala | 2 +- 5 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 test/files/pos/t6157.flags create mode 100644 test/files/pos/t6157.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/icode/analysis/TypeFlowAnalysis.scala b/src/compiler/scala/tools/nsc/backend/icode/analysis/TypeFlowAnalysis.scala index 962c47f443..e791936470 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/analysis/TypeFlowAnalysis.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/analysis/TypeFlowAnalysis.scala @@ -736,7 +736,13 @@ abstract class TypeFlowAnalysis { val succs = point.successors filter relevantBBs succs foreach { p => assert((p.predecessors filter isOnPerimeter).isEmpty) - val updated = lattice.lub(List(output, in(p)), p.exceptionHandlerStart) + val existing = in(p) + // TODO move the following assertion to typeFlowLattice.lub2 for wider applicability (ie MethodTFA in addition to MTFAGrowable). + assert(existing == lattice.bottom || + p.exceptionHandlerStart || + (output.stack.length == existing.stack.length), + "Trying to merge non-bottom type-stacks with different stack heights. For a possible cause see SI-6157.") + val updated = lattice.lub(List(output, existing), p.exceptionHandlerStart) if(updated != in(p)) { in(p) = updated enqueue(p) diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala index dd7676a371..22f0a9ca7c 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala @@ -981,8 +981,8 @@ abstract class Inliners extends SubComponent { return DontInlineHere("too low score (heuristics)") } - if(inc.hasHandlers && (stackLength != 0)) { - // TODO pending return DontInlineHere("callee contains exception handlers / finally clause, and is invoked with non-empty operand stack") // SI-6157 + if(inc.hasHandlers && (stackLength > inc.minimumStack)) { + return DontInlineHere("callee contains exception handlers / finally clause, and is invoked with non-empty operand stack") // SI-6157 } if(isKnownToInlineSafely) { return InlineableAtThisCaller } diff --git a/test/files/pos/t6157.flags b/test/files/pos/t6157.flags new file mode 100644 index 0000000000..0ebca3e7af --- /dev/null +++ b/test/files/pos/t6157.flags @@ -0,0 +1 @@ + -optimize diff --git a/test/files/pos/t6157.scala b/test/files/pos/t6157.scala new file mode 100644 index 0000000000..7463989b14 --- /dev/null +++ b/test/files/pos/t6157.scala @@ -0,0 +1,25 @@ +// SI-6157 - Compiler crash on inlined function and -optimize option + +object Test { + def main(args: Array[String]) { + Console.println( + ErrorHandler.defaultIfIOException("String")("String") + ) + } +} + +import java.io.IOException + +object ErrorHandler { + + @inline + def defaultIfIOException[T](default: => T)(closure: => T): T = { + try { + closure + } catch { + case e: IOException => + default + } + } +} + diff --git a/test/files/run/private-inline.scala b/test/files/run/private-inline.scala index a45300b026..a62007779c 100644 --- a/test/files/run/private-inline.scala +++ b/test/files/run/private-inline.scala @@ -30,7 +30,7 @@ final class A { } object Test { - def methodClasses = List("f1a", "f1b", "f2a", "f2b") map ("A$$anonfun$" + _ + "$1") + def methodClasses = List("f1a", "f2a") map ("A$$anonfun$" + _ + "$1") def main(args: Array[String]): Unit = { val a = new A -- cgit v1.2.3