12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605
David Holmes
david.holmes at oracle.com
Tue Dec 18 22:19:42 UTC 2018
On 19/12/2018 6:52 am, dean.long at oracle.com wrote:
> David, can I list you as a reviewer?
No, sorry, I only commented on the general issue.
David
> dl
>
> On 12/16/18 8:47 PM, dean.long at oracle.com wrote:
>> On 12/16/18 7:39 PM, dean.long at oracle.com wrote:
>>> On 12/16/18 7:03 PM, David Holmes wrote:
>>>> On 17/12/2018 12:49 pm, dean.long at oracle.com wrote:
>>>>> On 12/16/18 4:06 PM, David Holmes wrote:
>>>>>> On 15/12/2018 10:59 am, dean.long at oracle.com wrote:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8214583
>>>>>>> http://cr.openjdk.java.net/~dlong/8214583/webrev
>>>>>>>
>>>>>>> This change includes two new regression test that demonstrate the
>>>>>>> problem, and a fix that allows the tests
>>>>>>> to pass.
>>>>>>>
>>>>>>> The problem happens when the JIT compiler's escape analysis
>>>>>>> eliminates the allocation of the AccessControlContext object
>>>>>>> passed to doPrivileged. The compiler thinks this is safe because
>>>>>>> it does not see that the object "escapes".
>>>>>>
>>>>>> Then surely the compiler's notion of "escapes" needs to be updated!
>>>>>>
>>>>>
>>>>> The compiler can inline the callee method and see that the value
>>>>> doesn't escape. This is a valid optimization in cases where the
>>>>> callee method is known.
>>>>
>>>> But it's not a valid optimization in this case, so my comment stands.
>>>>
>>>> Is this stack walking something this is guaranteed by the spec to be
>>>> always valid (and hence the JIT is violating the rules), or is the
>>>> stack walking code making assumptions about whether it will find the
>>>> context object in the stack?
>>>>
>>>
>>> The stack walking is in the VM and is an internal implementation
>>> detail, not part of the AccessController API spec. A different
>>> thread running normal Java code would never be able to see a
>>> non-escaping value. The stack walking code does need to find the
>>> context object in the stack. Non-escaping objects won't show up in
>>> the stack.
>>>
>>>> If we have to hack around this with an annotation I'd rather see a
>>>> specific annotation that addresses the problematic usecase than a
>>>> generic "don't inline" one. E.g. @StackVisible or something like that.
>>>>
>>>
>>> That sounds like a good idea for 13, but would require changes to
>>> both C2 and Graal, and it seems a little risky compared to using
>>> existing mechanisms.
>>>
>>
>> I forgot to address this in my last reply, but I'm not suggesting a
>> @DontInline annotation. That was Claes. My fixes uses a native method.
>>
>> dl
>>
>>> dl
>>>
>>>> Cheers,
>>>> David
>>>>
>>>>>
>>>>> dl
>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>> However, getContext needs to be able to find
>>>>>>> the object using a stack walk, so we need a way to tell the
>>>>>>> compiler that it does indeed escape. To do this we pass the value
>>>>>>> to a native method that does nothing.
>>>>>>>
>>>>>>> Microbenchmark results:
>>>>>>>
>>>>>>> jdk12-b18:
>>>>>>>
>>>>>>> Benchmark Mode Cnt Score Error Units
>>>>>>> DoPrivileged.test avgt 25 255.626 ± 6.446 ns/op
>>>>>>> DoPrivileged.testInline avgt 25 250.968 ± 4.975 ns/op
>>>>>>>
>>>>>>>
>>>>>>> jdk12-b19:
>>>>>>>
>>>>>>> Benchmark Mode Cnt Score Error Units
>>>>>>> DoPrivileged.test avgt 25 5.689 ± 0.001 ns/op
>>>>>>> DoPrivileged.testInline avgt 25 2.765 ± 0.001 ns/op
>>>>>>>
>>>>>>> this fix:
>>>>>>>
>>>>>>> Benchmark Mode Cnt Score Error Units
>>>>>>> DoPrivileged.test avgt 25 5.020 ± 0.001 ns/op
>>>>>>> DoPrivileged.testInline avgt 25 2.774 ± 0.025 ns/op
>>>>>>>
>>>>>>>
>>>>>>> dl
>>>>>
>>>
>>
>
More information about the security-dev
mailing list