Skip to content
Permalink
Browse files
VMInspector::dumpRegisters() should not dump beyond the start of the …
…next frame.

https://bugs.webkit.org/show_bug.cgi?id=220136
rdar://64404201

Reviewed by Yusuke Suzuki.

JSTests:

* stress/dumpRegisters-should-stop-before-next-frame.js: Added.

Source/JavaScriptCore:

VMInspector::dumpRegisters() was dumping stack slots up for up to
codeBlock->numCalleeLocals() slots for any given CallFrame.  This is incorrect.
codeBlock->numCalleeLocals() indicates the maximum number of stack slots that the
codeBlock may use.  However, the executing codeBlock may not necessary use up that
number of slots before calling another function.

In the attached test case, the global program has 98 callee locals.  However, it
was only using a very small number of stack slots to call $vm.dumpRegisters().
On an ASAN build, iterating thru 98 stack slots of the global program (to dump
their contents) ended up reading beyond the top of the stack, and this made ASAN
very unhappy.  The fix is simply to ensure that VMInspector::dumpRegisters() never
dumps past the start of the next CallFrame.

* tools/VMInspector.cpp:
(JSC::VMInspector::dumpRegisters):


Canonical link: https://commits.webkit.org/232679@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271082 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Lam committed Dec 25, 2020
1 parent 1f6ac0a commit 56a0539eb277e6649149cd08b03c8220619d6645
Showing 4 changed files with 70 additions and 1 deletion.
@@ -1,3 +1,13 @@
2020-12-25 Mark Lam <mark.lam@apple.com>

VMInspector::dumpRegisters() should not dump beyond the start of the next frame.
https://bugs.webkit.org/show_bug.cgi?id=220136
rdar://64404201

Reviewed by Yusuke Suzuki.

* stress/dumpRegisters-should-stop-before-next-frame.js: Added.

2020-12-21 Yusuke Suzuki <ysuzuki@apple.com>

[JSC] Update elem.wast.js test
@@ -0,0 +1,15 @@
//@ runNoisyTestDefault

if (0) {
let z0 = []; let z1 = []; let z2 = []; let z3 = []; let z4 = []; let z5 = []; let z6 = []; let z7 = []; let z8 = []; let z9 = [];
let z10 = []; let z11 = []; let z12 = []; let z13 = []; let z14 = []; let z15 = []; let z16 = []; let z17 = []; let z18 = [];
let z19 = []; let z20 = []; let z21 = []; let z22 = []; let z23 = []; let z24 = []; let z25 = []; let z26 = []; let z27 = [];
let z28 = []; let z29 = []; let z30 = []; let z31 = []; let z32 = []; let z33 = []; let z34 = []; let z35 = []; let z36 = [];
let z37 = []; let z38 = []; let z39 = []; let z40 = []; let z41 = []; let z42 = []; let z43 = []; let z44 = []; let z45 = [];
let z46 = []; let z47 = []; let z48 = []; let z49 = []; let z50 = []; let z51 = []; let z52 = []; let z53 = []; let z54 = [];
let z55 = []; let z56 = []; let z57 = []; let z58 = []; let z59 = []; let z60 = []; let z61 = []; let z62 = []; let z63 = [];
let z64 = []; let z65 = []; let z66 = []; let z67 = []; let z68 = []; let z69 = []; let z70 = [];
for (let q of []) {}
}
[];
$vm.dumpRegisters();
@@ -1,3 +1,27 @@
2020-12-25 Mark Lam <mark.lam@apple.com>

VMInspector::dumpRegisters() should not dump beyond the start of the next frame.
https://bugs.webkit.org/show_bug.cgi?id=220136
rdar://64404201

Reviewed by Yusuke Suzuki.

VMInspector::dumpRegisters() was dumping stack slots up for up to
codeBlock->numCalleeLocals() slots for any given CallFrame. This is incorrect.
codeBlock->numCalleeLocals() indicates the maximum number of stack slots that the
codeBlock may use. However, the executing codeBlock may not necessary use up that
number of slots before calling another function.

In the attached test case, the global program has 98 callee locals. However, it
was only using a very small number of stack slots to call $vm.dumpRegisters().
On an ASAN build, iterating thru 98 stack slots of the global program (to dump
their contents) ended up reading beyond the top of the stack, and this made ASAN
very unhappy. The fix is simply to ensure that VMInspector::dumpRegisters() never
dumps past the start of the next CallFrame.

* tools/VMInspector.cpp:
(JSC::VMInspector::dumpRegisters):

2020-12-21 Jessica Tallon <jtallon@igalia.com>

[JSC] Add minimum parameter to the WASM JS-API for Memory & Table.
@@ -449,7 +449,27 @@ void VMInspector::dumpRegisters(CallFrame* callFrame)
}
dataLogF("-----------------------------------------------------------------------------\n");

end = it - codeBlock->numCalleeLocals() + codeBlock->numVars();
CallFrame* topCallFrame = vm.topCallFrame;
CallFrame* nextCallFrame = nullptr;

if (topCallFrame) {
topCallFrame->iterate(vm, [&] (StackVisitor& visitor) {
if (callFrame == visitor->callFrame())
return StackVisitor::Done;
nextCallFrame = visitor->callFrame();
return StackVisitor::Continue;
});
}

if (!nextCallFrame) {
// Don't know how much space on the stack frame is actually allocated for
// this frame. ASAN does not like it if we read beyond the frame.
end = it; // Stop the dump
} else {
end = bitwise_cast<const Register*>(nextCallFrame);
RELEASE_ASSERT(it - end < codeBlock->numCalleeLocals() - codeBlock->numVars());
}

if (it != end) {
do {
JSValue v = (*it).jsValue();

0 comments on commit 56a0539

Please sign in to comment.