Skip to content

Commit eb3fdd7

Browse files
rmacnak-googleCommit Queue
rmacnak-google
authored and
Commit Queue
committed
[vm] Default to non-atomic for heap accesses.
Avoids inadvertently covering up data races. TEST=tsan Change-Id: Icb001391638b9f467145cf889363d47527933cd9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446988 Reviewed-by: Alexander Aprelev <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 0f8250c commit eb3fdd7

12 files changed

+266
-100
lines changed

‎runtime/vm/code_patcher_arm64.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class PoolPointerCall : public ValueObject {
2929
}
3030

3131
void SetTarget(const Code& target) const {
32-
object_pool_.SetObjectAt(pp_index(), target);
32+
object_pool_.SetObjectAt<std::memory_order_release>(pp_index(), target);
3333
// No need to flush the instruction cache, since the code is not modified.
3434
}
3535

‎runtime/vm/code_patcher_riscv.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class PoolPointerCall : public ValueObject {
3939
}
4040

4141
void SetTarget(const Code& target) const {
42-
object_pool_.SetObjectAt(pp_index(), target);
42+
object_pool_.SetObjectAt<std::memory_order_release>(pp_index(), target);
4343
// No need to flush the instruction cache, since the code is not modified.
4444
}
4545

‎runtime/vm/code_patcher_x64.cc‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class UnoptimizedCall : public ValueObject {
147147
}
148148

149149
void set_target(const Code& target) const {
150-
object_pool_.SetObjectAt(code_index_, target);
150+
object_pool_.SetObjectAt<std::memory_order_release>(code_index_, target);
151151
// No need to flush the instruction cache, since the code is not modified.
152152
}
153153

@@ -181,7 +181,8 @@ class NativeCall : public ValueObject {
181181
}
182182

183183
void set_native_function(NativeFunction func) const {
184-
object_pool_.SetRawValueAt(argument_index(), reinterpret_cast<uword>(func));
184+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
185+
argument_index(), reinterpret_cast<uword>(func));
185186
}
186187

