> On 1 Feb 2017, at 22:19, Vladimir Kozlov <vladimir.koz...@oracle.com> wrote: > > AOT tool jaotc does not run with SecurityManager. We assume it runs in secure > environment and it does not access any external resources.
Great. Can I now consider this change reviewed and integrate it? -Doug >> On Feb 1, 2017, at 12:03 PM, Doug Simon <doug.si...@oracle.com> wrote: >> >> >>> 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 >>>> >> >
