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

Compiler generates code which crashes with VerifyError

    Details

      Description

      attempt to compile and run source in attachment cause next error:

      java.lang.VerifyError: (class: testverifyerror/CallCC$$anonfun$compose$1, method: apply signature: (I)Ltestverifyerror/ComputationBounds Inconsistent stack height 1 != 2
      at testverifyerror.CallCC$.compose(Main.scala:81)
      at testverifyerror.CallCC$.compose(Main.scala:111)
      at testverifyerror.ComputationBounds$$anonfun$map$1.apply(Main.scala:20)
      at testverifyerror.ComputationBounds$$anonfun$map$1.apply(Main.scala:20)
      at scala.Function1$class.apply$mcLI$sp(Function1.scala:39)
      at scala.runtime.AbstractFunction1.apply$mcLI$sp(AbstractFunction1.scala:12)
      at testverifyerror.Call.step(Main.scala:39)
      at testverifyerror.CallCC$.trampoline(Main.scala:67)
      at testverifyerror.ComputationBounds.get(Main.scala:18)
      at testverifyerror.Main$.main(Main.scala:140)
      at testverifyerror.Main.main(Main.scala)

      1. Main.scala
        4 kB
        ruslan shevchenko

        Activity

        Hide
        Jason Zaugg added a comment - - edited

        Reduced a ways:

        object Test {
          def foo(a: Int) {
            var bar: Int = 0
            bar = try { 0 } catch { case ex: Throwable => 0 }
            new { foo(bar) }
          }
        
          def main(args: Array[String]): Unit = foo(0)
        }
        
        java.lang.VerifyError: (class: Test$, method: foo signature: (I)V) Inconsistent stack height 1 != 2
        

        This cut down version crashes from (at least) 2.7.7.

        Show
        Jason Zaugg added a comment - - edited Reduced a ways: object Test { def foo(a: Int) { var bar: Int = 0 bar = try { 0 } catch { case ex: Throwable => 0 } new { foo(bar) } } def main(args: Array[String]): Unit = foo(0) } java.lang.VerifyError: (class: Test$, method: foo signature: (I)V) Inconsistent stack height 1 != 2 This cut down version crashes from (at least) 2.7.7.
        Hide
        Miguel Garcia added a comment - - edited

        This is interesting stuff but it's late so here go partial conclusions.

        To get rid of the verbose null push, pop, null push, pop (an artifact of GenICode's adapt(_, NullReference)) we can use:

        object Test {
        
          def foo(a: Any) {
            var bar: Any = a
        
            while(true) {
              bar = try { a } catch { case _ => a }
            }
        
            new { bar = a }
          }
        
          def main(args: Array[String]): Unit = foo(0)
        }
        

        The purpose of `new

        { bar = a }` is just to capture bar so that its type is ObjectRef.

        Resulting bytecode:
        public void foo(java.lang.Object);
          Code:
           0:   new     #16; //class scala/runtime/ObjectRef
           3:   dup
           4:   aload_1
           5:   invokespecial   #18; //Method scala/runtime/ObjectRef."<init>":(Ljava/lang/Object;)V
           8:   astore_2
           9:   goto    17
           12:  pop
           13:  aload_1
           14:  putfield        #22; //Field scala/runtime/ObjectRef.elem:Ljava/lang/Object;
           17:  aload_2
           18:  aload_1
           19:  goto    14
          Exception table:
           from   to  target type
            18    22    12   any
        


        The relevant sections are:
        - the catch-clause comprises instructions 12: and 13:, they leave the contents of a on the operand stack (only) because entering a catch empties the operand stack.
        - the next instruction (the assignment to the ObjectRef's elem) expects two stack slots (thus the error msg 1 != 2), ie z and the rhs value to assign to z.elem.

        For non-exceptional control flow, the putfield is reached with those two values, which are pushed in 17: and 18:. Those two instructions where emitted after GenICode's genStat() finds a Select in the lhs.

        Guessing somewhat, the "try lifting" trick in Uncurry didn't notice that a few phases later (LambdaLift) the lhs would stop being an Ident and become a Select. Thus the try expression wasn't turned into a local method. If so, not a backend problem.

        BTW `new { bar = a }

        ` is elided from the resulting bytecode. Perhaps ... enterIgnoreMode() ?

        Show
        Miguel Garcia added a comment - - edited This is interesting stuff but it's late so here go partial conclusions. To get rid of the verbose null push, pop, null push, pop (an artifact of GenICode's adapt(_, NullReference)) we can use: object Test { def foo(a: Any) { var bar: Any = a while(true) { bar = try { a } catch { case _ => a } } new { bar = a } } def main(args: Array[String]): Unit = foo(0) } The purpose of `new { bar = a }` is just to capture bar so that its type is ObjectRef . Resulting bytecode: public void foo(java.lang.Object); Code: 0: new #16; //class scala/runtime/ObjectRef 3: dup 4: aload_1 5: invokespecial #18; //Method scala/runtime/ObjectRef."<init>":(Ljava/lang/Object;)V 8: astore_2 9: goto 17 12: pop 13: aload_1 14: putfield #22; //Field scala/runtime/ObjectRef.elem:Ljava/lang/Object; 17: aload_2 18: aload_1 19: goto 14 Exception table: from to target type 18 22 12 any The relevant sections are: - the catch-clause comprises instructions 12: and 13:, they leave the contents of a on the operand stack (only) because entering a catch empties the operand stack. - the next instruction (the assignment to the ObjectRef's elem) expects two stack slots (thus the error msg 1 != 2), ie z and the rhs value to assign to z.elem . For non-exceptional control flow, the putfield is reached with those two values, which are pushed in 17: and 18:. Those two instructions where emitted after GenICode's genStat() finds a Select in the lhs. Guessing somewhat, the "try lifting" trick in Uncurry didn't notice that a few phases later (LambdaLift) the lhs would stop being an Ident and become a Select. Thus the try expression wasn't turned into a local method. If so, not a backend problem. BTW `new { bar = a } ` is elided from the resulting bytecode. Perhaps ... enterIgnoreMode() ?
        Hide
        Miguel Garcia added a comment -

        In UnCurry's mainTransform(), there are two cases intended to invokes liftTree() on the RHS of Assign:

              case Assign(Select(_, _), _) =>
                withNeedLift(true) { super.transform(tree) }
        
              case Assign(lhs, _) if lhs.symbol.owner != currentMethod || lhs.symbol.hasFlag(LAZY | ACCESSOR) =>
                withNeedLift(true) { super.transform(tree) }
        

        None of them covers the tree for

        bar = try { a } catch { case _ => a }
        

        which thus falls off to the default case, where no liftTree() is invoked. The reason it doesn't match the first case above is that at UnCurry-time the LHS is still an Ident.

        Show
        Miguel Garcia added a comment - In UnCurry's mainTransform() , there are two cases intended to invokes liftTree() on the RHS of Assign : case Assign(Select(_, _), _) => withNeedLift(true) { super.transform(tree) } case Assign(lhs, _) if lhs.symbol.owner != currentMethod || lhs.symbol.hasFlag(LAZY | ACCESSOR) => withNeedLift(true) { super.transform(tree) } None of them covers the tree for bar = try { a } catch { case _ => a } which thus falls off to the default case, where no liftTree() is invoked. The reason it doesn't match the first case above is that at UnCurry-time the LHS is still an Ident.
        Hide
        Paul Phillips added a comment -

        I turned over all testing to the buildbot, but since that identified the change pretty precisely, https://github.com/scala/scala/pull/1021

        Show
        Paul Phillips added a comment - I turned over all testing to the buildbot, but since that identified the change pretty precisely, https://github.com/scala/scala/pull/1021
        Hide
        Miguel Garcia added a comment -

        Merged into trunk, ie won't be in 2.10.

        Show
        Miguel Garcia added a comment - Merged into trunk, ie won't be in 2.10.
        Hide
        Adriaan Moors added a comment -

        reopening for 2.10.1-RC1 backport

        Show
        Adriaan Moors added a comment - reopening for 2.10.1-RC1 backport
        Hide
        Jason Zaugg added a comment -
        Show
        Jason Zaugg added a comment - Backport PR: https://github.com/scala/scala/pull/2007
        Hide
        Adriaan Moors added a comment -

        backport by Jason

        Show
        Adriaan Moors added a comment - backport by Jason

          People

          • Assignee:
            Miguel Garcia
            Reporter:
            ruslan shevchenko
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development