187188
CodePtr target() const {
@@ -191,7 +192,7 @@ class NativeCall : public ValueObject {
191192
}
192193

193194
void set_target(const Code& target) const {
194-
object_pool_.SetObjectAt(code_index_, target);
195+
object_pool_.SetObjectAt<std::memory_order_release>(code_index_, target);
195196
// No need to flush the instruction cache, since the code is not modified.
196197
}
197198

@@ -220,7 +221,7 @@ class InstanceCall : public UnoptimizedCall {
220221
ObjectPtr data() const { return object_pool_.ObjectAt(argument_index()); }
221222
void set_data(const Object& data) const {
222223
ASSERT(data.IsArray() || data.IsICData() || data.IsMegamorphicCache());
223-
object_pool_.SetObjectAt(argument_index(), data);
224+
object_pool_.SetObjectAt<std::memory_order_release>(argument_index(), data);
224225
}
225226

226227
private:
@@ -265,7 +266,7 @@ class PoolPointerCall : public ValueObject {
265266
}
266267

267268
void SetTarget(const Code& target) const {
268-
object_pool_.SetObjectAt(code_index_, target);
269+
object_pool_.SetObjectAt<std::memory_order_release>(code_index_, target);
269270
// No need to flush the instruction cache, since the code is not modified.
270271
}
271272

@@ -294,7 +295,7 @@ class SwitchableCallBase : public ValueObject {
294295

295296
void SetData(const Object& data) const {
296297
ASSERT(!Object::Handle(object_pool_.ObjectAt(data_index())).IsCode());
297-
object_pool_.SetObjectAt(data_index(), data);
298+
object_pool_.SetObjectAt<std::memory_order_release>(data_index(), data);
298299
// No need to flush the instruction cache, since the code is not modified.
299300
}
300301

@@ -353,7 +354,7 @@ class SwitchableCall : public SwitchableCallBase {
353354

354355
void SetTarget(const Code& target) const {
355356
ASSERT(Object::Handle(object_pool_.ObjectAt(target_index())).IsCode());
356-
object_pool_.SetObjectAt(target_index(), target);
357+
object_pool_.SetObjectAt<std::memory_order_release>(target_index(), target);
357358
// No need to flush the instruction cache, since the code is not modified.
358359
}
359360

@@ -424,7 +425,8 @@ class BareSwitchableCall : public SwitchableCallBase {
424425
void SetTarget(const Code& target) const {
425426
ASSERT(object_pool_.TypeAt(target_index()) ==
426427
ObjectPool::EntryType::kImmediate);
427-
object_pool_.SetRawValueAt(target_index(), target.MonomorphicEntryPoint());
428+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
429+
target_index(), target.MonomorphicEntryPoint());
428430
}
429431

430432
uword target_entry() const { return object_pool_.RawValueAt(target_index()); }

‎runtime/vm/instructions_arm.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ CodePtr NativeCallPattern::target() const {
7575
}
7676

7777
void NativeCallPattern::set_target(const Code& new_target) const {
78-
object_pool_.SetObjectAt(target_code_pool_index_, new_target);
78+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
79+
new_target);
7980
// No need to flush the instruction cache, since the code is not modified.
8081
}
8182

@@ -85,8 +86,8 @@ NativeFunction NativeCallPattern::native_function() const {
8586
}
8687

8788
void NativeCallPattern::set_native_function(NativeFunction func) const {
88-
object_pool_.SetRawValueAt(native_function_pool_index_,
89-
reinterpret_cast<uword>(func));
89+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
90+
native_function_pool_index_, reinterpret_cast<uword>(func));
9091
}
9192

9293
// Decodes a load sequence ending at 'end' (the last instruction of the load
@@ -216,7 +217,8 @@ CodePtr CallPattern::TargetCode() const {
216217
}
217218

218219
void CallPattern::SetTargetCode(const Code& target_code) const {
219-
object_pool_.SetObjectAt(target_code_pool_index_, target_code);
220+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
221+
target_code);
220222
}
221223

222224
ObjectPtr ICCallPattern::Data() const {
@@ -225,15 +227,16 @@ ObjectPtr ICCallPattern::Data() const {
225227

226228
void ICCallPattern::SetData(const Object& data) const {
227229
ASSERT(data.IsArray() || data.IsICData() || data.IsMegamorphicCache());
228-
object_pool_.SetObjectAt(data_pool_index_, data);
230+
object_pool_.SetObjectAt<std::memory_order_release>(data_pool_index_, data);
229231
}
230232

231233
CodePtr ICCallPattern::TargetCode() const {
232234
return static_cast<CodePtr>(object_pool_.ObjectAt(target_pool_index_));
233235
}
234236

235237
void ICCallPattern::SetTargetCode(const Code& target_code) const {
236-
object_pool_.SetObjectAt(target_pool_index_, target_code);
238+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
239+
target_code);
237240
}
238241

239242
SwitchableCallPatternBase::SwitchableCallPatternBase(
@@ -246,7 +249,7 @@ ObjectPtr SwitchableCallPatternBase::data() const {
246249

247250
void SwitchableCallPatternBase::SetData(const Object& data) const {
248251
ASSERT(!Object::Handle(object_pool_.ObjectAt(data_pool_index_)).IsCode());
249-
object_pool_.SetObjectAt(data_pool_index_, data);
252+
object_pool_.SetObjectAt<std::memory_order_release>(data_pool_index_, data);
250253
}
251254

252255
SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code)
@@ -270,7 +273,8 @@ ObjectPtr SwitchableCallPattern::target() const {
270273

271274
void SwitchableCallPattern::SetTarget(const Code& target) const {
272275
ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode());
273-
object_pool_.SetObjectAt(target_pool_index_, target);
276+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
277+
target);
274278
}
275279

276280
BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
@@ -296,8 +300,8 @@ uword BareSwitchableCallPattern::target_entry() const {
296300
void BareSwitchableCallPattern::SetTarget(const Code& target) const {
297301
ASSERT(object_pool_.TypeAt(target_pool_index_) ==
298302
ObjectPool::EntryType::kImmediate);
299-
object_pool_.SetRawValueAt(target_pool_index_,
300-
target.MonomorphicEntryPoint());
303+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
304+
target_pool_index_, target.MonomorphicEntryPoint());
301305
}
302306

303307
ReturnPattern::ReturnPattern(uword pc) : pc_(pc) {}

‎runtime/vm/instructions_arm64.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ CodePtr NativeCallPattern::target() const {
7676
}
7777

7878
void NativeCallPattern::set_target(const Code& target) const {
79-
object_pool_.SetObjectAt(target_code_pool_index_, target);
79+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
80+
target);
8081
// No need to flush the instruction cache, since the code is not modified.
8182
}
8283

@@ -86,8 +87,8 @@ NativeFunction NativeCallPattern::native_function() const {
8687
}
8788

8889
void NativeCallPattern::set_native_function(NativeFunction func) const {
89-
object_pool_.SetRawValueAt(native_function_pool_index_,
90-
reinterpret_cast<uword>(func));
90+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
91+
native_function_pool_index_, reinterpret_cast<uword>(func));
9192
}
9293

