RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

[Archie L. Cobbs]

On Wed, 11 Jan 2023 18:44:20 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> Also, looking at the loop test more closely, it seems to me that what the compiler needs to do is to prove that there can be possible paths by which a `this` can land into ref4.
>> 
>> If we build a graph of all the assignments, we get:
>> 
>> ref4 <- ref3 <- ref2 <- ref1 <- this
>> 
>> So, if we ask "can ref4 possibly contain `this`?" we could "walk" the variable dependencies backwards and discover that, yes, there exist a possible path in which `this` would get there.
>> 
>> Now, of course without a loop this can never be a real issue (since you can never send a `this` fully down the chain) - but again, this is a question of how much effort should be spend to handle false negatives in what look like a pathological case.
>
> Good idea. Looks like the difference is in the noise, at least on my Macbook:
> 
> Builds of master (jdk-21+3-69-gc6588d5bb3f)
> ==================================
> 
> Build times:
> 
>     real	2m24.650s
>     user	13m46.727s
>     sys	2m33.554s
> 
>     real	2m27.224s
>     user	13m43.464s
>     sys	2m37.251s
> 
>     real	2m26.658s
>     user	13m42.578s
>     sys	2m36.133s
> 
> Builds of ThisEscape (jdk-21+3-125-g6e96a7d76f8)
> ==================================
> 
> Modifications:
> 
>     - Reverted files in the make/ subdirectory to enable warning
>     - Commented out lines 363-382 in ThisEscapeAnalyzer.java
>       so no warnings are actually reported
> 
> Build times:
> 
>     real	2m25.912s
>     user	13m45.860s
>     sys	2m32.741s
> 
>     real	2m27.213s
>     user	13m44.830s
>     sys	2m36.596s
> 
>     real	2m25.756s
>     user	13m42.889s
>     sys	2m35.659s

Regarding the assignment graph approach, I think that would work if the references are bouncing around strictly between variables, but what if the chain includes any of the more complicated stuff that is currently being tracked, such as various Java expressions, method invocations, conditionals, etc.?

Consider this example:

public class ThisEscapeLoop { 
    
    public ThisEscapeLoop() { 
        ThisEscapeLoop ref1 = this;
        ThisEscapeLoop ref2 = null;
        ThisEscapeLoop ref3 = null;
        ThisEscapeLoop ref4 = null;
        for (int i = 0; i < 100; i++) {
            ref4 = this.returnMe(ref3);
            ref3 = ref2;
            ref2 = ref1;
            if (ref4 != null)
                ref4.mightLeak();
        }
    }

    public <T> T returnMe(T x) {
        return x;
    }

    public void mightLeak() {
    }
}

If you are only looking at variable assignments with values that are other variables, then the chain "breaks" when `ref4` is assigned indirectly via `returnMe()`, and you miss the leak.

-------------

PR: https://git.openjdk.org/jdk/pull/11874