stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8335390: C2 MergeStores: wrong result with Unsafe

Browse Source 
Reviewed-by: thartmann, chagedorn, kvn
This commit is contained in:
Emanuel Peter 2024-07-02 08:20:26 +00:00
parent 318d9acadf
commit 9046d7aee3
3 changed files with 141 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/hotspot/share/opto/memnode.cpp
View File

@ -2984,6 +2984,9 @@ StoreNode* MergePrimitiveArrayStores::run() {
type2aelembytes(bt) != _store->memory_size()) {
return nullptr;
}
if (_store->is_unsafe_access()) {
return nullptr;
}
// The _store must be the "last" store in a chain. If we find a use we could merge with
// then that use or a store further down is the "last" store.
@ -3017,11 +3020,13 @@ bool MergePrimitiveArrayStores::is_compatible_store(const StoreNode* other_store
int opc = _store->Opcode();
assert(opc == Op_StoreB || opc == Op_StoreC || opc == Op_StoreI, "precondition");
assert(_store->adr_type()->isa_aryptr() != nullptr, "must be array store");
assert(!_store->is_unsafe_access(), "no unsafe accesses");
if (other_store == nullptr ||
_store->Opcode() != other_store->Opcode() ||
other_store->adr_type() == nullptr ||
other_store->adr_type()->isa_aryptr() == nullptr) {
other_store->adr_type()->isa_aryptr() == nullptr ||
other_store->is_unsafe_access()) {
return false;
}

5
test/hotspot/jtreg/compiler/c2/TestMergeStores.java
View File

@ -611,8 +611,9 @@ public class TestMergeStores {
}
@Test
@IR(counts = {IRNode.STORE_L_OF_CLASS, "byte\\\\[int:>=0] \\\\(java/lang/Cloneable,java/io/Serializable\\\\)", "1"},
applyIf = {"UseUnalignedAccesses", "true"})
// Disabled by JDK-8335390, to be enabled again by JDK-8335392.
// @IR(counts = {IRNode.STORE_L_OF_CLASS, "byte\\\\[int:>=0] \\\\(java/lang/Cloneable,java/io/Serializable\\\\)", "1"},
// applyIf = {"UseUnalignedAccesses", "true"})
static Object[] test1f(byte[] a) {
UNSAFE.putByte(a, UNSAFE.ARRAY_BYTE_BASE_OFFSET + 0, (byte)0xbe);
UNSAFE.putByte(a, UNSAFE.ARRAY_BYTE_BASE_OFFSET + 1, (byte)0xba);

132
test/hotspot/jtreg/compiler/c2/TestMergeStoresUnsafeArrayPointer.java Normal file
View File

@ -0,0 +1,132 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8335390
* @summary Test merge stores for some Unsafe store address patterns.
* @modules java.base/jdk.internal.misc
* @requires vm.bits == 64
* @requires os.maxMemory > 8G
* @run main/othervm -XX:CompileCommand=compileonly,compiler.c2.TestMergeStoresUnsafeArrayPointer::test*
* -Xbatch
* -Xmx8g
* compiler.c2.TestMergeStoresUnsafeArrayPointer
* @run main/othervm -Xmx8g
* compiler.c2.TestMergeStoresUnsafeArrayPointer
*/
package compiler.c2;
import jdk.internal.misc.Unsafe;
public class TestMergeStoresUnsafeArrayPointer {
static final Unsafe UNSAFE = Unsafe.getUnsafe();
// We allocate a big int array of length:
static final int SIZE = (1 << 30) + 100;
// This gives us a memory region of 4x as many bytes:
static final long BYTE_SIZE = 4L * SIZE; // = 1L << 32 + 400L
// We set an "anchor" in the middle of this memory region, in bytes:
static final long ANCHOR = BYTE_SIZE / 2;
static int four = 4;
public static void main(String[] args) {
System.out.println("Allocate big array of SIZE = " + SIZE);
int[] big = new int[SIZE];
// Each test is executed a few times, so that we can see the difference between
// interpreter and compiler.
int errors = 0;
long val = 0;
System.out.println("test1");
for (int i = 0; i < 100_000; i++) {
testClear(big);
test1(big, ANCHOR);
long sum = testSum(big);
if (i == 0) {
val = sum;
} else {
if (sum != val) {
System.out.println("ERROR: test1 had wrong value: " + val + " != " + sum);
errors++;
break;
}
}
}
val = 0;
System.out.println("test2");
for (int i = 0; i < 100_000; i++) {
testClear(big);
test2(big, ANCHOR);
long sum = testSum(big);
if (i == 0) {
val = sum;
} else {
if (sum != val) {
System.out.println("ERROR: test2 had wrong value: " + val + " != " + sum);
errors++;
break;
}
}
}
if (errors > 0) {
throw new RuntimeException("ERRORS: " + errors);
}
System.out.println("PASSED");
}
// Only clear and sum over relevant parts of array to make the test fast.
static void testClear(int[] a) {
for (int j = 0 ; j < 100; j++) { a[j] = j; }
for (int j = a.length/2 - 100; j < a.length/2 + 100; j++) { a[j] = j; }
for (int j = a.length - 100; j < a.length + 0; j++) { a[j] = j; }
}
static long testSum(int[] a) {
long sum = 0;
for (int j = 0 ; j < 100; j++) { sum += a[j]; }
for (int j = a.length/2 - 100; j < a.length/2 + 100; j++) { sum += a[j]; }
for (int j = a.length - 100; j < a.length + 0; j++) { sum += a[j]; }
return sum;
}
// Reference: expected to merge.
static void test1(int[] a, long anchor) {
long base = UNSAFE.ARRAY_INT_BASE_OFFSET + anchor;
UNSAFE.putInt(a, base + 0, 0x42424242);
UNSAFE.putInt(a, base + 4, 0x66666666);
}
// Test: if MergeStores is applied this can lead to wrong results
static void test2(int[] a, long anchor) {
long base = UNSAFE.ARRAY_INT_BASE_OFFSET + ANCHOR;
UNSAFE.putInt(a, base + 0 + (long)(four + Integer.MAX_VALUE), 0x42424242);
UNSAFE.putInt(a, base + Integer.MAX_VALUE + (long)(four + 4 ), 0x66666666);
}
}