Scala Programming Language
  1. Scala Programming Language
  2. SI-6188

Optimizer incorrectly removes method invocations containing throw expressions

    Details

      Description

      When compiling the following snippet with and without -optimize, I get varying semantics and presumably wrong bytecode.

      Given:

      import scala.util.Success
      
      object Test {
       def main(args: Array[String]) {
         val e = new Exception("this is an exception")
         val res = Success(1).flatMap[Int](x => throw e)
         println(res)
       }
      }
      

      The expected output is:

      Failure(java.lang.Exception: this is an exception)
      

      (This is indeed what I get when running after compiling without -optimize)

      When running after compiling with -optimize, the exception is incorrectly thrown:

      java.lang.Exception: this is an exception
             at Test$.main(opt-bug.scala:5)
             at Test.main(opt-bug.scala)
             at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
             at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
             at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
             at java.lang.reflect.Method.invoke(Method.java:597)
             at scala.tools.nsc.util.ScalaClassLoader$$anonfun$run$1.apply(ScalaClassLoader.scala:71)
             at scala.tools.nsc.util.ScalaClassLoader$class.asContext(ScalaClassLoader.scala:31)
             at scala.tools.nsc.util.ScalaClassLoader$URLClassLoader.asContext(ScalaClassLoader.scala:139)
             at scala.tools.nsc.util.ScalaClassLoader$class.run(ScalaClassLoader.scala:71)
             at scala.tools.nsc.util.ScalaClassLoader$URLClassLoader.run(ScalaClassLoader.scala:139)
             at scala.tools.nsc.CommonRunner$class.run(ObjectRunner.scala:28)
             at scala.tools.nsc.ObjectRunner$.run(ObjectRunner.scala:45)
             at scala.tools.nsc.CommonRunner$class.runAndCatch(ObjectRunner.scala:35)
             at scala.tools.nsc.ObjectRunner$.runAndCatch(ObjectRunner.scala:45)
             at scala.tools.nsc.MainGenericRunner.runTarget$1(MainGenericRunner.scala:70)
             at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:92)
             at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:101)
             at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
      

      In inspecting the bytecode, it seems that the optimizer completely removes the flatMap invocation in the above snippet.

      Bytecode without optimize (CORRECT):

      public final class Test$ extends java.lang.Object{
      public static final Test$ MODULE$;
      
      public static {};
       Code:
        0:        new        #2; //class Test$
        3:        invokespecial        #12; //Method "<init>")V
        6:        return
      
      public void main(java.lang.String[]);
       Code:
        0:        new        #16; //class java/lang/Exception
        3:        dup
        4:        ldc        #18; //String this is an exception
        6:        invokespecial        #21; //Method java/lang/Exception."<init>":(Ljava/lang/String;)V
        9:        astore_2
        10:        new        #23; //class scala/util/Success
        13:        dup
        14:        iconst_1
        15:        invokestatic        #29; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
        18:        invokespecial        #32; //Method scala/util/Success."<init>":(Ljava/lang/Object;)V
        21:        new        #34; //class Test$$anonfun$1
        24:        dup
        25:        aload_2
        26:        invokespecial        #37; //Method Test$$anonfun$1."<init>":(Ljava/lang/Exception;)V
        29:        invokevirtual        #41; //Method scala/util/Success.flatMap:(Lscala/Function1;)Lscala/util/Try;
        32:        astore_3
        33:        getstatic        #46; //Field scala/Predef$.MODULE$:Lscala/Predef$;
        36:        aload_3
        37:        invokevirtual        #49; //Method scala/Predef$.println:(Ljava/lang/Object;)V
        40:        return
      
      }
      

      Bytecode with optimize (INCORRECT):

      public final class Test$ extends java.lang.Object{
      public static final Test$ MODULE$;
      
      public static {};
       Code:
        0:        new        #2; //class Test$
        3:        invokespecial        #12; //Method "<init>")V
        6:        return
      
      public void main(java.lang.String[]);
       Code:
        0:        new        #16; //class java/lang/Exception
        3:        dup
        4:        ldc        #18; //String this is an exception
        6:        invokespecial        #21; //Method java/lang/Exception."<init>":(Ljava/lang/String;)V
        9:        astore_2
        10:        new        #23; //class scala/util/Success
        13:        dup
        14:        iconst_1
        15:        invokestatic        #29; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
        18:        invokespecial        #32; //Method scala/util/Success."<init>":(Ljava/lang/Object;)V
        21:        pop
        22:        aload_2
        23:        athrow
      
      }
      

      The diff between the two:

      --- without-opt        2012-08-05 11:32:04.000000000 +0200
      +++ with-opt        2012-08-05 11:32:17.000000000 +0200
      @@ -20,16 +20,9 @@
         14:        iconst_1
         15:        invokestatic        #29; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
         18:        invokespecial        #32; //Method scala/util/Success."<init>":(Ljava/lang/Object;)V
      -   21:        new        #34; //class Test$$anonfun$1
      -   24:        dup
      -   25:        aload_2
      -   26:        invokespecial        #37; //Method Test$$anonfun$1."<init>":(Ljava/lang/Exception;)V
      -   29:        invokevirtual        #41; //Method scala/util/Success.flatMap:(Lscala/Function1;)Lscala/util/Try;
      -   32:        astore_3
      -   33:        getstatic        #46; //Field scala/Predef$.MODULE$:Lscala/Predef$;
      -   36:        aload_3
      -   37:        invokevirtual        #49; //Method scala/Predef$.println:(Ljava/lang/Object;)V
      -   40:        return
      +   21:        pop
      +   22:        aload_2
      +   23:        athrow
      
      }
      

        Issue Links

          Activity

          Hide
          Heather Miller added a comment - - edited

          Assuming that I'm correct and that this is purely an optimizer bug (and not anything wrong with Try), whenever this is resolved, the following test should be re-enabled:
          https://github.com/heathermiller/scala/commit/cb6066ee6136d78ceb8c3b5c06cefac998966dd0#diff-1

          Show
          Heather Miller added a comment - - edited Assuming that I'm correct and that this is purely an optimizer bug (and not anything wrong with Try), whenever this is resolved, the following test should be re-enabled: https://github.com/heathermiller/scala/commit/cb6066ee6136d78ceb8c3b5c06cefac998966dd0#diff-1
          Hide
          Jason Zaugg added a comment - - edited

          Decoupled the test from Try.

            ~/code/scala/test/files/run/t6188 cat t6188_1.scala 
          package t6188
          
          object Bug {
            def withFilter[U](f: Any => Any): Unit =
              try f(0)
              catch {
                case e: Throwable =>
              }
          }
            ~/code/scala/test/files/run/t6188 cat t6188_2.scala 
          package t6188
          
          object Test {
            def main(args: Array[String]) {
              val e = new Exception("this is an exception")
              Bug.withFilter((x) => throw e)
            }
          }
          
            ~/code/scala/test/files/run/t6188 export SB="$HOME/code/scala/build/quick/bin" &&  $SB/scalac -optimize t6188_1.scala && $SB/scalac -optimize t6188_2.scala && $SB/scala t6188.Test
          java.lang.Exception: this is an exception
          	at t6188.Test$.main(t6188_2.scala:5)
          

          The method name must be one of the monadic names, presumably to trigger Inliners#isMonadicMethod.

          Separate compilation is also required.

          Scala Version  Result
          ---------------------
          2.8.0          OK
          2.9.0          Compiler Crash ("java.lang.Error: Open block: 4 <dirtypreds> <dirtysuccs>")          
          2.9.1          As reported in this ticket.
          2.10.0-M1+     As reported in this ticket.
          
          [log inliner] Analyzing t6188.Test.main count 1 with 7 blocks
          [log inliner] shouldInline(t6188.Test$$anonfun$main$1.apply) score: 3
          [log inliner] Inlining t6188.Test$$anonfun$main$1.apply in t6188.Test.main at pos: 114
          [log inliner] Analyzing t6188.Test.main count 1 with 9 blocks
          [log inliner] shouldInline(t6188.Test$$anonfun$main$1.apply) score: 3
          [log inliner] Inlining t6188.Test$$anonfun$main$1.apply in t6188.Test.main at pos: 114
          [log inliner] Analyzing t6188.Test.main count 1 with 11 blocks
          [log inliner] !!! Found open but empty block while inlining t6188.Test.main: removing from block list.
          [log inliner]  t6188.Test.main blocks before inlining: 1 (17) after: 3 (36)
          
          Show
          Jason Zaugg added a comment - - edited Decoupled the test from Try. ~/code/scala/test/files/run/t6188 cat t6188_1.scala package t6188 object Bug { def withFilter[U](f: Any => Any): Unit = try f(0) catch { case e: Throwable => } } ~/code/scala/test/files/run/t6188 cat t6188_2.scala package t6188 object Test { def main(args: Array[String]) { val e = new Exception("this is an exception") Bug.withFilter((x) => throw e) } } ~/code/scala/test/files/run/t6188 export SB="$HOME/code/scala/build/quick/bin" && $SB/scalac -optimize t6188_1.scala && $SB/scalac -optimize t6188_2.scala && $SB/scala t6188.Test java.lang.Exception: this is an exception at t6188.Test$.main(t6188_2.scala:5) The method name must be one of the monadic names, presumably to trigger Inliners#isMonadicMethod . Separate compilation is also required. Scala Version Result --------------------- 2.8.0 OK 2.9.0 Compiler Crash ("java.lang.Error: Open block: 4 <dirtypreds> <dirtysuccs>") 2.9.1 As reported in this ticket. 2.10.0-M1+ As reported in this ticket. [log inliner] Analyzing t6188.Test.main count 1 with 7 blocks [log inliner] shouldInline(t6188.Test$$anonfun$main$1.apply) score: 3 [log inliner] Inlining t6188.Test$$anonfun$main$1.apply in t6188.Test.main at pos: 114 [log inliner] Analyzing t6188.Test.main count 1 with 9 blocks [log inliner] shouldInline(t6188.Test$$anonfun$main$1.apply) score: 3 [log inliner] Inlining t6188.Test$$anonfun$main$1.apply in t6188.Test.main at pos: 114 [log inliner] Analyzing t6188.Test.main count 1 with 11 blocks [log inliner] !!! Found open but empty block while inlining t6188.Test.main: removing from block list. [log inliner] t6188.Test.main blocks before inlining: 1 (17) after: 3 (36)
          Hide
          Miguel Garcia added a comment -

          The cause of this bug lies not so much in Inliner but in ICodeReader, which doesn't map bytecode-level exception handlers (in our case, that for Success.flatMap) to the resulting IMethod. Another bug in this category appears to be SI-6157.

          The short term solution consists in preventing inlining of callees containing EHs. Doing so also avoids class file format errors where a try expression is inlined at a non-empty-stack program point (same thing that "lift try" in UnCurry avoids).

          For reference, this is where ICodeReader meets bytecode-level EHs:

              while (i < exceptionEntries) {
                // skip start end PC
                in.skip(4)
                // read the handler PC
                code.jmpTargets += in.nextChar
                // skip the exception type
                in.skip(2)
                i += 1
              }
              skipAttributes()
          
          Show
          Miguel Garcia added a comment - The cause of this bug lies not so much in Inliner but in ICodeReader , which doesn't map bytecode-level exception handlers (in our case, that for Success.flatMap ) to the resulting IMethod . Another bug in this category appears to be SI-6157 . The short term solution consists in preventing inlining of callees containing EHs. Doing so also avoids class file format errors where a try expression is inlined at a non-empty-stack program point (same thing that "lift try" in UnCurry avoids). For reference, this is where ICodeReader meets bytecode-level EHs: while (i < exceptionEntries) { // skip start end PC in.skip(4) // read the handler PC code.jmpTargets += in.nextChar // skip the exception type in.skip(2) i += 1 } skipAttributes()
          Show
          Miguel Garcia added a comment - https://github.com/scala/scala/pull/1059
          Show
          Miguel Garcia added a comment - Fixed in https://github.com/scala/scala/commit/61cc8ff61c81a1276a921ad5288ee3bebea1c96e
          Show
          Adriaan Moors added a comment - https://github.com/scala/scala/pull/1764

            People

            • Assignee:
              Miguel Garcia
              Reporter:
              Heather Miller
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development