Closed Bug 1624842 Opened 5 years ago Closed 5 years ago

ThreadSanitizer: data race [@ baseUnowned] vs. [@ updateEdge<js::BaseShape>]

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla76

Tracking Flags:

Tracking

Status

firefox-esr68

---

wontfix

firefox74

---

wontfix

firefox75

---

wontfix

firefox76

---

fixed

People

(Reporter: decoder, Assigned: jonco)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase, Whiteboard: [bugmon:ignore])

Attachments

(2 files)

Testcase 5 years ago Christian Holler (:decoder) 1.05 KB, text/plain		Details
Bug 1624842 - Update base shapes on the main thread after compacting GC r?sfink 5 years ago Jon Coppeard (:jonco) (PTO until 5th Jan) 47 bytes, text/x-phabricator-request		Details \| Review

Christian Holler (:decoder)

Reporter

Description

•

5 years ago

The following testcase crashes on mozilla-central revision f985871b3e62+ (build with --enable-tests --enable-thread-sanitizer --disable-jemalloc --enable-optimize=-O2 --enable-fuzzing --disable-debug --enable-gczeal, run with --fuzzing-safe --cpu-count=2):

var JSVAL_INT_MIN = -Math.pow(2, 30);
var OMIT = {};
var VALUES = [ -Infinity, JSVAL_INT_MIN, -0 ];
var GETS = [ undefined ];
var SETS = [ undefined, null, 5, OMIT ];
var ENUMERABLES = [ true, OMIT ];
var CONFIGURABLES = [ true ];
var WRITABLES = [ true ];
function mapTestDescriptors(filter) {
  var descs = [];
  function put(field, value)  {
    if (value !== OMIT)
      desc[field] = value;
  }
  VALUES.forEach(function(value)  {
    GETS.forEach(function(get)    {
      SETS.forEach(function(set)      {
        ENUMERABLES.forEach(function(enumerable)        {
          CONFIGURABLES.forEach(function(configurable)          {
            WRITABLES.forEach(function(writable)            {
              desc = {};
              put("value", value);
              put("set", set);
              put("enumerable", enumerable);
              put("configurable", configurable);
                descs.push(desc);
            });
          });
        });
      });
    });
  });
  return descs;
}
var ALL_DESCRIPTORS = mapTestDescriptors();
gczeal(10, 2);
newGlobal();

Backtrace:

