Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-07 Thread Doug Simon

> On 1 Feb 2017, at 22:19, Vladimir Kozlov  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  wrote:
>> 
>> 
>>> On 1 Feb 2017, at 20:54, Sean Mullan  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  wrote:
>> 
>> 
>> On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:
>> 
>> 
>>> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
>>> 
>>> 
 On Jan 30, 2017, at 10:38 AM, Doug Simon  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
 
>> 
> 



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-07 Thread Doug Simon

> On 1 Feb 2017, at 20:54, Sean Mullan  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  wrote:
>>> 
 
 On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:
 
 
> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
> 
> 
>> On Jan 30, 2017, at 10:38 AM, Doug Simon  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
>> 



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-02 Thread Alan Bateman



On 02/02/2017 02:12, Mandy Chung wrote:

On Feb 1, 2017, at 3: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/

Looks good.


Looks okay to me too.

-Alan.


Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Mandy Chung

> On Feb 1, 2017, at 3: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/

Looks good.

Mandy

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Sean Mullan

On 2/1/17 4:27 PM, Doug Simon wrote:

Can I now consider this change reviewed and integrate it?


Yes.

--Sean


Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Vladimir Kozlov
AOT tool jaotc does not run with SecurityManager. We assume it runs in secure 
environment and it does not access any external resources.

Thanks
Vladimir

> On Feb 1, 2017, at 12:03 PM, Doug Simon  wrote:
> 
> 
>> On 1 Feb 2017, at 20:54, Sean Mullan  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  wrote:
> 
> 
> On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:
> 
> 
>> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
>> 
>> 
>>> On Jan 30, 2017, at 10:38 AM, Doug Simon  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
>>> 
> 



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Sean Mullan

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.


- 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.


--Sean

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  wrote:



On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:



On 30 Jan 2017, at 21:55, Mandy Chung  wrote:



On Jan 30, 2017, at 10:38 AM, Doug Simon  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




Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Doug Simon

> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
> 
> 
>> On Jan 30, 2017, at 10:38 AM, Doug Simon  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”?

I think I should get at least one sign-off from the security team.

Also, since this is effectively making jdk.vm.compiler an upgradeable module, 
what’s the implication for it being a hash-checked module? It seems like these 
changes effectively achieve what I was requesting with 
https://bugs.openjdk.java.net/browse/JDK-8171448.

-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



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-02-01 Thread Doug Simon
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  wrote:
> 
>> 
>> On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:
>> 
>> 
>>> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
>>> 
>>> 
 On Jan 30, 2017, at 10:38 AM, Doug Simon  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



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-30 Thread Mandy Chung

> On Jan 30, 2017, at 1:36 PM, Doug Simon  wrote:
> 
> 
>> On 30 Jan 2017, at 21:55, Mandy Chung  wrote:
>> 
>> 
>>> On Jan 30, 2017, at 10:38 AM, Doug Simon  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
> 



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-30 Thread Doug Simon

> On 30 Jan 2017, at 17:11, Mandy Chung  wrote:
> 
> Moving it to platform class loader is okay with me.  I’m still unsure if 
> jdk.vm.compiler should be defined by the boot loader instead and you may want 
> to look into in a future release.

We also want jdk.vm.compiler to be upgradeable (as discussed in another thread) 
so that we can test newer development versions of Graal as the VM JIT via 
--upgrade-module-path instead of having to rebuild the JDK. As far as I 
understand, this means it cannot be defined by the boot loader.

> You can change make/common/Modules.gmk to move it to the PLATFORM_MODULES 
> list.

I’ve extended the webrev with that change - please re-review:

http://cr.openjdk.java.net/~dnsimon/8145337_make/webrev

Strangely, there was no existing declaration of jdk.vm.compiler in Modules.gmk.

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.

-Doug

[1] 
http://hg.openjdk.java.net/jdk9/hs/hotspot/file/2c44cff993b8/src/jdk.vm.ci/share/classes/jdk.vm.ci.services/src/jdk/vm/ci/services/JVMCIServiceLocator.java#l29

