From 591db5d98bc69a8d08f28285a7ab28b0c094cdf4 Mon Sep 17 00:00:00 2001 From: Clemens Backes Date: Wed, 27 Jan 2021 16:48:19 +0100 Subject: [PATCH] [wasm] Fix data race in lazy compilation Instead of updating the detected features set directly, use the synchronized {OnCompilationStopped} method. In order to avoid this error in the future, the whole {detected_features()} getter is removed, as it returns a pointer which can only be accessed when holding the mutex anyway. Also, the refactored code was the only user of this dangerous method. Drive-by: Pass the WasmFeatures set by value, since it's just an EnumSet. Drive-by 2: Remove a print line from the regression test which can be confusing if the test is picked up again by foozzie. R=ahaas@chromium.org CC=zhin@chromium.org Bug: v8:11357 Change-Id: I75b5c8f35983d2bc1fd2b61adcb2ecfc18564f39 Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2653226 Reviewed-by: Zhi An Ng Reviewed-by: Andreas Haas Commit-Queue: Clemens Backes Cr-Commit-Position: refs/heads/master@{#72375} --- src/wasm/module-compiler.cc | 9 +++++---- test/mjsunit/mjsunit.status | 3 --- test/mjsunit/regress/wasm/regress-1161555.js | 1 - 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc index f0db78f41c..62eedd4f0a 100644 --- a/src/wasm/module-compiler.cc +++ b/src/wasm/module-compiler.cc @@ -589,7 +589,7 @@ class CompilationStateImpl { void OnFinishedUnits(Vector); void OnFinishedJSToWasmWrapperUnits(int num); - void OnCompilationStopped(const WasmFeatures& detected); + void OnCompilationStopped(WasmFeatures detected); void PublishDetectedFeatures(Isolate*); void SchedulePublishCompilationResults( std::vector> unpublished_code); @@ -627,7 +627,6 @@ class CompilationStateImpl { CompileMode compile_mode() const { return compile_mode_; } Counters* counters() const { return async_counters_.get(); } - WasmFeatures* detected_features() { return &detected_features_; } void SetWireBytesStorage( std::shared_ptr wire_bytes_storage) { @@ -1121,9 +1120,11 @@ bool CompileLazy(Isolate* isolate, Handle module_object, WasmCompilationUnit baseline_unit{func_index, tiers.baseline_tier, kNoDebugging}; CompilationEnv env = native_module->CreateCompilationEnv(); + WasmFeatures detected_features; WasmCompilationResult result = baseline_unit.ExecuteCompilation( isolate->wasm_engine(), &env, compilation_state->GetWireBytesStorage(), - counters, compilation_state->detected_features()); + counters, &detected_features); + compilation_state->OnCompilationStopped(detected_features); // During lazy compilation, we can only get compilation errors when // {--wasm-lazy-validation} is enabled. Otherwise, the module was fully @@ -3216,7 +3217,7 @@ void CompilationStateImpl::TriggerCallbacks( } } -void CompilationStateImpl::OnCompilationStopped(const WasmFeatures& detected) { +void CompilationStateImpl::OnCompilationStopped(WasmFeatures detected) { base::MutexGuard guard(&mutex_); detected_features_.Add(detected); } diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 8a4abb7619..035871623e 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -630,9 +630,6 @@ # BUG(v8:9506): times out. 'wasm/shared-memory-worker-explicit-gc-stress': [SKIP], - - # BUG(v8:11357): data race. - 'regress/wasm/regress-1161555': [SKIP], }], # 'tsan == True' ############################################################################## diff --git a/test/mjsunit/regress/wasm/regress-1161555.js b/test/mjsunit/regress/wasm/regress-1161555.js index c6b2badb2a..186aa626dc 100644 --- a/test/mjsunit/regress/wasm/regress-1161555.js +++ b/test/mjsunit/regress/wasm/regress-1161555.js @@ -8,7 +8,6 @@ // where we are not correctly pushing the full 128-bits of a SIMD register. load('test/mjsunit/wasm/wasm-module-builder.js'); -print('v8-foozzie source: v8/test/mjsunit/wasm/simd-call.js'); const __v_0 = new WasmModuleBuilder(); __v_0.addImportedMemory('m', 'imported_mem'); __v_0.addFunction('main', makeSig([], [])).addBodyWithEnd([