fix illegal-access behaviour #22
Conversation
|
@tajila Have you filled out an ECA as mentioned in https://github.com/eclipse/openj9/blob/master/CONTRIBUTING.md? Was the ECA filed with this |
|
Not to nitpick but the commit comment references an IBM-internal bug number. Can you remove the |
runtime/util/modularityHelper.c
Outdated
| @@ -267,6 +267,10 @@ isPackageExportedToModuleHelper(J9VMThread *currentThread, J9Module *fromModule, | |||
| */ | |||
| isExported = (*targetPtr == toModule); | |||
| } | |||
| } else if (J9_ARE_NO_BITS_SET(vm->runtimeFlags, J9_RUNTIME_DENY_ILLEGAL_ACCESS)) { | |||
| /* in Java9 --illegal-access=permit is turned on by default. This opens | |||
| * each package to all-unnamed modules unless illegal-access=deny is specified */ | |||
There was a problem hiding this comment.
This looks right with the minor nitpick of moving the end of comment delimiter to its own line.
| @@ -64,7 +64,6 @@ checkVisibility(J9VMThread *currentThread, J9Class* sourceClass, J9Class* destCl | |||
| && (J2SE_VERSION(vm) >= J2SE_19) | |||
| && J9_ARE_ALL_BITS_SET(vm->runtimeFlags, J9_RUNTIME_JAVA_BASE_MODULE_CREATED) | |||
| && !J9ROMCLASS_IS_PRIMITIVE_TYPE(destClass->romClass) | |||
| && J9_ARE_ALL_BITS_SET(vm->runtimeFlags, J9_RUNTIME_DENY_ILLEGAL_ACCESS) | |||
There was a problem hiding this comment.
I'm not sure I follow why this doesn't need to be checked as part of the visibility check. Can you elaborate?
There was a problem hiding this comment.
It is still being checked as part of the visibility check but just at a later stage. We check it further down when isPackageExportedToModuleWithName(...) is called.
The --illegal-access=[permit|debug|warn] opens all packages to all unnamed modules. So it is similar to add-exports but allows deeper reflection. The 'deeper reflection' part is handled by the JCL but we still have to make those packages visible in the vm.
tajila
force-pushed
the
illegal
branch
2 times, most recently
from
48a35c2 to
40124bd
Compare
|
My mistake with the commit message, I forgot to change it when I migrated the PR. |
|
@tajila Please post a comment when the commit message is updated (or indicate if you don't plan to) and then we can get this merged |
tajila
force-pushed
the
illegal
branch
2 times, most recently
from
c822497 to
4a0326f
Compare
This is the first set of changes to address illegal-access behaviour. The illegal-access (--illegal-access=[permit|debug|warn]) option opens all packages in the runtime image to unnamed modules if those package existed in JDK8. The previous implementation also provided add-reads access to all modules which is incorrect. The 'if that package existed in JDK8' part will be addressed in a future PR. The default behaviour in JDK9 is '--illegal-access=permit'. In order to turn this off one must supply '--illegal-access=deny', which will be the default for Java10+ java releases. Signed-off-by: tajila <atobia@ca.ibm.com>
|
@DanHeidinga I fixed the commit message. I also added more info regarding what I will do next. |
|
Looks good @tajila! Thanks for your help to get this landed. |
* added register class path entries Signed-off-by: Vinoshan Tharmalenkam <Vinoshan.Tharmalenkam@ibm.com>
…a-sandbox Checkcast null to valuetype
fix illegal-access behaviour
This is the first set of changes to address illegal-access behaviour.
The illegal-access (--illegal-access=[permit|debug|warn]) option opens
all packages in the runtime image to unnamed modules if those package
existed in JDK8.
The previous implementation also provided add-reads access to all
modules which is incorrect. The 'if that package existed in JDK8' part
will be addressed in a future PR.
The default behaviour in JDK9 is '--illegal-access=permit'. In order to
turn this off one must supply '--illegal-access=deny', which will be the
default for Java10+ java releases.
Signed-off-by: tajila atobia@ca.ibm.com
Reviewer: @pshipton