12 RFR(M) 8214583: AccessController.getContext may return wrong value after JDK-8212605

[David Holmes]

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?

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.

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
>