> 
>> On Jan 29, 2017, at 12:40 PM, Doug Simon  wrote:
>> 
>> Both jdk.vm.ci and jdk.vm.compiler should be considered part of the VM since 
>> they have the same capabilities as Unsafe. Arguably they could be considered 
>> even more trusted as they make it easier to do what one could (in theory) do 
>> with Unsafe. However, maybe they can still be defined by the platform class 
>> loader as 17 of 27 modules in default.policy[1] are granted AllPermission.
>> 
>> -Doug
>> 
>> [1] 
>> http://hg.openjdk.java.net/jdk9/dev/jdk/file/tip/src/java.base/share/lib/security/default.policy
>> 
>>> On 28 Jan 2017, at 02:00, Mandy Chung  wrote:
>>> 
>>> To clarify my question, I am not objecting fo jdk.vm.compiler be included 
>>> in the runtime. I’m trying to figure what the right loader should be.  It 
>>> seems to me that it should be defined by the boot loader, as it is part of 
>>> the VM.  However, another thread suggests that you want jdk.vm.compiler to 
>>> be upgradeable as Graal version requires other dependencies.  We only 
>>> support modules defined by the platform class loader be upgradeable.
>>> 
>>> Mandy
>>> 
 On Jan 27, 2017, at 9:52 AM, Mandy Chung  wrote:
 
 I have been wondering what defining loader jdk.vm.compiler should be, if 
 used for JIT at runtime.  That’s one reason I mentioned that the VM does 
 not delegate to any of its child loader.
 
 How does JVMCI call out to jdk.vm.compiler?  does it load classes using 
 Class::forName with the system class loader?
 
 Mandy
 
> On Jan 27, 2017, at 9:23 AM, Doug Simon  wrote:
> 
> Since jdk.vm.compiler can be used as the VM compiler, shouldn't it be 
> loaded by the platform class loader? I hope jdk.vm.ci is using the 
> platform class loader. Where is this decision encoded?
> 
> Sent from my iPhone
> 
>> On Jan 27, 2017, at 5:45 PM, Sean Mullan  wrote:
>> 
>> I would need more time to understand the issue, but default.policy is 
>> not the right place to be granting these permissions, since this module 
>> is not loaded by the platform class loader. From the first 3 lines of 
>> default.policy:
>> 
>> //
>> // Permissions required by modules stored in a run-time image and loaded
>> // by the platform class loader.
>> 
>> --Sean
>> 
>>> On 1/26/17 7:36 PM, Mandy Chung wrote:
>>> I’ll let Drew and Sean to advise on this. (Drew, Sean - sorry for 
>>> missing to include you in this offlist thread).
>>> 
>>> jdk.vm.compiler is defined by the application class loader.  It’d be 
>>> the first one in JDK if we grant it with AllPermissions and so the 
>>> security team should be the experts to review this.
>>> 
>>> Mandy
>>> 
 On Jan 26, 2017, at 1:17 PM, Thomas Wuerthinger 
  wrote:
 
 While the addition of "-Djava.security.policy=jit.policy” is not an 
 issue, we would like to avoid the need for additional files.
 
 To me -XX:+UseJVMCICompiler should pretty much imply that 
 “jdk.vm.compiler” will be granted java.security.AllPermission. The 
 code running in the “jdk.vm.compiler” module can install machine code 
 in the VM. I don’t think there is a scenario in which one would like 
 to run any “untrusted” code as part of this module.
 
 - thomas
 
 
> On 26 Jan 2017, at 19:05, Mandy Chung  wrote:
> 
> Sicen it’s experimental, when 

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-30 Thread Doug Simon

> On 29 Jan 2017, at 20:34, Igor Veresov  wrote:
> 
>> 
>> On Jan 27, 2017, at 3:40 PM, Christian Thalinger  
>> wrote:
>> 
>>> 
>>> On Jan 26, 2017, at 7:40 AM, Doug Simon  wrote:
>>> 
 
 On 26 Jan 2017, at 17:55, Mandy Chung  wrote:
 
 
> On Jan 26, 2017, at 3:13 AM, Doug Simon  wrote:
> 
>> 
>> jdk.vm.compiler is defined by the application class loader and it’s used 
>> by AOT tool.  I wonder why it has to run with security manager.
> 
> Without java.security.AllPermission, the policy for jdk.vm.compiler 
> required to get through a bootstrap (i.e., java -server 
> -XX:+UnlockExperimentalVMOptions -Djava.security.manager 
> -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler -version) is show below 
> (annotated with comments denoting the methods requiring the permissions):
> 
> :
 
 Are -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler supported to use at runtime?
>>> 
>>> Not sure I understand your question - they cannot be used at any other time 
>>> apart from runtime.
>> 
>> The question is if these command line options are supported by Oracle in JDK 
>> 9.  The answer used to be no but that might have changed.  Someone from 
>> Oracle needs to chime in.
> 
> 
> I would imagine that the permissions Doug mentions are required regardless of 
> whether it’s supported on not.

Correct.

Also, sorry for misinterpreting the question Mandy. I thought being 
experimental means not officially supported. However, I’ve heard that AOT may 
be supported even though being experimental. In any case, I’m fairly sure using 
Graal as a JIT is not supported.

