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.

Sign up for GitHub

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

Jump to bottom

Selectively enforce lookup class package check #10351

Merged
merged 1 commit into from
Aug 8, 2020
Merged

Selectively enforce lookup class package check #10351

merged 1 commit into from
Aug 8, 2020
+34 −13

Conversation

JasonFengJ9
Copy link
Member

@JasonFengJ9 JasonFengJ9 commented Aug 6, 2020 •
edited
Loading

Only following APIs are subject to lookup class package check (matching RI behaviours):

MethodHandles.Lookup.in​(Class<?> lookupClass)
MethodHandles.Lookup.dropLookupMode​(int modeToDrop)
MethodHandles.Lookup.privateLookupIn​(Class<?> targetClass, MethodHandles.Lookup caller)

The change only applies to JDK15+. The package check stays same for pre-JDK15 levels.

Signed-off-by: Jason Feng fengj@ca.ibm.com

@JasonFengJ9
Copy link
Member Author

@DanHeidinga this is a replay of #10329, please review.

DanHeidinga
DanHeidinga reviewed Aug 7, 2020
View reviewed changes
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing this will have follow on effects so I'll further after that

jcl/src/java.base/share/classes/java/lang/invoke/MethodHandles.java Show resolved Hide resolved
@DanHeidinga DanHeidinga added this to the Release 0.22 (Java 15) milestone Aug 7, 2020
DanHeidinga
DanHeidinga approved these changes Aug 7, 2020
View reviewed changes
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@DanHeidinga
Copy link
Member

Jenkins test sanity plinux jdk11

@DanHeidinga
Copy link
Member

Jenkins test sanity zlinux jdk15

@DanHeidinga
Copy link
Member

Jenkins test sanity xlinux jdk8

@DanHeidinga DanHeidinga self-assigned this Aug 7, 2020
@DanHeidinga
Copy link
Member

@JasonFengJ9 Can you look at the failing JDK15 tests? This may need to be coupled with some test changes as the assumptions the tests were written under are no longer correct

@JasonFengJ9
Selectively enforce lookup class package check
514b23b 
Only following APIs are subject to lookup class package check (matching
RI behaviours):

MethodHandles.Lookup.in​(Class<?> lookupClass)
MethodHandles.Lookup.dropLookupMode​(int modeToDrop)
MethodHandles.Lookup.privateLookupIn​(Class<?> targetClass,
MethodHandles.Lookup caller)
The change only applies to JDK15. The package check stays same for
pre-JDK15 levels.

Signed-off-by: Jason Feng <fengj@ca.ibm.com>
@JasonFengJ9
Copy link
Member Author

This may need to be coupled with some test changes as the assumptions the tests were written under are no longer correct

Right, the test expects an IllegalAccessException at https://github.com/eclipse/openj9/blob/ec0e7656c48de123994f05fa210999029962e2fa/jcl/src/java.base/share/classes/java/lang/invoke/MethodHandles.java#L1911 which won't be hit because performSecurityCheck is false for the MethodHandles.Lookup object returned from MethodHandles.lookup() at https://github.com/eclipse/openj9/blob/ec0e7656c48de123994f05fa210999029962e2fa/jcl/src/java.base/share/classes/java/lang/invoke/MethodHandles.java#L1895

Excluded test_RevealDirect_Security for JDK15+, and tested in a personal build. sorry not catching this earlier.

@DanHeidinga
Copy link
Member

Jenkins test sanity zlinux jdk15

@DanHeidinga
Copy link
Member

Just rerunning the 15 build as the only changes since successful 8&11 builds were to test excludes for 15

@DanHeidinga DanHeidinga merged commit f5d1b83 into eclipse-openj9:master Aug 8, 2020
@JasonFengJ9 JasonFengJ9 deleted the lookupobjV2 branch October 15, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanHeidinga DanHeidinga DanHeidinga approved these changes

Assignees

@DanHeidinga DanHeidinga

Labels
comp:vm
Projects
None yet
Milestone
Release 0.22 (Java 15)
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@JasonFengJ9 @DanHeidinga