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

[Claes Redestad]

Fair enough, I wasn't aware EA was looking beyond the inlined code like
this (which means it can't be "dead" or the JIT might see through the
trick at some point). I've skimmed the existing usages in the JDK and
can't find anything that seems to be dependent on DontInline defeating
EA - maybe there was in the past, or I simply misremembered.

Either way, it might be nice to have a more explicit facility for this
in a future release, as David suggested. Say an @Escaping local
variable/parameter annotation.

/Claes

On 2018-12-17 05:45, dean.long at oracle.com wrote:
> Unfortunately, I don't think @DontInline on an empty method is sufficient
> here.  If other code is relying on @DontInline for the same purpose then
> we might need to reexamine that code.  My understanding from discussing
> with other compiler engineers is that using a native method is the safest
> technique that the compilers can't see through.  The problem with
> @DontInline is that C2 looks at the bytecodes of the target method, even
> if it isn't inlined (see BCEscapeAnalyzer and the EstimateArgEscape flag).
> There may be a way to make it work, but that would require more
> investigation, and I'm not sure the benefit outweighs the risk.
> 
> dl
> 
> On 12/15/18 6:48 AM, Claes Redestad wrote:
>> Hi Dean,
>>
>> to avoid escape analysis-eliminated allocations in the past @DontInline
>> has been sufficient. This means a simpler patch (no changes to native
>> code needed - added assertions notwithstanding) and passes your tests
>> with C2 (it'd concern me if Graal's EA sees through this trick, as it
>> might break some existing places where DontInline is used to this
>> effect):
>>
>>     /**
>>      * The value needs to be physically located in the frame, so that it
>>      * can be found by a stack walk.
>>      */
>>     @Hidden
>>     @DontInline
>>     private static void ensureMaterializedForStackWalk(Object o) {}
>>
>> Thanks!
>>
>> /Claes
>>
>> On 2018-12-15 01:59, 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".  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
>