-Doug

>> 
>> Having said that, it would be a shame if we don’t make jdk.vm.compiler a 
>> trusted system component because it obviously is.
>> 
>> We have to do it at some point anyway so why not now…
>> 
>>> 
> There’s no guarantee that this is all the permissions required since not 
> all code paths are exercised during bootstrap.
> 
>> You can reference JDK tools such as jdk.compiler and jdk.jlink that are 
>> not granted with any permission.
> 
> Neither of those tools create code and install it in the VM. I don’t 
> think a fine grained SecurityManager policy makes sense for a VM compiler 
> since it could subvert security by compiling/installing malicious code. 
> That is, a VM compiler has to be a trusted component. Keep in mind that 
> user code cannot get to jdk.vm.compiler.
 
 My question is not about granting fine-grained permissions vs 
 AllPermissions.  I expect jdk.vm.compiler is used with jdk.aot which does 
 not run with security manager.
 
 If jdk.vm.compiler is run with VM as JIT and with security manager, the 
 user can set -Djava.security.policy to a security policy configuring the 
 permission for jdk.vm.compiler.
 
 grant codeBase "jrt:/jdk.vm.compiler" {
  permission java.security.AllPermission;
 };
 
 If -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler are supported, the other 
 question I have is which loader jdk.vm.compiler should be defined?
>>> 
>>> I’m no expert on class loaders, but I would guess the same loader as 
>>> jdk.vm.ci.
>>> 
>>> -Doug



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-30 Thread Christian Thalinger

> On Jan 26, 2017, at 7:40 AM, Doug Simon  wrote:
> 
>> 
>> On 26 Jan 2017, at 17:55, Mandy Chung  wrote:
>> 
>> 
>>> On Jan 26, 2017, at 3:13 AM, Doug Simon  wrote:
>>> 
 
 jdk.vm.compiler is defined by the application class loader and it’s used 
 by AOT tool.  I wonder why it has to run with security manager.
>>> 
>>> Without java.security.AllPermission, the policy for jdk.vm.compiler 
>>> required to get through a bootstrap (i.e., java -server 
>>> -XX:+UnlockExperimentalVMOptions -Djava.security.manager 
>>> -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler -version) is show below 
>>> (annotated with comments denoting the methods requiring the permissions):
>>> 
>>> :
>> 
>> Are -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler supported to use at runtime?
> 
> Not sure I understand your question - they cannot be used at any other time 
> apart from runtime.

The question is if these command line options are supported by Oracle in JDK 9. 
 The answer used to be no but that might have changed.  Someone from Oracle 
needs to chime in.

Having said that, it would be a shame if we don’t make jdk.vm.compiler a 
trusted system component because it obviously is.

We have to do it at some point anyway so why not now…

> 
>>> There’s no guarantee that this is all the permissions required since not 
>>> all code paths are exercised during bootstrap.
>>> 
 You can reference JDK tools such as jdk.compiler and jdk.jlink that are 
 not granted with any permission.
>>> 
>>> Neither of those tools create code and install it in the VM. I don’t think 
>>> a fine grained SecurityManager policy makes sense for a VM compiler since 
>>> it could subvert security by compiling/installing malicious code. That is, 
>>> a VM compiler has to be a trusted component. Keep in mind that user code 
>>> cannot get to jdk.vm.compiler.
>> 
>> My question is not about granting fine-grained permissions vs 
>> AllPermissions.  I expect jdk.vm.compiler is used with jdk.aot which does 
>> not run with security manager.
>> 
>> If jdk.vm.compiler is run with VM as JIT and with security manager, the user 
>> can set -Djava.security.policy to a security policy configuring the 
>> permission for jdk.vm.compiler.
>> 
>> grant codeBase "jrt:/jdk.vm.compiler" {
>>  permission java.security.AllPermission;
>> };
>> 
>> If -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler are supported, the other 
>> question I have is which loader jdk.vm.compiler should be defined?
> 
> I’m no expert on class loaders, but I would guess the same loader as 
> jdk.vm.ci.
> 
> -Doug



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-30 Thread Mandy Chung

> On Jan 30, 2017, at 10:38 AM, Doug Simon  wrote:
> 
> I’ve extended the webrev with that change - please re-review:
> 
> http://cr.openjdk.java.net/~dnsimon/8145337_make/webrev
> 

+1

> 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

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-26 Thread Doug Simon

