OpenJDK / amber / amber
changeset 40669:252f9d8272af
8157904: Atomic::cmpxchg for jbyte is missing a fence on initial failure
Reviewed-by: simonis, aph, kbarrett
author | dholmes |
---|---|
date | Wed, 24 Aug 2016 19:54:03 -0400 |
parents | 60d799be5dda |
children | 76bf2fc655cd f20df9375ac9 |
files | hotspot/src/share/vm/runtime/atomic.hpp hotspot/src/share/vm/utilities/globalDefinitions.hpp |
diffstat | 2 files changed, 36 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- a/hotspot/src/share/vm/runtime/atomic.hpp Wed Aug 24 20:38:21 2016 +0200 +++ b/hotspot/src/share/vm/runtime/atomic.hpp Wed Aug 24 19:54:03 2016 -0400 @@ -150,26 +150,39 @@ * as well as defining VM_HAS_SPECIALIZED_CMPXCHG_BYTE. This will cause the platform specific * implementation to be used instead. */ -inline jbyte Atomic::cmpxchg(jbyte exchange_value, volatile jbyte *dest, jbyte comparand, cmpxchg_memory_order order) -{ - assert(sizeof(jbyte) == 1, "assumption."); - uintptr_t dest_addr = (uintptr_t)dest; - uintptr_t offset = dest_addr % sizeof(jint); - volatile jint* dest_int = (volatile jint*)(dest_addr - offset); +inline jbyte Atomic::cmpxchg(jbyte exchange_value, volatile jbyte* dest, + jbyte compare_value, cmpxchg_memory_order order) { + STATIC_ASSERT(sizeof(jbyte) == 1); + volatile jint* dest_int = + static_cast<volatile jint*>(align_ptr_down(dest, sizeof(jint))); + size_t offset = pointer_delta(dest, dest_int, 1); jint cur = *dest_int; - jbyte* cur_as_bytes = (jbyte*)(&cur); - jint new_val = cur; - jbyte* new_val_as_bytes = (jbyte*)(&new_val); - new_val_as_bytes[offset] = exchange_value; - while (cur_as_bytes[offset] == comparand) { - jint res = cmpxchg(new_val, dest_int, cur, order); - if (res == cur) break; + jbyte* cur_as_bytes = reinterpret_cast<jbyte*>(&cur); + + // current value may not be what we are looking for, so force it + // to that value so the initial cmpxchg will fail if it is different + cur_as_bytes[offset] = compare_value; + + // always execute a real cmpxchg so that we get the required memory + // barriers even on initial failure + do { + // value to swap in matches current value ... + jint new_value = cur; + // ... except for the one jbyte we want to update + reinterpret_cast<jbyte*>(&new_value)[offset] = exchange_value; + + jint res = cmpxchg(new_value, dest_int, cur, order); + if (res == cur) break; // success + + // at least one jbyte in the jint changed value, so update + // our view of the current jint cur = res; - new_val = cur; - new_val_as_bytes[offset] = exchange_value; - } + // if our jbyte is still as cur we loop and try again + } while (cur_as_bytes[offset] == compare_value); + return cur_as_bytes[offset]; } + #endif // VM_HAS_SPECIALIZED_CMPXCHG_BYTE inline unsigned Atomic::xchg(unsigned int exchange_value, volatile unsigned int* dest) {
--- a/hotspot/src/share/vm/utilities/globalDefinitions.hpp Wed Aug 24 20:38:21 2016 +0200 +++ b/hotspot/src/share/vm/utilities/globalDefinitions.hpp Wed Aug 24 19:54:03 2016 -0400 @@ -327,11 +327,12 @@ // and then additions like // ... top() + size ... // are safe because we know that top() is at least size below end(). -inline size_t pointer_delta(const void* left, - const void* right, +inline size_t pointer_delta(const volatile void* left, + const volatile void* right, size_t element_size) { return (((uintptr_t) left) - ((uintptr_t) right)) / element_size; } + // A version specialized for HeapWord*'s. inline size_t pointer_delta(const HeapWord* left, const HeapWord* right) { return pointer_delta(left, right, sizeof(HeapWord)); @@ -516,6 +517,10 @@ return (void*)align_size_down((intptr_t)ptr, (intptr_t)alignment); } +inline volatile void* align_ptr_down(volatile void* ptr, size_t alignment) { + return (volatile void*)align_size_down((intptr_t)ptr, (intptr_t)alignment); +} + // Align metaspace objects by rounding up to natural word boundary inline intptr_t align_metadata_size(intptr_t size) {