[jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

[Vicente Romero]

On Fri, 11 Jun 2021 15:01:20 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) which was intended to openjdk/jdk.
> 
> Please review this PR which is syncing the implementation of `DirectMethodHandleDesc.Kind.valueOf(int)` and `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading of the method's spec is that if the value of the refKind parameter is: MethodHandleInfo.REF_invokeInterface, then DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a value of true for its second argument, currently it is invoked with false regardless of the value of the refKind parameter
> 
> TIA

src/java.base/share/classes/java/lang/constant/DirectMethodHandleDesc.java line 143:

> 141:                 }
> 142:                 if (kind.refKind == refKind &&
> 143:                         (refKind != REF_invokeStatic || refKind != REF_invokeSpecial || kind.isInterface == isInterface)){

@mlchung 13 hours ago Member

It reads to me that the static initializer tries to set up the table such that valueOf(refKind, isInterface) should find the proper kind to return except this:

// There is not a specific Kind for interfaces
if (kind == VIRTUAL) kind = INTERFACE_VIRTUAL;

This changes the entry for REF_invokeVirtual to kind INTERFACE_VIRTUAL. Do you know why? If this entry is set to VIRTUAL, then each refKind has two entries in the table corresponding to the correct result for valueOf.

test/jdk/java/lang/constant/MethodHandleDescTest.java line 364:

> 362:     public void testKind() {
> 363:         for (Kind k : Kind.values()) {
> 364:             assertEquals(Kind.valueOf(k.refKind), Kind.valueOf(k.refKind, k.refKind == MethodHandleInfo.REF_invokeInterface));

@mlchung mlchung 2 days ago Member

Looks like the test does not verify the cases specified by valueOf(int refKind, boolean isInterface).
i.e. For most values of refKind, there is an exact match regardless of the value of isInterface except REF_invokeStatic and REF_invokeSpecial.

Do you mind adding those cases?

@vicente-romero-oracle vicente-romero-oracle 22 hours ago •

hum, the implementation for valueOf(int refKind, boolean isInterface) is incorrect, the behavior does depends on the value of isInterface for example: Kind.valueOf(1, false) returns GETTER while Kind.valueOf(1, true) fails with java.lang.IllegalArgumentException will fix the implementation of valueOf(int refKind, boolean isInterface) for it to match its spec

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

PR: https://git.openjdk.java.net/jdk17/pull/29