Issue metadata
Sign in to add a comment
|
Data race in LayoutDescriptorHelper::IsTagged |
||||||||||||||||||||||
Issue descriptionThis fails on V8 bots: https://build.chromium.org/p/client.v8.fyi/builders/Linux%20Release%20%28NVIDIA%29/builds/4214 # # Fatal error in ../../v8/src/layout-descriptor-inl.h, line 225 # Debug check failed: header_size_ >= 0 (-40 vs. 0). # #0 0x7f705c5ab217 base::debug::StackTrace::StackTrace() #1 0x7f705f218c15 gin::(anonymous namespace)::PrintStackTrace() #2 0x7f705f0e0e2c V8_Fatal() #3 0x7f705f0e0c02 v8::base::(anonymous namespace)::DefaultDcheckHandler() #4 0x7f705ba78b2a v8::internal::LayoutDescriptorHelper::LayoutDescriptorHelper() #5 0x7f705ba78b95 v8::internal::BodyDescriptorBase::IterateBodyImpl<>() #6 0x7f705ba77c4b v8::internal::ConcurrentMarkingVisitor::VisitJSObject() #7 0x7f705ba74ad0 v8::internal::ConcurrentMarking::Run() #8 0x7f705c5ac701 base::debug::TaskAnnotator::RunTask() #9 0x7f705c624c6a base::internal::TaskTracker::RunOrSkipTask() #10 0x7f705c6257fa base::internal::TaskTrackerPosix::RunOrSkipTask() #11 0x7f705c623ed8 base::internal::TaskTracker::RunNextTask() #12 0x7f705c6214e7 base::internal::SchedulerWorker::Thread::ThreadMain() #13 0x7f705c62d80c base::(anonymous namespace)::ThreadFunc() #14 0x7f70576db184 start_thread #15 0x7f70516cb37d clone
,
Nov 17 2017
I'll take a look.
,
Nov 17 2017
Thank you, Michael. This is a great catch by the GPU bot. It uncovered a data race between the concurrent marker calling LayoutDescriptorHelper and finalization of slack tracking. The critical part is LayoutDescriptorHelper constructor querying map->GetInObjectProperties() and map->instance_size() to initialize the header_size_: int inobject_properties = map->GetInObjectProperties(); DCHECK_GT(inobject_properties, 0); header_size_ = map->instance_size() - (inobject_properties * kPointerSize); This is not safe on the background thread because the main thread can be finalizing the slack tracking which mutates these fields of the map. My understanding of the layout descriptor was that it stores a bit for each slot of an object, but it actually tries to skip bits corresponding to the header slots. That's why it needs to query the instance size and the counters of the map. I think we should just store header bits in the layout descriptor too.
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/61bf2cc69217a4e9c7a40bb74269508fa26c2062 commit 61bf2cc69217a4e9c7a40bb74269508fa26c2062 Author: Ulan Degenbaev <ulan@chromium.org> Date: Fri Nov 17 21:57:23 2017 [runtime] Make layout descriptor helper safe for concurrent marking. The layout descriptor helper computes the object header size using map->instance_size() and map->GetInObjectProperties(). It races with finalization of slack tracking, which changes both the instance size and the in-object properties count. This patch replaces the in-object properties count byte in the map with the byte that stores the start offset of in-object properties. The new byte can be used in the layout descriptor to compute the object header size and it is immutable. This patch also renames InstanceSize to InstanceSizeInWords where the instance size is represented in words. Bug: chromium:786069 , chromium:694255 Change-Id: I4b48c6944d3fe8a950bd7b0ba43d75216b177a78 Reviewed-on: https://chromium-review.googlesource.com/776720 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#49461} [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/bootstrapper.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/builtins/builtins-async-gen.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/builtins/builtins-constructor-gen.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/code-stub-assembler.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/code-stub-assembler.h [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/heap/heap.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/ic/keyed-store-generic.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/layout-descriptor-inl.h [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/objects-inl.h [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/objects.cc [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/src/objects/map.h [modify] https://crrev.com/61bf2cc69217a4e9c7a40bb74269508fa26c2062/tools/gen-postmortem-metadata.py
,
Nov 20 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by machenb...@chromium.org
, Nov 16 2017