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

[SwathiKalahastri]

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

[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.