From 9e28e19002045fdf6f1cfd1794c59591082c0fcc Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 24 Oct 2024 03:24:08 +0000 Subject: [PATCH] [test] Synthetic versions of the mutator-marker race. TEST=vm/cc/MutatorMarkerRace_* Bug: https://github.com/dart-lang/sdk/issues/56895 Change-Id: I3e3f25b2c43aec09a409377f77c6d2fec287bccb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/390263 Reviewed-by: Siva Annamalai Commit-Queue: Ryan Macnak --- runtime/platform/BUILD.gn | 20 +++ runtime/platform/no_tsan.cc | 33 ++++ runtime/platform/no_tsan.h | 48 +++++ runtime/tests/vm/vm.status | 2 + runtime/vm/class_table.cc | 4 + runtime/vm/heap/heap_test.cc | 332 +++++++++++++++++++++++++++++++++++ 6 files changed, 439 insertions(+) create mode 100644 runtime/platform/no_tsan.cc create mode 100644 runtime/platform/no_tsan.h diff --git a/runtime/platform/BUILD.gn b/runtime/platform/BUILD.gn index 0bd32ee9ae2..0b229b58ee3 100644 --- a/runtime/platform/BUILD.gn +++ b/runtime/platform/BUILD.gn @@ -11,6 +11,7 @@ library_for_all_configs("libdart_platform") { sources = platform_sources include_dirs = [ ".." ] extra_deps = [] + configurable_deps = [ ":libdart_platform_no_tsan" ] if (is_fuchsia) { extra_deps += [ @@ -19,3 +20,22 @@ library_for_all_configs("libdart_platform") { ] } } + +config("no_tsan_config") { + if (!is_win) { + cflags = [ "-fno-sanitize=thread" ] + } +} + +library_for_all_configs("libdart_platform_no_tsan") { + target_type = "source_set" + public_configs = [ + "../vm:libdart_vm_config", + ":no_tsan_config", + ] + sources = [ + "no_tsan.cc", + "no_tsan.h", + ] + include_dirs = [ ".." ] +} diff --git a/runtime/platform/no_tsan.cc b/runtime/platform/no_tsan.cc new file mode 100644 index 00000000000..9611f4e97ce --- /dev/null +++ b/runtime/platform/no_tsan.cc @@ -0,0 +1,33 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#include "platform/no_tsan.h" + +namespace dart { + +#if defined(__clang__) + +#if defined(__has_feature) +#if __has_feature(thread_sanitizer) +#error Misconfigured build +#endif +#endif + +uintptr_t FetchAndRelaxedIgnoreRace(std::atomic* ptr, + uintptr_t value) { + return ptr->fetch_and(value, std::memory_order_relaxed); +} + +uintptr_t FetchOrRelaxedIgnoreRace(std::atomic* ptr, + uintptr_t value) { + return ptr->fetch_or(value, std::memory_order_relaxed); +} + +uintptr_t LoadRelaxedIgnoreRace(const std::atomic* ptr) { + return ptr->load(std::memory_order_relaxed); +} + +#endif // defined(__clang__) + +} // namespace dart diff --git a/runtime/platform/no_tsan.h b/runtime/platform/no_tsan.h new file mode 100644 index 00000000000..9e65a1b93ce --- /dev/null +++ b/runtime/platform/no_tsan.h @@ -0,0 +1,48 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#ifndef RUNTIME_PLATFORM_NO_TSAN_H_ +#define RUNTIME_PLATFORM_NO_TSAN_H_ + +#include + +#include + +namespace dart { + +#if defined(__clang__) +// Clang does not honor no_sanitize(thread) for std::atomic, so we place +// the implementation in a separate compilation unit with TSAN disabled. +uintptr_t FetchAndRelaxedIgnoreRace(std::atomic* ptr, + uintptr_t value); +uintptr_t FetchOrRelaxedIgnoreRace(std::atomic* ptr, + uintptr_t value); +uintptr_t LoadRelaxedIgnoreRace(const std::atomic* ptr); +#else +#if defined(__GNUC__) +__attribute__((no_sanitize("thread"))) +#endif +inline uintptr_t +FetchAndRelaxedIgnoreRace(std::atomic* ptr, uintptr_t value) { + return ptr->fetch_and(value, std::memory_order_relaxed); +} +#if defined(__GNUC__) +__attribute__((no_sanitize("thread"))) +#endif +inline uintptr_t +FetchOrRelaxedIgnoreRace(std::atomic* ptr, uintptr_t value) { + return ptr->fetch_or(value, std::memory_order_relaxed); +} +#if defined(__GNUC__) +__attribute__((no_sanitize("thread"))) +#endif +inline uintptr_t +LoadRelaxedIgnoreRace(const std::atomic* ptr) { + return ptr->load(std::memory_order_relaxed); +} +#endif + +} // namespace dart + +#endif // RUNTIME_PLATFORM_NO_TSAN_H_ diff --git a/runtime/tests/vm/vm.status b/runtime/tests/vm/vm.status index b9ae28fffdf..a34e1d31ca2 100644 --- a/runtime/tests/vm/vm.status +++ b/runtime/tests/vm/vm.status @@ -11,6 +11,8 @@ cc/IsolateReload_PendingStaticCall_DefinedToNSM: Fail # Issue 32981 cc/IsolateReload_PendingStaticCall_NSMToDefined: Fail, Crash # Issue 32981. Fails on non-Windows, crashes on Windows (because of test.py special handline) cc/IsolateReload_PendingUnqualifiedCall_InstanceToStatic: Fail # Issue 32981 cc/IsolateReload_PendingUnqualifiedCall_StaticToInstance: Fail # Issue 32981 +cc/MutatorMarkerRace_Relaxed: Pass, Fail # Comparison to demonstrate race, failure seen on Mac M1. +cc/MutatorMarkerRace_ReleaseHeader: Pass, Fail # Comparison to demonstrate race, failure seen on Windows Snapdragon. cc/TTS_STC_ManyAsserts: Pass, Slow # Generates 10k classes that are put into an STC via assert checks. cc/TypeArguments_Cache_ManyInstantiations: Pass, Slow dart/analyze_snapshot_binary_test: Pass, Slow # Runs various subprocesses for testing AOT. diff --git a/runtime/vm/class_table.cc b/runtime/vm/class_table.cc index 38bffb82d5a..e5a017e9c3c 100644 --- a/runtime/vm/class_table.cc +++ b/runtime/vm/class_table.cc @@ -85,6 +85,10 @@ void ClassTable::Register(const Class& cls) { classes_.GetColumn()); UpdateCachedAllocationTracingStateTablePointer(); } else { + // GCC warns that TSAN doesn't understand thread fences. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic ignored "-Wtsan" +#endif std::atomic_thread_fence(std::memory_order_release); } } diff --git a/runtime/vm/heap/heap_test.cc b/runtime/vm/heap/heap_test.cc index 1088a264b3a..645fba1417e 100644 --- a/runtime/vm/heap/heap_test.cc +++ b/runtime/vm/heap/heap_test.cc @@ -10,6 +10,7 @@ #include "platform/globals.h" #include "platform/assert.h" +#include "platform/no_tsan.h" #include "vm/class_finalizer.h" #include "vm/dart_api_impl.h" #include "vm/globals.h" @@ -1367,4 +1368,335 @@ ISOLATE_UNIT_TEST_CASE(CardRememberedWeakArray) { TestCardRememberedWeakArray(false); } +struct ExistingObject; + +static constexpr uword kMarkBit = 1; +static constexpr uword kCidBit = 2; +static constexpr size_t kNewObjectSlotCount = 3; +struct NewObject { + std::atomic header; + std::atomic slots[kNewObjectSlotCount]; +}; + +static constexpr size_t kExistingObjectSlotCount = 64 * KB; +struct ExistingObject { + std::atomic slots[kExistingObjectSlotCount]; +}; + +struct NewPage { + std::atomic top; + std::atomic end; + NewObject objects[kExistingObjectSlotCount]; +}; +static constexpr size_t kNewPageAlignment = + Utils::RoundUpToPowerOfTwo(sizeof(NewPage)); +static constexpr size_t kNewPageMask = kNewPageAlignment - 1; + +typedef void (*MutatorFunction)(NewPage*, ExistingObject*); +typedef void (*MarkerFunction)(ExistingObject*); + +struct MarkerArguments { + ExistingObject* existing_object; + MarkerFunction function; + Monitor* monitor; + ThreadJoinId join_id; +}; + +static void MutatorMarkerRace(MutatorFunction mutator, MarkerFunction marker) { + VirtualMemory* existing_vm = VirtualMemory::Allocate( + Utils::RoundUp(sizeof(ExistingObject), VirtualMemory::PageSize()), false, + false, "dart-heap"); + ExistingObject* existing_object = + reinterpret_cast(existing_vm->address()); + + Monitor monitor; + MarkerArguments arguments; + arguments.existing_object = existing_object; + arguments.function = marker; + arguments.monitor = &monitor; + + for (intptr_t k = 0; k < 1000; k++) { + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + existing_object->slots[i] = nullptr; + } + arguments.join_id = OSThread::kInvalidThreadJoinId; + + OSThread::Start( + "FakeMarker", + [](uword parameter) { + MarkerArguments* arguments = + reinterpret_cast(parameter); + + arguments->function(arguments->existing_object); + + MonitorLocker ml(arguments->monitor); + arguments->join_id = + OSThread::GetCurrentThreadJoinId(OSThread::Current()); + ml.Notify(); + }, + reinterpret_cast(&arguments)); + + VirtualMemory* new_vm = VirtualMemory::AllocateAligned( + kNewPageAlignment, kNewPageAlignment, false, false, "dart-heap"); + NewPage* new_page = reinterpret_cast(new_vm->address()); + new_page->end = new_vm->end(); + new_page->top.store(reinterpret_cast(new_page->objects), + std::memory_order_release); + + mutator(new_page, existing_object); + + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + uword header = new_object->header.load(std::memory_order_relaxed); + EXPECT_EQ(kCidBit, header & kCidBit); + } + + { + MonitorLocker ml(&monitor); + while (arguments.join_id == OSThread::kInvalidThreadJoinId) { + ml.Wait(); + } + } + OSThread::Join(arguments.join_id); + + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + uword header = new_object->header.load(std::memory_order_relaxed); + EXPECT_EQ(kCidBit | kMarkBit, header); + } + + delete new_vm; + } + + delete existing_vm; +} + +// Skip tests with races on weak-memory model architecture to avoid meta-flaking +// the test status. +#if defined(HOST_ARCH_IA32) || defined(HOST_ARCH_X64) + +// This has a race: the initializing store of the header and the publishing +// store of the new object's pointers might get reordered as seen by the marker. +// Seen in practice on an M1. +VM_UNIT_TEST_CASE(MutatorMarkerRace_Relaxed) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_relaxed); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_relaxed); + } + }, + [](ExistingObject* existing_object) { + // Marker: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + uword header = FetchOrRelaxedIgnoreRace(&target->header, kMarkBit); + EXPECT_EQ(kCidBit, header); + } + }); +} + +// This has a race: the release orders stores before the header initialization +// with the header initialization, but still lets the header initialization and +// publishing store get reordered. +// Seen in practice on Windows ARM64 Snapdragon. +VM_UNIT_TEST_CASE(MutatorMarkerRace_ReleaseHeader) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_release); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_relaxed); + } + }, + [](ExistingObject* existing_object) { + // Marker: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + uword header = FetchOrRelaxedIgnoreRace(&target->header, kMarkBit); + EXPECT_EQ(kCidBit, header); + } + }); +} + +#endif // defined(HOST_ARCH_IA32) || defined(HOST_ARCH_X64) + +VM_UNIT_TEST_CASE(MutatorMarkerRace_ReleasePublish) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_relaxed); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_release); + } + }, + [](ExistingObject* existing_object) { + // Marker: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + uword header = FetchOrRelaxedIgnoreRace(&target->header, kMarkBit); + EXPECT_EQ(kCidBit, header); + } + }); +} + +// TSAN doesn't support std::atomic_thread_fence. +#if !defined(USING_THREAD_SANITIZER) +VM_UNIT_TEST_CASE(MutatorMarkerRace_Fence) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_relaxed); + std::atomic_thread_fence(std::memory_order_release); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_relaxed); + } + }, + [](ExistingObject* existing_object) { + // Marker: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + uword header = FetchOrRelaxedIgnoreRace(&target->header, kMarkBit); + EXPECT_EQ(kCidBit, header); + } + }); +} +#endif // !defined(USING_THREAD_SANITIZER) + +VM_UNIT_TEST_CASE(MutatorMarkerRace_DetectPreviousValue) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_relaxed); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_relaxed); + } + }, + [](ExistingObject* existing_object) { + // Marker: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + while (LoadRelaxedIgnoreRace(&target->header) == 0) { + // Wait. + } + + uword header = FetchOrRelaxedIgnoreRace(&target->header, kMarkBit); + EXPECT_EQ(kCidBit, header); + } + }); +} + +VM_UNIT_TEST_CASE(MutatorMarkerRace_DetectInTLAB) { + MutatorMarkerRace( + [](NewPage* new_page, ExistingObject* existing_object) { + // Mutator: + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* new_object = &new_page->objects[i]; + new_object->header.store(2u, std::memory_order_relaxed); + for (size_t j = 0; j < kNewObjectSlotCount; j++) { + new_object->slots[j].store(existing_object, + std::memory_order_relaxed); + } + existing_object->slots[i].store(new_object, + std::memory_order_relaxed); + + if ((i % 8) == 0) { + new_page->top.store( + reinterpret_cast(&new_page->objects[i + 1]), + std::memory_order_release); + } + } + + new_page->top.store( + reinterpret_cast( + &new_page->objects[kExistingObjectSlotCount + 1]), + std::memory_order_release); + }, + [](ExistingObject* existing_object) { + // Marker: + MallocGrowableArray deferred(kExistingObjectSlotCount); + + for (size_t i = 0; i < kExistingObjectSlotCount; i++) { + NewObject* target; + do { + target = existing_object->slots[i].load(std::memory_order_relaxed); + } while (target == nullptr); + + uword addr = reinterpret_cast(target); + NewPage* new_page = reinterpret_cast(addr & ~kNewPageMask); + if (addr < new_page->top.load(std::memory_order_acquire)) { + uword header = + target->header.fetch_or(kMarkBit, std::memory_order_relaxed); + EXPECT_EQ(kCidBit, header); + } else { + deferred.Add(target); + } + } + + for (intptr_t i = 0; i < deferred.length(); i++) { + NewObject* target = deferred[i]; + + uword addr = reinterpret_cast(target); + NewPage* new_page = reinterpret_cast(addr & ~kNewPageMask); + while (addr >= new_page->top.load(std::memory_order_acquire)) { + // Wait. Would be a STW phase in the full thing. + } + uword header = + target->header.fetch_or(kMarkBit, std::memory_order_relaxed); + EXPECT_EQ(kCidBit, header); + } + }); +} + } // namespace dart