[3.1.2] DLS_DEAD_LOCAL_STORE false-positive (regression)

[boris-petrov]

We have a Reference<T> class like:

public final class Reference<T> {
	public T value;

	public Reference(T value) {
		this.value = value;
	}
}

Having an instance of Reference<Integer> and doing a reference.value++ gives a DLS_DEAD_LOCAL_STORE which is wrong. Didn't happen before 3.1.2.

[iloveeclipse]

Can you post a full example, where ++ is used?

[boris-petrov]

Well, I guess this would be enough:

void foo(Reference<Integer> reference) { reference.value++; }

[iloveeclipse]

You guess or does it produce the false positive?

[boris-petrov]

It does.

[iloveeclipse]

Can you provide a full, self contained example? I've tried a simple example without issues:

package a;
public class Foo {
	void foo(Reference<Integer> reference) {
		reference.value++;
	}
}

package a;
public final class Reference<T> {
	public T value;
	public Reference(T value) {
		this.value = value;
	}
}

[boris-petrov]

The (almost) exact same example gives the warning for me:

package a;

public class Foo {
	void foo(Reference<Integer> reference) {
		reference.value++;
	}
}

final class Reference<T> {
	public T value;
	public Reference(T value) {
		this.value = value;
	}
}

I've just put them in a single file. I'm not sure how to help more. Adding a new file Foo.java with this inside triggers the warning.

[iloveeclipse]

Still can't reproduce :-(
Which compiler do you use, and which version? I guess you are not using Eclipse?

[boris-petrov]

Not using Eclipse, no:

%  gradle --version

------------------------------------------------------------
Gradle 4.5.1
------------------------------------------------------------

Build time:   2018-02-05 13:22:49 UTC
Revision:     37007e1c012001ff09973e0bd095139239ecd3b3

Groovy:       2.4.12
Ant:          Apache Ant(TM) version 1.9.9 compiled on February 2 2017
JVM:          1.8.0_162 (Oracle Corporation 25.162-b12)
OS:           Linux 4.15.5-1-ARCH amd64

P.S. I'm also using the ErrorProne Gradle plugin, not sure if it matters.

[iloveeclipse]

I can reproduce now with Oracle javac. It seem to generate some real garbage at the end of the method, see:

  foo(Foo$Reference) : void
   L0
    LINENUMBER 5 L0
    ALOAD 1: reference
    ASTORE 2
    ALOAD 2
    GETFIELD Foo$Reference.value : Object
    CHECKCAST Integer
    ASTORE 3
    ALOAD 2
    ALOAD 2
    GETFIELD Foo$Reference.value : Object
    CHECKCAST Integer
    INVOKEVIRTUAL Integer.intValue() : int
    ICONST_1
    IADD
    INVOKESTATIC Integer.valueOf(int) : Integer
    DUP_X1
    PUTFIELD Foo$Reference.value : Object
    ASTORE 4
    ALOAD 3
    POP
   L1
    LINENUMBER 6 L1
    RETURN
   L2
    LOCALVARIABLE this Foo L0 L2 0
    LOCALVARIABLE reference Foo$Reference L0 L2 1
      // declaration: a.Foo$Reference<java.lang.Integer>
    MAXSTACK = 3
    MAXLOCALS = 5

What should those operations after putfield do?!?:

    PUTFIELD Foo$Reference.value : Object
    ASTORE 4
    ALOAD 3
    POP

Here is what Eclipse compiler generates for the same code:

  foo(Foo$Reference) : void
   L0
    LINENUMBER 5 L0
    ALOAD 1: reference
    DUP
    GETFIELD Foo$Reference.value : Object
    CHECKCAST Integer
    INVOKEVIRTUAL Integer.intValue() : int
    ICONST_1
    IADD
    INVOKESTATIC Integer.valueOf(int) : Integer
    PUTFIELD Foo$Reference.value : Object
   L1
    LINENUMBER 6 L1
    RETURN
   L2
    LOCALVARIABLE this Foo L0 L2 0
    LOCALVARIABLE reference Foo$Reference L0 L2 1
      // declaration: a.Foo$Reference<java.lang.Integer>
    MAXSTACK = 3
    MAXLOCALS = 2

So it looks like you've found a bug in javac.

This bug was uncovered by commit ef14b65 for issue #516. I haven't checked oracle bug database yet, but this bytecode looks really buggy (or way to complex), compared with the ecj version it needs 3 (!) local variables more and does many unneeded things.

[iloveeclipse]

BTW changing the reference.value++ to reference.value = reference.value + 1; fixes the broken bytecode and mutes the warning, so your code will be faster :o)

[boris-petrov]

I enjoy and appreciate the fact that you're actually reading the bytecode generated by javac. :) Which compiler are you checking against for comparison (apart from Eclipse)?

P,S. And are you going to raise a bug in JDK?

[iloveeclipse]

Looks like they use some bytecode generator which generates something like:

Object var1 = reference; //     ASTORE 2
Integer var2 = (Integer) unnamedCall??? (var1, Foo$Reference.value);  // ALOAD 2; GETFIELD Foo$Reference.value : Object; CHECKCAST Integer; ASTORE 3

// ???    ALOAD 2;  ALOAD 2

Object var3 = Foo$Reference.value = Integer.valueOf( ((Integer) Foo$Reference.value).intValue() + 1 );

//    GETFIELD Foo$Reference.value : Object
//    CHECKCAST Integer
//    INVOKEVIRTUAL Integer.intValue() : int
//    ICONST_1
//   IADD
//   INVOKESTATIC Integer.valueOf(int) : Integer
//    DUP_X1
//    PUTFIELD Foo$Reference.value : Object
//     ASTORE 4

And the left over bytecode is just pure nonsense which loads var2 on stack again and immediately discards the load again:

    ALOAD 3
    POP

P.S.
I'm Eclipse guy, so I mostly use the second Java compiler written by other Eclipse guys :-)