[Compiler] VerifyError for record whose canonical constructor unconditionally throws exception #487

Marcono1234 · 2022-10-22T12:54:47Z

Version

Eclipse Compiler 3.31.0
(Maven: org.eclipse.jdt:ecj:3.31.0)

Description

The Eclipse compiler seems to produce invalid byte code for a record class with at least one component whose canonical constructor unconditionally throws an exception:

Error: Unable to initialize main class RecordTest
Caused by: java.lang.VerifyError: Control flow falls through code end
Exception Details:
  Location:
    RecordTest.<init>(Ljava/lang/String;)V @17: <invalid>
  Reason:
    Error exists in the bytecode
  Bytecode:
    0000000: 2ab7 000a bb00 0d59 b700 0fbf 2a2b b500
    0000010: 10
  Stackmap Table:
    full_frame(@12,{Object[#1],Object[#20]},{})

I am not very familiar with Java byte code, but I assume the issue is that the compiler generates unreachable default code which tries to assign the record component fields. javap output:

 0: aload_0
 1: invokespecial #10                 // Method java/lang/Record."<init>":()V
 4: new           #13                 // class java/lang/RuntimeException
 7: dup
 8: invokespecial #15                 // Method java/lang/RuntimeException."<init>":()V
11: athrow
12: aload_0
13: aload_1
14: putfield      #16                 // Field s:Ljava/lang/String;

This is a quite contrived case, but it should probably nonetheless produce valid Java byte code.
This issue also occurs when the throw statement is wrapped with if (true) { ... }.

Reproduction steps

Create a file called RecordTest.java with the following content

record RecordTest(String s) {
    RecordTest {
        throw new RuntimeException();
    }
    
    public static void main(String... args) throws Exception {
        new RecordTest("");
    }
}

Compile the class with the Eclipse compiler:

java -jar ./ecj-3.31.0.jar -17 ./RecordTest.java

Try to run the compiled class
```
java RecordTest
```
❌ Bug: It fails with a VerifyError

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

trancexpress · 2023-03-15T17:46:59Z

javap -verbose output for the .class compiled by javac

  RecordTest(java.lang.String);
    descriptor: (Ljava/lang/String;)V
    flags: (0x0000)
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: invokespecial #1                  // Method java/lang/Record."<init>":()V
         4: new           #7                  // class java/lang/RuntimeException
         7: dup
         8: invokespecial #9                  // Method java/lang/RuntimeException."<init>":()V
        11: athrow

And for the one from ecj:

  RecordTest(java.lang.String);
    descriptor: (Ljava/lang/String;)V
    flags: (0x0000)
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: invokespecial #10                 // Method java/lang/Record."<init>":()V
         4: new           #13                 // class java/lang/RuntimeException
         7: dup
         8: invokespecial #15                 // Method java/lang/RuntimeException."<init>":()V
        11: athrow
        12: aload_0
        13: aload_1
        14: putfield      #16                 // Field s:Ljava/lang/String;

Stack trace that generates the putfield after the athrow:

"Compiler Processing Task" #199 daemon prio=5 os_prio=0 cpu=1.52ms elapsed=35.86s tid=0x00007fbd9c2429e0 nid=0x611e at breakpoint [0x00007fbe66bfe000]
   java.lang.Thread.State: RUNNABLE
        at org.eclipse.jdt.internal.compiler.ast.Reference.fieldStore(Reference.java:124)
        at org.eclipse.jdt.internal.compiler.ast.FieldReference.generateAssignment(FieldReference.java:227)
        at org.eclipse.jdt.internal.compiler.ast.Assignment.generateCode(Assignment.java:139)
        at org.eclipse.jdt.internal.compiler.ast.Expression.generateCode(Expression.java:767)
        at org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration.internalGenerateCode(ConstructorDeclaration.java:458)
        at org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration.generateCode(ConstructorDeclaration.java:310)
        at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:761)
        at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:831)
        at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.generateCode(CompilationUnitDeclaration.java:413)
        at org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:916)
        at org.eclipse.jdt.internal.compiler.ProcessTaskManager.run(ProcessTaskManager.java:145)
        at java.lang.Thread.run([email protected]/Thread.java:833)

On the stack trace, I see codeStream.methodDeclaration (of type CompactConstructorDeclaration) with contents (at Expression.generateCode()):

RecordTest(String s) {
  super();
  throw new RuntimeException();
  this.s = s;
}

this.s = s is appended with this stack trace:

"Compiler Processing Task" #239 daemon prio=5 os_prio=0 cpu=9.28ms elapsed=58.51s tid=0x00007fbdb412fba0 nid=0x6841 runnable  [0x00007fbe943da000]
   java.lang.Thread.State: RUNNABLE
        at org.eclipse.jdt.internal.compiler.ast.CompactConstructorDeclaration.checkAndGenerateFieldAssignment(CompactConstructorDeclaration.java:93)
        at org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration.analyseCode(ConstructorDeclaration.java:207)
        at org.eclipse.jdt.internal.compiler.ast.CompactConstructorDeclaration.analyseCode(CompactConstructorDeclaration.java:43)
        at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.internalAnalyseCode(TypeDeclaration.java:951)
        at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.analyseCode(TypeDeclaration.java:304)
        at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.analyseCode(CompilationUnitDeclaration.java:137)
        at org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:911)
        at org.eclipse.jdt.internal.compiler.ProcessTaskManager.run(ProcessTaskManager.java:145)
        at java.lang.Thread.run([email protected]/Thread.java:833)

