> On 1 Feb 2017, at 20:54, Sean Mullan <sean.mul...@oracle.com> wrote: > > Couple of comments: > > - jdk.vm.ci is already loaded by the boot loader so it is implicitly granted > AllPermission and does not need an entry in default.policy.
Thanks - I removed it. > - all internal APIs in the jdk.vm.compiler module will now be restricted by > default by SecurityManager::checkPackageAccess(), so if you have any code or > tests running with a SecurityManager that are accessing internal APIs in the > jdk.vm.compiler module, you will need to grant them an appropriate > "accessClassInPackage" RuntimePermission in addition to any --add-exports > option you are using to break through encapsulation. Vladimir, does the AOT need to run with a SecurityManager and if so, I assume the qualified exports from jdk.vm.compiler to jdk.aot will allow it to run without needed an extra policy file? -Doug > On 2/1/17 6:07 AM, Doug Simon wrote: >> I’ve reworked the webrev as requested to make jdk.vm.compiler a >> non-upgradeable platform module, this allowing it to be mentioned in >> default.policy: >> >> http://cr.openjdk.java.net/~dnsimon/8145337/ >> >> -Doug >> >>> On 30 Jan 2017, at 22:53, Mandy Chung <mandy.ch...@oracle.com> wrote: >>> >>>> >>>> On Jan 30, 2017, at 1:36 PM, Doug Simon <doug.si...@oracle.com> wrote: >>>> >>>> >>>>> On 30 Jan 2017, at 21:55, Mandy Chung <mandy.ch...@oracle.com> wrote: >>>>> >>>>> >>>>>> On Jan 30, 2017, at 10:38 AM, Doug Simon <doug.si...@oracle.com> wrote: >>>>>> >>>>>> I’ve extended the webrev with that change - please re-review: >>>>>> >>>>>> http://cr.openjdk.java.net/~dnsimon/8145337_make/webrev >>>>>> >>>>> >>>>> +1 >>>> >>>> Thanks. Is that a “Reviewed”? >>>> >>> >>> Sorry. I only noticed now that you added this to UPGRADEABLE_MODULE. >>> Please add it only to PLATFORM_MODULES list instead. >>> >>> Making it an upgradeable module is a separate issue. I suggest you reopen >>> JDK-8171448. Specifically, since upgradeable modules are not tied with >>> java.base, our goal for JDK 9 is to eliminate qualified exports from JDK >>> modules to upgradeable modules, e.g. JDK-8170116, JDK-8166745, JDK-8161549. >>> >>>> I think I should get at least one sign-off from the security team. >>>> >>> >>> Hope Sean will review this one. Please send an updated webrev. >>> >>>> Also, since this is effectively making jdk.vm.compiler an upgradeable >>>> module, >>> >>> No it does not. >>> >>>> what’s the implication for it being a hash-checked module? >>> >>> When a module M is recorded in the ModuleHashes attribute of java.base, the >>> runtime will check if module M resolved in the graph matches the one tied >>> with java.base when created at build time; if not, it will fail. If an >>> upgradeable module >>> >>>> It seems like these changes effectively achieve what I was requesting with >>>> https://bugs.openjdk.java.net/browse/JDK-8171448. >>> >>> JDK-8145337 is about the security permission. It’s better to separate this >>> review from JDK-8171448. >>> >>> Mandy >>> >>>> >>>> -Doug >>>> >>>>> >>>>>> Strangely, there was no existing declaration of jdk.vm.compiler in >>>>>> Modules.gmk. >>>>>> >>>>> >>>>> Default is to be defined by the application class loader. The build will >>>>> find all modules from the source. There is no need to list all modules. >>>>> >>>>>> BTW, I never answered your question: >>>>>> >>>>>> "How does JVMCI call out to jdk.vm.compiler? does it load classes using >>>>>> Class::forName with the system class loader?” >>>>>> >>>>>> It uses JVMCIServiceLocator[1] which is a mechanism built on the >>>>>> standard ServiceLoader. >>>>> >>>>> Thanks for the pointer. That confirms my understanding that loads the >>>>> service providers using the system class loader. >>>>> >>>>> Mandy >>