From dcac60b67277ee20f9a12d43b3ca562a6b002957 Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Tue, 28 Nov 2023 13:15:56 +0000 Subject: [PATCH] [vm/aot] Fix BoxInt64 in deferred units When generating code for deferred units compiler can't emit PC relative call to the shared Mint allocation stub. This causes compiler to emit an indirect call through a Code object. Such calls clobber CODE_REG which is actually an allocatable register. Fix this by using non-allocatable register instead of CODE_REG when generating indirect calls through Code object in AOT mode. Callees don't expect anything useful in CODE_REG anyway because AOT calling convetion does not use it. TEST=vm/cc/{BranchLinkPreservesRegisters,JumpAndLinkPreservesRegisters,CallCodePreservesRegisters} Bug: b/242559057 Cq-Include-Trybots: luci.dart.try:vm-aot-linux-release-arm64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-linux-debug-simriscv64-try,vm-ffi-qemu-linux-release-riscv64-try Change-Id: Ib1fdc1c104d0269d41bb1ab9cbe292ad28c7cd49 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338127 Reviewed-by: Alexander Markov Commit-Queue: Slava Egorov --- runtime/platform/utils.h | 52 +++++++ runtime/vm/code_patcher_x64.cc | 147 +++++++++++------- .../vm/compiler/assembler/assembler_arm.cc | 30 ++-- runtime/vm/compiler/assembler/assembler_arm.h | 7 +- .../vm/compiler/assembler/assembler_arm64.cc | 26 ++-- .../vm/compiler/assembler/assembler_arm64.h | 7 +- .../assembler/assembler_arm64_test.cc | 54 +++++++ .../compiler/assembler/assembler_arm_test.cc | 51 ++++++ .../vm/compiler/assembler/assembler_riscv.cc | 23 +-- .../vm/compiler/assembler/assembler_riscv.h | 7 +- .../assembler/assembler_riscv_test.cc | 54 +++++++ .../vm/compiler/assembler/assembler_x64.cc | 21 ++- runtime/vm/compiler/assembler/assembler_x64.h | 3 + .../compiler/assembler/assembler_x64_test.cc | 51 ++++++ .../compiler/backend/flow_graph_compiler.cc | 15 +- runtime/vm/constants.h | 8 + runtime/vm/instructions_arm.cc | 13 +- runtime/vm/instructions_arm64.cc | 13 +- runtime/vm/instructions_riscv.cc | 25 +-- runtime/vm/unit_test.cc | 11 ++ runtime/vm/unit_test.h | 4 + 21 files changed, 488 insertions(+), 134 deletions(-) diff --git a/runtime/platform/utils.h b/runtime/platform/utils.h index 79df0e6d700..ace7f012107 100644 --- a/runtime/platform/utils.h +++ b/runtime/platform/utils.h @@ -551,6 +551,58 @@ class Utils { return ((mask >> position) & 1) != 0; } + template + class BitsIterator { + public: + explicit BitsIterator(uint32_t bits) : bits_(bits), bit_(bits & -bits) {} + + DART_FORCE_INLINE T operator*() const { + return static_cast(BitPosition(bit_)); + } + + DART_FORCE_INLINE bool operator==(const BitsIterator& other) const { + return bits_ == other.bits_ && bit_ == other.bit_; + } + + DART_FORCE_INLINE bool operator!=(const BitsIterator& other) const { + return !(*this == other); + } + + DART_FORCE_INLINE BitsIterator& operator++() { + bits_ ^= bit_; + bit_ = bits_ & -bits_; + return *this; + } + + private: + // Returns position of the given bit. Unlike CountTrailingZeroes assumes + // that bit is not zero without checking! + static DART_FORCE_INLINE intptr_t BitPosition(uint32_t bit) { +#if defined(DART_HOST_OS_WINDOWS) + unsigned long position; // NOLINT + BitScanForward(&position, bit); + return static_cast(position); +#else + return __builtin_ctz(bit); +#endif + } + + uint32_t bits_; + intptr_t bit_; + }; + + template + class BitsRange { + public: + explicit BitsRange(uint32_t bits) : bits_(bits) {} + + BitsIterator begin() { return BitsIterator(bits_); } + BitsIterator end() { return BitsIterator(0); } + + public: + const uint32_t bits_; + }; + static char* StrError(int err, char* buffer, size_t bufsize); // Not all platforms support strndup. diff --git a/runtime/vm/code_patcher_x64.cc b/runtime/vm/code_patcher_x64.cc index f32a7e3bca9..6d98a806435 100644 --- a/runtime/vm/code_patcher_x64.cc +++ b/runtime/vm/code_patcher_x64.cc @@ -16,6 +16,95 @@ namespace dart { +// callq [CODE_REG + entry_point_offset (disp8)] +static const int16_t kCallPatternJIT[] = { + 0x41, 0xff, 0x54, 0x24, -1, +}; + +// callq [TMP + entry_point_offset (disp8)] +static const int16_t kCallPatternAOT[] = { + 0x41, + 0xff, + 0x53, + -1, +}; + +static const intptr_t kLoadCodeFromPoolInstructionLength = 3; +static const intptr_t kLoadCodeFromPoolDisp8PatternLength = + kLoadCodeFromPoolInstructionLength + 1; +static const intptr_t kLoadCodeFromPoolDisp32PatternLength = + kLoadCodeFromPoolInstructionLength + 4; + +// movq CODE_REG, [PP + disp8] +static const int16_t + kLoadCodeFromPoolDisp8JIT[kLoadCodeFromPoolDisp8PatternLength] = { + 0x4d, + 0x8b, + 0x67, + -1, +}; + +// movq CODE_REG, [PP + disp32] +static const int16_t + kLoadCodeFromPoolDisp32JIT[kLoadCodeFromPoolDisp32PatternLength] = { + 0x4d, 0x8b, 0xa7, -1, -1, -1, -1, +}; + +// movq TMP, [PP + disp8] +static const int16_t + kLoadCodeFromPoolDisp8AOT[kLoadCodeFromPoolDisp8PatternLength] = { + 0x4d, + 0x8b, + 0x5f, + -1, +}; + +// movq TMP, [PP + disp32] +static const int16_t + kLoadCodeFromPoolDisp32AOT[kLoadCodeFromPoolDisp32PatternLength] = { + 0x4d, 0x8b, 0x9f, -1, -1, -1, -1, +}; + +static void MatchCallPattern(uword* pc) { + const int16_t* call_pattern = + FLAG_precompiled_mode ? kCallPatternAOT : kCallPatternJIT; + const intptr_t call_pattern_length = FLAG_precompiled_mode + ? ARRAY_SIZE(kCallPatternAOT) + : ARRAY_SIZE(kCallPatternJIT); + + // callq [reg + entry_point_offset] + if (MatchesPattern(*pc, call_pattern, call_pattern_length)) { + *pc -= call_pattern_length; + } else { + FATAL("Expected `call [%s + offs]` at %" Px, + FLAG_precompiled_mode ? "TMP" : "CODE_REG", *pc); + } +} + +static void MatchCodeLoadFromPool(uword* pc, intptr_t* code_index) { + const int16_t* load_code_disp8_pattern = FLAG_precompiled_mode + ? kLoadCodeFromPoolDisp8AOT + : kLoadCodeFromPoolDisp8JIT; + const int16_t* load_code_disp32_pattern = FLAG_precompiled_mode + ? kLoadCodeFromPoolDisp32AOT + : kLoadCodeFromPoolDisp32JIT; + + if (MatchesPattern(*pc, load_code_disp8_pattern, + kLoadCodeFromPoolDisp8PatternLength)) { + *pc -= kLoadCodeFromPoolDisp8PatternLength; + *code_index = + IndexFromPPLoadDisp8(*pc + kLoadCodeFromPoolInstructionLength); + } else if (MatchesPattern(*pc, load_code_disp32_pattern, + kLoadCodeFromPoolDisp32PatternLength)) { + *pc -= kLoadCodeFromPoolDisp32PatternLength; + *code_index = + IndexFromPPLoadDisp32(*pc + kLoadCodeFromPoolInstructionLength); + } else { + FATAL("Expected `movq %s, [PP + imm8|imm32]` at %" Px, + FLAG_precompiled_mode ? "TMP" : "CODE_REG", *pc); + } +} + class UnoptimizedCall : public ValueObject { public: UnoptimizedCall(uword return_address, const Code& code) @@ -24,33 +113,8 @@ class UnoptimizedCall : public ValueObject { argument_index_(-1) { uword pc = return_address; - // callq [CODE_REG + entry_point_offset] - static int16_t call_pattern[] = { - 0x41, 0xff, 0x54, 0x24, -1, - }; - if (MatchesPattern(pc, call_pattern, ARRAY_SIZE(call_pattern))) { - pc -= ARRAY_SIZE(call_pattern); - } else { - FATAL("Failed to decode at %" Px, pc); - } - - // movq CODE_REG, [PP + offset] - static int16_t load_code_disp8[] = { - 0x4d, 0x8b, 0x67, -1, // - }; - static int16_t load_code_disp32[] = { - 0x4d, 0x8b, 0xa7, -1, -1, -1, -1, - }; - if (MatchesPattern(pc, load_code_disp8, ARRAY_SIZE(load_code_disp8))) { - pc -= ARRAY_SIZE(load_code_disp8); - code_index_ = IndexFromPPLoadDisp8(pc + 3); - } else if (MatchesPattern(pc, load_code_disp32, - ARRAY_SIZE(load_code_disp32))) { - pc -= ARRAY_SIZE(load_code_disp32); - code_index_ = IndexFromPPLoadDisp32(pc + 3); - } else { - FATAL("Failed to decode at %" Px, pc); - } + MatchCallPattern(&pc); + MatchCodeLoadFromPool(&pc, &code_index_); ASSERT(Object::Handle(object_pool_.ObjectAt(code_index_)).IsCode()); // movq RBX, [PP + offset] @@ -164,33 +228,8 @@ class PoolPointerCall : public ValueObject { code_index_(-1) { uword pc = return_address; - // callq [CODE_REG + entry_point_offset] - static int16_t call_pattern[] = { - 0x41, 0xff, 0x54, 0x24, -1, - }; - if (MatchesPattern(pc, call_pattern, ARRAY_SIZE(call_pattern))) { - pc -= ARRAY_SIZE(call_pattern); - } else { - FATAL("Failed to decode at %" Px, pc); - } - - // movq CODE_REG, [PP + offset] - static int16_t load_code_disp8[] = { - 0x4d, 0x8b, 0x67, -1, // - }; - static int16_t load_code_disp32[] = { - 0x4d, 0x8b, 0xa7, -1, -1, -1, -1, - }; - if (MatchesPattern(pc, load_code_disp8, ARRAY_SIZE(load_code_disp8))) { - pc -= ARRAY_SIZE(load_code_disp8); - code_index_ = IndexFromPPLoadDisp8(pc + 3); - } else if (MatchesPattern(pc, load_code_disp32, - ARRAY_SIZE(load_code_disp32))) { - pc -= ARRAY_SIZE(load_code_disp32); - code_index_ = IndexFromPPLoadDisp32(pc + 3); - } else { - FATAL("Failed to decode at %" Px, pc); - } + MatchCallPattern(&pc); + MatchCodeLoadFromPool(&pc, &code_index_); ASSERT(Object::Handle(object_pool_.ObjectAt(code_index_)).IsCode()); } diff --git a/runtime/vm/compiler/assembler/assembler_arm.cc b/runtime/vm/compiler/assembler/assembler_arm.cc index ae02ce8fbcb..b731fa98910 100644 --- a/runtime/vm/compiler/assembler/assembler_arm.cc +++ b/runtime/vm/compiler/assembler/assembler_arm.cc @@ -2715,20 +2715,24 @@ void Assembler::Vdivqs(QRegister qd, QRegister qn, QRegister qm) { vmulqs(qd, qn, qd); } -void Assembler::Branch(const Code& target, - ObjectPoolBuilderEntry::Patchability patchable, - Register pp, - Condition cond) { - const intptr_t index = - object_pool_builder().FindObject(ToObject(target), patchable); - LoadWordFromPoolIndex(CODE_REG, index, pp, cond); - Branch(FieldAddress(CODE_REG, target::Code::entry_point_offset()), cond); -} - void Assembler::Branch(const Address& address, Condition cond) { ldr(PC, address, cond); } +void Assembler::BranchLink(intptr_t target_code_pool_index, + CodeEntryKind entry_kind) { + CLOBBERS_LR({ + // Avoid clobbering CODE_REG when invoking code in precompiled mode. + // We don't actually use CODE_REG in the callee and caller might + // be using CODE_REG for a live value (e.g. a value that is alive + // across invocation of a shared stub like the one we use for + // allocating Mint boxes). + const Register code_reg = FLAG_precompiled_mode ? LR : CODE_REG; + LoadWordFromPoolIndex(code_reg, target_code_pool_index, PP, AL); + Call(FieldAddress(code_reg, target::Code::entry_point_offset(entry_kind))); + }); +} + void Assembler::BranchLink( const Code& target, ObjectPoolBuilderEntry::Patchability patchable, @@ -2740,8 +2744,7 @@ void Assembler::BranchLink( // use 'blx ip' in a non-patchable sequence (see other BranchLink flavors). const intptr_t index = object_pool_builder().FindObject( ToObject(target), patchable, snapshot_behavior); - LoadWordFromPoolIndex(CODE_REG, index, PP, AL); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + BranchLink(index, entry_kind); } void Assembler::BranchLinkPatchable( @@ -2761,8 +2764,7 @@ void Assembler::BranchLinkWithEquivalence(const Code& target, // use 'blx ip' in a non-patchable sequence (see other BranchLink flavors). const intptr_t index = object_pool_builder().FindObject(ToObject(target), equivalence); - LoadWordFromPoolIndex(CODE_REG, index, PP, AL); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + BranchLink(index, entry_kind); } void Assembler::BranchLink(const ExternalLabel* label) { diff --git a/runtime/vm/compiler/assembler/assembler_arm.h b/runtime/vm/compiler/assembler/assembler_arm.h index 85858660e1d..1836e818271 100644 --- a/runtime/vm/compiler/assembler/assembler_arm.h +++ b/runtime/vm/compiler/assembler/assembler_arm.h @@ -788,12 +788,6 @@ class Assembler : public AssemblerBase { void bx(Register rm, Condition cond = AL); void blx(Register rm, Condition cond = AL); - void Branch(const Code& code, - ObjectPoolBuilderEntry::Patchability patchable = - ObjectPoolBuilderEntry::kNotPatchable, - Register pp = PP, - Condition cond = AL); - void Branch(const Address& address, Condition cond = AL); void BranchLink(const Code& code, @@ -1678,6 +1672,7 @@ class Assembler : public AssemblerBase { void BindARMv7(Label* label); void BranchLink(const ExternalLabel* label); + void BranchLink(intptr_t target_code_pool_index, CodeEntryKind entry_kind); void LoadObjectHelper( Register rd, diff --git a/runtime/vm/compiler/assembler/assembler_arm64.cc b/runtime/vm/compiler/assembler/assembler_arm64.cc index 31050cd9f6a..0cb41b29ac9 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64.cc +++ b/runtime/vm/compiler/assembler/assembler_arm64.cc @@ -727,14 +727,18 @@ void Assembler::LoadQImmediate(VRegister vd, simd128_value_t immq) { LoadQFromOffset(vd, PP, offset); } -void Assembler::Branch(const Code& target, - Register pp, - ObjectPoolBuilderEntry::Patchability patchable) { - const intptr_t index = - object_pool_builder().FindObject(ToObject(target), patchable); - LoadWordFromPoolIndex(CODE_REG, index, pp); - ldr(TMP, FieldAddress(CODE_REG, target::Code::entry_point_offset())); - br(TMP); +void Assembler::BranchLink(intptr_t target_code_pool_index, + CodeEntryKind entry_kind) { + CLOBBERS_LR({ + // Avoid clobbering CODE_REG when invoking code in precompiled mode. + // We don't actually use CODE_REG in the callee and caller might + // be using CODE_REG for a live value (e.g. a value that is alive + // across invocation of a shared stub like the one we use for + // allocating Mint boxes). + const Register code_reg = FLAG_precompiled_mode ? LR : CODE_REG; + LoadWordFromPoolIndex(code_reg, target_code_pool_index); + Call(FieldAddress(code_reg, target::Code::entry_point_offset(entry_kind))); + }); } void Assembler::BranchLink( @@ -744,8 +748,7 @@ void Assembler::BranchLink( ObjectPoolBuilderEntry::SnapshotBehavior snapshot_behavior) { const intptr_t index = object_pool_builder().FindObject( ToObject(target), patchable, snapshot_behavior); - LoadWordFromPoolIndex(CODE_REG, index); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + BranchLink(index, entry_kind); } void Assembler::BranchLinkWithEquivalence(const Code& target, @@ -753,8 +756,7 @@ void Assembler::BranchLinkWithEquivalence(const Code& target, CodeEntryKind entry_kind) { const intptr_t index = object_pool_builder().FindObject(ToObject(target), equivalence); - LoadWordFromPoolIndex(CODE_REG, index); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + BranchLink(index, entry_kind); } void Assembler::AddImmediate(Register dest, diff --git a/runtime/vm/compiler/assembler/assembler_arm64.h b/runtime/vm/compiler/assembler/assembler_arm64.h index 91a9fcb2922..50e056c94d6 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64.h +++ b/runtime/vm/compiler/assembler/assembler_arm64.h @@ -1788,11 +1788,6 @@ class Assembler : public AssemblerBase { tbz(label, reg, kSmiTag); } - void Branch(const Code& code, - Register pp, - ObjectPoolBuilderEntry::Patchability patchable = - ObjectPoolBuilderEntry::kNotPatchable); - void BranchLink(const Code& code, ObjectPoolBuilderEntry::Patchability patchable = ObjectPoolBuilderEntry::kNotPatchable, @@ -3120,6 +3115,8 @@ class Assembler : public AssemblerBase { Emit(encoding); } + void BranchLink(intptr_t target_code_pool_index, CodeEntryKind entry_kind); + friend class dart::FlowGraphCompiler; std::function generate_invoke_write_barrier_wrapper_; std::function generate_invoke_array_write_barrier_; diff --git a/runtime/vm/compiler/assembler/assembler_arm64_test.cc b/runtime/vm/compiler/assembler/assembler_arm64_test.cc index 186d319a490..1dc53bcacae 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64_test.cc +++ b/runtime/vm/compiler/assembler/assembler_arm64_test.cc @@ -6,6 +6,7 @@ #if defined(TARGET_ARCH_ARM64) #include "vm/compiler/assembler/assembler.h" +#include "vm/compiler/backend/locations.h" #include "vm/cpu.h" #include "vm/os.h" #include "vm/unit_test.h" @@ -7687,6 +7688,59 @@ ASSEMBLER_TEST_RUN(RangeCheckWithTempReturnValue, test) { EXPECT_EQ(kMintCid, result); } +// Tests that BranchLink only clobbers CODE_REG in JIT mode and does not +// clobber any allocatable registers in AOT mode. +ASSEMBLER_TEST_GENERATE(BranchLinkPreservesRegisters, assembler) { + const auto& do_nothing_just_return = + AssemblerTest::Generate("DoNothing", [](auto assembler) { __ Ret(); }); + + EnterTestFrame(assembler); + SPILLS_LR_TO_FRAME({ __ PushRegister(LR); }); + + const RegisterSet clobbered_regs( + kDartAvailableCpuRegs & ~(static_cast(1) << R0), + /*fpu_register_mask=*/0); + __ PushRegisters(clobbered_regs); + + Label done; + + const auto check_all_allocatable_registers_are_preserved_by_call = [&]() { + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + __ LoadImmediate(reg, static_cast(reg)); + } + __ BranchLink(do_nothing_just_return); + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + // We expect CODE_REG to be clobbered in JIT mode. + if (!FLAG_precompiled_mode && reg == CODE_REG) continue; + + Label ok; + __ CompareImmediate(reg, static_cast(reg)); + __ b(&ok, EQ); + __ LoadImmediate(R0, reg); + __ b(&done); + __ Bind(&ok); + } + }; + + check_all_allocatable_registers_are_preserved_by_call(); + + FLAG_precompiled_mode = true; + check_all_allocatable_registers_are_preserved_by_call(); + FLAG_precompiled_mode = false; + + __ LoadImmediate(R0, 42); // 42 is SUCCESS. + __ Bind(&done); + __ PopRegisters(clobbered_regs); + RESTORES_LR_FROM_FRAME({ __ PopRegister(LR); }); + LeaveTestFrame(assembler); + __ Ret(); +} + +ASSEMBLER_TEST_RUN(BranchLinkPreservesRegisters, test) { + const intptr_t result = test->InvokeWithCodeAndThread(); + EXPECT_EQ(42, result); +} + } // namespace compiler } // namespace dart diff --git a/runtime/vm/compiler/assembler/assembler_arm_test.cc b/runtime/vm/compiler/assembler/assembler_arm_test.cc index 9335f09e603..55dfe6a1d7f 100644 --- a/runtime/vm/compiler/assembler/assembler_arm_test.cc +++ b/runtime/vm/compiler/assembler/assembler_arm_test.cc @@ -6,6 +6,7 @@ #if defined(TARGET_ARCH_ARM) #include "vm/compiler/assembler/assembler.h" +#include "vm/compiler/backend/locations.h" #include "vm/cpu.h" #include "vm/os.h" #include "vm/unit_test.h" @@ -3932,6 +3933,56 @@ void LeaveTestFrame(Assembler* assembler) { __ LeaveFrame(1 << THR | 1 << PP | 1 << CODE_REG); } +// Tests that BranchLink only clobbers CODE_REG in JIT mode and does not +// clobber any allocatable registers in AOT mode. +ASSEMBLER_TEST_GENERATE(BranchLinkPreservesRegisters, assembler) { + const auto& do_nothing_just_return = + AssemblerTest::Generate("DoNothing", [](auto assembler) { __ Ret(); }); + + EnterTestFrame(assembler); + SPILLS_LR_TO_FRAME({ __ PushRegister(LR); }); + + const RegisterSet clobbered_regs( + kDartAvailableCpuRegs & ~(static_cast(1) << R0), + /*fpu_register_mask=*/0); + __ PushRegisters(clobbered_regs); + + Label done; + + const auto check_all_allocatable_registers_are_preserved_by_call = [&]() { + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + __ LoadImmediate(reg, static_cast(reg)); + } + __ BranchLink(do_nothing_just_return); + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + // We expect CODE_REG to be clobbered in JIT mode. + if (!FLAG_precompiled_mode && reg == CODE_REG) continue; + + __ CompareImmediate(reg, static_cast(reg)); + __ LoadImmediate(R0, reg, NE); + __ b(&done, NE); + } + }; + + check_all_allocatable_registers_are_preserved_by_call(); + + FLAG_precompiled_mode = true; + check_all_allocatable_registers_are_preserved_by_call(); + FLAG_precompiled_mode = false; + + __ LoadImmediate(R0, 42); // 42 is SUCCESS. + __ Bind(&done); + __ PopRegisters(clobbered_regs); + RESTORES_LR_FROM_FRAME({ __ PopRegister(LR); }); + LeaveTestFrame(assembler); + __ Ret(); +} + +ASSEMBLER_TEST_RUN(BranchLinkPreservesRegisters, test) { + const intptr_t result = test->InvokeWithCodeAndThread(); + EXPECT_EQ(42, result); +} + } // namespace compiler } // namespace dart diff --git a/runtime/vm/compiler/assembler/assembler_riscv.cc b/runtime/vm/compiler/assembler/assembler_riscv.cc index 703a477f307..f5beeed4d09 100644 --- a/runtime/vm/compiler/assembler/assembler_riscv.cc +++ b/runtime/vm/compiler/assembler/assembler_riscv.cc @@ -2983,13 +2983,16 @@ void Assembler::CompareWords(Register reg1, beq(temp, TMP, &loop, Assembler::kNearJump); } -void Assembler::Jump(const Code& target, - Register pp, - ObjectPoolBuilderEntry::Patchability patchable) { - const intptr_t index = - object_pool_builder().FindObject(ToObject(target), patchable); - LoadWordFromPoolIndex(CODE_REG, index, pp); - Jump(FieldAddress(CODE_REG, target::Code::entry_point_offset())); +void Assembler::JumpAndLink(intptr_t target_code_pool_index, + CodeEntryKind entry_kind) { + // Avoid clobbering CODE_REG when invoking code in precompiled mode. + // We don't actually use CODE_REG in the callee and caller might + // be using CODE_REG for a live value (e.g. a value that is alive + // across invocation of a shared stub like the one we use for + // allocating Mint boxes). + const Register code_reg = FLAG_precompiled_mode ? TMP : CODE_REG; + LoadWordFromPoolIndex(code_reg, target_code_pool_index); + Call(FieldAddress(code_reg, target::Code::entry_point_offset(entry_kind))); } void Assembler::JumpAndLink( @@ -2999,8 +3002,7 @@ void Assembler::JumpAndLink( ObjectPoolBuilderEntry::SnapshotBehavior snapshot_behavior) { const intptr_t index = object_pool_builder().FindObject( ToObject(target), patchable, snapshot_behavior); - LoadWordFromPoolIndex(CODE_REG, index); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + JumpAndLink(index, entry_kind); } void Assembler::JumpAndLinkWithEquivalence(const Code& target, @@ -3008,8 +3010,7 @@ void Assembler::JumpAndLinkWithEquivalence(const Code& target, CodeEntryKind entry_kind) { const intptr_t index = object_pool_builder().FindObject(ToObject(target), equivalence); - LoadWordFromPoolIndex(CODE_REG, index); - Call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + JumpAndLink(index, entry_kind); } void Assembler::Call(Address target) { diff --git a/runtime/vm/compiler/assembler/assembler_riscv.h b/runtime/vm/compiler/assembler/assembler_riscv.h index 57632941e2b..740683efd81 100644 --- a/runtime/vm/compiler/assembler/assembler_riscv.h +++ b/runtime/vm/compiler/assembler/assembler_riscv.h @@ -1006,11 +1006,6 @@ class Assembler : public MicroAssembler { Register temp, Label* equals) override; - void Jump(const Code& code, - Register pp, - ObjectPoolBuilderEntry::Patchability patchable = - ObjectPoolBuilderEntry::kNotPatchable); - void JumpAndLink(const Code& code, ObjectPoolBuilderEntry::Patchability patchable = ObjectPoolBuilderEntry::kNotPatchable, @@ -1670,6 +1665,8 @@ class Assembler : public MicroAssembler { ObjectPoolBuilderEntry::SnapshotBehavior snapshot_behavior = ObjectPoolBuilderEntry::kSnapshotable); + void JumpAndLink(intptr_t target_code_pool_index, CodeEntryKind entry_kind); + friend class dart::FlowGraphCompiler; std::function generate_invoke_write_barrier_wrapper_; std::function generate_invoke_array_write_barrier_; diff --git a/runtime/vm/compiler/assembler/assembler_riscv_test.cc b/runtime/vm/compiler/assembler/assembler_riscv_test.cc index 6d2c43b0f79..945285ac047 100644 --- a/runtime/vm/compiler/assembler/assembler_riscv_test.cc +++ b/runtime/vm/compiler/assembler/assembler_riscv_test.cc @@ -6,6 +6,7 @@ #if defined(TARGET_ARCH_RISCV32) || defined(TARGET_ARCH_RISCV64) #include "vm/compiler/assembler/assembler.h" +#include "vm/compiler/backend/locations.h" #include "vm/cpu.h" #include "vm/os.h" #include "vm/unit_test.h" @@ -7729,6 +7730,59 @@ void LeaveTestFrame(Assembler* assembler) { __ LeaveFrame(); } +// Tests that JumpAndLink only clobbers CODE_REG in JIT mode and does not +// clobber any allocatable registers in AOT mode. +ASSEMBLER_TEST_GENERATE(JumpAndLinkPreservesRegisters, assembler) { + const auto& do_nothing_just_return = + AssemblerTest::Generate("DoNothing", [](auto assembler) { __ Ret(); }); + + EnterTestFrame(assembler); + __ PushRegister(RA); + + const RegisterSet clobbered_regs( + kDartAvailableCpuRegs & ~(static_cast(1) << A0), + /*fpu_register_mask=*/0); + __ PushRegisters(clobbered_regs); + + Label done; + + const auto check_all_allocatable_registers_are_preserved_by_call = [&]() { + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + __ LoadImmediate(reg, static_cast(reg)); + } + __ JumpAndLink(do_nothing_just_return); + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + // We expect CODE_REG to be clobbered in JIT mode. + if (!FLAG_precompiled_mode && reg == CODE_REG) continue; + + Label ok; + __ CompareImmediate(reg, static_cast(reg)); + __ BranchIf(EQ, &ok, Assembler::kNearJump); + __ LoadImmediate(A0, reg); + __ j(&done); + __ Bind(&ok); + } + }; + + check_all_allocatable_registers_are_preserved_by_call(); + + FLAG_precompiled_mode = true; + check_all_allocatable_registers_are_preserved_by_call(); + FLAG_precompiled_mode = false; + + __ LoadImmediate(A0, 42); // 42 is SUCCESS. + __ Bind(&done); + __ PopRegisters(clobbered_regs); + __ PopRegister(RA); + LeaveTestFrame(assembler); + __ Ret(); +} + +ASSEMBLER_TEST_RUN(JumpAndLinkPreservesRegisters, test) { + const intptr_t result = test->InvokeWithCodeAndThread(); + EXPECT_EQ(42, result); +} + } // namespace compiler } // namespace dart diff --git a/runtime/vm/compiler/assembler/assembler_x64.cc b/runtime/vm/compiler/assembler/assembler_x64.cc index ede2cff9a8b..302dafe7862 100644 --- a/runtime/vm/compiler/assembler/assembler_x64.cc +++ b/runtime/vm/compiler/assembler/assembler_x64.cc @@ -62,6 +62,18 @@ void Assembler::call(const ExternalLabel* label) { call(TMP); } +void Assembler::CallCodeThroughPool(intptr_t target_code_pool_index, + CodeEntryKind entry_kind) { + // Avoid clobbering CODE_REG when invoking code in precompiled mode. + // We don't actually use CODE_REG in the callee and caller might + // be using CODE_REG for a live value (e.g. a value that is alive + // across invocation of a shared stub like the one we use for + // allocating Mint boxes). + const Register code_reg = FLAG_precompiled_mode ? TMP : CODE_REG; + LoadWordFromPoolIndex(code_reg, target_code_pool_index); + call(FieldAddress(code_reg, target::Code::entry_point_offset(entry_kind))); +} + void Assembler::CallPatchable( const Code& target, CodeEntryKind entry_kind, @@ -69,8 +81,7 @@ void Assembler::CallPatchable( ASSERT(constant_pool_allowed()); const intptr_t idx = object_pool_builder().AddObject( ToObject(target), ObjectPoolBuilderEntry::kPatchable, snapshot_behavior); - LoadWordFromPoolIndex(CODE_REG, idx); - call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + CallCodeThroughPool(idx, entry_kind); } void Assembler::CallWithEquivalence(const Code& target, @@ -79,8 +90,7 @@ void Assembler::CallWithEquivalence(const Code& target, ASSERT(constant_pool_allowed()); const intptr_t idx = object_pool_builder().FindObject(ToObject(target), equivalence); - LoadWordFromPoolIndex(CODE_REG, idx); - call(FieldAddress(CODE_REG, target::Code::entry_point_offset(entry_kind))); + CallCodeThroughPool(idx, entry_kind); } void Assembler::Call( @@ -90,8 +100,7 @@ void Assembler::Call( const intptr_t idx = object_pool_builder().FindObject( ToObject(target), ObjectPoolBuilderEntry::kNotPatchable, snapshot_behavior); - LoadWordFromPoolIndex(CODE_REG, idx); - call(FieldAddress(CODE_REG, target::Code::entry_point_offset())); + CallCodeThroughPool(idx, CodeEntryKind::kNormal); } void Assembler::pushq(Register reg) { diff --git a/runtime/vm/compiler/assembler/assembler_x64.h b/runtime/vm/compiler/assembler/assembler_x64.h index 2a0f7d66b89..9edef395910 100644 --- a/runtime/vm/compiler/assembler/assembler_x64.h +++ b/runtime/vm/compiler/assembler/assembler_x64.h @@ -1483,6 +1483,9 @@ class Assembler : public AssemblerBase { private: bool constant_pool_allowed_; + void CallCodeThroughPool(intptr_t target_code_pool_index, + CodeEntryKind entry_kind); + bool CanLoadFromObjectPool(const Object& object) const; void LoadObjectHelper( Register dst, diff --git a/runtime/vm/compiler/assembler/assembler_x64_test.cc b/runtime/vm/compiler/assembler/assembler_x64_test.cc index 265ff59ce0a..bcb973d64f4 100644 --- a/runtime/vm/compiler/assembler/assembler_x64_test.cc +++ b/runtime/vm/compiler/assembler/assembler_x64_test.cc @@ -6446,6 +6446,57 @@ ASSEMBLER_TEST_RUN(RangeCheckWithTempReturnValue, test) { EXPECT_EQ(kMintCid, result); } +// Tests that BranchLink only clobbers CODE_REG in JIT mode and does not +// clobber any allocatable registers in AOT mode. +ASSEMBLER_TEST_GENERATE(CallCodePreservesRegisters, assembler) { + const auto& do_nothing_just_return = + AssemblerTest::Generate("DoNothing", [](auto assembler) { __ ret(); }); + + EnterTestFrame(assembler); + + const RegisterSet clobbered_regs( + kDartAvailableCpuRegs & ~(static_cast(1) << RAX), + /*fpu_register_mask=*/0); + __ PushRegisters(clobbered_regs); + + Label done; + + const auto check_all_allocatable_registers_are_preserved_by_call = [&]() { + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + __ LoadImmediate(reg, static_cast(reg)); + } + __ Call(do_nothing_just_return); + for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { + // We expect CODE_REG to be clobbered in JIT mode. + if (!FLAG_precompiled_mode && reg == CODE_REG) continue; + + Label ok; + __ CompareImmediate(reg, static_cast(reg)); + __ j(EQUAL, &ok); + __ LoadImmediate(RAX, reg); + __ jmp(&done); + __ Bind(&ok); + } + }; + + check_all_allocatable_registers_are_preserved_by_call(); + + FLAG_precompiled_mode = true; + check_all_allocatable_registers_are_preserved_by_call(); + FLAG_precompiled_mode = false; + + __ LoadImmediate(RAX, 42); // 42 is SUCCESS. + __ Bind(&done); + __ PopRegisters(clobbered_regs); + LeaveTestFrame(assembler); + __ Ret(); +} + +ASSEMBLER_TEST_RUN(CallCodePreservesRegisters, test) { + const intptr_t result = test->InvokeWithCodeAndThread(); + EXPECT_EQ(42, result); +} + } // namespace compiler } // namespace dart diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc index 7525f910370..9652fc41925 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc @@ -57,6 +57,10 @@ DEFINE_FLAG(int, 2000, "The scale of invocation count, by size of the function."); DEFINE_FLAG(bool, source_lines, false, "Emit source line as assembly comment."); +DEFINE_FLAG(bool, + force_indirect_calls, + false, + "Do not emit PC relative calls."); DECLARE_FLAG(charp, deoptimize_filter); DECLARE_FLAG(bool, intrinsify); @@ -3492,18 +3496,21 @@ void FlowGraphCompiler::EmitMoveConst(const compiler::ffi::NativeLocation& dst, } bool FlowGraphCompiler::CanPcRelativeCall(const Function& target) const { - return FLAG_precompiled_mode && (LoadingUnit::LoadingUnitOf(function()) == - LoadingUnit::LoadingUnitOf(target)); + return FLAG_precompiled_mode && !FLAG_force_indirect_calls && + (LoadingUnit::LoadingUnitOf(function()) == + LoadingUnit::LoadingUnitOf(target)); } bool FlowGraphCompiler::CanPcRelativeCall(const Code& target) const { - return FLAG_precompiled_mode && !target.InVMIsolateHeap() && + return FLAG_precompiled_mode && !FLAG_force_indirect_calls && + !target.InVMIsolateHeap() && (LoadingUnit::LoadingUnitOf(function()) == LoadingUnit::LoadingUnitOf(target)); } bool FlowGraphCompiler::CanPcRelativeCall(const AbstractType& target) const { - return FLAG_precompiled_mode && !target.InVMIsolateHeap() && + return FLAG_precompiled_mode && !FLAG_force_indirect_calls && + !target.InVMIsolateHeap() && (LoadingUnit::LoadingUnitOf(function()) == LoadingUnit::LoadingUnit::kRootId); } diff --git a/runtime/vm/constants.h b/runtime/vm/constants.h index bb55baa1191..029d7a53fc2 100644 --- a/runtime/vm/constants.h +++ b/runtime/vm/constants.h @@ -103,6 +103,14 @@ static inline ScaleFactor ToScaleFactor(intptr_t index_scale, return static_cast(shift); } +// Helper for using register sets with range loops: +// +// for (auto reg : RegisterRange(kDartAvailableCpuRegs)) { ... } +// +static inline Utils::BitsRange RegisterRange(uint32_t regs) { + return Utils::BitsRange(regs); +} + } // namespace dart #endif // RUNTIME_VM_CONSTANTS_H_ diff --git a/runtime/vm/instructions_arm.cc b/runtime/vm/instructions_arm.cc index 9a3f92fc94d..667857b01af 100644 --- a/runtime/vm/instructions_arm.cc +++ b/runtime/vm/instructions_arm.cc @@ -16,6 +16,11 @@ namespace dart { +static bool IsBranchLinkScratch(Register reg) { + // See Assembler::BranchLink + return FLAG_precompiled_mode ? reg == LINK_REGISTER : reg == CODE_REG; +} + CallPattern::CallPattern(uword pc, const Code& code) : object_pool_(ObjectPool::Handle(code.GetObjectPool())), target_code_pool_index_(-1) { @@ -26,7 +31,7 @@ CallPattern::CallPattern(uword pc, const Code& code) Register reg; InstructionPattern::DecodeLoadWordFromPool(pc - 2 * Instr::kInstrSize, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); } ICCallPattern::ICCallPattern(uword pc, const Code& code) @@ -40,7 +45,7 @@ ICCallPattern::ICCallPattern(uword pc, const Code& code) Register reg; uword data_load_end = InstructionPattern::DecodeLoadWordFromPool( pc - 2 * Instr::kInstrSize, ®, &target_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); InstructionPattern::DecodeLoadWordFromPool(data_load_end, ®, &data_pool_index_); @@ -59,7 +64,7 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) Register reg; uword native_function_load_end = InstructionPattern::DecodeLoadWordFromPool( end_ - 2 * Instr::kInstrSize, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); InstructionPattern::DecodeLoadWordFromPool(native_function_load_end, ®, &native_function_pool_index_); ASSERT(reg == R9); @@ -256,7 +261,7 @@ SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code) ASSERT(reg == R9); InstructionPattern::DecodeLoadWordFromPool(data_load_end - Instr::kInstrSize, ®, &target_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); } uword SwitchableCallPattern::target_entry() const { diff --git a/runtime/vm/instructions_arm64.cc b/runtime/vm/instructions_arm64.cc index 323559e5fda..b45e47f8500 100644 --- a/runtime/vm/instructions_arm64.cc +++ b/runtime/vm/instructions_arm64.cc @@ -16,6 +16,11 @@ namespace dart { +static bool IsBranchLinkScratch(Register reg) { + // See Assembler::BranchLink + return FLAG_precompiled_mode ? reg == LINK_REGISTER : reg == CODE_REG; +} + CallPattern::CallPattern(uword pc, const Code& code) : object_pool_(ObjectPool::Handle(code.GetObjectPool())), target_code_pool_index_(-1) { @@ -26,7 +31,7 @@ CallPattern::CallPattern(uword pc, const Code& code) Register reg; InstructionPattern::DecodeLoadWordFromPool(pc - 2 * Instr::kInstrSize, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); } ICCallPattern::ICCallPattern(uword pc, const Code& code) @@ -42,7 +47,7 @@ ICCallPattern::ICCallPattern(uword pc, const Code& code) InstructionPattern::DecodeLoadDoubleWordFromPool( pc - 2 * Instr::kInstrSize, &data_reg, &code_reg, &pool_index); ASSERT(data_reg == R5); - ASSERT(code_reg == CODE_REG); + ASSERT(IsBranchLinkScratch(code_reg)); data_pool_index_ = pool_index; target_pool_index_ = pool_index + 1; @@ -60,7 +65,7 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) Register reg; uword native_function_load_end = InstructionPattern::DecodeLoadWordFromPool( end_ - 2 * Instr::kInstrSize, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsBranchLinkScratch(reg)); InstructionPattern::DecodeLoadWordFromPool(native_function_load_end, ®, &native_function_pool_index_); ASSERT(reg == R5); @@ -413,7 +418,7 @@ SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code) InstructionPattern::DecodeLoadDoubleWordFromPool( pc - 2 * Instr::kInstrSize, &ic_data_reg, &code_reg, &pool_index); ASSERT(ic_data_reg == R5); - ASSERT(code_reg == CODE_REG); + ASSERT(IsBranchLinkScratch(code_reg)); data_pool_index_ = pool_index; target_pool_index_ = pool_index + 1; diff --git a/runtime/vm/instructions_riscv.cc b/runtime/vm/instructions_riscv.cc index 73980395397..0f9b939ba35 100644 --- a/runtime/vm/instructions_riscv.cc +++ b/runtime/vm/instructions_riscv.cc @@ -16,12 +16,17 @@ namespace dart { +static bool IsJumpAndLinkScratch(Register reg) { + return reg == (FLAG_precompiled_mode ? TMP : CODE_REG); +} + CallPattern::CallPattern(uword pc, const Code& code) : object_pool_(ObjectPool::Handle(code.GetObjectPool())), target_code_pool_index_(-1) { ASSERT(code.ContainsInstructionAt(pc)); - // [lui,add,]lx CODE_REG, ##(pp) - // xxxxxxxx lx ra, ##(CODE_REG) + // R is either CODE_REG (JIT) or TMP (AOT) + // [lui,add,]lx R, ##(pp) + // xxxxxxxx lx ra, ##(R) // xxxx jalr ra // Last instruction: jalr ra. @@ -29,7 +34,7 @@ CallPattern::CallPattern(uword pc, const Code& code) Register reg; InstructionPattern::DecodeLoadWordFromPool(pc - 6, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsJumpAndLinkScratch(reg)); } ICCallPattern::ICCallPattern(uword pc, const Code& code) @@ -37,9 +42,10 @@ ICCallPattern::ICCallPattern(uword pc, const Code& code) target_pool_index_(-1), data_pool_index_(-1) { ASSERT(code.ContainsInstructionAt(pc)); + // R is either CODE_REG (JIT) or TMP (AOT) // [lui,add,]lx IC_DATA_REG, ##(pp) - // [lui,add,]lx CODE_REG, ##(pp) - // xxxxxxxx lx ra, ##(CODE_REG) + // [lui,add,]lx R, ##(pp) + // xxxxxxxx lx ra, ##(R) // xxxx jalr ra // Last instruction: jalr ra. @@ -48,7 +54,7 @@ ICCallPattern::ICCallPattern(uword pc, const Code& code) Register reg; uword data_load_end = InstructionPattern::DecodeLoadWordFromPool( pc - 6, ®, &target_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsJumpAndLinkScratch(reg)); InstructionPattern::DecodeLoadWordFromPool(data_load_end, ®, &data_pool_index_); @@ -61,9 +67,10 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) native_function_pool_index_(-1), target_code_pool_index_(-1) { ASSERT(code.ContainsInstructionAt(pc)); + // R is either CODE_REG (JIT) or TMP (AOT) // [lui,add,]lx t5, ##(pp) - // [lui,add,]lx CODE_REG, ##(pp) - // xxxxxxxx lx ra, ##(CODE_REG) + // [lui,add,]lx R, ##(pp) + // xxxxxxxx lx ra, ##(R) // xxxx jalr ra // Last instruction: jalr ra. @@ -72,7 +79,7 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) Register reg; uword native_function_load_end = InstructionPattern::DecodeLoadWordFromPool( pc - 6, ®, &target_code_pool_index_); - ASSERT(reg == CODE_REG); + ASSERT(IsJumpAndLinkScratch(reg)); InstructionPattern::DecodeLoadWordFromPool(native_function_load_end, ®, &native_function_pool_index_); ASSERT(reg == T5); diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index c7bbcfb9f60..6d8f4cdfd74 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -753,6 +753,17 @@ void AssemblerTest::Assemble() { #endif // !PRODUCT } +const Code& AssemblerTest::Generate( + const char* name, + const std::function& generator) { + compiler::ObjectPoolBuilder object_pool_builder; + compiler::Assembler assembler(&object_pool_builder, /*far_branch_level=*/0); + AssemblerTest test(name, &assembler, Thread::Current()->zone()); + assembler.Ret(); + test.Assemble(); + return test.code(); +} + bool CompilerTest::TestCompileFunction(const Function& function) { Thread* thread = Thread::Current(); ASSERT(thread != nullptr); diff --git a/runtime/vm/unit_test.h b/runtime/vm/unit_test.h index b31cd71be54..29a0f24321c 100644 --- a/runtime/vm/unit_test.h +++ b/runtime/vm/unit_test.h @@ -626,6 +626,10 @@ class AssemblerTest { // Disassembly of the code with relative branch/jump targets. char* RelativeDisassembly() { return disassembly_; } + static const Code& Generate( + const char* name, + const std::function& generator); + private: const char* name_; compiler::Assembler* assembler_;