[backport] Prevent infinite recursion in ProdConsAnalyzer

When an instruction is its own producer or consumer, the `initialProducer` / `ultimateConsumer` methods would loop. While loops or @tailrec annotated methods can generate such bytecode.
adriaanm · Jul 23, 2015 · 404e862 · 404e862
1 parent 1b0703e
commit 404e862
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 9 deletions.
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala
@@ -10,6 +10,7 @@ import java.io.{StringWriter, PrintWriter}
 import scala.tools.asm.util.{CheckClassAdapter, TraceClassVisitor, TraceMethodVisitor, Textifier}
 import scala.tools.asm.{ClassWriter, Attribute, ClassReader}
 import scala.collection.convert.decorateAsScala._
+import scala.tools.nsc.backend.jvm.analysis.InitialProducer
 import scala.tools.nsc.backend.jvm.opt.InlineInfoAttributePrototype
 
 object AsmUtils {
@@ -81,13 +82,16 @@ object AsmUtils {
   /**
    * Returns a human-readable representation of the given instruction.
    */
-  def textify(insn: AbstractInsnNode): String = {
-    val trace = new TraceMethodVisitor(new Textifier)
-    insn.accept(trace)
-    val sw = new StringWriter
-    val pw = new PrintWriter(sw)
-    trace.p.print(pw)
-    sw.toString.trim
+  def textify(insn: AbstractInsnNode): String = insn match {
+    case _: InitialProducer =>
+      insn.toString
+    case _ =>
+      val trace = new TraceMethodVisitor(new Textifier)
+      insn.accept(trace)
+      val sw = new StringWriter
+      val pw = new PrintWriter(sw)
+      trace.p.print(pw)
+      sw.toString.trim
   }
 
   /**

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/ProdConsAnalyzer.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/ProdConsAnalyzer.scala
@@ -103,7 +103,11 @@ class ProdConsAnalyzer(methodNode: MethodNode, classInternalName: InternalName)
   def initialProducersForValueAt(insn: AbstractInsnNode, slot: Int): Set[AbstractInsnNode] = {
     def initialProducers(insn: AbstractInsnNode, producedSlot: Int): Set[AbstractInsnNode] = {
       if (isCopyOperation(insn)) {
-        _initialProducersCache.getOrElseUpdate((insn, producedSlot), {
+        val key = (insn, producedSlot)
+        _initialProducersCache.getOrElseUpdate(key, {
+          // prevent infinite recursion if an instruction is its own producer or consumer
+          // see cyclicProdCons in ProdConsAnalyzerTest
+          _initialProducersCache(key) = Set.empty
           val (sourceValue, sourceValueSlot) = copyOperationSourceValue(insn, producedSlot)
           sourceValue.insns.iterator.asScala.flatMap(initialProducers(_, sourceValueSlot)).toSet
         })
@@ -121,7 +125,11 @@ class ProdConsAnalyzer(methodNode: MethodNode, classInternalName: InternalName)
   def ultimateConsumersOfValueAt(insn: AbstractInsnNode, slot: Int): Set[AbstractInsnNode] = {
     def ultimateConsumers(insn: AbstractInsnNode, consumedSlot: Int): Set[AbstractInsnNode] = {
       if (isCopyOperation(insn)) {
-        _ultimateConsumersCache.getOrElseUpdate((insn, consumedSlot), {
+        val key = (insn, consumedSlot)
+        _ultimateConsumersCache.getOrElseUpdate(key, {
+          // prevent infinite recursion if an instruction is its own producer or consumer
+          // see cyclicProdCons in ProdConsAnalyzerTest
+          _ultimateConsumersCache(key) = Set.empty
           for {
             producedSlot     <- copyOperationProducedValueSlots(insn, consumedSlot)
             consumer         <- consumersOfValueAt(insn.getNext, producedSlot)

diff --git a/test/junit/scala/tools/nsc/backend/jvm/analysis/ProdConsAnalyzerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/analysis/ProdConsAnalyzerTest.scala
@@ -246,4 +246,46 @@ class ProdConsAnalyzerTest extends ClearAfterClass {
     testSingleInsn(a.consumersOfOutputsFrom(l2i), "IRETURN")
     testSingleInsn(a.producersForInputsOf(ret), "L2I")
   }
+
+  @Test
+  def cyclicProdCons(): Unit = {
+    import Opcodes._
+    val m = genMethod(descriptor = "(I)I")(
+      Label(1),
+      VarOp(ILOAD, 1),
+      IntOp(BIPUSH, 10),
+      Op(IADD),          // consumer of the above ILOAD
+
+      Op(ICONST_0),
+      Jump(IF_ICMPNE, Label(2)),
+
+      VarOp(ILOAD, 1),
+      VarOp(ISTORE, 1),
+      Jump(GOTO, Label(1)),
+
+      Label(2),
+      IntOp(BIPUSH, 9),
+      Op(IRETURN)
+    )
+    m.maxLocals = 2
+    m.maxStack = 2
+    val a = new ProdConsAnalyzer(m, "C")
+
+    val List(iadd) = findInstr(m, "IADD")
+    val firstLoad = iadd.getPrevious.getPrevious
+    assert(firstLoad.getOpcode == ILOAD)
+    val secondLoad = findInstr(m, "ISTORE").head.getPrevious
+    assert(secondLoad.getOpcode == ILOAD)
+
+    testSingleInsn(a.producersForValueAt(iadd, 2), "ILOAD")
+    testSingleInsn(a.initialProducersForValueAt(iadd, 2), "ParameterProducer(1)")
+    testMultiInsns(a.producersForInputsOf(firstLoad), List("ParameterProducer", "ISTORE"))
+    testMultiInsns(a.producersForInputsOf(secondLoad), List("ParameterProducer", "ISTORE"))
+
+    testSingleInsn(a.ultimateConsumersOfOutputsFrom(firstLoad), "IADD")
+    testSingleInsn(a.ultimateConsumersOfOutputsFrom(secondLoad), "IADD")
+
+    testSingleInsn(a.consumersOfOutputsFrom(firstLoad), "IADD")
+    testSingleInsn(a.consumersOfOutputsFrom(secondLoad), "ISTORE")
+  }
 }