Fwd: RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException
David Holmes
david.holmes at oracle.com
Fri May 26 01:04:56 UTC 2017
On 26/05/2017 4:55 AM, Martin Buchholz wrote:
> [+jdk8u-dev]
>
> We've been hunting the elusive spurious NPEs as well; the following seems
> to be working for us (but we don't have any small repro recipe); something
> like this should be put into jdk8:
In other words you want a backport of: 8061964: Insufficient compiler
barriers for GCC in OrderAccess functions …
https://bugs.openjdk.java.net/browse/JDK-8061964
IIRC what stopped this from being an 'automatic' backport candidate was
the potential problem of older gcc's needing to be validated.
Cheers,
David
-----
> --- hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2016-11-22
> 15:30:39.000000000 -0800
> +++ hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2017-04-27
> 18:12:33.000000000 -0700
> @@ -32,6 +32,11 @@
>
> // Implementation of class OrderAccess.
>
> +// A compiler barrier, forcing the C++ compiler to invalidate all memory
> assumptions
> +static inline void compiler_barrier() {
> + __asm__ volatile ("" : : : "memory");
> +}
> +
> inline void OrderAccess::loadload() { acquire(); }
> inline void OrderAccess::storestore() { release(); }
> inline void OrderAccess::loadstore() { acquire(); }
> @@ -47,9 +52,7 @@
> }
>
> inline void OrderAccess::release() {
> - // Avoid hitting the same cache-line from
> - // different threads.
> - volatile jint local_dummy = 0;
> + compiler_barrier();
> }
>
> inline void OrderAccess::fence() {
> @@ -63,34 +66,34 @@
> }
> }
>
> -inline jbyte OrderAccess::load_acquire(volatile jbyte* p) { return
> *p; }
> -inline jshort OrderAccess::load_acquire(volatile jshort* p) { return
> *p; }
> -inline jint OrderAccess::load_acquire(volatile jint* p) { return
> *p; }
> -inline jlong OrderAccess::load_acquire(volatile jlong* p) { return
> Atomic::load(p); }
> -inline jubyte OrderAccess::load_acquire(volatile jubyte* p) { return
> *p; }
> -inline jushort OrderAccess::load_acquire(volatile jushort* p) { return
> *p; }
> -inline juint OrderAccess::load_acquire(volatile juint* p) { return
> *p; }
> -inline julong OrderAccess::load_acquire(volatile julong* p) { return
> Atomic::load((volatile jlong*)p); }
> -inline jfloat OrderAccess::load_acquire(volatile jfloat* p) { return
> *p; }
> -inline jdouble OrderAccess::load_acquire(volatile jdouble* p) { return
> jdouble_cast(Atomic::load((volatile jlong*)p)); }
> -
> -inline intptr_t OrderAccess::load_ptr_acquire(volatile intptr_t* p) {
> return *p; }
> -inline void* OrderAccess::load_ptr_acquire(volatile void* p) {
> return *(void* volatile *)p; }
> -inline void* OrderAccess::load_ptr_acquire(const volatile void* p) {
> return *(void* const volatile *)p; }
> -
> -inline void OrderAccess::release_store(volatile jbyte* p, jbyte v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile jshort* p, jshort v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile jint* p, jint v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile jlong* p, jlong v)
> { Atomic::store(v, p); }
> -inline void OrderAccess::release_store(volatile jubyte* p, jubyte v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile jushort* p, jushort v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile juint* p, juint v)
> { *p = v; }
> -inline void OrderAccess::release_store(volatile julong* p, julong v)
> { Atomic::store((jlong)v, (volatile jlong*)p); }
> -inline void OrderAccess::release_store(volatile jfloat* p, jfloat v)
> { *p = v; }
> +inline jbyte OrderAccess::load_acquire(volatile jbyte* p) { jbyte v
> = *p; compiler_barrier(); return v; }
> +inline jshort OrderAccess::load_acquire(volatile jshort* p) { jshort v
> = *p; compiler_barrier(); return v; }
> +inline jint OrderAccess::load_acquire(volatile jint* p) { jint v
> = *p; compiler_barrier(); return v; }
> +inline jlong OrderAccess::load_acquire(volatile jlong* p) { jlong v
> = Atomic::load(p); compiler_barrier(); return v; }
> +inline jubyte OrderAccess::load_acquire(volatile jubyte* p) { jubyte v
> = *p; compiler_barrier(); return v; }
> +inline jushort OrderAccess::load_acquire(volatile jushort* p) { jushort v
> = *p; compiler_barrier(); return v; }
> +inline juint OrderAccess::load_acquire(volatile juint* p) { juint v
> = *p; compiler_barrier(); return v; }
> +inline julong OrderAccess::load_acquire(volatile julong* p) { julong v
> = Atomic::load((volatile jlong*)p); compiler_barrier(); return v; }
> +inline jfloat OrderAccess::load_acquire(volatile jfloat* p) { jfloat v
> = *p; compiler_barrier(); return v; }
> +inline jdouble OrderAccess::load_acquire(volatile jdouble* p) { jdouble v
> = jdouble_cast(Atomic::load((volatile jlong*)p)); compiler_barrier();
> return v; }
> +
> +inline intptr_t OrderAccess::load_ptr_acquire(volatile intptr_t* p) {
> intptr_t v = *p; compiler_barrier(); return v; }
> +inline void* OrderAccess::load_ptr_acquire(volatile void* p) {
> void* v = *(void* volatile *)p; compiler_barrier(); return v; }
> +inline void* OrderAccess::load_ptr_acquire(const volatile void* p) {
> void* v = *(void* const volatile *)p; compiler_barrier(); return v; }
> +
> +inline void OrderAccess::release_store(volatile jbyte* p, jbyte v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile jshort* p, jshort v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile jint* p, jint v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile jlong* p, jlong v)
> { compiler_barrier(); Atomic::store(v, p); }
> +inline void OrderAccess::release_store(volatile jubyte* p, jubyte v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile jushort* p, jushort v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile juint* p, juint v)
> { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store(volatile julong* p, julong v)
> { compiler_barrier(); Atomic::store((jlong)v, (volatile jlong*)p); }
> +inline void OrderAccess::release_store(volatile jfloat* p, jfloat v)
> { compiler_barrier(); *p = v; }
> inline void OrderAccess::release_store(volatile jdouble* p, jdouble v)
> { release_store((volatile jlong *)p, jlong_cast(v)); }
>
> -inline void OrderAccess::release_store_ptr(volatile intptr_t* p,
> intptr_t v) { *p = v; }
> -inline void OrderAccess::release_store_ptr(volatile void* p, void*
> v) { *(void* volatile *)p = v; }
> +inline void OrderAccess::release_store_ptr(volatile intptr_t* p,
> intptr_t v) { compiler_barrier(); *p = v; }
> +inline void OrderAccess::release_store_ptr(volatile void* p, void*
> v) { compiler_barrier(); *(void* volatile *)p = v; }
>
> inline void OrderAccess::store_fence(jbyte* p, jbyte v) {
> __asm__ volatile ( "xchgb (%2),%0"
>
>
> On Thu, May 25, 2017 at 9:25 AM, Andrew Dinn <adinn at redhat.com> wrote:
>
>> Apologies but this RFR is retracted -- the problem only applies to jdk8.
>>
>> I will be posting a revised RFR to jdk8u.
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>> Senior Principal Software Engineer
>> Red Hat UK Ltd
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>>
>> On 25/05/17 14:16, Andrew Dinn wrote:
>>> Forwarding this to hotpsot-dev which is probably the more appropriate
>>> destination.
>>>
>>>
>>> -------- Forwarded Message --------
>>> Subject: RFR: 8181085: Race condition in method resolution may produce
>>> spurious NullPointerException
>>> Date: Thu, 25 May 2017 14:12:53 +0100
>>> From: Andrew Dinn <adinn at redhat.com>
>>> To: jdk10-dev <jdk10-dev at openjdk.java.net>
>>>
>>> The following webrev fixes a race condition that is present in jdk10 and
>>> also jdk9 and jdk8. It is caused by a misplaced volatile keyword that
>>> faild to ensure correct ordering of writes by the compiler. Reviews
>> welcome.
>>>
>>> http://cr.openjdk.java.net/~adinn/8181085/webrev.00/
>>>
>>> Backporting:
>>> This same fix is required in jdk9 and jdk8.
>>>
>>> Testing:
>>> The reproducer posted with the original issue manifests the NPE reliably
>>> on jdk8. It does not manifest on jdk9/10 but that is only thanks to
>>> changes introduced into the resolution process in jdk9 which change the
>>> timing of execution. However, without this fix the out-of-order write
>>> problem is still present in jdk9/10, as can be seen by eyeballing the
>>> compiled code for ConstantPoolCacheEntry::set_direct_or_vtable_call.
>>>
>>> The patch has been validated on jdk8 by running the reproducer. It stops
>>> any resulting NPEs.
>>>
>>> The code for ConstantPoolCacheEntry::set_direct_or_vtable_call on
>>> jdk8-10 has been eyeballed to ensure that post-patch the assignments now
>>> occur in the correct order.
>>>
>>> regards,
>>>
>>>
>>> Andrew Dinn
>>> -----------
>>> Senior Principal Software Engineer
>>> Red Hat UK Ltd
>>> Registered in England and Wales under Company Registration No. 03798903
>>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>>>
>>>
>>
>>
More information about the hotspot-dev
mailing list