The "--no-inline" option caused Kawa to generate invalid bytecode

The "--no-inline" option caused Kawa to generate invalid bytecode

[Kay Zheng]

Hi Per,

I think I hit a bug regarding "(define-early-constant ...)" and the
"--no-inline" option. This is the problematic program:

    (define-library (kawa-test)
      (import (kawa base)
              (class java.lang System))
      (export Main)

      (begin
        (define-early-constant MSG :: string
                               (string-append "This" "is" "a" "dummy" "log"))

        (define-simple-class Main ()
          ((main (args :: String[])) :: void
              allocation: 'class
            (invoke System:out 'println MSG)))))

Save it as kawa-test.scm, and compile it with

    kawa --no-inline -C kawa-test.scm

Then try to run the main method:

    java -cp .:kawa.jar Main

The program will fail with this error message:

    Exception in thread "main" java.lang.VerifyError: Bad local
variable type
    Exception Details:
      Location:
        kawa-test.<clinit>()V @50: aload_0
      Reason:
        Type top (current frame, locals[0]) is not assignable to reference type
      Current Frame:
        bci: @50
        flags: { }
        locals: { }
        stack: { }
      Bytecode:
        0x0000000: 124d b800 53b3 0033 1255 b800 53b3 002c
        0x0000010: 1257 b800 53b3 0029 1259 b800 53b3 0026
        0x0000020: 125b b800 53b3 0023 bb00 0259 b700 15b3
        0x0000030: 0019 2ab2 001f b200 23b2 0026 b200 29b2
        0x0000040: 002c b600 302a b200 33b6 0036 2ab6 003a
        0x0000050: 123c b800 42c0 003c b300 4612 48b3 004b
        0x0000060: b1

            at Main.main(kawa-test.scm:13)

I then disassembled kawa-test.class (the result is appended at the end
of this email), and found that there was a "aload_0" instruction in
the static initialization block. I'm not familiar with the JVM, but
search results said the instruction was to read the argument array,
yet there was no argument list for the static initialization block at
all.

Regards,
Kay Z.


========= Disassembled kawa-test.class starts here =========

    Compiled from "kawa-test.scm"
    public class kawa-test extends gnu.expr.ModuleBody implements
java.lang.Runnable,gnu.expr.RunnableModule {
      public static final java.lang.Class Main;

      static final java.lang.CharSequence MSG;

      public static kawa-test $instance;

      static final gnu.lists.IString Lit0;

      static final gnu.lists.IString Lit1;

      static final gnu.lists.IString Lit2;

      static final gnu.lists.IString Lit3;

      static final gnu.lists.IString Lit4;

      public final void run(gnu.mapping.CallContext);
        Code:
           0: aload_1
           1: getfield      #8                  // Field
gnu/mapping/CallContext.consumer:Lgnu/lists/Consumer;
           4: astore_2
           5: return

      public static {};
        Code:
           0: ldc           #77                 // String log
           2: invokestatic  #83                 // Method
gnu/lists/IString.valueOf:(Ljava/lang/CharSequence;)Lgnu/lists/IString;
           5: putstatic     #51                 // Field
Lit4:Lgnu/lists/IString;
           8: ldc           #85                 // String dummy
          10: invokestatic  #83                 // Method
gnu/lists/IString.valueOf:(Ljava/lang/CharSequence;)Lgnu/lists/IString;
          13: putstatic     #44                 // Field
Lit3:Lgnu/lists/IString;
          16: ldc           #87                 // String a
          18: invokestatic  #83                 // Method
gnu/lists/IString.valueOf:(Ljava/lang/CharSequence;)Lgnu/lists/IString;
          21: putstatic     #41                 // Field
Lit2:Lgnu/lists/IString;
          24: ldc           #89                 // String is
          26: invokestatic  #83                 // Method
gnu/lists/IString.valueOf:(Ljava/lang/CharSequence;)Lgnu/lists/IString;
          29: putstatic     #38                 // Field
Lit1:Lgnu/lists/IString;
          32: ldc           #91                 // String This
          34: invokestatic  #83                 // Method
gnu/lists/IString.valueOf:(Ljava/lang/CharSequence;)Lgnu/lists/IString;
          37: putstatic     #35                 // Field
Lit0:Lgnu/lists/IString;
          40: new           #2                  // class "kawa-test"
          43: dup
          44: invokespecial #21                 // Method "<init>":()V
          47: putstatic     #25                 // Field $instance:Lkawa-test;
          50: aload_0
          51: getstatic     #31                 // Field
kawa/lib/kawa/istrings."string-append":Lgnu/expr/CompiledProc;
          54: getstatic     #35                 // Field
Lit0:Lgnu/lists/IString;
          57: getstatic     #38                 // Field
Lit1:Lgnu/lists/IString;
          60: getstatic     #41                 // Field
Lit2:Lgnu/lists/IString;
          63: getstatic     #44                 // Field
Lit3:Lgnu/lists/IString;
          66: invokevirtual #48                 // Method
gnu/mapping/CallContext.setupApply:(Lgnu/mapping/Procedure;Ljava/lang/Object;Ljava/
          69: aload_0
          70: getstatic     #51                 // Field
Lit4:Lgnu/lists/IString;
          73: invokevirtual #54                 // Method
gnu/mapping/CallContext.addArg:(Ljava/lang/Object;)V
          76: aload_0
          77: invokevirtual #58                 // Method
gnu/mapping/CallContext.runUntilValue:()Ljava/lang/Object;
          80: ldc           #60                 // class java/lang/CharSequence
          82: invokestatic  #66                 // Method
gnu/mapping/Promise.force:(Ljava/lang/Object;Ljava/lang/Class;)Ljava/lang/Object;
          85: checkcast     #60                 // class java/lang/CharSequence
          88: putstatic     #70                 // Field
MSG:Ljava/lang/CharSequence;
          91: ldc           #72                 // class Main
          93: putstatic     #75                 // Field Main:Ljava/lang/Class;
          96: return

      public kawa-test();
        Code:
           0: aload_0
           1: invokespecial #14                 // Method
gnu/expr/ModuleBody."<init>":()V
           4: aload_0
           5: invokestatic  #20                 // Method
gnu/expr/ModuleInfo.register:(Ljava/lang/Object;)V
           8: return
    }

========= Disassembled kawa-test.class ends here =========

Re: The "--no-inline" option caused Kawa to generate invalid bytecode

[Per Bothner]

On 09/04/2017 03:25 AM, Kay Zheng wrote:
> Hi Per,
> 
> I think I hit a bug regarding "(define-early-constant ...)" and the
> "--no-inline" option.

I was able to reproduce the problem.

> I then disassembled kawa-test.class (the result is appended at the end
> of this email), and found that there was a "aload_0" instruction in
> the static initialization block. I'm not familiar with the JVM, but
> search results said the instruction was to read the argument array,
> yet there was no argument list for the static initialization block at
> all.

aload_0 does not necessarily means to read the argument array.
It means to push the contents of local variable 0 (which must
be a reference to an object or null) onto the operand stack.
The first few local variables are commonly used for parameters,
but <clinit> takes no parameters, not even the implicit 'this'.

In this case I'm guessing a bug in the Compilation.loadCallContext method.
-- 
	--Per Bothner
per@bothner.com   http://per.bothner.com/

Re: The "--no-inline" option caused Kawa to generate invalid bytecode

[Kay Zheng]

2017-09-05 11:28 GMT+08:00 Per Bothner <per@bothner.com>:
>
> aload_0 does not necessarily means to read the argument array.
> It means to push the contents of local variable 0 (which must
> be a reference to an object or null) onto the operand stack.
> The first few local variables are commonly used for parameters,
> but <clinit> takes no parameters, not even the implicit 'this'.
>

Indeed I mistook the meaning of the aload_0 instruction, I'm sorry.
Thank you for explaining.it.

Re: The "--no-inline" option caused Kawa to generate invalid bytecode

[Per Bothner]

On 09/04/2017 03:25 AM, Kay Zheng wrote:
> Hi Per,
> 
> I think I hit a bug regarding "(define-early-constant ...)" and the
> "--no-inline" option. This is the problematic program:

... [caused a VerifyError].

The problem was that one point Kawa thought the current bytecode
location was "unreachable" - as it would be after a goto instruction.
That is why it didn't generate the necessary instructions.
Fixing that problem was relatively easy, but then I ran into another
problem in generating the stack-map tables (which are used by the verifier)
Fixing that problem looked messy and difficult.

I tried the test-case using the bytecode2 branch (announced in the
previous message) - and that worked.

So I decided to "punt" on this problem for now, and wait for it to be
fixed when bytecode2 is merged into master.  Sorry about that.
Until the code is merged, I welcome testing of the bytecode2 branch.
-- 
	--Per Bothner
per@bothner.com   http://per.bothner.com/