> On 26 Jan 2017, at 17:55, Mandy Chung  wrote:
> 
> 
>> On Jan 26, 2017, at 3:13 AM, Doug Simon  wrote:
>> 
>>> 
>>> jdk.vm.compiler is defined by the application class loader and it’s used by 
>>> AOT tool.  I wonder why it has to run with security manager.
>> 
>> Without java.security.AllPermission, the policy for jdk.vm.compiler required 
>> to get through a bootstrap (i.e., java -server 
>> -XX:+UnlockExperimentalVMOptions -Djava.security.manager -XX:+BootstrapJVMCI 
>> -XX:+UseJVMCICompiler -version) is show below (annotated with comments 
>> denoting the methods requiring the permissions):
>> 
>> :
> 
> Are -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler supported to use at runtime?

Not sure I understand your question - they cannot be used at any other time 
apart from runtime.

>> There’s no guarantee that this is all the permissions required since not all 
>> code paths are exercised during bootstrap.
>> 
>>> You can reference JDK tools such as jdk.compiler and jdk.jlink that are not 
>>> granted with any permission.
>> 
>> Neither of those tools create code and install it in the VM. I don’t think a 
>> fine grained SecurityManager policy makes sense for a VM compiler since it 
>> could subvert security by compiling/installing malicious code. That is, a VM 
>> compiler has to be a trusted component. Keep in mind that user code cannot 
>> get to jdk.vm.compiler.
> 
> My question is not about granting fine-grained permissions vs AllPermissions. 
>  I expect jdk.vm.compiler is used with jdk.aot which does not run with 
> security manager.
> 
> If jdk.vm.compiler is run with VM as JIT and with security manager, the user 
> can set -Djava.security.policy to a security policy configuring the 
> permission for jdk.vm.compiler.
> 
> grant codeBase "jrt:/jdk.vm.compiler" {
>   permission java.security.AllPermission;
> };
> 
> If -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler are supported, the other 
> question I have is which loader jdk.vm.compiler should be defined?

I’m no expert on class loaders, but I would guess the same loader as jdk.vm.ci.

-Doug

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-26 Thread Doug Simon

> On 26 Jan 2017, at 03:32, Mandy Chung  wrote:
> 
> (dropping jdk9-dev.  security-libs is the appropriate list to review security 
> permission)
> 
>> On Jan 23, 2017, at 1:56 PM, Doug Simon  wrote:
>> 
>> Both jdk.vm.ci and jdk.vm.compiler require a number of permissions when a 
>> security manager is present. Since neither of these modules is accessible to 
>> application code, it should be ok to give them all permissions. This seems 
>> to be the approach for a number of other modules including 
>> jdk.scripting.nashorn, jdk.dynalink, jdk.jsobject etc. Please review this 
>> small change that configures this proposed permission level.
>> 
>> http://cr.openjdk.java.net/~dnsimon/8145337/webrev/
>> https://bugs.openjdk.java.net/browse/JDK-8145337
> 
> jdk.vm.compiler is defined by the application class loader and it’s used by 
> AOT tool.  I wonder why it has to run with security manager.

Without java.security.AllPermission, the policy for jdk.vm.compiler required to 
get through a bootstrap (i.e., java -server -XX:+UnlockExperimentalVMOptions 
-Djava.security.manager -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler -version) is 
show below (annotated with comments denoting the methods requiring the 
permissions):

grant codeBase "jrt:/jdk.vm.compiler" {
// org.graalvm.compiler.hotspot.HotSpotGraalJVMCIServiceLocator.
permission jdk.vm.ci.services.JVMCIPermission;

// 
org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.getSavedProperties
permission java.lang.RuntimePermission 
"accessClassInPackage.jdk.internal.misc";
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";

// org.graalvm.compiler.core.common.util.ModuleAPI.
permission java.lang.RuntimePermission "accessDeclaredMembers";
permission java.lang.RuntimePermission 
"accessClassInPackage.jdk.internal.module";

// org.graalvm.compiler.options.OptionValue.
permission java.util.PropertyPermission "graal.profileOptionValue", "read";

// org.graalvm.compiler.debug.Debug.
permission java.util.PropertyPermission "*", "read,write";

// org.graalvm.compiler.core.common.UnsafeAccess.initUnsafe
permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";

// org.graalvm.compiler.hotspot.BootstrapWatchDog.
permission java.lang.RuntimePermission "modifyThread";
permission java.lang.RuntimePermission "modifyThreadGroup";

// org.graalvm.compiler.hotspot.replacements.SHA2Substitutions.
permission java.lang.RuntimePermission 
"accessClassInPackage.sun.security.provider";

// 
org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugins.resolveClass
permission java.lang.RuntimePermission 
"accessClassInPackage.jdk.internal.reflect";
permission java.lang.RuntimePermission 
"accessClassInPackage.oracle.jrockit.jfr.jdkevents";
};

