Thanks Karen!
Once my re-testing has completed I'll look at pushing this.
David
On 27/04/2018 7:21 AM, Karen Kinnear wrote:
Looks good!
Thank you so much David and Vladimir for all the work to cover the corner cases.
thanks,
Karen
On Apr 26, 2018, at 12:12 AM, David Holmes <david.hol...@oracle.com> wrote:
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
-----
