Bytecode verification failure when using abstract class and inlined lambda (kotlin) #280

exFalso · 2017-06-22T12:56:05Z

This is as far as I got reducing the test case:

abstract class AbstractClass {
    @Suspendable
    abstract fun call(): Unit
}

class MyFiber(val ac: AbstractClass) : Fiber<Unit>() {
    @Suspendable
    override fun run() {
        function { ac.call() }
    }
}

class SomeClass(a: Unit)

inline fun function(block: () -> Unit) {
    SomeClass(block())
}

fun main(args: Array<String>) {
    MyFiber(object : AbstractClass() {
        @Suspendable
        override fun call() {
        }
    })
}

Running the above main produces:

Exception in thread "main" java.lang.VerifyError: Bad type on operand stack
Exception Details:
  Location:
    MyFiber.run()V @88: invokestatic
  Reason:
    Type uninitialized 55 (current frame, stack[0]) is not assignable to 'java/lang/Object'
  Current Frame:
    bci: @88
    flags: { }
    locals: { 'MyFiber', top, top, uninitialized 55, uninitialized 55, top, 'co/paralleluniverse/fibers/Stack', integer, null }
    stack: { uninitialized 55, 'co/paralleluniverse/fibers/Stack', integer }
  Bytecode:
    0x0000000: 013a 08b8 005f 593a 06c6 002a 1906 0436
    0x0000010: 07b6 0063 aa00 0000 0000 0014 0000 0001
    0x0000020: 0000 0001 0000 0052 1906 b600 679a 0006
    0x0000030: 013a 0603 3607 00bb 0013 593a 044e 2ab4
    0x0000040: 0017 1906 c600 4019 0604 06b6 006b 1906
    0x0000050: 03b8 006f 2d19 0604 b800 6f19 0419 0605
    0x0000060: b800 6f03 3607 1906 04b6 0073 c000 134e
    0x0000070: 1906 05b6 0073 c000 133a 0419 0603 b600
    0x0000080: 73c0 0019 b600 1cb2 0010 3a05 2d19 0419
    0x0000090: 05b7 0020 5719 06c6 0008 1906 b600 76b1
    0x00000a0: 59b6 007c c100 5599 0009 b600 7ca7 000d
    0x00000b0: 1906 c600 0819 06b6 0076 bf            
  Exception Handler Table:
    bci [51, 160] => handler: 186
    bci [51, 160] => handler: 186
    bci [51, 160] => handler: 160
    bci [51, 160] => handler: 176
  Stackmap Table:
    full_frame(@40,{Object[#2],Top,Top,Top,Top,Top,Object[#91],Integer,Null},{})
    full_frame(@51,{Object[#2],Top,Top,Top,Top,Top,Object[#91],Top,Null},{})
    full_frame(@102,{Object[#2],Top,Top,Top,Top,Top,Object[#91],Integer,Null},{})
    same_locals_1_stack_item_frame(@132,Object[#25])
    full_frame(@159,{Object[#2],Top,Top,Top,Top,Object[#12],Object[#91],Integer,Null},{})
    full_frame(@160,{Object[#2],Top,Top,Top,Top,Top,Object[#91],Top,Null},{Object[#89]})
    same_locals_1_stack_item_frame(@176,Object[#120])
    same_locals_1_stack_item_frame(@186,Object[#120])

The text was updated successfully, but these errors were encountered:

circlespainter · 2017-06-24T16:41:46Z

Thanks, this indeed reproduces the issue.

pron · 2017-07-25T16:03:50Z

This seems to be a tough one, as Kotlin produces some very weird code:

  protected void run();
    descriptor: ()V
    flags: ACC_PROTECTED
    Code:
      stack=3, locals=6, args_size=1
         0: nop
         1: new           #19                 // class SomeClass
         4: dup
         5: astore        4
         7: astore_3
         8: aload_0
         9: getfield      #23                 // Field ac:LAbstractClass;
        12: invokevirtual #28                 // Method AbstractClass.call:()V
        15: getstatic     #16                 // Field kotlin/Unit.INSTANCE:Lkotlin/Unit;
        18: astore        5
        20: aload_3
        21: aload         4
        23: aload         5
        25: invokespecial #32                 // Method SomeClass."<init>":(Lkotlin/Unit;)V
        28: pop
        29: return
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            8       7     1 $i$a$1$function   I
            1      28     2 $i$f$function   I
            0      30     0  this   LMyFiber;

The SomeClass instance is allocated in bci 1, but the constructor is only called in bci 25, after the suspendable call. When we want to capture the stack before the call, there is an uninitialized object there, that is basically inaccessible to any Java operation. I am not quite sure how to resolve this.

This code looks so weird that it may be a Kotlin bug.

pron · 2017-07-25T16:16:31Z

OTOH, this code:

inline fun function(block: () -> Unit) {
    val x = block()
    SomeClass(x)
}

is compiled to the more reasonable bytecode:

  protected void run();
    descriptor: ()V
    flags: ACC_PROTECTED
    Code:
      stack=3, locals=4, args_size=1
         0: nop
         1: nop
         2: aload_0
         3: getfield      #21                 // Field ac:LAbstractClass;
         6: invokevirtual #26                 // Method AbstractClass.call:()V
         9: getstatic     #16                 // Field kotlin/Unit.INSTANCE:Lkotlin/Unit;
        12: astore_2
        13: new           #28                 // class SomeClass
        16: dup
        17: aload_2
        18: invokespecial #32                 // Method SomeClass."<init>":(Lkotlin/Unit;)V
        21: pop
        22: return
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            2       7     1 $i$a$1$function   I
           13       9     2  x$iv   Lkotlin/Unit;
            1      21     3 $i$f$function   I
            0      23     0  this   LMyFiber;

Which works fine with Quasar's instrumentation.

exFalso · 2017-07-25T20:35:08Z

Interesting!
Am I understanding it correctly that inlined lambda calls in argument lists are somehow expanded between the allocation and the initialisation of the outer object? This does smell like a Kotlin bug...
Could a "workaround" be to move all news that don't have a corresponding init before the suspendable call to somewhere after the call (e.g. just before the init)? Although this would require some non-trivial analysis I guess. I'll raise an issue with JB

pron · 2017-07-25T21:04:00Z

I guess -- that's certainly what's happening in this case -- but I haven't taken a close look at other cases of Kotlin compilation.

In general, calls to new and to invokespecial <init> should be close together. That they're not is technically valid (in terms of the JVM specification), but weird and probably unnecessary. As the new operation triggers no user code, there is no reason to issue it so early. BTW, this is precisely why Quasar does not support blocking inside constructors, because the object is in an uninitialized state, and the JVM does not allow saving any reference to it, aside from temporary references on the operand stack -- an uninitialized object has this temporary type, uninitialized, that, as the error state, cannot be considered an Object.

exFalso · 2017-07-26T12:12:45Z

For reference: https://youtrack.jetbrains.com/issue/KT-19251

See https://youtrack.jetbrains.com/issue/KT-19251 puniverse/quasar#280 Inline function calls (as well as try/catch expressions) in constructor arguments produce bytecode that spills stack, and stores uninitialized objects (created by 'NEW C', but not initialized by 'C.<init>') to local variables. Such bytecode is valid according to the JVM spec, but confuses Quasar (and other bytecode postprocessing tools). In order to avoid that, we apply 'processUnitializedStores' already implemented for coroutines. It moves 'NEW' instructions after the constructor arguments evaluation, producing code like <evaluate constructor arguments> <store constructor arguments to variables> NEW C DUP <load constructor arguments from variables> INVOKESPECIAL C.<init>(...) This transformation changes evaluation order for the expression, because 'NEW C' might cause 'C.<clinit>' invocation, thus we can enable it only in 1.2+. NB some other expressions, such as break/continue in the constructor arguments, also can produce "weird" bytecode: object is created by a 'NEW C' instruction, but later (conditionally) POPped from stack and left uninitialized. This, as we know, also can screw bytecode postprocessing. However, it looks like we can get away with it ATM. Otherwise it looks like we'd have to analyze constructor arguments, see if the evaluation can "jump out", and perform argument linearization in codegen. TODO KT-19251 Fixed Target versions 1.2

See https://youtrack.jetbrains.com/issue/KT-19251 puniverse/quasar#280 Inline function calls (as well as try/catch expressions) in constructor arguments produce bytecode that spills stack, and stores uninitialized objects (created by 'NEW C', but not initialized by 'C.<init>') to local variables. Such bytecode is valid according to the JVM spec, but confuses Quasar (and other bytecode postprocessing tools). In order to avoid that, we apply 'processUnitializedStores' already implemented for coroutines. It moves 'NEW' instructions after the constructor arguments evaluation, producing code like <initialize class C using Class.forName> <evaluate constructor arguments> <store constructor arguments to variables> NEW C DUP <load constructor arguments from variables> INVOKESPECIAL C.<init>(...) NB some other expressions, such as break/continue in the constructor arguments, also can produce "weird" bytecode: object is created by a 'NEW C' instruction, but later (conditionally) POPped from stack and left uninitialized. This, as we know, also can screw bytecode postprocessing. However, it looks like we can get away with it ATM. Otherwise it looks like we'd have to analyze constructor arguments, see if the evaluation can "jump out", and perform argument linearization in codegen.

See https://youtrack.jetbrains.com/issue/KT-19251 puniverse/quasar#280 https://bugs.openjdk.java.net/browse/JDK-8046233 Inline function calls (as well as try/catch expressions) in constructor arguments produce bytecode that spills stack, and stores uninitialized objects (created by 'NEW C', but not initialized by 'C.<init>') to local variables. Such bytecode is valid according to the JVM spec, but confuses Quasar (and other bytecode postprocessing tools), and fails to verify under some (buggy) versions of JDK 8. In order to avoid that, we apply 'processUnitializedStores' already implemented for coroutines. It moves 'NEW' instructions after the constructor arguments evaluation, producing code like <initialize class C using Class.forName> <evaluate constructor arguments> <store constructor arguments to variables> NEW C DUP <load constructor arguments from variables> INVOKESPECIAL C.<init>(...) NB some other expressions, such as break/continue in the constructor arguments, also can produce "weird" bytecode: object is created by a 'NEW C' instruction, but later (conditionally) POPped from stack and left uninitialized. This, as we know, also can screw bytecode postprocessing. However, it looks like we can get away with it ATM. Otherwise it looks like we'd have to analyze constructor arguments, see if the evaluation can "jump out", and perform argument linearization in codegen.

mikehearn · 2017-10-10T08:32:05Z

I think the way this is going, is that it'll actually be considered a Quasar bug.

What Kotlin is doing is - as you observe - technically valid. They have a change to the compiler that can push new closer to invokespecial, but it changes evaluation ordering and is thus in their view not backwards compatible. The issue being that new does run user code; the <clinit>. So changing the emission ordering changes when the class c'tor runs.

My guess is that for Quasar to be fully JVMS compliant in this edge case will require more assistance from the JVM side to be able to process stacks containing uninitialised classes. Kotlin may change its behaviour in future, and I guess we can opt-in to the required behaviour now, but it'd also be required for our users to set this flag and that ... would be annoying.

At the very least, can Quasar perhaps detect this scenario early and bail out with an error message explaining to the user about the Kotlin compiler flag?

circlespainter added the bug label Jun 24, 2017

pron closed this as completed in 977edd2 Jul 24, 2017

pron reopened this Jul 24, 2017

pron removed the bug label Jul 25, 2017

pron closed this as completed Jul 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bytecode verification failure when using abstract class and inlined lambda (kotlin) #280

Bytecode verification failure when using abstract class and inlined lambda (kotlin) #280

exFalso commented Jun 22, 2017

circlespainter commented Jun 24, 2017

pron commented Jul 25, 2017 •

edited

Loading

pron commented Jul 25, 2017 •

edited

Loading

exFalso commented Jul 25, 2017

pron commented Jul 25, 2017 •

edited

Loading

exFalso commented Jul 26, 2017

mikehearn commented Oct 10, 2017 •

edited

Loading

Bytecode verification failure when using abstract class and inlined lambda (kotlin) #280

Bytecode verification failure when using abstract class and inlined lambda (kotlin) #280

Comments

exFalso commented Jun 22, 2017

circlespainter commented Jun 24, 2017

pron commented Jul 25, 2017 • edited Loading

pron commented Jul 25, 2017 • edited Loading

exFalso commented Jul 25, 2017

pron commented Jul 25, 2017 • edited Loading

exFalso commented Jul 26, 2017

mikehearn commented Oct 10, 2017 • edited Loading

pron commented Jul 25, 2017 •

edited

Loading

pron commented Jul 25, 2017 •

edited

Loading

pron commented Jul 25, 2017 •

edited

Loading

mikehearn commented Oct 10, 2017 •

edited

Loading