Suspect code generation for "try-with-resource" functionality

[SwathiKalahastri]

Generated java class and its output is different using eclipse with traditional Java

As per specification, In Throwable.addSuppressed() method, if exception is this throwable; a throwable cannot suppress itself in such scenario it should throw java.lang.IllegalArgumentException but the result is varies with using eclipse compiler.

https://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#addSuppressed-java.lang.Throwable-

package test;

public class Try_with_Resource {
	public static void main(String[] args) {
		RuntimeException e = new RuntimeException("My Exception");
		try (TestB A = new TestB(e)) {
			throw e;
		}
	}
}

class TestB implements AutoCloseable {
	RuntimeException e;
	public TestB(RuntimeException e) {
		this.e = e;
	}

	@Override
	public void close() {
		throw e;
	}
}

Output using regular java (with java 7 version & above)

Output from eclipse

The bytecode of class generated by eclipse vs regular javac

Using eclipse compiler

$ javap -c Try_with_Resource.class 
Compiled from "Try_with_Resource.java"
public class test.Try_with_Resource {
  public test.Try_with_Resource();
    Code:
       0: aload_0
       1: invokespecial #8                  // Method java/lang/Object."<init>":()V
       4: return

  public static void main(java.lang.String[]);
    Code:
       0: new           #16                 // class java/lang/RuntimeException
       3: dup
       4: ldc           #18                 // String My Exception
       6: invokespecial #20                 // Method java/lang/RuntimeException."<init>":(Ljava/lang/String;)V
       9: astore_1
      10: aconst_null
      11: astore_2
      12: aconst_null
      13: astore_3
      14: new           #23                 // class test/TestB
      17: dup
      18: aload_1
      19: invokespecial #25                 // Method test/TestB."<init>":(Ljava/lang/RuntimeException;)V
      22: astore        4
      24: aload_1
      25: athrow
      26: astore_2
      27: aload         4
      29: ifnull        37
      32: aload         4
      34: invokevirtual #28                 // Method test/TestB.close:()V
      37: aload_2
      38: athrow
      39: astore_3
      40: aload_2
      41: ifnonnull     49
      44: aload_3
      45: astore_2
      46: goto          59
      49: aload_2
      50: aload_3
      51: if_acmpeq     59
      54: aload_2
      55: aload_3
      56: invokevirtual #31                 // Method java/lang/Throwable.addSuppressed:(Ljava/lang/Throwable;)V
      59: aload_2
      60: athrow
    Exception table:
       from    to  target type
          24    26    26   any
          14    39    39   any
}

Using Traditional java compiler

 $ javap -c test/Try_with_Resource
Compiled from "Try_with_Resource.java"
public class test.Try_with_Resource {
  public test.Try_with_Resource();
    Code:
       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":()V
       4: return

  public static void main(java.lang.String[]);
    Code:
       0: new           #7                  // class java/lang/RuntimeException
       3: dup
       4: ldc           #9                  // String My Exception
       6: invokespecial #11                 // Method java/lang/RuntimeException."<init>":(Ljava/lang/String;)V
       9: astore_1
      10: new           #14                 // class test/TestB
      13: dup
      14: aload_1
      15: invokespecial #16                 // Method test/TestB."<init>":(Ljava/lang/RuntimeException;)V
      18: astore_2
      19: aload_1
      20: athrow
      21: astore_3
      22: aload_2
      23: invokevirtual #21                 // Method test/TestB.close:()V
      26: goto          37
      29: astore        4
      31: aload_3
      32: aload         4
      34: invokevirtual #24                 // Method java/lang/Throwable.addSuppressed:(Ljava/lang/Throwable;)V
      37: aload_3
      38: athrow
    Exception table:
       from    to  target type
          19    21    21   Class java/lang/Throwable
          22    26    29   Class java/lang/Throwable
}

I am not sure it might be the reason, I could see generated “Exception table” in java class with eclipse compiler is different from java class generated by the traditional java compiler.

[mpalat]

Thanks @SwathiKalahastri for raising this

[srikanth-sankaran]

The segment

49: aload_2
50: aload_3
51: if_acmpeq     59

in ECJ generated code look suspect.

JLS 20 gives this translation scheme:

14.20.3.1

...

The meaning of a basic try-with-resources statement of the form:

try ({VariableModifier} R Identifier = Expression ...)
Block

is given by by the following translation to a local variable declaration and a try-catchfinally
statement:

{
    final {VariableModifierNoFinal} R Identifier = Expression;
    Throwable #primaryExc = null;
    try ResourceSpecification_tail
        Block
    catch (Throwable #t) {
        #primaryExc = #t;
        throw #t;
    } finally {
        if (Identifier != null) {
            if (#primaryExc != null) {
                try {
                    Identifier.close();
                } catch (Throwable #suppressedExc) {
                   #primaryExc.addSuppressed(#suppressedExc);
                }
            } else {
                Identifier.close();
            }
        }
    }
}

There is no provision for the comparison and jump in this scheme. I wonder if draft JLS7 wording somehow suggested this.

[srikanth-sankaran]

PR here: #1033

[iloveeclipse]

This seem to cause a massive amount of regressions in our internal (Advantest) tests.

The code that uses "try with resources" (or wrapped into such try block) now fails in many cases reporting

java.lang.IllegalArgumentException: Self-suppression not permitted at java.base/java.lang.Throwable.addSuppressed(Throwable.java:1072) at

in places where we never call Throwable.addSuppressed().

We don't see the problem with I20230504-1800 and we see it with I20230511-1800.
PR #1033 seem to be the best candidate.

I will try to create a standalone reproducer.

[iloveeclipse]

I will try to create a standalone reproducer.

Turns out, it is trivial:

import java.io.Closeable;
import java.io.IOException;

public class X {
	public static void main(String[] args) throws IOException {
		try (DummyClosable closable = new DummyClosable()) {
			throw new IOException("OMG!!!");
		} catch (IOException e) {
			throw e;
		}
	}

	static class DummyClosable implements Closeable {
		@Override
		public void close() throws IOException {
			System.out.println("Closed!");
		}
	}
}

Executed with ecj from master it throws IllegalArgumentException:

Closed!
Exception in thread "main" java.lang.IllegalArgumentException: Self-suppression not permitted
	at java.base/java.lang.Throwable.addSuppressed(Throwable.java:1072)
	at X.main(X.java:8)
Caused by: java.io.IOException: OMG!!!
	at X.main(X.java:7)

With commit 2bac9e4 reverted, it prints what is expected to print:

Closed!
Exception in thread "main" java.io.IOException: OMG!!!
	at X.main(X.java:7)

@srikanth-sankaran : this must be fixed or 2bac9e4 reverted before M3 which is this Thursday.

[srikanth-sankaran]

Thanks for the reduced test case. I will get on this right away.

[iloveeclipse]

@srikanth-sankaran : anything here to do, or why this issue is still opened?

[srikanth-sankaran]

@srikanth-sankaran : anything here to do, or why this issue is still opened?

The original problem reported by SwathiKalahastri is still there on master. My attempted solution was reverted due to a regression and I haven't had an opportunity to revisit yet.

[iloveeclipse]

Ah, I remember now, I've removed 4.29 milestone now.