1624563 - ThreadSanitizer: data race [@ fun_resolve] vs. [@ hasFlags]

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Christian Holler (:decoder)]

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

function testTraceNativeConversion(nativeFunc) {
  var nativeFuncName = nativeFunc.name;
  try {
    nativeFunc(thrower);
  } catch (e) {}
}
testTraceNativeConversion(Math.max);
testTraceNativeConversion(Math.min);
evaluate("");

Backtrace:

WARNING: ThreadSanitizer: data race (pid=24578)
  Read of size 2 at 0x7efe5b1928e2 by thread T3:
    #0 hasFlags js/src/vm/JSFunction.h:169:48 (js+0x118771e)
    #1 isInterpreted js/src/vm/JSFunction.h:187:12 (js+0x118771e)
    #2 isNative js/src/vm/JSFunction.h:189:35 (js+0x118771e)
    #3 isBuiltinNative js/src/vm/JSFunction.h:211:12 (js+0x118771e)
    #4 isBuiltinNative js/src/vm/JSFunction.h:467:48 (js+0x118771e)
    #5 hasJitInfo js/src/vm/JSFunction.h:842:12 (js+0x118771e)
    #6 hasJitInfo js/src/jit/MIR.h:2613:42 (js+0x118771e)
    #7 js::jit::CodeGenerator::visitCallNative(js::jit::LCallNative*) js/src/jit/CodeGenerator.cpp:4820:45 (js+0x118771e)
    #8 js::jit::CodeGenerator::generateBody() js/src/jit/CodeGenerator.cpp:6511:9 (js+0x1190603)
    #9 js::jit::CodeGenerator::generate() js/src/jit/CodeGenerator.cpp:10624:8 (js+0x11c3b0b)
    #10 js::jit::GenerateCode(js::jit::MIRGenerator*, js::jit::LIRGraph*) js/src/jit/Ion.cpp:1458:17 (js+0x11fce27)
    #11 js::jit::CompileBackEnd(js::jit::MIRGenerator*, js::jit::WarpSnapshot*) js/src/jit/Ion.cpp:1488:10 (js+0x11fcef3)
    #12 js::jit::IonCompileTask::runTask() js/src/jit/IonCompileTask.cpp:26:24 (js+0x12977b4)
    #13 js::HelperThread::handleIonWorkload(js::AutoLockHelperThreadState&) js/src/vm/HelperThreads.cpp:2141:11 (js+0x88d2c7)
    #14 js::HelperThread::threadLoop() js/src/vm/HelperThreads.cpp:2522:5 (js+0x88c978)
    #15 js::HelperThread::ThreadMain(void*) js/src/vm/HelperThreads.cpp:2044:11 (js+0x888151)
    #16 callMain<0> js/src/threading/Thread.h:218:5 (js+0x8d6734)
    #17 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) js/src/threading/Thread.h:207:11 (js+0x8d6734)

  Previous write of size 2 at 0x7efe5b1928e2 by main thread:
    #0 fun_resolve(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, bool*) js/src/vm/JSFunction.cpp (js+0x915c36)
    #1 CallResolveOp js/src/vm/NativeObject-inl.h:730:8 (js+0x966949)
    #2 LookupOwnPropertyInline<js::CanGC> js/src/vm/NativeObject-inl.h:812:12 (js+0x966949)
    #3 NativeGetPropertyInline<js::CanGC> js/src/vm/NativeObject.cpp:2430:10 (js+0x966949)
    #4 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)
    #5 GetProperty js/src/vm/ObjectOperations-inl.h:117:10 (js+0x5dfff7)
    #6 GetProperty js/src/vm/ObjectOperations-inl.h:124:10 (js+0x5dfff7)
    #7 js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:4410:10 (js+0x5dfff7)
    #8 ComputeGetPropResult js/src/jit/BaselineIC.cpp:2496:12 (js+0xf8d460)
    #9 js::jit::DoGetPropFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICGetProp_Fallback*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) js/src/jit/BaselineIC.cpp:2524:8 (js+0xf8d460)
    #10 <null> <null> (0x7efdd8e56014)
    #11 Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3073:40 (js+0x5cd478)
    #12 js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:448:10 (js+0x5b95f0)
    #13 js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:824:13 (js+0x5dd93d)
    #14 js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:857:10 (js+0x5ddc99)
    #15 ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) js/src/vm/CompilationAndEvaluation.cpp:453:10 (js+0x7d8d90)
    #16 JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:486:10 (js+0x7d8ee4)
    #17 RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool) js/src/shell/js.cpp:913:10 (js+0x45185c)
    #18 Process(JSContext*, char const*, bool, FileKind) js/src/shell/js.cpp:1530:14 (js+0x4511f5)
    #19 ProcessArgs js/src/shell/js.cpp:10303:10 (js+0x42bf35)
    #20 Shell(JSContext*, js::cli::OptionParser*, char**) js/src/shell/js.cpp:10924:10 (js+0x42bf35)
    #21 main js/src/shell/js.cpp:11614:12 (js+0x42528a)

  Thread T3 'JS Helper' (tid=24584, 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/JSFunction.h:169:48 in hasFlags

This was found in fuzzing, the js shell I used is from a full browser build, but it should also work with our TSan JS shell builds. The testcase reproduces fairly reliable but can intermittently fail to reproduce (due to the nature of TSan).

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Nicolas B. Pierron [:nbp]]

(In reply to Christian Holler (:decoder) from comment #0)

This was found in fuzzing, the js shell I used is from a full browser build, but it should also work with our TSan JS shell builds. The testcase reproduces fairly reliable but can intermittently fail to reproduce (due to the nature of TSan).

For what is worth rr works with TSan.

ni? Jan because of the frame #11: js::jit::CompileBackEnd(js::jit::MIRGenerator*, js::jit::WarpSnapshot*)

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1624563 - Add hasJitInfo_ flag to WrappedFunction to avoid reading JSFunction flags off-thread. r?tcampbell!

Some of the flags are mutable so reading this flag off-thread can cause TSan issues.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d464253a6d67
Add hasJitInfo_ flag to WrappedFunction to avoid reading JSFunction flags off-thread. r=tcampbell

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d464253a6d67