Skip to content

Commit b7afce9

Browse files
chicoxyzzyConstellation
authored andcommitted
[JSC][Wasm] Prefer lowest function index for concurrent CompileError
https://bugs.webkit.org/show_bug.cgi?id=283476 Reviewed by Yusuke Suzuki. When multiple functions fail validation under concurrent compilation, keep the error message for the lowest function index instead of whichever worker finishes first. Function-scoped failures record the error without completing the plan immediately so later lower-index failures can replace it. Non-function failures such as OOM are not replaced by later function errors. EntryPlan stops assigning more work and ThreadCountHolder completes when active workers drain. * Source/JavaScriptCore/wasm/WasmPlan.h: * Source/JavaScriptCore/wasm/WasmPlan.cpp: * Source/JavaScriptCore/wasm/WasmEntryPlan.h: * Source/JavaScriptCore/wasm/WasmEntryPlan.cpp: * Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp: * JSTests/wasm/stress/consistent-compile-error-function-index.js: Added. Canonical link: https://commits.webkit.org/319430@main
1 parent 4cc12fa commit b7afce9

6 files changed

Lines changed: 104 additions & 6 deletions

File tree

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Concurrent function validation should report the CompileError for the lowest
2+
// failing function index, not whichever worker finishes first.
3+
// https://bugs.webkit.org/show_bug.cgi?id=283476
4+
//@ requireOptions("--useConcurrentJIT=true")
5+
6+
import * as assert from "../assert.js";
7+
8+
function leb(value) {
9+
let bytes = [];
10+
do {
11+
let byte = value & 0x7f;
12+
value >>>= 7;
13+
if (value)
14+
byte |= 0x80;
15+
bytes.push(byte);
16+
} while (value);
17+
return bytes;
18+
}
19+
20+
function section(id, payload) {
21+
return [id, ...leb(payload.length), ...payload];
22+
}
23+
24+
// Many invalid functions. Function 0 is large (slow to validate) so a higher-index
25+
// worker is likely to finish first under concurrent compilation. The reported
26+
// error must still be for function 0.
27+
function invalidModuleBytes() {
28+
const types = [0x01, 0x60, 0x00, 0x00];
29+
const functionCount = 16;
30+
const functions = [functionCount, ...Array(functionCount).fill(0x00)];
31+
32+
// Function 0: many nops then empty-stack i32.add (fails late).
33+
const body0 = [0x00];
34+
for (let i = 0; i < 8000; ++i)
35+
body0.push(0x01); // nop
36+
body0.push(0x6a, 0x0b); // i32.add end
37+
38+
// Functions 1..N: immediate empty-stack f32.add (fails quickly).
39+
const bodyFast = [0x00, 0x92, 0x0b];
40+
41+
const codePayload = [functionCount, ...leb(body0.length), ...body0];
42+
for (let i = 1; i < functionCount; ++i)
43+
codePayload.push(...leb(bodyFast.length), ...bodyFast);
44+
45+
return new Uint8Array([
46+
0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00,
47+
...section(1, types),
48+
...section(3, functions),
49+
...section(10, codePayload),
50+
]);
51+
}
52+
53+
const bytes = invalidModuleBytes();
54+
const iterations = 80;
55+
56+
for (let i = 0; i < iterations; ++i) {
57+
let message;
58+
try {
59+
new WebAssembly.Module(bytes);
60+
throw new Error("expected CompileError");
61+
} catch (error) {
62+
assert.truthy(error instanceof WebAssembly.CompileError, `expected CompileError, got ${error}`);
63+
message = String(error);
64+
}
65+
assert.truthy(
66+
message.includes("function at index 0"),
67+
`iteration ${i}: expected error for function 0, got: ${message}`
68+
);
69+
for (let j = 1; j < 16; ++j) {
70+
assert.truthy(
71+
!message.includes(`function at index ${j}`),
72+
`iteration ${i}: should not report function ${j} when function 0 also fails: ${message}`
73+
);
74+
}
75+
}

‎Source/JavaScriptCore/wasm/WasmEntryPlan.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,14 @@ bool EntryPlan::failIfMixedExceptionHandlingProposals()
273273
return false;
274274
}
275275

