From a677c369d93c11038f71d5f953cf08e091c04228 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Thu, 29 Jan 2026 13:18:24 -0800 Subject: [PATCH] [vm] Use atomics when accessing patchable object pool entries Although the data races on patchable object pool entries are benign, without proper release-acquire ordering there is no happens-before relationship between store and load and there is a data race as defined by the C++ standard (and detected by TSAN), which is an undefined behavior. It seems like additional memory_order_acquire loads only happen in runtime entries when we need to patch calls, which is rather infrequent. Unless additional barriers measurably affect performance, we should prefer to avoid UB. TEST=ci Fixes https://github.com/dart-lang/sdk/issues/62236 Change-Id: I3a4d8c3dca7ce1ee1a75efd1ea8ba4e9c06c9dd6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/476500 Reviewed-by: Alexander Aprelev Commit-Queue: Alexander Markov --- runtime/vm/code_patcher_arm64.cc | 3 ++- runtime/vm/code_patcher_riscv.cc | 3 ++- runtime/vm/code_patcher_x64.cc | 38 +++++++++++++++++++++++--------- runtime/vm/instructions_arm.cc | 19 +++++++++++----- runtime/vm/instructions_arm64.cc | 19 +++++++++++----- runtime/vm/instructions_riscv.cc | 19 +++++++++++----- 6 files changed, 70 insertions(+), 31 deletions(-) diff --git a/runtime/vm/code_patcher_arm64.cc b/runtime/vm/code_patcher_arm64.cc index e44c6785ed6..d2db73cfc72 100644 --- a/runtime/vm/code_patcher_arm64.cc +++ b/runtime/vm/code_patcher_arm64.cc @@ -25,7 +25,8 @@ class PoolPointerCall : public ValueObject { intptr_t pp_index() const { return index_; } CodePtr Target() const { - return static_cast(object_pool_.ObjectAt(pp_index())); + return static_cast( + object_pool_.ObjectAt(pp_index())); } void SetTarget(const Code& target) const { diff --git a/runtime/vm/code_patcher_riscv.cc b/runtime/vm/code_patcher_riscv.cc index 035ba31c54c..5272236697e 100644 --- a/runtime/vm/code_patcher_riscv.cc +++ b/runtime/vm/code_patcher_riscv.cc @@ -35,7 +35,8 @@ class PoolPointerCall : public ValueObject { intptr_t pp_index() const { return index_; } CodePtr Target() const { - return static_cast(object_pool_.ObjectAt(pp_index())); + return static_cast( + object_pool_.ObjectAt(pp_index())); } void SetTarget(const Code& target) const { diff --git a/runtime/vm/code_patcher_x64.cc b/runtime/vm/code_patcher_x64.cc index 8a3ffd747dc..6c60d826e76 100644 --- a/runtime/vm/code_patcher_x64.cc +++ b/runtime/vm/code_patcher_x64.cc @@ -138,14 +138,16 @@ class UnoptimizedCall : public ValueObject { MatchCallPattern(&pc); MatchDataLoadFromPool(&pc, &argument_index_); MatchCodeLoadFromPool(&pc, &code_index_); - ASSERT(Object::Handle(object_pool_.ObjectAt(code_index_)).IsCode()); + ASSERT(Object::Handle( + object_pool_.ObjectAt(code_index_)) + .IsCode()); } intptr_t argument_index() const { return argument_index_; } CodePtr target() const { Code& code = Code::Handle(); - code ^= object_pool_.ObjectAt(code_index_); + code ^= object_pool_.ObjectAt(code_index_); return code.ptr(); } @@ -173,7 +175,9 @@ class NativeCall : public ValueObject { MatchCallPattern(&pc); MatchCodeLoadFromPool(&pc, &code_index_); MatchDataLoadFromPool(&pc, &argument_index_); - ASSERT(Object::Handle(object_pool_.ObjectAt(code_index_)).IsCode()); + ASSERT(Object::Handle( + object_pool_.ObjectAt(code_index_)) + .IsCode()); } intptr_t argument_index() const { return argument_index_; } @@ -190,7 +194,7 @@ class NativeCall : public ValueObject { CodePtr target() const { Code& code = Code::Handle(); - code ^= object_pool_.ObjectAt(code_index_); + code ^= object_pool_.ObjectAt(code_index_); return code.ptr(); } @@ -261,12 +265,14 @@ class PoolPointerCall : public ValueObject { MatchCallPattern(&pc); MatchCodeLoadFromPool(&pc, &code_index_); - ASSERT(Object::Handle(object_pool_.ObjectAt(code_index_)).IsCode()); + ASSERT(Object::Handle( + object_pool_.ObjectAt(code_index_)) + .IsCode()); } CodePtr Target() const { Code& code = Code::Handle(); - code ^= object_pool_.ObjectAt(code_index_); + code ^= object_pool_.ObjectAt(code_index_); return code.ptr(); } @@ -301,7 +307,9 @@ class SwitchableCallBase : public ValueObject { } void SetDataRelease(const Object& data) const { - ASSERT(!Object::Handle(object_pool_.ObjectAt(data_index())).IsCode()); + ASSERT(!Object::Handle( + object_pool_.ObjectAt(data_index())) + .IsCode()); object_pool_.SetObjectAt(data_index(), data); // No need to flush the instruction cache, since the code is not modified. } @@ -359,16 +367,22 @@ class SwitchableCall : public SwitchableCallBase { } else { FATAL("Failed to decode at %" Px, pc); } - ASSERT(Object::Handle(object_pool_.ObjectAt(target_index_)).IsCode()); + ASSERT(Object::Handle( + object_pool_.ObjectAt(target_index_)) + .IsCode()); } void SetTargetRelease(const Code& target) const { - ASSERT(Object::Handle(object_pool_.ObjectAt(target_index())).IsCode()); + ASSERT(Object::Handle( + object_pool_.ObjectAt(target_index())) + .IsCode()); object_pool_.SetObjectAt(target_index(), target); // No need to flush the instruction cache, since the code is not modified. } - ObjectPtr target() const { return object_pool_.ObjectAt(target_index()); } + ObjectPtr target() const { + return object_pool_.ObjectAt(target_index()); + } }; // See [SwitchableCallBase] for a switchable calls in general. @@ -413,7 +427,9 @@ class BareSwitchableCall : public SwitchableCallBase { } else { FATAL("Failed to decode at %" Px, pc); } - ASSERT(!Object::Handle(object_pool_.ObjectAt(data_index_)).IsCode()); + ASSERT(!Object::Handle( + object_pool_.ObjectAt(data_index_)) + .IsCode()); // movq RCX, [PP + offset] static int16_t load_code_disp8[] = { diff --git a/runtime/vm/instructions_arm.cc b/runtime/vm/instructions_arm.cc index a7f38ee51e5..bfa036e1066 100644 --- a/runtime/vm/instructions_arm.cc +++ b/runtime/vm/instructions_arm.cc @@ -71,7 +71,8 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) } CodePtr NativeCallPattern::target() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void NativeCallPattern::set_target(const Code& new_target) const { @@ -213,7 +214,8 @@ bool DecodeLoadObjectFromPoolOrThread(uword pc, const Code& code, Object* obj) { } CodePtr CallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void CallPattern::SetTargetCode(const Code& target_code) const { @@ -231,7 +233,8 @@ void ICCallPattern::SetData(const Object& data) const { } CodePtr ICCallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_pool_index_)); + return static_cast( + object_pool_.ObjectAt(target_pool_index_)); } void ICCallPattern::SetTargetCode(const Code& target_code) const { @@ -248,7 +251,9 @@ ObjectPtr SwitchableCallPatternBase::data() const { } void SwitchableCallPatternBase::SetDataRelease(const Object& data) const { - ASSERT(!Object::Handle(object_pool_.ObjectAt(data_pool_index_)).IsCode()); + ASSERT(!Object::Handle(object_pool_.ObjectAt( + data_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(data_pool_index_, data); } @@ -268,11 +273,13 @@ SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code) } ObjectPtr SwitchableCallPattern::target() const { - return object_pool_.ObjectAt(target_pool_index_); + return object_pool_.ObjectAt(target_pool_index_); } void SwitchableCallPattern::SetTargetRelease(const Code& target) const { - ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode()); + ASSERT(Object::Handle(object_pool_.ObjectAt( + target_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(target_pool_index_, target); } diff --git a/runtime/vm/instructions_arm64.cc b/runtime/vm/instructions_arm64.cc index d5b85c09015..3eb898d9f37 100644 --- a/runtime/vm/instructions_arm64.cc +++ b/runtime/vm/instructions_arm64.cc @@ -72,7 +72,8 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) } CodePtr NativeCallPattern::target() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void NativeCallPattern::set_target(const Code& target) const { @@ -395,7 +396,8 @@ void InstructionPattern::EncodeLoadWordFromPoolFixed(uword end, } CodePtr CallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void CallPattern::SetTargetCode(const Code& target) const { @@ -414,7 +416,8 @@ void ICCallPattern::SetData(const Object& data) const { } CodePtr ICCallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_pool_index_)); + return static_cast( + object_pool_.ObjectAt(target_pool_index_)); } void ICCallPattern::SetTargetCode(const Code& target) const { @@ -432,7 +435,9 @@ ObjectPtr SwitchableCallPatternBase::data() const { } void SwitchableCallPatternBase::SetDataRelease(const Object& data) const { - ASSERT(!Object::Handle(object_pool_.ObjectAt(data_pool_index_)).IsCode()); + ASSERT(!Object::Handle(object_pool_.ObjectAt( + data_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(data_pool_index_, data); } @@ -454,11 +459,13 @@ SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code) } ObjectPtr SwitchableCallPattern::target() const { - return object_pool_.ObjectAt(target_pool_index_); + return object_pool_.ObjectAt(target_pool_index_); } void SwitchableCallPattern::SetTargetRelease(const Code& target) const { - ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode()); + ASSERT(Object::Handle(object_pool_.ObjectAt( + target_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(target_pool_index_, target); } diff --git a/runtime/vm/instructions_riscv.cc b/runtime/vm/instructions_riscv.cc index fcff5bc1533..9446e0ea330 100644 --- a/runtime/vm/instructions_riscv.cc +++ b/runtime/vm/instructions_riscv.cc @@ -86,7 +86,8 @@ NativeCallPattern::NativeCallPattern(uword pc, const Code& code) } CodePtr NativeCallPattern::target() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void NativeCallPattern::set_target(const Code& target) const { @@ -294,7 +295,8 @@ void InstructionPattern::EncodeLoadWordFromPoolFixed(uword end, } CodePtr CallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_code_pool_index_)); + return static_cast(object_pool_.ObjectAt( + target_code_pool_index_)); } void CallPattern::SetTargetCode(const Code& target) const { @@ -313,7 +315,8 @@ void ICCallPattern::SetData(const Object& data) const { } CodePtr ICCallPattern::TargetCode() const { - return static_cast(object_pool_.ObjectAt(target_pool_index_)); + return static_cast( + object_pool_.ObjectAt(target_pool_index_)); } void ICCallPattern::SetTargetCode(const Code& target) const { @@ -331,7 +334,9 @@ ObjectPtr SwitchableCallPatternBase::data() const { } void SwitchableCallPatternBase::SetDataRelease(const Object& data) const { - ASSERT(!Object::Handle(object_pool_.ObjectAt(data_pool_index_)).IsCode()); + ASSERT(!Object::Handle(object_pool_.ObjectAt( + data_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(data_pool_index_, data); } @@ -357,11 +362,13 @@ SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code) } ObjectPtr SwitchableCallPattern::target() const { - return object_pool_.ObjectAt(target_pool_index_); + return object_pool_.ObjectAt(target_pool_index_); } void SwitchableCallPattern::SetTargetRelease(const Code& target) const { - ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode()); + ASSERT(Object::Handle(object_pool_.ObjectAt( + target_pool_index_)) + .IsCode()); object_pool_.SetObjectAt(target_pool_index_, target); }