diff --git a/src/hotspot/share/opto/memnode.cpp b/src/hotspot/share/opto/memnode.cpp index 36de7732a55..587a6778d80 100644 --- a/src/hotspot/share/opto/memnode.cpp +++ b/src/hotspot/share/opto/memnode.cpp @@ -596,7 +596,7 @@ Node* LoadNode::find_previous_arraycopy(PhaseTransform* phase, Node* ld_alloc, N ArrayCopyNode* MemNode::find_array_copy_clone(PhaseTransform* phase, Node* ld_alloc, Node* mem) const { if (mem->is_Proj() && mem->in(0) != NULL && (mem->in(0)->Opcode() == Op_MemBarStoreStore || - mem->in(0)->Opcode() == Op_MemBarCPUOrder)) { + mem->in(0)->Opcode() == Op_MemBarCPUOrder)) { if (ld_alloc != NULL) { // Check if there is an array copy for a clone Node* mb = mem->in(0); @@ -1061,7 +1061,6 @@ Node* MemNode::can_see_stored_value(Node* st, PhaseTransform* phase) const { // This is more general than load from boxing objects. if (skip_through_membars(atp, tp, phase->C->eliminate_boxing())) { uint alias_idx = atp->index(); - bool final = !atp->is_rewritable(); Node* result = NULL; Node* current = st; // Skip through chains of MemBarNodes checking the MergeMems for @@ -1069,17 +1068,19 @@ Node* MemNode::can_see_stored_value(Node* st, PhaseTransform* phase) const { // kind of node is encountered. Loads from final memory can skip // through any kind of MemBar but normal loads shouldn't skip // through MemBarAcquire since the could allow them to move out of - // a synchronized region. + // a synchronized region. It is not safe to step over MemBarCPUOrder, + // because alias info above them may be inaccurate (e.g., due to + // mixed/mismatched unsafe accesses). + bool is_final_mem = !atp->is_rewritable(); while (current->is_Proj()) { int opc = current->in(0)->Opcode(); - if ((final && (opc == Op_MemBarAcquire || - opc == Op_MemBarAcquireLock || - opc == Op_LoadFence)) || + if ((is_final_mem && (opc == Op_MemBarAcquire || + opc == Op_MemBarAcquireLock || + opc == Op_LoadFence)) || opc == Op_MemBarRelease || opc == Op_StoreFence || opc == Op_MemBarReleaseLock || - opc == Op_MemBarStoreStore || - opc == Op_MemBarCPUOrder) { + opc == Op_MemBarStoreStore) { Node* mem = current->in(0)->in(TypeFunc::Memory); if (mem->is_MergeMem()) { MergeMemNode* merge = mem->as_MergeMem(); diff --git a/test/hotspot/jtreg/compiler/unsafe/MismatchedUnsafeAccessTest.java b/test/hotspot/jtreg/compiler/unsafe/MismatchedUnsafeAccessTest.java new file mode 100644 index 00000000000..be07a3bfc8e --- /dev/null +++ b/test/hotspot/jtreg/compiler/unsafe/MismatchedUnsafeAccessTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2021, 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 8223923 + * @modules java.base/jdk.internal.misc + * @run main/othervm -Xbatch compiler.unsafe.MismatchedUnsafeAccessTest + */ +package compiler.unsafe; + +import jdk.internal.misc.Unsafe; + +public class MismatchedUnsafeAccessTest { + private static final Unsafe UNSAFE = Unsafe.getUnsafe(); + + public static class Test { + public int x = 0; + public int y = 0; + + public static final long offsetX; + public static final long offsetY; + + static { + try { + offsetX = UNSAFE.objectFieldOffset(Test.class.getField("x")); + offsetY = UNSAFE.objectFieldOffset(Test.class.getField("y")); + } catch (NoSuchFieldException e) { + throw new InternalError(e); + } + // Validate offsets + if (offsetX >= offsetY || offsetY - offsetX != 4) { + throw new InternalError("Wrong offsets: " + offsetX + " " + offsetY); + } + } + } + + public static int test(long l) { + Test a = new Test(); + UNSAFE.putLong(a, Test.offsetX, l); // mismatched access; interferes with subsequent load + return UNSAFE.getInt(a, Test.offsetY); + } + + public static void main(String[] args) { + for (int i = 0; i < 20_000; i++) { + int res = test(-1L); + if (res != -1) { + throw new AssertionError("Wrong result: " + res); + } + } + System.out.println("TEST PASSED"); + } +}