WARNING: ThreadSanitizer: data race (pid=28706)
  Read of size 8 at 0x7fcc64dc7670 by main thread:
    #0 baseUnowned js/src/vm/Shape.h:863:10 (js+0x9ed65f)
    #1 unowned js/src/vm/Shape.h:853:22 (js+0x9ed65f)
    #2 js::Shape::fixupShapeTreeAfterMovingGC() js/src/vm/Shape.cpp:1968:39 (js+0x9ed65f)
    #3 js::Shape::fixupAfterMovingGC() js/src/vm/Shape.cpp:1994:5 (js+0x9ed8b9)
    #4 UpdateCellPointers<js::Shape> js/src/gc/GC.cpp:2175:9 (js+0xe9bdc8)
    #5 UpdateArenaPointersTyped<js::Shape> js/src/gc/GC.cpp:2182:5 (js+0xe9bdc8)
    #6 UpdateArenaPointers js/src/gc/GC.cpp:2198:5 (js+0xe9bdc8)
    #7 UpdateArenaListSegmentPointers(js::gc::GCRuntime*, ArenaListSegment const&) js/src/gc/GC.cpp:2222:5 (js+0xe9bdc8)
    #8 js::gc::GCRuntime::updateCellPointers(JS::Zone*, mozilla::EnumSet<js::gc::AllocKind, unsigned long>) js/src/gc/GC.cpp:2374:5 (js+0xe9a443)
    #9 updateAllCellPointers js/src/gc/GC.cpp:2435:3 (js+0xe9c359)
    #10 js::gc::GCRuntime::updateZonePointersToRelocatedCells(JS::Zone*) js/src/gc/GC.cpp:2474:3 (js+0xe9c359)
    #11 js::gc::GCRuntime::compactPhase(JS::GCReason, js::SliceBudget&, js::gc::AutoGCSession&) js/src/gc/GC.cpp:6130:7 (js+0xeb5cf5)
    #12 js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason, js::gc::AutoGCSession&) js/src/gc/GC.cpp:6577:13 (js+0xeb7c87)
    #13 js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) js/src/gc/GC.cpp:6940:3 (js+0xeb98b2)
    #14 js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) js/src/gc/GC.cpp:7123:9 (js+0xebb42a)
    #15 js::gc::GCRuntime::runDebugGC() js/src/gc/GC.cpp:7695:5 (js+0xe88290)
    #16 js::gc::GCRuntime::gcIfNeededAtAllocation(JSContext*) js/src/gc/Allocator.cpp:440:5 (js+0xe87bca)
    #17 checkAllocatorState<js::CanGC> js/src/gc/Allocator.cpp:402:10 (js+0xec6451)
    #18 JSObject* js::AllocateObject<(js::AllowGC)1>(JSContext*, js::gc::AllocKind, unsigned long, js::gc::InitialHeap, JSClass const*) js/src/gc/Allocator.cpp:61:15 (js+0xec6451)
    #19 JSFunction::create(JSContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::ObjectGroup*>) js/src/vm/JSFunction-inl.h:123:19 (js+0x934f71)
    #20 NewObject(JSContext*, JS::Handle<js::ObjectGroup*>, js::gc::AllocKind, js::NewObjectKind, unsigned int) js/src/vm/JSObject.cpp:797:5 (js+0x8f13ea)
    #21 js::NewObjectWithClassProtoCommon(JSContext*, JSClass const*, JS::Handle<JSObject*>, js::gc::AllocKind, js::NewObjectKind) js/src/vm/JSObject.cpp:934:19 (js+0x8f1956)
    #22 NewObjectWithClassProto js/src/vm/JSObject-inl.h:491:10 (js+0x8e098c)
    #23 NewObjectWithClassProto<JSFunction> js/src/vm/JSObject-inl.h:513:7 (js+0x8e098c)
    #24 js::NewFunctionWithProto(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), unsigned int, FunctionFlags, JS::Handle<JSObject*>, JS::Handle<JSAtom*>, JS::Handle<JSObject*>, js::gc::AllocKind, js::NewObjectKind) js/src/vm/JSFunction.cpp:2061:7 (js+0x8e098c)
    #25 JS::NewFunctionFromSpec(JSContext*, JSFunctionSpec const*, JS::Handle<JS::PropertyKey>) js/src/jsapi.cpp (js+0x714bda)
    #26 DefineFunctionFromSpec js/src/vm/JSObject.cpp:3076:21 (js+0x8ff990)
    #27 js::DefineFunctions(JSContext*, JS::Handle<JSObject*>, JSFunctionSpec const*, js::DefineAsIntrinsic) js/src/vm/JSObject.cpp:3093:10 (js+0x8ff990)
    #28 JS_DefineFunctions(JSContext*, JS::Handle<JSObject*>, JSFunctionSpec const*) js/src/jsapi.cpp:3389:10 (js+0x715116)
    #29 js::GlobalObject::resolveConstructor(JSContext*, JS::Handle<js::GlobalObject*>, JSProtoKey, js::GlobalObject::IfClassIsDisabled) js/src/vm/GlobalObject.cpp:345:12 (js+0x81290d)
    #30 ensureConstructor js/src/vm/GlobalObject.h:179:12 (js+0x854def)
    #31 getOrCreateObjectPrototype js/src/vm/GlobalObject.h:309:12 (js+0x854def)
    #32 CreateReflectObject(JSContext*, JSProtoKey) js/src/builtin/Reflect.cpp:219:26 (js+0x854def)
    #33 js::GlobalObject::resolveConstructor(JSContext*, JS::Handle<js::GlobalObject*>, JSProtoKey, js::GlobalObject::IfClassIsDisabled) js/src/vm/GlobalObject.cpp:314:25 (js+0x81269e)
    #34 ensureConstructor js/src/vm/GlobalObject.h:179:12 (js+0x7015a0)
    #35 JS_ResolveStandardClass(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, bool*) js/src/jsapi.cpp:954:12 (js+0x7015a0)
    #36 global_resolve(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, bool*) js/src/shell/js.cpp:9630:8 (js+0x434570)
    #37 CallResolveOp js/src/vm/NativeObject-inl.h:730:8 (js+0x966949)
    #38 LookupOwnPropertyInline<js::CanGC> js/src/vm/NativeObject-inl.h:812:12 (js+0x966949)
    #39 NativeGetPropertyInline<js::CanGC> js/src/vm/NativeObject.cpp:2430:10 (js+0x966949)
    #40 js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) js/src/vm/NativeObject.cpp:2480:10 (js+0x966949)
    #41 GetProperty js/src/vm/ObjectOperations-inl.h:117:10 (js+0x8282bf)
    #42 GetProperty js/src/vm/ObjectOperations-inl.h:124:10 (js+0x8282bf)
    #43 GetProperty js/src/vm/ObjectOperations-inl.h:138:10 (js+0x8282bf)
    #44 JS_InitReflectParse(JSContext*, JS::Handle<JSObject*>) js/src/builtin/ReflectParse.cpp:3758:8 (js+0x8282bf)
    #45 NewGlobalObject(JSContext*, JS::RealmOptions&, JSPrincipals*, ShellGlobalKind) js/src/shell/js.cpp:10015:10 (js+0x432661)
    #46 NewGlobal(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:6612:27 (js+0x442dcf)
    #47 CallJSNative js/src/vm/Interpreter.cpp:476:13 (js+0x5dadd0)
    #48 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:568:12 (js+0x5dadd0)
    #49 InternalCall js/src/vm/Interpreter.cpp:631:10 (js+0x5ccc9a)
    #50 CallFromStack js/src/vm/Interpreter.cpp:635:10 (js+0x5ccc9a)
    #51 Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3027:16 (js+0x5ccc9a)
    #52 js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:448:10 (js+0x5b95f0)
    #53 js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:824:13 (js+0x5dd93d)
    #54 js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:857:10 (js+0x5ddc99)
    #55 ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) js/src/vm/CompilationAndEvaluation.cpp:453:10 (js+0x7d8d90)
    #56 JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:486:10 (js+0x7d8ee4)
    #57 RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool) js/src/shell/js.cpp:913:10 (js+0x45185c)
    #58 Process(JSContext*, char const*, bool, FileKind) js/src/shell/js.cpp:1530:14 (js+0x4511f5)
    #59 ProcessArgs js/src/shell/js.cpp:10303:10 (js+0x42bf35)
    #60 Shell(JSContext*, js::cli::OptionParser*, char**) js/src/shell/js.cpp:10924:10 (js+0x42bf35)
    #61 main js/src/shell/js.cpp:11614:12 (js+0x42528a)

  Previous write of size 8 at 0x7fcc64dc7670 by thread T2:
    #0 updateEdge<js::BaseShape> js/src/gc/GC.cpp:2092:13 (js+0xe98b1e)
    #1 js::gc::MovingTracer::onBaseShapeEdge(js::BaseShape**) js/src/gc/GC.cpp:2107:10 (js+0xe98b1e)
    #2 dispatchToOnEdge objdir-ff-fuzzing-tsan/dist/include/js/TracingAPI.h:271:12 (js+0xf30f7d)
    #3 bool DoCallback<js::BaseShape>(JS::CallbackTracer*, js::BaseShape**, char const*) js/src/gc/Tracer.cpp:45:15 (js+0xf30f7d)
    #4 bool js::gc::TraceEdgeInternal<js::BaseShape*>(JSTracer*, js::BaseShape**, char const*) js/src/gc/Marking.cpp:718:10 (js+0xf13f92)
    #5 TraceEdge<js::UnownedBaseShape *> js/src/gc/Tracer.h:125:3 (js+0x9eba27)
    #6 traceChildrenSkipShapeCache js/src/vm/Shape.cpp:1650:5 (js+0x9eba27)
    #7 js::BaseShape::traceChildren(JSTracer*) js/src/vm/Shape.cpp:1644:3 (js+0x9eba27)
    #8 UpdateCellPointers<js::BaseShape> js/src/gc/GC.cpp:2176:9 (js+0xe9b759)
    #9 UpdateArenaPointersTyped<js::BaseShape> js/src/gc/GC.cpp:2182:5 (js+0xe9b759)
    #10 UpdateArenaPointers js/src/gc/GC.cpp:2198:5 (js+0xe9b759)
    #11 UpdateArenaListSegmentPointers(js::gc::GCRuntime*, ArenaListSegment const&) js/src/gc/GC.cpp:2222:5 (js+0xe9b759)
    #12 js::gc::ParallelWorker<ArenaListSegment, ArenasToUpdate>::run() js/src/gc/ParallelWork.h:55:22 (js+0xee8fef)
    #13 js::GCParallelTask::runTask() js/src/gc/GCParallelTask.cpp:146:3 (js+0xec5c43)
    #14 js::GCParallelTask::runFromHelperThread(js::AutoLockHelperThreadState&) js/src/gc/GCParallelTask.cpp:131:5 (js+0xec5ae0)
    #15 js::HelperThread::handleGCParallelWorkload(js::AutoLockHelperThreadState&) js/src/vm/HelperThreads.cpp:1712:21 (js+0x88a8ee)
    #16 js::HelperThread::threadLoop() js/src/vm/HelperThreads.cpp:2522:5 (js+0x88c978)
    #17 js::HelperThread::ThreadMain(void*) js/src/vm/HelperThreads.cpp:2044:11 (js+0x888151)
    #18 callMain<0> js/src/threading/Thread.h:218:5 (js+0x8d6734)
    #19 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) js/src/threading/Thread.h:207:11 (js+0x8d6734)

  Thread T2 'JS Helper' (tid=28709, running) created by main thread at:
    #0 pthread_create /srv/repos/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:962 (js+0x377945)
    #1 js::Thread::create(void* (*)(void*), void*) js/src/threading/posix/PosixThread.cpp:52:7 (js+0x77b93d)
    #2 bool js::Thread::init<void (&)(void*), js::HelperThread*>(void (&)(void*), js::HelperThread*&&) js/src/threading/Thread.h:91:12 (js+0x8b2e56)
    #3 js::GlobalHelperThreadState::ensureInitialized() js/src/vm/HelperThreads.cpp:1158:27 (js+0x882698)
    #4 js::EnsureHelperThreadsInitialized() js/src/vm/HelperThreads.cpp:94:30 (js+0x882256)
    #5 JSRuntime::init(JSContext*, unsigned int) js/src/vm/Runtime.cpp:200:32 (js+0x9b2fd5)
    #6 js::NewContext(unsigned int, JSRuntime*) js/src/vm/JSContext.cpp:170:17 (js+0x8d94a7)
    #7 JS_NewContext(unsigned int, JSRuntime*) js/src/jsapi.cpp:391:10 (js+0x6fdc54)
    #8 main js/src/shell/js.cpp:11479:25 (js+0x424a05)