9394
// Decodes a load sequence ending at 'end' (the last instruction of the load
@@ -398,7 +399,8 @@ CodePtr CallPattern::TargetCode() const {
398399
}
399400

400401
void CallPattern::SetTargetCode(const Code& target) const {
401-
object_pool_.SetObjectAt(target_code_pool_index_, target);
402+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
403+
target);
402404
// No need to flush the instruction cache, since the code is not modified.
403405
}
404406

@@ -408,15 +410,16 @@ ObjectPtr ICCallPattern::Data() const {
408410

409411
void ICCallPattern::SetData(const Object& data) const {
410412
ASSERT(data.IsArray() || data.IsICData() || data.IsMegamorphicCache());
411-
object_pool_.SetObjectAt(data_pool_index_, data);
413+
object_pool_.SetObjectAt<std::memory_order_release>(data_pool_index_, data);
412414
}
413415

414416
CodePtr ICCallPattern::TargetCode() const {
415417
return static_cast<CodePtr>(object_pool_.ObjectAt(target_pool_index_));
416418
}
417419

418420
void ICCallPattern::SetTargetCode(const Code& target) const {
419-
object_pool_.SetObjectAt(target_pool_index_, target);
421+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
422+
target);
420423
// No need to flush the instruction cache, since the code is not modified.
421424
}
422425

@@ -430,7 +433,7 @@ ObjectPtr SwitchableCallPatternBase::data() const {
430433

431434
void SwitchableCallPatternBase::SetData(const Object& data) const {
432435
ASSERT(!Object::Handle(object_pool_.ObjectAt(data_pool_index_)).IsCode());
433-
object_pool_.SetObjectAt(data_pool_index_, data);
436+
object_pool_.SetObjectAt<std::memory_order_release>(data_pool_index_, data);
434437
}
435438

436439
SwitchableCallPattern::SwitchableCallPattern(uword pc, const Code& code)
@@ -456,7 +459,8 @@ ObjectPtr SwitchableCallPattern::target() const {
456459

457460
void SwitchableCallPattern::SetTarget(const Code& target) const {
458461
ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode());
459-
object_pool_.SetObjectAt(target_pool_index_, target);
462+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
463+
target);
460464
}
461465

462466
BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
@@ -483,8 +487,8 @@ uword BareSwitchableCallPattern::target_entry() const {
483487
void BareSwitchableCallPattern::SetTarget(const Code& target) const {
484488
ASSERT(object_pool_.TypeAt(target_pool_index_) ==
485489
ObjectPool::EntryType::kImmediate);
486-
object_pool_.SetRawValueAt(target_pool_index_,
487-
target.MonomorphicEntryPoint());
490+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
491+
target_pool_index_, target.MonomorphicEntryPoint());
488492
}
489493

490494
ReturnPattern::ReturnPattern(uword pc) : pc_(pc) {}

‎runtime/vm/instructions_riscv.cc‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ CodePtr NativeCallPattern::target() const {
9090
}
9191

9292
void NativeCallPattern::set_target(const Code& target) const {
93-
object_pool_.SetObjectAt(target_code_pool_index_, target);
93+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
94+
target);
9495
// No need to flush the instruction cache, since the code is not modified.
9596
}
9697

@@ -100,8 +101,8 @@ NativeFunction NativeCallPattern::native_function() const {
100101
}
101102

102103
void NativeCallPattern::set_native_function(NativeFunction func) const {
103-
object_pool_.SetRawValueAt(native_function_pool_index_,
104-
reinterpret_cast<uword>(func));
104+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
105+
native_function_pool_index_, reinterpret_cast<uword>(func));
105106
}
106107

