Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JDK8 OJDK-MH] IAE from test_FindSpecial_Default_CrossPackage_Interface #14987

Closed
babsingh opened this issue May 2, 2022 · 5 comments · Fixed by #18398 or #18441
Closed

[JDK8 OJDK-MH] IAE from test_FindSpecial_Default_CrossPackage_Interface #14987

babsingh opened this issue May 2, 2022 · 5 comments · Fixed by #18398 or #18441
Labels
jdk8 project:MH Used to track Method Handles related work triageRequired

Comments

@babsingh
Copy link
Contributor

babsingh commented May 2, 2022

The below failure(s) is(are) only seen when OJDK MHs are enabled in OpenJ9.

Errors

FAILED: test_FindSpecial_Default_CrossPackage_Interface
java.lang.IllegalAccessException: no private access for invokespecial: interface examples.CrossPackageDefaultMethodInterface, from com.ibm.j9.jsr292.LookupAPITests_Find
	at java.lang.invoke.MemberName.makeAccessException(MemberName.java:850)
	at java.lang.invoke.MethodHandles$Lookup.checkSpecialCaller(MethodHandles.java:1572)
	at java.lang.invoke.MethodHandles$Lookup.findSpecial(MethodHandles.java:1002)
	at com.ibm.j9.jsr292.LookupAPITests_Find.test_FindSpecial_Default_CrossPackage_Interface(LookupAPITests_Find.java:2109)

Failing Test Targets

  • BUILD_LIST=functional
    • jsr292Test_jdk8_0
    • jsr292Test_JitCount0_jdk8_0
    • jsr292Test_jdk8_special_0

How-to Run Tests?

https://github.com/eclipse/openj9/blob/master/test/docs/OpenJ9TestUserGuide.md

Steps to build an OpenJ9 JDK8 with OJDK MHs enabled

Refer to #14541.

@babsingh babsingh added jdk8 project:MH Used to track Method Handles related work triageRequired labels May 2, 2022
@babsingh
Copy link
Contributor Author

babsingh commented May 2, 2022

I believe that the RI also throws IllegalAccessException for this test. When OJDK-MHs are enabled in OJ9, we automatically adopted the RI's behaviour. These statements should be re-confirmed. Afterwards, it should be verified if the RI's behaviour is correct. If the RI's behaviour is as-per spec or Java doc, then the test should be updated or disabled accordingly. Otherwise, we will have to look into updating the RI's behaviour.

@ThanHenderson
Copy link
Contributor

ThanHenderson commented Nov 3, 2023

w.r.t. #14987 (comment)

I believe that the RI also throws IllegalAccessException for this test. When OJDK-MHs are enabled in OJ9, we automatically adopted the RI's behaviour. These statements should be re-confirmed.

These statements are true.

it should be verified if the RI's behaviour is correct.

The RI's behaviour seems correct, the spec says:

Before method resolution, if the explicitly specified caller class is not identical with the lookup class, or if this lookup object does not have private access privileges, the access fails.

Which is what this does:
https://github.com/openjdk/jdk8/blob/6a383433a9f4661a96a90b2a4c7b5b9a85720031/jdk/src/share/classes/java/lang/invoke/MethodHandles.java#L1554-L1557

The behaviour of findSpecial changed with JEP 274 (https://openjdk.org/jeps/274) stating:

Lookups
The implementation of the method MethodHandles.Lookup.findSpecial(Class refc, String name, MethodType type, Class specialCaller) will be modified to allow for finding super-callable methods on interfaces. While this is not a change of the API as such, its documented behaviour changes significantly.

Here is the code:
https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/java.base/share/classes/java/lang/invoke/MethodHandles.java#L2235-L2238

and JEP 274 shipped with Java 9.

the test should be updated or disabled accordingly

The test that is failing tests the behaviour of JEP 274 (finding methods on interfaces). Given the statements above, this test should be disabled for Java 8 with OJDK MHs, or edited to expect the exception in such case.

@babsingh
Copy link
Contributor Author

babsingh commented Nov 6, 2023

The RI's behaviour seems correct, the spec says: ...

This implies that OpenJ9's current MH impl doesn't satisfy the spec in JDK8. #3978 might have changed the behaviour in JDK 8. Will ifdef'ing #3978 to JDK 9+ fix the OpenJ9 MH behaviour in JDK 8?

@ThanHenderson
Copy link
Contributor

I'll look more into that. That would likely be the solution; just ifdef'ing the latter part of the condition here: https://github.com/eclipse-openj9/openj9/pull/3978/files#diff-0a684f4145fdb97062bd8f290586a3a737634cb776bcf1a7adea2a394a9777d8R563 would do the trick. I can open an Issue

ThanHenderson added a commit to ThanHenderson/openj9 that referenced this issue Nov 6, 2023
This patch fixes eclipse-openj9#14987. OJDK MHs use OJDK's
version of findSpecial. Before JEP 274, which was implemented for
Java 9, findSpecial did not support interface method access. This
patch handles the IllegalAccessException that is thrown for OpenJ9
with OJDK MHs enabled.

Closes: eclipse-openj9#14987
Signed-off-by: Nathan Henderson <[email protected]>
@babsingh babsingh reopened this Nov 7, 2023
@babsingh
Copy link
Contributor Author

babsingh commented Nov 7, 2023

ThanHenderson added a commit to ThanHenderson/openj9 that referenced this issue Nov 10, 2023
This patch closes eclipse-openj9#14987. OpenJ9 MHs'
`checkSpecialAccess` incorrectly supported JEP 274 for all
Java versions despite JEP 274 targeting Java 9+.

Closes: eclipse-openj9#14987
Signed-off-by: Nathan Henderson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jdk8 project:MH Used to track Method Handles related work triageRequired
Projects
2 participants