(M) RFR: 8200167: Validate more special case invocations

[David Holmes]

Revised webrev: http://cr.openjdk.java.net/~dholmes/8200167/webrev.v2/

Karen formulated an additional test scenario invoking Object methods 
through an interface reference, which when turned into a new testcase 
demonstrated that the receiver typecheck was also missing in that case 
for Methodhandle's from findSpecial.

Again Vladimir Ivanov formulated the fix for this. Thanks Vladimir!

Summary of changes from original webrev:

- We introduce a new version of DirectMethodHandle.preparedlambdaForm 
that takes the caller class as a parameter, and that class is checked 
for being an interface (not the method's declaring class) to trigger the 
switch to LF_SPECIAL_IFC. (This obviously addresses one problem with 
invoking Object methods - as Object is not an interface - but by itself 
did not fix the problem.)

- We introduce a new LambdaForm kind, DIRECT_INVOKE_SPECIAL_IFC, which 
we use when dealing with LF_INVSPECIAL_IFC. (This was the key in 
ensuring the receiver typecheck via Special.checkReceiver remained in 
the generated code.)

- In the test we:
   - introduce a new invokeDirect testcase for Object.toString(), but we 
need to do that via a modified jcod file (as javac generates an 
invokevirtual)
   - introduce the new invokeSpecialObjectMH(I2 i) call for the MH case.

Thanks,
David

On 17/04/2018 6:48 PM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8200167
> webrev: http://cr.openjdk.java.net/~dholmes/8200167/webrev/
> 
> Credit to John Rose and Vladimir Ivanov for the form of the MH fix; and 
> to Tobias Hartmann for the C1 fix. Their help was very much appreciated 
> (and needed!).
> 
> tl;dr version: there are some places where badly formed interface method 
> calls (which are not detected by the verifier) were missing runtime 
> checks on the type of the receiver. Similar issues have been fixed in 
> the past and this was a remaining hole in relation to invokespecial 
> semantics and interface calls via MethodHandles. It also turned out 
> there was an issue in C1 that affected direct invokespecial calls.
> 
> Description of changes:
> 
> - src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java
> 
> Added a new form LF_INVSPECIAL_IFC for invokespecial of interface methods.
> 
> - src/java.base/share/classes/java/lang/invoke/MethodHandles.java
> 
> Renamed callerClass parameter to boundCallerClass, where it originates 
> from findBoundCallerClass. The name "callerClass" is misleading because 
> most of the time it is NULL!
> 
> In getDirectMethodCommon, pass the actual caller class (lookupClass) to 
> DirectMethodHandle.make. And correct the comment about restrictReceiver.
> 
> - src/java.base/share/classes/java/lang/invoke/DirectMethodHandle.java
> 
> DirectMethodHandle.make now takes a callerClass parameter (which may be 
> null).
> 
> DirectMethodHandle make "receiver" parameter is renamed "refc" as it is 
> not the receiver (it's the resolved type of the method holder ie REFC).
> 
> The Special subclass now takes a "checkClass" parameter which is either 
> refc, or callerClass. This is what we will check the receiver against.
> 
> In preparedLambdaForm we switch from LF_INVSPECIAL to LF_INVSPECIAL_IFC 
> if the target method is in an interface.
> 
> In makePreparedLambdaForm we augment the needsReceiverCheck test to 
> include LF_INVSPECIAL_IFC. (So we will not be doing a new receiver type 
> check on all invokespecials, just those involving interface methods.)
> 
> Added DMH.checkReceiver which is overridden by both the Special and 
> Interface subclasses.
> 
> - src/hotspot/share/c1/c1_Canonicalizer.cpp
> 
> Don't optimize away the checkcast when it is an invokespecial receiver 
> check.
> 
> - test/jdk/java/lang/invoke/SpecialInterfaceCall.java
> 
> (I've moved this test back and forth between hotspot/runtime and 
> j.l.invoke as it spans direct calls and MH calls. But I think on balance 
> it lands better here.)
> 
> The test sets up direct interface method calls for which invokespecial 
> is generated by javac, and similarly it creates MethodHandles with 
> invokespecial semantics. The should-not-throw cases are trivial. The 
> should-throw cases involve MH wizardry.
> 
> The test is broken into three parts to check the behaviour on first use 
> (when the method has to be initially resolved); on second use (to test 
> we don't use the cpcache in a way that is invalid); and again after 
> forcing JIT compilation of the calls.
> 
> The test is run 5 times to exercise the interpreter (yes the multiple 
> runs internal to the test are redundant in this case, but it's much 
> simpler to just run it all than try to work out what needs to be run); 
> the three variants of using C1, plus a C2 only run.
> 
> ---
> 
> Testing:
>    - the new test of course
>    - hotspot/runtime/*
>    - hotspot/compiler/jsr292
>    - jdk/java/lang/invoke
> 
>    - hs tiers 1 and 2
>    - jdk tiers 1, 2 and 3
> 
> Plus some related closed tests.
> 
> Thanks,
> David
> -----