There’s no guarantee that this is all the permissions required since not all 
code paths are exercised during bootstrap.

>  You can reference JDK tools such as jdk.compiler and jdk.jlink that are not 
> granted with any permission.

Neither of those tools create code and install it in the VM. I don’t think a 
fine grained SecurityManager policy makes sense for a VM compiler since it 
could subvert security by compiling/installing malicious code. That is, a VM 
compiler has to be a trusted component. Keep in mind that user code cannot get 
to jdk.vm.compiler.

For comparison, below are all the modules currently given 
java.security.AllPermission by lib/security/default.policy:

grant codeBase "jrt:/java.activation" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/java.compiler" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/java.corba" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.incubator.httpclient" {
};

grant codeBase "jrt:/java.scripting" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/java.security.jgss" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/java.sql" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/java.sql.rowset" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.dynalink" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.internal.le" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.jsobject" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.naming.dns" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.scripting.nashorn" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.scripting.nashorn.shell" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.security.auth" {
permission java.security.AllPermission;
};

grant codeBase "jrt:/jdk.security.jgss" {
permission java.security.AllPermission;
};



Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-26 Thread Mandy Chung

> On Jan 26, 2017, at 3:13 AM, Doug Simon  wrote:
> 
>> 
>> jdk.vm.compiler is defined by the application class loader and it’s used by 
>> AOT tool.  I wonder why it has to run with security manager.
> 
> Without java.security.AllPermission, the policy for jdk.vm.compiler required 
> to get through a bootstrap (i.e., java -server 
> -XX:+UnlockExperimentalVMOptions -Djava.security.manager -XX:+BootstrapJVMCI 
> -XX:+UseJVMCICompiler -version) is show below (annotated with comments 
> denoting the methods requiring the permissions):
> 
> :

Are -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler supported to use at runtime?

> There’s no guarantee that this is all the permissions required since not all 
> code paths are exercised during bootstrap.
> 
>> You can reference JDK tools such as jdk.compiler and jdk.jlink that are not 
>> granted with any permission.
> 
> Neither of those tools create code and install it in the VM. I don’t think a 
> fine grained SecurityManager policy makes sense for a VM compiler since it 
> could subvert security by compiling/installing malicious code. That is, a VM 
> compiler has to be a trusted component. Keep in mind that user code cannot 
> get to jdk.vm.compiler.

My question is not about granting fine-grained permissions vs AllPermissions.  
I expect jdk.vm.compiler is used with jdk.aot which does not run with security 
manager.

If jdk.vm.compiler is run with VM as JIT and with security manager, the user 
can set -Djava.security.policy to a security policy configuring the permission 
for jdk.vm.compiler.

grant codeBase "jrt:/jdk.vm.compiler" {
   permission java.security.AllPermission;
};

If -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler are supported, the other question 
I have is which loader jdk.vm.compiler should be defined?

Mandy

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-25 Thread Mandy Chung
(dropping jdk9-dev.  security-libs is the appropriate list to review security 
permission)

> On Jan 23, 2017, at 1:56 PM, Doug Simon  wrote:
> 
> Both jdk.vm.ci and jdk.vm.compiler require a number of permissions when a 
> security manager is present. Since neither of these modules is accessible to 
> application code, it should be ok to give them all permissions. This seems to 
> be the approach for a number of other modules including 
> jdk.scripting.nashorn, jdk.dynalink, jdk.jsobject etc. Please review this 
> small change that configures this proposed permission level.
> 
> http://cr.openjdk.java.net/~dnsimon/8145337/webrev/
> https://bugs.openjdk.java.net/browse/JDK-8145337

jdk.vm.compiler is defined by the application class loader and it’s used by AOT 
tool.  I wonder why it has to run with security manager.  You can reference JDK 
tools such as jdk.compiler and jdk.jlink that are not granted with any 
permission.

Mandy

RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

2017-01-25 Thread Doug Simon
Both jdk.vm.ci and jdk.vm.compiler require a number of permissions when a 
security manager is present. Since neither of these modules is accessible to 
application code, it should be ok to give them all permissions. This seems to 
be the approach for a number of other modules including jdk.scripting.nashorn, 
jdk.dynalink, jdk.jsobject etc. Please review this small change that configures 
this proposed permission level.

http://cr.openjdk.java.net/~dnsimon/8145337/webrev/
https://bugs.openjdk.java.net/browse/JDK-8145337

-Doug