Possibly the fix is as simple as changing the addition order:

diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java
index 391b860990..e9c4eb35b2 100644
--- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java
+++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java
@@ -87,8 +87,8 @@ public class CompactConstructorDeclaration extends ConstructorDeclaration {
                int len = this.statements.length;
                int fLen = fa.length;
                Statement[] stmts = new Statement[len + fLen];
-               System.arraycopy(this.statements, 0, stmts, 0, len);
-               System.arraycopy(fa, 0, stmts, len, fLen);
+               System.arraycopy(fa, 0, stmts, 0, fLen);
+               System.arraycopy(this.statements, 0, stmts, fLen, len);
                this.statements = stmts;
        }
 }
\ No newline at end of file

The following code results in generating instructions after an athrow byte code statement: record X(String s) { X { throw new RuntimeException(); } } This is due to CompactConstructorDeclaration.checkAndGenerateFieldAssignment() appending field assignments after the record constructor body. This change adjusts CompactConstructorDeclaration to not generate field assignments if the execution flow is unreachable or dead after adding the statements from the record constructor. This prevents generating code after an athrow statement in the byte code. Fixes: eclipse-jdt#487 Signed-off-by: Simeon Andreev <[email protected]>

Marcono1234 · 2023-03-15T21:38:39Z

Possibly the fix is as simple as changing the addition order

That would probably violate the language specification:

After the last statement, if any, in the body of the compact constructor has completed normally (§14.1), all component fields of the record class are implicitly initialized to the values of the corresponding formal parameters.

The question would be whether that difference is really noticeable, but there might be contrived cases, such as leaking this in the canonical constructor, where this difference might have an effect.

Either way, since you did not chose this approach for your pull request it does not matter much.
Also thanks for fixing this!

trancexpress · 2023-03-15T21:42:24Z

That would probably violate the language specification

Thanks for the hint!

The following code results in generating instructions after an athrow byte code statement: record X(String s) { X { throw new RuntimeException(); } } This is due to CompactConstructorDeclaration.checkAndGenerateFieldAssignment() appending field assignments after the record constructor body. This change adjusts CompactConstructorDeclaration to not generate field assignments if the execution flow is unreachable or dead after adding the statements from the record constructor. This prevents generating code after an athrow statement in the byte code. Fixes: eclipse-jdt#487 Signed-off-by: Simeon Andreev <[email protected]>

The following code results in generating instructions after an athrow byte code statement: record X(String s) { X { throw new RuntimeException(); } } This is due to CompactConstructorDeclaration.checkAndGenerateFieldAssignment() appending field assignments after the record constructor body. This change adjusts CompactConstructorDeclaration to not generate field assignments if the execution flow is unreachable or dead after adding the statements from the record constructor. This prevents generating code after an athrow statement in the byte code. Fixes: #487 Signed-off-by: Simeon Andreev <[email protected]>

The following code results in generating instructions after an athrow byte code statement: record X(String s) { X { throw new RuntimeException(); } } This is due to CompactConstructorDeclaration.checkAndGenerateFieldAssignment() appending field assignments after the record constructor body. This change adjusts CompactConstructorDeclaration to not generate field assignments if the execution flow is unreachable or dead after adding the statements from the record constructor. This prevents generating code after an athrow statement in the byte code. Fixes: eclipse-jdt#487 Signed-off-by: Simeon Andreev <[email protected]>

Marcono1234 mentioned this issue Oct 22, 2022

Adjust Record adapter and extend test coverage google/gson#2224

Merged

trancexpress mentioned this issue Mar 15, 2023

Record with exception in constructor results in invalid byte code #840

Merged

3 tasks

trancexpress added compiler Eclipse Java Compiler (ecj) related issues bug Something isn't working labels Mar 17, 2023

iloveeclipse closed this as completed in #840 Mar 30, 2023

trancexpress self-assigned this Mar 30, 2023

trancexpress added this to the 4.28 M1 milestone Mar 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Compiler] VerifyError for record whose canonical constructor unconditionally throws exception #487

[Compiler] VerifyError for record whose canonical constructor unconditionally throws exception #487

Marcono1234 commented Oct 22, 2022

trancexpress commented Mar 15, 2023

Marcono1234 commented Mar 15, 2023 •

edited

Loading

trancexpress commented Mar 15, 2023

[Compiler] VerifyError for record whose canonical constructor unconditionally throws exception #487

[Compiler] VerifyError for record whose canonical constructor unconditionally throws exception #487

Comments

Marcono1234 commented Oct 22, 2022

Version

Description

Reproduction steps

trancexpress commented Mar 15, 2023

Marcono1234 commented Mar 15, 2023 • edited Loading

trancexpress commented Mar 15, 2023

Marcono1234 commented Mar 15, 2023 •

edited

Loading