Commit c5c5d144 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm] Fix tier down on streaming with error

Recompilation for tier down should not be triggered if the module had a
compile error. This CL ensures that by moving the recompilation a bit
later in the async compilation, to a place where a compile error would
have been detected already. An added DCHECK would catch similar bugs
earlier (crashing instead of timing out).

R=ahaas@chromium.org

Bug: chromium:1160031
Change-Id: I7eb3d2921db0f28bb39e9ec6150fd98fd4b99089
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2649028
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72329}
parent 4512066b
......@@ -1707,6 +1707,7 @@ void RecompileNativeModule(NativeModule* native_module,
compilation_state->InitializeRecompilation(
tiering_state,
[recompilation_finished_semaphore](CompilationEvent event) {
DCHECK_NE(CompilationEvent::kFailedCompilation, event);
if (event == CompilationEvent::kFinishedRecompilation) {
recompilation_finished_semaphore->Signal();
}
......@@ -1960,6 +1961,12 @@ void AsyncCompileJob::FinishCompile(bool is_after_cache_hit) {
// We can only update the feature counts once the entire compile is done.
compilation_state->PublishDetectedFeatures(isolate_);
// We might need to recompile the module for debugging, if the debugger was
// enabled while streaming compilation was running. Since handling this while
// compiling via streaming is tricky, we just tier down now, before publishing
// the module.
if (native_module_->IsTieredDown()) native_module_->RecompileForTiering();
// Finally, log all generated code (it does not matter if this happens
// repeatedly in case the script is shared).
native_module_->LogWasmCodes(isolate_, module_object_->script());
......@@ -2709,13 +2716,6 @@ void AsyncStreamingProcessor::OnFinishedStream(OwnedVector<uint8_t> bytes) {
}
const bool needs_finish = job_->DecrementAndCheckFinisherCount();
DCHECK_IMPLIES(!has_code_section, needs_finish);
// We might need to recompile the module for debugging, if the debugger was
// enabled while streaming compilation was running. Since handling this while
// compiling via streaming is tricky, we just tier down now, before publishing
// the module.
if (job_->native_module_->IsTieredDown()) {
job_->native_module_->RecompileForTiering();
}
if (needs_finish) {
const bool failed = job_->native_module_->compilation_state()->failed();
if (!cache_hit) {
......
......@@ -1372,6 +1372,28 @@ STREAM_TEST(TestProfilingMidStreaming) {
cpu_profiler->Dispose();
}
STREAM_TEST(TierDownWithError) {
// https://crbug.com/1160031
StreamTester tester(isolate);
Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
Zone* zone = tester.zone();
ZoneBuffer buffer(zone);
{
TestSignatures sigs;
WasmModuleBuilder builder(zone);
// Type error at i32.add.
builder.AddFunction(sigs.v_v())->Emit(kExprI32Add);
builder.WriteTo(&buffer);
}
i_isolate->wasm_engine()->TierDownAllModulesPerIsolate(i_isolate);
tester.OnBytesReceived(buffer.begin(), buffer.size());
tester.FinishStream();
tester.RunCompilerTasks();
}
#undef STREAM_TEST
} // namespace wasm
......
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