SUMMARY: ThreadSanitizer: data race js/src/vm/Shape.h:863:10 in baseUnowned

This is likely bug 1622969 but with a testcase and full stack. The second stack was suppressed by these suppressions:

         // Bug 1600895
         "race:js::gc::MovingTracer::onBaseShapeEdge\n"
         "race:js::gc::MovingTracer::onScopeEdge\n"
         "race:js::gc::MovingTracer::onShapeEdge\n"

Removing those made this bug reproducible 100% and provided the full stack.

Christian Holler (:decoder)

Reporter

Comment 1

•

5 years ago

Attached file Testcase — Details

Christian Holler (:decoder)

Reporter

Updated

•

5 years ago

Blocks: tsan

Flags: needinfo?(jcoppeard)

Jon Coppeard (:jonco) (PTO until 5th Jan)

Assignee

Updated

•

5 years ago

Assignee: nobody → jcoppeard

Flags: needinfo?(jcoppeard)

Priority: -- → P1

Jon Coppeard (:jonco) (PTO until 5th Jan)

Assignee

Comment 2

•

5 years ago

Attached file Bug 1624842 - Update base shapes on the main thread after compacting GC r?sfink — Details

Previously we updated base shapes in parallel on helper threads, but this can cause a race with updating shapes. The race occurs between getting a base shape's unowned base shape in fixupShapeTreeAfterMovingGC and updating that field of the base shape. I don't think this race is currently causing problems.

The fix is to update base shapes on the main thread along with shapes (which are already updated on the main thread). It's unlikely that this will make much difference to performance due to the relatively small number of base shapes.

I took the opportunity to refactor Shape::fixupShapeTreeAfterMovingGC slightly.

Pulsebot

Comment 3

•

5 years ago

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/57e989c8a37c Update base shapes on the main thread after compacting GC r=sfink

Alexandru Michis [:malexandru]

Comment 4

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/57e989c8a37c

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox76: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla76

Ryan VanderMeulen [:RyanVM]

Updated

•

5 years ago

status-firefox74: --- → wontfix

status-firefox75: --- → wontfix

status-firefox-esr68: --- → wontfix

You need to log in before you can comment on or make changes to this bug.

Bugzilla

ThreadSanitizer: data race [@ baseUnowned] vs. [@ updateEdge<js::BaseShape>]

Categories

(Core :: JavaScript: GC, defect, P1)

Tracking

()

People

(Reporter: decoder, Assigned: jonco)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase, Whiteboard: [bugmon:ignore])

Crash Data

Security

(public)

User Story

Attachments

(2 files)

Description

Comment 1

Updated

Updated

Comment 2

Comment 3

Comment 4

Updated

Attachment

General

Description

File Name

Content Type