dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Disallow simultaneous switchable call updates.

Browse Source 
Note it is okay for the lookup part of a switchable call miss to race with the update of another thread's miss.

TEST=tsan
Bug: https://github.com/dart-lang/sdk/issues/61670
Bug: https://github.com/dart-lang/sdk/issues/61671
Change-Id: Iad28f0f180f311de69e982a64b872f773f9eaaba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/453680
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Ryan Macnak
2025-10-07 10:44:02 -07:00
committed by Commit Queue
parent cff98f605b
commit aea69bc76e
10 changed files with 30 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/code_patcher.h
-2
View File
@@ -79,8 +79,6 @@ class CodePatcher : public AllStatic {
const Code& target);
static ObjectPtr GetSwitchableCallDataAt(uword return_address,
const Code& caller_code);
static ObjectPtr GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code);
static uword GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code);
runtime/vm/code_patcher_arm.cc
+3 -10
View File
@@ -86,9 +86,12 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
const Code& caller_code,
const Object& data,
const Code& target) {
// We lock to block other writers but don't start a safepoint to block readers
// (i.e., Dart execution).
// First update target to a stub that does not read 'data' so that concurrent
// Dart execution cannot observe the new stub with the old data or the old
// stub with the new data.
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
if (FLAG_precompiled_mode) {
BareSwitchableCallPattern call(return_address);
call.SetTargetRelease(StubCode::SwitchableCallMiss());
@@ -102,16 +105,6 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
}
}
ObjectPtr CodePatcher::GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
UNREACHABLE();
} else {
SwitchableCallPattern call(return_address, caller_code);
return call.target();
}
}
uword CodePatcher::GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
runtime/vm/code_patcher_arm64.cc
+3 -10
View File
@@ -114,9 +114,12 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
const Code& caller_code,
const Object& data,
const Code& target) {
// We lock to block other writers but don't start a safepoint to block readers
// (i.e., Dart execution).
// First update target to a stub that does not read 'data' so that concurrent
// Dart execution cannot observe the new stub with the old data or the old
// stub with the new data.
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
if (FLAG_precompiled_mode) {
BareSwitchableCallPattern call(return_address);
call.SetTargetRelease(StubCode::SwitchableCallMiss());
@@ -130,16 +133,6 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
}
}
ObjectPtr CodePatcher::GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
UNREACHABLE();
} else {
SwitchableCallPattern call(return_address, caller_code);
return call.target();
}
}
uword CodePatcher::GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
runtime/vm/code_patcher_ia32.cc
-6
View File
@@ -257,12 +257,6 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
PatchInstanceCallAt(return_address, caller_code, data, target);
}
ObjectPtr CodePatcher::GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code) {
InstanceCall call(return_address, caller_code);
return call.target();
}
uword CodePatcher::GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code) {
// Switchable instance calls only generated for precompilation.
runtime/vm/code_patcher_riscv.cc
+3 -10
View File
@@ -123,9 +123,12 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
const Code& caller_code,
const Object& data,
const Code& target) {
// We lock to block other writers but don't start a safepoint to block readers
// (i.e., Dart execution).
// First update target to a stub that does not read 'data' so that concurrent
// Dart execution cannot observe the new stub with the old data or the old
// stub with the new data.
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
if (FLAG_precompiled_mode) {
BareSwitchableCallPattern call(return_address);
call.SetTargetRelease(StubCode::SwitchableCallMiss());
@@ -139,16 +142,6 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
}
}
ObjectPtr CodePatcher::GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
UNREACHABLE();
} else {
SwitchableCallPattern call(return_address, caller_code);
return call.target();
}
}
uword CodePatcher::GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
runtime/vm/code_patcher_x64.cc
+9 -12
View File
@@ -294,7 +294,9 @@ class SwitchableCallBase : public ValueObject {
intptr_t data_index() const { return data_index_; }
intptr_t target_index() const { return target_index_; }
ObjectPtr data() const { return object_pool_.ObjectAt(data_index()); }
ObjectPtr data() const {
return object_pool_.ObjectAt<std::memory_order_relaxed>(data_index());
}
void SetDataRelease(const Object& data) const {
ASSERT(!Object::Handle(object_pool_.ObjectAt(data_index())).IsCode());
@@ -442,7 +444,9 @@ class BareSwitchableCall : public SwitchableCallBase {
target_index(), target.MonomorphicEntryPoint());
}
uword target_entry() const { return object_pool_.RawValueAt(target_index()); }
uword target_entry() const {
return object_pool_.RawValueAt<std::memory_order_relaxed>(target_index());
}
};
CodePtr CodePatcher::GetStaticCallTargetAt(uword return_address,
@@ -518,9 +522,12 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
const Code& caller_code,
const Object& data,
const Code& target) {
// We lock to block other writers but don't start a safepoint to block readers
// (i.e., Dart execution).
// First update target to a stub that does not read 'data' so that concurrent
// Dart execution cannot observe the new stub with the old data or the old
// stub with the new data.
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
if (FLAG_precompiled_mode) {
BareSwitchableCall call(return_address);
call.SetTargetRelease(StubCode::SwitchableCallMiss());
@@ -534,16 +541,6 @@ void CodePatcher::PatchSwitchableCallAt(uword return_address,
}
}
ObjectPtr CodePatcher::GetSwitchableCallTargetAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
UNREACHABLE();
} else {
SwitchableCall call(return_address, caller_code);
return call.target();
}
}
uword CodePatcher::GetSwitchableCallTargetEntryAt(uword return_address,
const Code& caller_code) {
if (FLAG_precompiled_mode) {
runtime/vm/instructions_arm.cc
+2 -2
View File
@@ -244,7 +244,7 @@ SwitchableCallPatternBase::SwitchableCallPatternBase(
: object_pool_(object_pool), data_pool_index_(-1), target_pool_index_(-1) {}
ObjectPtr SwitchableCallPatternBase::data() const {
return object_pool_.ObjectAt(data_pool_index_);
return object_pool_.ObjectAt<std::memory_order_relaxed>(data_pool_index_);
}
void SwitchableCallPatternBase::SetDataRelease(const Object& data) const {
@@ -294,7 +294,7 @@ BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
}
uword BareSwitchableCallPattern::target_entry() const {
return object_pool_.RawValueAt(target_pool_index_);
return object_pool_.RawValueAt<std::memory_order_relaxed>(target_pool_index_);
}
void BareSwitchableCallPattern::SetTargetRelease(const Code& target) const {
runtime/vm/instructions_arm64.cc
+2 -2
View File
@@ -428,7 +428,7 @@ SwitchableCallPatternBase::SwitchableCallPatternBase(
: object_pool_(object_pool), data_pool_index_(-1), target_pool_index_(-1) {}
ObjectPtr SwitchableCallPatternBase::data() const {
return object_pool_.ObjectAt(data_pool_index_);
return object_pool_.ObjectAt<std::memory_order_relaxed>(data_pool_index_);
}
void SwitchableCallPatternBase::SetDataRelease(const Object& data) const {
@@ -481,7 +481,7 @@ BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
}
uword BareSwitchableCallPattern::target_entry() const {
return object_pool_.RawValueAt(target_pool_index_);
return object_pool_.RawValueAt<std::memory_order_relaxed>(target_pool_index_);
}
void BareSwitchableCallPattern::SetTargetRelease(const Code& target) const {
runtime/vm/instructions_riscv.cc
+2 -2
View File
@@ -327,7 +327,7 @@ SwitchableCallPatternBase::SwitchableCallPatternBase(
: object_pool_(object_pool), data_pool_index_(-1), target_pool_index_(-1) {}
ObjectPtr SwitchableCallPatternBase::data() const {
return object_pool_.ObjectAt(data_pool_index_);
return object_pool_.ObjectAt<std::memory_order_relaxed>(data_pool_index_);
}
void SwitchableCallPatternBase::SetDataRelease(const Object& data) const {
@@ -387,7 +387,7 @@ BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
}
uword BareSwitchableCallPattern::target_entry() const {
return object_pool_.RawValueAt(target_pool_index_);
return object_pool_.RawValueAt<std::memory_order_relaxed>(target_pool_index_);
}
void BareSwitchableCallPattern::SetTargetRelease(const Code& target) const {
runtime/vm/object.h
+6 -1
View File
@@ -5745,7 +5745,12 @@ class ObjectPool : public Object {
uword RawValueAt(intptr_t index) const {
ASSERT(TypeAt(index) != EntryType::kTaggedObject);
return EntryAddr(index)->raw_value_;
return LoadNonPointer<uword>(&EntryAddr(index)->raw_value_);
}
template <std::memory_order order>
uword RawValueAt(intptr_t index) const {
ASSERT(TypeAt(index) != EntryType::kTaggedObject);
return LoadNonPointer<uword, order>(&EntryAddr(index)->raw_value_);
}
void SetRawValueAt(intptr_t index, uword raw_value) const {
ASSERT(TypeAt(index) != EntryType::kTaggedObject);