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

[Archie L. Cobbs]

On Tue, 10 Jan 2023 23:45:59 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> It's slightly different from that.
>> 
>> Considering any of the various things in scope that can point to an object (these are: the current 'this' instance, the current outer 'this' instance, method parameter/variable, method return value, top of Java stack), such a thing has a "direct" reference if it might possibly _directly_ point to the 'this' we're tracking, while a thing has an "indirect" reference if it might possibly point to the 'this' we're tracking through _at least one level of indirection_.
>> 
>> This is just an attempt to eliminate some false positives by distinguishing between those two cases. Originally I was going to try to track fields (in a very limited way), and so this distinction was going to be more important, but even without tracking fields it's still useful. For example, if some method invokes `x.y.foo()` and `x` represents a direct but not an indirect 'this' reference, then there is no leak declared.
>> 
>> Considering the other options... (a) if you only track direct references, then you suffer from more false negatives (how many? unclear); (b) if you lump direct and indirect references into one bucket, then you suffer from more false positives (as in previous example `x.y.foo()`).
>> 
>> You can see an example of an indirect reference being tracked and exposed in the unit test `ThisEscapeArrayElement.java`.
>> 
>> Another motivating example is lambdas. The act of simply _creating_ a lambda never creates a leak, and a lambda never represents a _direct_ reference. But it might represent an _indirect_ reference.
>
> So, if I understand correctly your array element test, when we have `new Object[][] { { this } }` the analysis is able to detect that there might be some direct reference nested inside the array. So the outer array is an indirect reference. The inner array is also an indirect reference - but any element accessed on the inner array are direct reference - and so you detect a leak there. Correct?

Yes - although it's not tracked being tracked any more precisely than "one or more levels of indirection".

And of course by "reference" we only mean "the possibility of a reference" - in general we don't know for sure.

Here's the logic:
* The analysis knows `this` is a direct reference
* Therefore the array expression `{ this }` has an indirect reference (but no direct reference)
* The outer array expression `{ { this } }` therefore _also_ has an indirect reference (but no direct reference)

At this point, we don't know how many levels of indirection there are in `array` though.... only that its ≥ 1.

* There expression `array[0]` therefore is determined to have both direct and indirect references (they are both _possible_ because we've dereferenced something with an indirect reference)
* As does `array[0][0]` for the same reason

So invoking `array[0][0].hashCode()` means invoking `hashCode()` on something that has a possible direct reference, and is therefore a leak.

Note that because of the imprecision in the number of levels of indirection, invoking `array[0].hashCode()` would also have generated a warning - but in that case, a false positive.

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

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