107108
// Decodes a load sequence ending at 'end' (the last instruction of the load
@@ -297,7 +298,8 @@ CodePtr CallPattern::TargetCode() const {
297298
}
298299

299300
void CallPattern::SetTargetCode(const Code& target) const {
300-
object_pool_.SetObjectAt(target_code_pool_index_, target);
301+
object_pool_.SetObjectAt<std::memory_order_release>(target_code_pool_index_,
302+
target);
301303
// No need to flush the instruction cache, since the code is not modified.
302304
}
303305

@@ -307,15 +309,16 @@ ObjectPtr ICCallPattern::Data() const {
307309

308310
void ICCallPattern::SetData(const Object& data) const {
309311
ASSERT(data.IsArray() || data.IsICData() || data.IsMegamorphicCache());
310-
object_pool_.SetObjectAt(data_pool_index_, data);
312+
object_pool_.SetObjectAt<std::memory_order_release>(data_pool_index_, data);
311313
}
312314

313315
CodePtr ICCallPattern::TargetCode() const {
314316
return static_cast<CodePtr>(object_pool_.ObjectAt(target_pool_index_));
315317
}
316318

317319
void ICCallPattern::SetTargetCode(const Code& target) const {
318-
object_pool_.SetObjectAt(target_pool_index_, target);
320+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
321+
target);
319322
// No need to flush the instruction cache, since the code is not modified.
320323
}
321324

@@ -359,7 +362,8 @@ ObjectPtr SwitchableCallPattern::target() const {
359362

360363
void SwitchableCallPattern::SetTarget(const Code& target) const {
361364
ASSERT(Object::Handle(object_pool_.ObjectAt(target_pool_index_)).IsCode());
362-
object_pool_.SetObjectAt(target_pool_index_, target);
365+
object_pool_.SetObjectAt<std::memory_order_release>(target_pool_index_,
366+
target);
363367
}
364368

365369
BareSwitchableCallPattern::BareSwitchableCallPattern(uword pc)
@@ -389,8 +393,8 @@ uword BareSwitchableCallPattern::target_entry() const {
389393
void BareSwitchableCallPattern::SetTarget(const Code& target) const {
390394
ASSERT(object_pool_.TypeAt(target_pool_index_) ==
391395
ObjectPool::EntryType::kImmediate);
392-
object_pool_.SetRawValueAt(target_pool_index_,
393-
target.MonomorphicEntryPoint());
396+
object_pool_.SetRawValueAt<std::memory_order_relaxed>(
397+
target_pool_index_, target.MonomorphicEntryPoint());
394398
}
395399

396400
ReturnPattern::ReturnPattern(uword pc) : pc_(pc) {}

‎runtime/vm/object.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8396,8 +8396,6 @@ void Function::SetInstructionsSafe(const Code& value) const {
83968396

83978397
void Function::AttachCode(const Code& value) const {
83988398
ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter());
8399-
// Finish setting up code before activating it.
8400-
value.set_owner(*this);
84018399
SetInstructions(value);
84028400
ASSERT(Function::Handle(value.function()).IsNull() ||
84038401
(value.function() == this->ptr()));
@@ -12275,7 +12273,7 @@ const char* FunctionType::ToCString() const {
1227512273
}
1227612274

1227712275
void ClosureData::set_context_scope(const ContextScope& value) const {
12278-
untag()->set_context_scope(value.ptr());
12276+
untag()->set_context_scope<std::memory_order_release>(value.ptr());
1227912277
}
1228012278

1228112279
void ClosureData::set_implicit_static_closure(const Closure& closure) const {
@@ -12824,7 +12822,8 @@ void Field::set_dependent_code(const WeakArray& array) const {
1282412822
ASSERT(IsOriginal());
1282512823
DEBUG_ASSERT(
1282612824
IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter());
12827-
untag()->set_dependent_code(array.ptr());
12825+
// relaxed: races with Field::Clone.
12826+
untag()->set_dependent_code<std::memory_order_relaxed>(array.ptr());
1282812827
}
1282912828

1283012829
class FieldDependentArray : public WeakCodeReferences {

0 commit comments

Comments
 (0)