Commit 591db5d9 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[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/+/2653226Reviewed-by: 's avatarZhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72375}
parent 7331739f
...@@ -589,7 +589,7 @@ class CompilationStateImpl { ...@@ -589,7 +589,7 @@ class CompilationStateImpl {
void OnFinishedUnits(Vector<WasmCode*>); void OnFinishedUnits(Vector<WasmCode*>);
void OnFinishedJSToWasmWrapperUnits(int num); void OnFinishedJSToWasmWrapperUnits(int num);
void OnCompilationStopped(const WasmFeatures& detected); void OnCompilationStopped(WasmFeatures detected);
void PublishDetectedFeatures(Isolate*); void PublishDetectedFeatures(Isolate*);
void SchedulePublishCompilationResults( void SchedulePublishCompilationResults(
std::vector<std::unique_ptr<WasmCode>> unpublished_code); std::vector<std::unique_ptr<WasmCode>> unpublished_code);
...@@ -627,7 +627,6 @@ class CompilationStateImpl { ...@@ -627,7 +627,6 @@ class CompilationStateImpl {
CompileMode compile_mode() const { return compile_mode_; } CompileMode compile_mode() const { return compile_mode_; }
Counters* counters() const { return async_counters_.get(); } Counters* counters() const { return async_counters_.get(); }
WasmFeatures* detected_features() { return &detected_features_; }
void SetWireBytesStorage( void SetWireBytesStorage(
std::shared_ptr<WireBytesStorage> wire_bytes_storage) { std::shared_ptr<WireBytesStorage> wire_bytes_storage) {
...@@ -1121,9 +1120,11 @@ bool CompileLazy(Isolate* isolate, Handle<WasmModuleObject> module_object, ...@@ -1121,9 +1120,11 @@ bool CompileLazy(Isolate* isolate, Handle<WasmModuleObject> module_object,
WasmCompilationUnit baseline_unit{func_index, tiers.baseline_tier, WasmCompilationUnit baseline_unit{func_index, tiers.baseline_tier,
kNoDebugging}; kNoDebugging};
CompilationEnv env = native_module->CreateCompilationEnv(); CompilationEnv env = native_module->CreateCompilationEnv();
WasmFeatures detected_features;
WasmCompilationResult result = baseline_unit.ExecuteCompilation( WasmCompilationResult result = baseline_unit.ExecuteCompilation(
isolate->wasm_engine(), &env, compilation_state->GetWireBytesStorage(), 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 // During lazy compilation, we can only get compilation errors when
// {--wasm-lazy-validation} is enabled. Otherwise, the module was fully // {--wasm-lazy-validation} is enabled. Otherwise, the module was fully
...@@ -3216,7 +3217,7 @@ void CompilationStateImpl::TriggerCallbacks( ...@@ -3216,7 +3217,7 @@ void CompilationStateImpl::TriggerCallbacks(
} }
} }
void CompilationStateImpl::OnCompilationStopped(const WasmFeatures& detected) { void CompilationStateImpl::OnCompilationStopped(WasmFeatures detected) {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&mutex_);
detected_features_.Add(detected); detected_features_.Add(detected);
} }
......
...@@ -630,9 +630,6 @@ ...@@ -630,9 +630,6 @@
# BUG(v8:9506): times out. # BUG(v8:9506): times out.
'wasm/shared-memory-worker-explicit-gc-stress': [SKIP], 'wasm/shared-memory-worker-explicit-gc-stress': [SKIP],
# BUG(v8:11357): data race.
'regress/wasm/regress-1161555': [SKIP],
}], # 'tsan == True' }], # 'tsan == True'
############################################################################## ##############################################################################
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
// where we are not correctly pushing the full 128-bits of a SIMD register. // where we are not correctly pushing the full 128-bits of a SIMD register.
load('test/mjsunit/wasm/wasm-module-builder.js'); load('test/mjsunit/wasm/wasm-module-builder.js');
print('v8-foozzie source: v8/test/mjsunit/wasm/simd-call.js');
const __v_0 = new WasmModuleBuilder(); const __v_0 = new WasmModuleBuilder();
__v_0.addImportedMemory('m', 'imported_mem'); __v_0.addImportedMemory('m', 'imported_mem');
__v_0.addFunction('main', makeSig([], [])).addBodyWithEnd([ __v_0.addFunction('main', makeSig([], [])).addBodyWithEnd([
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment