New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 790004 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

False positive race with used_or_unused_instance_size_in_words

Project Member Reported by u...@chromium.org, Nov 30 2017

Issue description

TSAN reports false positive data race for computation of used instance size in concurrent marker.

Scenario:
1. StoreIC triggers for object.field = value.
2. Store transition from the old_map to the newly created new_map is added to the map transition tree.
3. The object map is updated with store-release: object.map = new_map
4. Generated code performs old_map => new_map transition for some other object2.
5. Concurrent marker visits object2 and loads its map with acquire-load.

TSAN see unmatched acquire-load for object2 and reports a race.

The race is OK in practice because the store-release in step 3 ensures that all initializing stores of new_map happen before the store-release.


WARNING: ThreadSanitizer: data race (pid=1717)
  Read of size 1 at 0x7fd30d0d7f2a by thread T4 (mutexes: write M114):
    #0 used_or_unused_instance_size_in_words src/objects-inl.h:3152:10 (d8+0x8d76e6)
    #1 UsedInstanceSize src/objects-inl.h:3163 (d8+0x8d76e6)
    #2 v8::internal::ConcurrentMarkingVisitor::VisitJSObject(v8::internal::Map*, v8::internal::JSObject*) src/heap/concurrent-marking.cc:117 (d8+0x8d76e6)
    #3 v8::internal::ConcurrentMarking::Run(int, v8::internal::ConcurrentMarking::TaskState*) src/heap/worklist.h (d8+0x8d14dc)
    #4 v8::internal::ConcurrentMarking::Task::RunInternal() src/heap/concurrent-marking.cc:398:26 (d8+0x8d6c87)
    #5 Run src/cancelable-task.h:148:7 (d8+0x4cb043)
    #6 non-virtual thunk to v8::internal::CancelableTask::Run() src/cancelable-task.h (d8+0x4cb043)
    #7 v8::platform::WorkerThread::Run() src/libplatform/worker-thread.cc:26:11 (d8+0xfcabb0)
    #8 NotifyStartedAndRun src/base/platform/platform.h:382:5 (d8+0xfb9512)
    #9 v8::base::ThreadEntry(void*) src/base/platform/platform-posix.cc:683 (d8+0xfb9512)
  Previous write of size 1 at 0x7fd30d0d7f2a by main thread:
    #0 set_used_or_unused_instance_size_in_words src/objects-inl.h:3158:3 (d8+0xab6e44)
    #1 AccountAddedOutOfObjectPropertyField src/objects-inl.h:3232 (d8+0xab6e44)
    #2 AccountAddedPropertyField src/objects-inl.h:3214 (d8+0xab6e44)
    #3 v8::internal::Map::CopyWithField(v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::FieldType>, v8::internal::PropertyAttributes, v8::internal::PropertyConstness, v8::internal::Representation, v8::internal::TransitionFlag) src/objects.cc:3812 (d8+0xab6e44)
    #4 v8::internal::Map::TransitionToDataProperty(v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes, v8::internal::PropertyConstness, v8::internal::Object::StoreFromKeyed, bool*) src/objects.cc:9724:17 (d8+0xadbb01)
    #5 v8::internal::LookupIterator::PrepareTransitionToDataProperty(v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes, v8::internal::Object::StoreFromKeyed) src/lookup.cc:459:28 (d8+0xa77242)
    #6 v8::internal::StoreIC::LookupForWrite(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::Object::StoreFromKeyed) src/ic/ic.cc:1286:11 (d8+0x9cbd1d)
    #7 v8::internal::StoreIC::UpdateCaches(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::Object::StoreFromKeyed, v8::internal::MaybeHandle<v8::internal::Object>) src/ic/ic.cc:1396:14 (d8+0x9ccced)
    #8 v8::internal::StoreIC::Store(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::Object::StoreFromKeyed) src/ic/ic.cc:1374:20 (d8+0x9cc9ee)
    #9 v8::internal::KeyedStoreIC::Store(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) src/ic/ic.cc:1946:5 (d8+0x9cf809)
    #10 __RT_impl_Runtime_KeyedStoreIC_Miss src/ic/ic.cc:2244:3 (d8+0x9d579c)
    #11 v8::internal::Runtime_KeyedStoreIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) src/ic/ic.cc:2231 (d8+0x9d579c)
    #12 <null> <null> (0x7fd321a0431d)
    #13 CallInternal src/execution.cc:178:10 (d8+0x86f38d)
    #14 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) src/execution.cc:188 (d8+0x86f38d)
    #15 v8::Script::Run(v8::Local<v8::Context>) src/api.cc:2092:7 (d8+0x3ac9e2)
    #16 v8::Shell::ExecuteString(v8::Isolate*, v8::Local<v8::String>, v8::Local<v8::Value>, bool, bool) src/d8.cc:703:28 (d8+0x377008)
    #17 v8::SourceGroup::Execute(v8::Isolate*) src/d8.cc:2486:12 (d8+0x3811c7)
    #18 v8::Shell::RunMain(v8::Isolate*, int, char**, bool) src/d8.cc:2961:34 (d8+0x3844ac)
    #19 v8::Shell::Main(int, char**) src/d8.cc:3422:16 (d8+0x3865d0)
    #20 main src/d8.cc:3456:10 (d8+0x3866fe)
  Mutex M114 (0x7b64000005f8) created at:
    #0 pthread_mutex_init /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1137:3 (d8+0x310b33)
    #1 InitializeNativeHandle src/base/platform/mutex.cc:28:12 (d8+0xfb5c7b)
    #2 v8::base::Mutex::Mutex() src/base/platform/mutex.cc:81 (d8+0xfb5c7b)
    #3 TaskState src/heap/concurrent-marking.h:61:10 (d8+0x8d0d07)
    #4 v8::internal::ConcurrentMarking::ConcurrentMarking(v8::internal::Heap*, v8::internal::Worklist<v8::internal::HeapObject*, 64>*, v8::internal::Worklist<v8::internal::HeapObject*, 64>*, v8::internal::Worklist<v8::internal::HeapObject*, 64>*, v8::internal::WeakObjects*) src/heap/concurrent-marking.cc:407 (d8+0x8d0d07)
    #5 v8::internal::Heap::SetUp() src/heap/heap.cc (d8+0x8fd38f)
    #6 v8::internal::Isolate::Init(v8::internal::StartupDeserializer*) src/isolate.cc:2835:14 (d8+0xa34e47)
    #7 v8::internal::Snapshot::Initialize(v8::internal::Isolate*) src/snapshot/snapshot-common.cc:52:27 (d8+0xd9fac9)
    #8 v8::IsolateNewImpl(v8::internal::Isolate*, v8::Isolate::CreateParams const&) src/api.cc:8609:29 (d8+0x3cbe62)
    #9 v8::Isolate::New(v8::Isolate::CreateParams const&) src/api.cc:8556:10 (d8+0x3cbb74)
    #10 v8::Shell::Main(int, char**) src/d8.cc:3370:22 (d8+0x385cc2)
    #11 main src/d8.cc:3456:10 (d8+0x3866fe)
  Thread T4 'V8 WorkerThread' (tid=1767, running) created by main thread at:
    #0 pthread_create /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:921:3 (d8+0x30fa23)
    #1 v8::base::Thread::Start() src/base/platform/platform-posix.cc:716:14 (d8+0xfb9469)
    #2 v8::platform::WorkerThread::WorkerThread(v8::platform::TaskQueue*) src/libplatform/worker-thread.cc:15:3 (d8+0xfcaa90)
    #3 make_unique<v8::platform::WorkerThread, v8::platform::TaskQueue *> src/base/template-utils.h:56:33 (d8+0xfbcfc8)
    #4 v8::platform::DefaultBackgroundTaskRunner::DefaultBackgroundTaskRunner(unsigned int) src/libplatform/default-background-task-runner.cc:16 (d8+0xfbcfc8)
    #5 __compressed_pair_elem<int &, 0> buildtools/third_party/libc++/trunk/include/memory:2056:9 (d8+0xfba24a)
    #6 __compressed_pair<std::__1::allocator<v8::platform::DefaultBackgroundTaskRunner> &, int &> buildtools/third_party/libc++/trunk/include/memory:2153 (d8+0xfba24a)
    #7 __shared_ptr_emplace<int &> buildtools/third_party/libc++/trunk/include/memory:3566 (d8+0xfba24a)
    #8 make_shared<int &> buildtools/third_party/libc++/trunk/include/memory:4224 (d8+0xfba24a)
    #9 make_shared<v8::platform::DefaultBackgroundTaskRunner, int &> buildtools/third_party/libc++/trunk/include/memory:4594 (d8+0xfba24a)
    #10 v8::platform::DefaultPlatform::EnsureBackgroundTaskRunnerInitialized() src/libplatform/default-platform.cc:117 (d8+0xfba24a)
    #11 v8::platform::NewDefaultPlatform(int, v8::platform::IdleTaskSupport, v8::platform::InProcessStackDumping, std::__1::unique_ptr<v8::TracingController, std::__1::default_delete<v8::TracingController> >) src/libplatform/default-platform.cc:43:13 (d8+0xfba172)
    #12 v8::Shell::Main(int, char**) src/d8.cc:3322:16 (d8+0x3858d8)
    #13 main src/d8.cc:3456:10 (d8+0x3866fe)
SUMMARY: ThreadSanitizer: data race src/objects-inl.h:3152:10 in used_or_unused_instance_size_in_words
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5b7e1a01f489fe294650c710b1eef07680c4f546

commit 5b7e1a01f489fe294650c710b1eef07680c4f546
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Nov 30 17:50:49 2017

[heap] Annotate accesses to used instance size field of a map as atomic.

This fixes false positive data race reported by TSAN.

Bug:  chromium:790004 
Change-Id: I6335af1735fe9ea77a26cc9919ec4f089b004cdf
Reviewed-on: https://chromium-review.googlesource.com/800936
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49766}
[modify] https://crrev.com/5b7e1a01f489fe294650c710b1eef07680c4f546/src/objects-inl.h

Comment 2 by u...@chromium.org, Nov 30 2017

Status: Fixed (was: Assigned)

Sign in to add a comment