From 0bad445941832843531331d8b0204eaa5e3243b1 Mon Sep 17 00:00:00 2001 From: Paul Sandoz Date: Wed, 18 May 2016 18:46:14 +0200 Subject: [PATCH] 8157152: Atomic add for VarHandle byte[]/ByteBuffer views is incorrect for endian conversion Reviewed-by: fyuan, shade, vlivanov --- .../X-VarHandleByteArrayView.java.template | 72 ++++++++++++------- .../VarHandleTestByteArrayAsChar.java | 2 +- .../VarHandleTestByteArrayAsDouble.java | 2 +- .../VarHandleTestByteArrayAsFloat.java | 2 +- .../VarHandleTestByteArrayAsInt.java | 2 +- .../VarHandleTestByteArrayAsLong.java | 2 +- .../VarHandleTestByteArrayAsShort.java | 2 +- .../invoke/VarHandles/generate-vh-tests.sh | 16 +++-- 8 files changed, 63 insertions(+), 37 deletions(-) diff --git a/jdk/src/java.base/share/classes/java/lang/invoke/X-VarHandleByteArrayView.java.template b/jdk/src/java.base/share/classes/java/lang/invoke/X-VarHandleByteArrayView.java.template index a24e6c82186..66bf267476e 100644 --- a/jdk/src/java.base/share/classes/java/lang/invoke/X-VarHandleByteArrayView.java.template +++ b/jdk/src/java.base/share/classes/java/lang/invoke/X-VarHandleByteArrayView.java.template @@ -271,22 +271,33 @@ final class VarHandleByteArrayAs$Type$s extends VarHandleByteArrayBase { #if[AtomicAdd] @ForceInline - static $type$ getAndAdd(ArrayHandle handle, Object oba, int index, $type$ value) { + static $type$ getAndAdd(ArrayHandle handle, Object oba, int index, $type$ delta) { byte[] ba = (byte[]) oba; - return convEndian(handle.be, - UNSAFE.getAndAdd$RawType$( - ba, - address(ba, index(ba, index)), - convEndian(handle.be, value))); + if (handle.be == BE) { + return UNSAFE.getAndAdd$RawType$( + ba, + address(ba, index(ba, index)), + delta); + } else { + return getAndAddConvEndianWithCAS(ba, index, delta); + } } @ForceInline - static $type$ addAndGet(ArrayHandle handle, Object oba, int index, $type$ value) { - byte[] ba = (byte[]) oba; - return convEndian(handle.be, UNSAFE.getAndAdd$RawType$( - ba, - address(ba, index(ba, index)), - convEndian(handle.be, value))) + value; + static $type$ getAndAddConvEndianWithCAS(byte[] ba, int index, $type$ delta) { + $type$ nativeExpectedValue, expectedValue; + long offset = address(ba, index(ba, index)); + do { + nativeExpectedValue = UNSAFE.get$RawType$Volatile(ba, offset); + expectedValue = $RawBoxType$.reverseBytes(nativeExpectedValue); + } while (!UNSAFE.weakCompareAndSwap$RawType$Volatile(ba, offset, + nativeExpectedValue, $RawBoxType$.reverseBytes(expectedValue + delta))); + return expectedValue; + } + + @ForceInline + static $type$ addAndGet(ArrayHandle handle, Object oba, int index, $type$ delta) { + return getAndAdd(handle, oba, index, delta) + delta; } #end[AtomicAdd] @@ -503,23 +514,34 @@ final class VarHandleByteArrayAs$Type$s extends VarHandleByteArrayBase { #if[AtomicAdd] @ForceInline - static $type$ getAndAdd(ByteBufferHandle handle, Object obb, int index, $type$ value) { + static $type$ getAndAdd(ByteBufferHandle handle, Object obb, int index, $type$ delta) { ByteBuffer bb = (ByteBuffer) obb; - return convEndian(handle.be, - UNSAFE.getAndAdd$RawType$( - UNSAFE.getObject(bb, BYTE_BUFFER_HB), - address(bb, indexRO(bb, index)), - convEndian(handle.be, value))); + if (handle.be == BE) { + return UNSAFE.getAndAdd$RawType$( + UNSAFE.getObject(bb, BYTE_BUFFER_HB), + address(bb, indexRO(bb, index)), + delta); + } else { + return getAndAddConvEndianWithCAS(bb, index, delta); + } } @ForceInline - static $type$ addAndGet(ByteBufferHandle handle, Object obb, int index, $type$ value) { - ByteBuffer bb = (ByteBuffer) obb; - return convEndian(handle.be, - UNSAFE.getAndAdd$RawType$( - UNSAFE.getObject(bb, BYTE_BUFFER_HB), - address(bb, indexRO(bb, index)), - convEndian(handle.be, value))) + value; + static $type$ getAndAddConvEndianWithCAS(ByteBuffer bb, int index, $type$ delta) { + $type$ nativeExpectedValue, expectedValue; + Object base = UNSAFE.getObject(bb, BYTE_BUFFER_HB); + long offset = address(bb, indexRO(bb, index)); + do { + nativeExpectedValue = UNSAFE.get$RawType$Volatile(base, offset); + expectedValue = $RawBoxType$.reverseBytes(nativeExpectedValue); + } while (!UNSAFE.weakCompareAndSwap$RawType$Volatile(base, offset, + nativeExpectedValue, $RawBoxType$.reverseBytes(expectedValue + delta))); + return expectedValue; + } + + @ForceInline + static $type$ addAndGet(ByteBufferHandle handle, Object obb, int index, $type$ delta) { + return getAndAdd(handle, obb, index, delta) + delta; } #end[AtomicAdd] diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java index 806dd1ce4f8..a2221b50a20 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsChar extends VarHandleBaseByteArrayTest { static final char VALUE_2 = (char)0x1112; - static final char VALUE_3 = (char)0x2122; + static final char VALUE_3 = (char)0xFFFE; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsDouble.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsDouble.java index ccb0028c472..79305710ce2 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsDouble.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsDouble.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsDouble extends VarHandleBaseByteArrayTest { static final double VALUE_2 = 0x1112131415161718L; - static final double VALUE_3 = 0x2122232425262728L; + static final double VALUE_3 = 0xFFFEFDFCFBFAF9F8L; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsFloat.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsFloat.java index 2ff6427aec7..b3545b4f69c 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsFloat.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsFloat.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsFloat extends VarHandleBaseByteArrayTest { static final float VALUE_2 = 0x11121314; - static final float VALUE_3 = 0x21222324; + static final float VALUE_3 = 0xFFFEFDFC; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsInt.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsInt.java index 3087a14f049..b733bb34054 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsInt.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsInt.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsInt extends VarHandleBaseByteArrayTest { static final int VALUE_2 = 0x11121314; - static final int VALUE_3 = 0x21222324; + static final int VALUE_3 = 0xFFFEFDFC; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsLong.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsLong.java index 8388a9cdd6c..88bf198272b 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsLong.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsLong.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsLong extends VarHandleBaseByteArrayTest { static final long VALUE_2 = 0x1112131415161718L; - static final long VALUE_3 = 0x2122232425262728L; + static final long VALUE_3 = 0xFFFEFDFCFBFAF9F8L; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java index b13bce1e1da..5ed9d6099f7 100644 --- a/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java +++ b/jdk/test/java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java @@ -51,7 +51,7 @@ public class VarHandleTestByteArrayAsShort extends VarHandleBaseByteArrayTest { static final short VALUE_2 = (short)0x1112; - static final short VALUE_3 = (short)0x2122; + static final short VALUE_3 = (short)0xFFFE; @Override diff --git a/jdk/test/java/lang/invoke/VarHandles/generate-vh-tests.sh b/jdk/test/java/lang/invoke/VarHandles/generate-vh-tests.sh index 7d6afc7b206..436d2d371d9 100644 --- a/jdk/test/java/lang/invoke/VarHandles/generate-vh-tests.sh +++ b/jdk/test/java/lang/invoke/VarHandles/generate-vh-tests.sh @@ -113,36 +113,40 @@ do ;; esac + # The value of `value3` is chosen such that when added to `value1` or `value2` + # it will result in carrying of bits over to the next byte, thereby detecting + # possible errors in endianness conversion e.g. if say for atomic addition the + # augend is incorrectly processed case $type in short) value1=(short)0x0102 value2=(short)0x1112 - value3=(short)0x2122 + value3=(short)0xFFFE ;; char) value1=(char)0x0102 value2=(char)0x1112 - value3=(char)0x2122 + value3=(char)0xFFFE ;; int) value1=0x01020304 value2=0x11121314 - value3=0x21222324 + value3=0xFFFEFDFC ;; long) value1=0x0102030405060708L value2=0x1112131415161718L - value3=0x2122232425262728L + value3=0xFFFEFDFCFBFAF9F8L ;; float) value1=0x01020304 value2=0x11121314 - value3=0x21222324 + value3=0xFFFEFDFC ;; double) value1=0x0102030405060708L value2=0x1112131415161718L - value3=0x2122232425262728L + value3=0xFFFEFDFCFBFAF9F8L ;; esac