276+
void EntryPlan::failFunctionCompilation(FunctionCodeIndex functionIndex, String&& errorMessage, CompilationError error)
277+
{
278+
failAtFunction(functionIndex, WTF::move(errorMessage), error);
279+
m_currentIndex = m_numberOfFunctions;
280+
if (hasWork())
281+
moveToState(State::Compiled);
282+
}
283+
276284
bool EntryPlan::completeSyncIfPossible()
277285
{
278286
Locker locker { m_lock };

‎Source/JavaScriptCore/wasm/WasmEntryPlan.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ class EntryPlan : public Plan, public StreamingParserClient {
118118
void complete() WTF_REQUIRES_LOCK(m_lock) override;
119119

120120
bool failIfMixedExceptionHandlingProposals() WTF_REQUIRES_LOCK(m_lock);
121+
// Records a function validation error (lowest index wins) and stops assigning more work.
122+
// Completion is deferred to ThreadCountHolder / completeInStreaming so concurrent
123+
// workers can still report a lower-index failure.
124+
void failFunctionCompilation(FunctionCodeIndex, String&& errorMessage, CompilationError = CompilationError::Default) WTF_REQUIRES_LOCK(m_lock);
121125

122126
virtual bool prepareImpl() = 0;
123127
virtual void compileFunction(FunctionCodeIndex functionIndex) = 0;

‎Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,7 @@ void IPIntPlan::compileFunction(FunctionCodeIndex functionIndex)
9797

9898
if (!parseAndCompileResult) [[unlikely]] {
9999
Locker locker { m_lock };
100-
if (!m_errorMessage) {
101-
// Multiple compiles could fail simultaneously. We arbitrarily choose the first.
102-
fail(makeString(parseAndCompileResult.error(), ", in function at index "_s, functionIndex.rawIndex())); // FIXME make this an Expected.
103-
}
104-
m_currentIndex = m_moduleInformation->functions.size();
100+
failFunctionCompilation(functionIndex, makeString(parseAndCompileResult.error(), ", in function at index "_s, functionIndex.rawIndex()));
105101
return;
106102
}
107103

@@ -117,7 +113,7 @@ void IPIntPlan::compileFunction(FunctionCodeIndex functionIndex)
117113

118114
if (usesSIMD && !Options::useBBQJIT() && !Options::useWasmIPIntSIMD()) {
119115
Locker locker { m_lock };
120-
Base::fail(makeString("JIT is disabled, but the entrypoint for "_s, functionIndex.rawIndex(), " requires JIT"_s));
116+
failFunctionCompilation(functionIndex, makeString("JIT is disabled, but the entrypoint for "_s, functionIndex.rawIndex(), " requires JIT"_s));
121117
return;
122118
}
123119

‎Source/JavaScriptCore/wasm/WasmPlan.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,18 @@ void Plan::fail(String&& errorMessage, CompilationError error)
127127
complete();
128128
}
129129

130+
void Plan::failAtFunction(FunctionCodeIndex functionIndex, String&& errorMessage, CompilationError error)
131+
{
132+
ASSERT(errorMessage);
133+
// Non-function failures (OOM, parse, cancel) win over function validation errors.
134+
if (failed() && (!m_errorFunctionIndex || *m_errorFunctionIndex <= functionIndex))
135+
return;
136+
dataLogLnIf(WasmPlanInternal::verbose, "failing function ", functionIndex, " with message: ", errorMessage);
137+
m_errorMessage = WTF::move(errorMessage);
138+
m_errorFunctionIndex = functionIndex;
139+
m_error = error;
140+
}
141+
130142
Plan::~Plan() = default;
131143

132144
CString Plan::signpostMessage(CompilationMode compilationMode, uint32_t functionIndexSpace) const

‎Source/JavaScriptCore/wasm/WasmPlan.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "WasmJS.h"
3333
#include "WasmModuleInformation.h"
3434
#include "WasmOMGIRGenerator.h"
35+
#include <optional>
3536
#include <wtf/Bag.h>
3637
#include <wtf/CrossThreadCopier.h>
3738
#include <wtf/SharedTask.h>
@@ -81,6 +82,7 @@ class Plan : public ThreadSafeRefCounted<Plan> {
8182
protected:
8283
void runCompletionTasks() WTF_REQUIRES_LOCK(m_lock);
8384
void fail(String&& errorMessage, CompilationError = CompilationError::Default) WTF_REQUIRES_LOCK(m_lock);
85+
void failAtFunction(FunctionCodeIndex, String&& errorMessage, CompilationError = CompilationError::Default) WTF_REQUIRES_LOCK(m_lock);
8486

8587
virtual bool isComplete() const = 0;
8688
virtual void complete() WTF_REQUIRES_LOCK(m_lock) = 0;
@@ -100,6 +102,7 @@ class Plan : public ThreadSafeRefCounted<Plan> {
100102
Vector<std::pair<VM*, CompletionTask>, 1> m_completionTasks;
101103

102104
String m_errorMessage;
105+
std::optional<FunctionCodeIndex> m_errorFunctionIndex;
103106
CompilationError m_error { CompilationError::Default };
104107
};
105108

0 commit comments

Comments
 (0)