RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException

[David Holmes]

On 26/05/2017 6:29 AM, Paul Hohensee wrote:
> I don't know that you want to retract the patch. There's still a bug here
> imo that your patch fixes.

I agree. This is a common error when dealing with pointer variables, 
especially when looking at surrounding usage on non-pointer variables. 
We need the _f1 pointer to be volatile, not the thing to which the _f1 
pointer points (well it's possible we may need both, I haven't dived 
that deep).

Any variable passed to an OrderAccess, or Atomic, function should be 
volatile to minimise the chances the C compiler will do something 
unexpected with it.

I don't even know what to make of the vmStructs.cpp existing code!

David

> The pointer formal parameter types for all orderAccess methods are volatile
> in order to force the compiler, at the point of method invocation, to order
> the memory access through the pointer within the method with respect to
> other volatile accesses. Accesses in the caller won't be ordered by the
> compiler with respect to the access in the orderAccess method unless the
> caller accesses are also volatile. And that's the bug.
> 
> If we want the compiler to not reorder accesses to _f1, _f1 must be
> declared volatile.
> 
> volatile MethodData* _f1;
> 
> says that the MethodData is volatile (i.e., all accesses to parts of the
> MethodData object are volatile), which isn't what we want if we're intent
> on ordering with respect to accesses to _f1.
> 
> MethodData* volatile _f1;
> 
> is the way to do that.
> 
> Thanks,
> 
> Paul
> 
> On Thu, May 25, 2017 at 8:33 AM, Andrew Haley <aph at redhat.com> wrote:
> 
>> On 25/05/17 16:03, Andrew Dinn wrote:
>>> The jdk10 implementation of release_Store_ptr has indeed already been
>>> reworked to insert a compiler barrier (using "asm volatile memory") but
>>> not a memory store barrier.
>>
>> Cool.  An asm volatile memory barrier is a bit stronger than is
>> perhaps needed, but it almost certainly will make no difference,
>> and is compatible with old releases of GCC.
>>
>> Andrew.
>>