Skip to content

Commit

Permalink
Add missing stack check to bbq->omg OSR
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270605
rdar://124060272

Reviewed by Keith Miller.

In https://commits.webkit.org/272448.466@safari-7618-branch, we turned
a stack overflow during OSR entry into a crash, preventing a security
issue. While the crash prevents memory corruption, it should never
happen. This patch fixes a case that was missed in the first patch.

Note: the test case currently runs forever, so it is skipped until
we fix the watchdog in wasm.

* JSTests/wasm/stress/omg-stack-overflow.js: Added.
(globalThis.callerIsBBQOrOMGCompiled.instantiateJsc):
(else.instantiateBrowser):
(async let):
* JSTests/wasm/stress/omg-stack-overflow.wasm: Added.
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):

Originally-landed-as: 272448.704@safari-7618-branch (36930ea). rdar://125261536
Canonical link: https://commits.webkit.org/276645@main
  • Loading branch information
Justin Michaud authored and JonWBedard committed Mar 25, 2024
1 parent d689b8e commit 3a0671f
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 3 deletions.
92 changes: 92 additions & 0 deletions JSTests/wasm/stress/omg-stack-overflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//@skip
// TODO: unskip after rdar://103455312 ([wasm] —watchdog should work for WebAssembly code)
//@runDefault("--maxPerThreadStackUsage=1572864", "--useConcurrentJIT=0")
let tools;
if (globalThis.callerIsBBQOrOMGCompiled) {
function instantiateJsc(filename, importObject) {
let bytes = read(filename, 'binary');
return WebAssembly.instantiate(bytes, importObject, 'x');
}
const log = function() {};
const report = $.agent.report;
const isJIT = callerIsBBQOrOMGCompiled;
tools = {log, report, isJIT, instantiate: instantiateJsc};
} else {
function instantiateBrowser(filename, importObject) {
let bytes = fetch(filename);
return WebAssembly.instantiateStreaming(bytes, importObject, 'x');
}
const log = function() {};
const report = function() {};
const isJIT = () => 1;
tools = {log, report, isJIT, instantiate: instantiateBrowser};
}
const {log, report, isJIT, instantiate} = tools;
const extra = {isJIT};
(async function () {
let tag3 = new WebAssembly.Tag({parameters: []});
let tag5 = new WebAssembly.Tag({parameters: ['anyfunc']});
let tag10 = new WebAssembly.Tag({parameters: []});
let global0 = new WebAssembly.Global({value: 'i64', mutable: true}, 199896032n);
let global1 = new WebAssembly.Global({value: 'externref', mutable: true}, {});
let global4 = new WebAssembly.Global({value: 'i64', mutable: true}, 161444932n);
let m2 = {global2: new WebAssembly.Global({value: 'externref', mutable: false}, {}), global4, global5: 623044.865719049, tag3, tag7: tag3, tag8: tag3, tag10};
let m0 = {global0, global3: global1, tag4: tag3, tag11: tag5};
let m1 = {global1, tag5, tag6: tag3, tag9: tag3};
let importObject0 = /** @type {Imports2} */ ({extra, m0, m1, m2});
let i0 = await instantiate('omg-stack-overflow.wasm', importObject0);
let {fn0, global6, global7, global8, memory0, table0, table1, tag0, tag1, tag2} = /**
@type {{
fn0: () => I32,
global6: WebAssembly.Global,
global7: WebAssembly.Global,
global8: WebAssembly.Global,
memory0: WebAssembly.Memory,
table0: WebAssembly.Table,
table1: WebAssembly.Table,
tag0: WebAssembly.Tag,
tag1: WebAssembly.Tag,
tag2: WebAssembly.Tag
}} */ (i0.instance.exports);
global8.value = 0;
global1.value = 'a';
log('calling fn0');
report('progress');
try {
for (let k=0; k<12; k++) {
let zzz = fn0();
zzz?.toString();
}
} catch (e) {
if (e instanceof WebAssembly.Exception) {
log(e); if (e.stack) { log(e.stack); }
} else if (e instanceof TypeError) {
if (e.message === 'an exported wasm function cannot contain a v128 parameter or return value') { log(e); } else { throw e; }
} else if (e instanceof WebAssembly.RuntimeError || e instanceof RangeError) { log(e); } else { throw e; }
}
// log('calling fn0');
// report('progress');
// try {
// for (let k=0; k<22; k++) {
// let zzz = fn0();
// zzz?.toString();
// }
// } catch (e) {
// if (e instanceof WebAssembly.Exception) {
// log(e); if (e.stack) { log(e.stack); }
// } else if (e instanceof TypeError) {
// if (e.message === 'an exported wasm function cannot contain a v128 parameter or return value') { log(e); } else { throw e; }
// } else if (e instanceof WebAssembly.RuntimeError || e instanceof RangeError) { log(e); } else { throw e; }
// }
// let tables = [table1, table0];
// for (let table of tables) {
// for (let k=0; k < table.length; k++) { table.get(k)?.toString(); }
// }
})().then(() => {
log('after')
report('after');
}).catch(e => {
log(e)
log('error')
report('error');
})
Binary file added JSTests/wasm/stress/omg-stack-overflow.wasm
Binary file not shown.
27 changes: 24 additions & 3 deletions Source/JavaScriptCore/wasm/WasmOperations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,18 @@ JSC_DEFINE_JIT_OPERATION(operationWasmTriggerOSREntryNow, void, (Probe::Context&
context.gpr(GPRInfo::nonPreservedNonArgumentGPR0) = 0;
};

auto doStackCheck = [instance](OSREntryCallee* callee) -> bool {
uintptr_t stackPointer = reinterpret_cast<uintptr_t>(currentStackPointer());
ASSERT(callee->stackCheckSize());
uintptr_t stackExtent = stackPointer - callee->stackCheckSize();
uintptr_t stackLimit = reinterpret_cast<uintptr_t>(instance->softStackLimit());
if (UNLIKELY(stackExtent >= stackPointer || stackExtent <= stackLimit)) {
dataLogLnIf(Options::verboseOSR(), "Skipping OMG loop tier up due to stack check; ", RawHex(stackPointer), " -> ", RawHex(stackExtent), " is past soft limit ", RawHex(stackLimit));
return false;
}
return true;
};

Wasm::CalleeGroup& calleeGroup = *instance->calleeGroup();
ASSERT(instance->memory()->mode() == calleeGroup.mode());

Expand Down Expand Up @@ -399,8 +411,11 @@ JSC_DEFINE_JIT_OPERATION(operationWasmTriggerOSREntryNow, void, (Probe::Context&
}

if (OSREntryCallee* osrEntryCallee = callee.osrEntryCallee()) {
if (osrEntryCallee->loopIndex() == loopIndex)
if (osrEntryCallee->loopIndex() == loopIndex) {
if (!doStackCheck(osrEntryCallee))
return returnWithoutOSREntry();
return doOSREntry(instance, context, callee, *osrEntryCallee, osrEntryData);
}
}

if (!shouldTriggerOMGCompile(tierUp, callee.replacement(), functionIndex) && !triggeredSlowPathToStartCompilation)
Expand All @@ -414,8 +429,11 @@ JSC_DEFINE_JIT_OPERATION(operationWasmTriggerOSREntryNow, void, (Probe::Context&
}

if (OSREntryCallee* osrEntryCallee = callee.osrEntryCallee()) {
if (osrEntryCallee->loopIndex() == loopIndex)
if (osrEntryCallee->loopIndex() == loopIndex) {
if (!doStackCheck(osrEntryCallee))
return returnWithoutOSREntry();
return doOSREntry(instance, context, callee, *osrEntryCallee, osrEntryData);
}
tierUp.dontOptimizeAnytimeSoon(functionIndex);
return returnWithoutOSREntry();
}
Expand Down Expand Up @@ -495,8 +513,11 @@ JSC_DEFINE_JIT_OPERATION(operationWasmTriggerOSREntryNow, void, (Probe::Context&
return returnWithoutOSREntry();
}

if (osrEntryCallee->loopIndex() == loopIndex)
if (osrEntryCallee->loopIndex() == loopIndex) {
if (!doStackCheck(osrEntryCallee))
return returnWithoutOSREntry();
return doOSREntry(instance, context, callee, *osrEntryCallee, osrEntryData);
}

tierUp.dontOptimizeAnytimeSoon(functionIndex);
return returnWithoutOSREntry();
Expand Down

0 comments on commit 3a0671f

Please sign in to comment.