From f480e95774ef6417ef8d855ca699a00c35d71adf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Sikstr=C3=B6m?= Date: Tue, 19 May 2026 14:56:12 +0000 Subject: [PATCH] 8384520: AArch64: Prevent out-of-CodeBuffer read in MacroAssembler::membar Reviewed-by: shade, aseoane --- .../cpu/aarch64/macroAssembler_aarch64.cpp | 7 ++-- .../gtest/aarch64/test_assembler_aarch64.cpp | 36 ++++++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 8f5eb702714..f385a3d2eff 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -2687,10 +2687,11 @@ void MacroAssembler::membar(Membar_mask_bits order_constraint) { BLOCK_COMMENT("merged membar"); return; } else { - // A special case like "DMB ST;DMB LD;DMB ST", the last DMB can be skipped - // We need check the last 2 instructions + // A special case like "DMB ST;DMB LD;DMB ST", the last DMB can be skipped. + // We need to check the second-to-last instruction, only if it is inside + // the current code section. address prev2 = prev - NativeMembar::instruction_size; - if (last != code()->last_label() && nativeInstruction_at(prev2)->is_Membar()) { + if (prev2 >= begin() && last != code()->last_label() && nativeInstruction_at(prev2)->is_Membar()) { NativeMembar *bar2 = NativeMembar_at(prev2); assert(bar2->get_kind() == order_constraint, "it should be merged before"); BLOCK_COMMENT("merged membar(elided)"); diff --git a/test/hotspot/gtest/aarch64/test_assembler_aarch64.cpp b/test/hotspot/gtest/aarch64/test_assembler_aarch64.cpp index 686f1ed4f3a..a3d856de0df 100644 --- a/test/hotspot/gtest/aarch64/test_assembler_aarch64.cpp +++ b/test/hotspot/gtest/aarch64/test_assembler_aarch64.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2026, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2020, Red Hat Inc. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -157,6 +157,35 @@ void test_merge_dmb() { BufferBlob::free(b); } +void test_merge_dmb_does_not_cross_section_start() { + unsigned int insns[12] = {}; + + // Add a StoreStore membar just before the start of the CodeBuffer's + // instruction section to see that it doesn't affect any merging decisions + // inside the code buffer. + insns[0] = test_encode_dmb_st; + + unsigned int* insns_actual = (insns + 1); + + CodeBuffer code((address)insns_actual, sizeof(insns) - sizeof(insns[0])); + MacroAssembler _masm(&code); + { + __ membar(Assembler::Membar_mask_bits::LoadStore); + __ membar(Assembler::Membar_mask_bits::StoreStore); + } + + asm_dump(code.insts()->start(), code.insts()->end()); + + // The last StoreStore in the CodeBuffer should not have been removed. + static const unsigned int insns_expected[] = { + test_encode_dmb_ld, + test_encode_dmb_st, + }; + + EXPECT_EQ(code.insts()->size(), (CodeSection::csize_t)(sizeof(insns_expected))); + asm_check((const unsigned int *)code.insts()->start(), insns_expected, sizeof(insns_expected) / sizeof(insns_expected[0])); +} + TEST_VM(AssemblerAArch64, merge_dmb_1) { FlagSetting fs(AlwaysMergeDMB, true); test_merge_dmb(); @@ -167,6 +196,11 @@ TEST_VM(AssemblerAArch64, merge_dmb_2) { test_merge_dmb(); } +TEST_VM(AssemblerAArch64, merge_dmb_does_not_cross_section_start) { + FlagSetting fs(AlwaysMergeDMB, false); + test_merge_dmb_does_not_cross_section_start(); +} + TEST_VM(AssemblerAArch64, merge_dmb_block_by_label) { BufferBlob* b = BufferBlob::create("aarch64Test", 400); CodeBuffer code(b);