[wasm] Add more checks around streaming compilation

We see failures in the wild when finishing streaming wasm compilation.
This CL adds CHECKs that we do not accidentally finish or abort a job
multiple times, since many methods actually delete the
{AsyncCompileJob}, so calling such methods twice would lead to a
use-after-free.

R=jkummerow@chromium.org

Bug: chromium:1399790, chromium:1400066
Change-Id: I0b83b1e402444afd4444638d5c9a2fd31a78b056
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4110762
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84905}
This commit is contained in:
Clemens Backes 2022-12-15 18:34:27 +01:00 committed by V8 LUCI CQ
parent ad2fc65680
commit 9b3e66263b
2 changed files with 13 additions and 2 deletions

View File

@ -2292,7 +2292,7 @@ void AsyncCompileJob::FinishCompile(bool is_after_cache_hit) {
void AsyncCompileJob::Failed() {
// {job} keeps the {this} pointer alive.
std::shared_ptr<AsyncCompileJob> job =
std::unique_ptr<AsyncCompileJob> job =
GetWasmEngine()->RemoveCompileJob(this);
// Revalidate the whole module to produce a deterministic error message.
@ -2837,6 +2837,9 @@ void AsyncStreamingProcessor::OnFinishedStream(
if (validate_functions_job_data_.found_error) after_error = true;
}
// Check that we did not abort or finish the job before.
CHECK(job_);
job_->wire_bytes_ = ModuleWireBytes(bytes.as_vector());
job_->bytes_copy_ = std::move(bytes);
@ -2856,6 +2859,7 @@ void AsyncStreamingProcessor::OnFinishedStream(
GetWasmEngine()->StreamingCompilationFailed(prefix_hash_);
}
job_->Failed();
job_ = nullptr;
return;
}
@ -2929,6 +2933,9 @@ void AsyncStreamingProcessor::OnFinishedStream(
} else {
job_->FinishCompile(cache_hit);
}
// Calling either {Failed} or {FinishCompile} will invalidate the
// {AsyncCompileJob}.
job_ = nullptr;
}
}
@ -2942,7 +2949,9 @@ void AsyncStreamingProcessor::OnAbort() {
// Clean up the temporary cache entry.
GetWasmEngine()->StreamingCompilationFailed(prefix_hash_);
}
// {Abort} invalidates the {AsyncCompileJob}.
job_->Abort();
job_ = nullptr;
}
bool AsyncStreamingProcessor::Deserialize(
@ -2970,6 +2979,8 @@ bool AsyncStreamingProcessor::Deserialize(
job_->native_module_ = job_->module_object_->shared_native_module();
job_->wire_bytes_ = ModuleWireBytes(job_->native_module_->wire_bytes());
job_->FinishCompile(false);
// Calling {FinishCompile} invalidates the {AsyncCompileJob}.
job_ = nullptr;
return true;
}

View File

@ -256,7 +256,7 @@ size_t AsyncStreamingDecoder::DecodingState::ReadBytes(
void AsyncStreamingDecoder::Finish(bool can_use_compiled_module) {
TRACE_STREAMING("Finish\n");
DCHECK(!stream_finished_);
CHECK(!stream_finished_);
stream_finished_ = true;
if (ok() && deserializing()) {
// Try to deserialize the module from wire bytes and module bytes.