- Revision
- 211479
- Author
- utatane....@gmail.com
- Date
- 2017-02-01 03:29:25 -0800 (Wed, 01 Feb 2017)
Log Message
ArityFixup should adjust SP first
https://bugs.webkit.org/show_bug.cgi?id=167239
Reviewed by Michael Saboff.
JSTests:
Significantly large arity fixup reliably causes this crash.
* stress/arity-fixup-should-not-touch-stack-area-below-sp.js: Added.
Source/_javascript_Core:
Arity fixup extends the stack and copy/fill the stack with
the values. At that time, we accidentally read/write stack
space below the stack pointer. As a result, we touch the area
of the stack space below the x64 red zone. These areas are unsafe.
OS may corrupt this space when constructing a signal stack.
The Linux kernel could not populate the pages for this space
and causes segmentation fault. This patch changes the stack
pointer before performing the arity fixup.
* jit/ThunkGenerators.cpp:
(JSC::arityFixupGenerator):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (211478 => 211479)
--- trunk/JSTests/ChangeLog 2017-02-01 10:59:17 UTC (rev 211478)
+++ trunk/JSTests/ChangeLog 2017-02-01 11:29:25 UTC (rev 211479)
@@ -1,3 +1,14 @@
+2017-02-01 Yusuke Suzuki <utatane....@gmail.com>
+
+ ArityFixup should adjust SP first
+ https://bugs.webkit.org/show_bug.cgi?id=167239
+
+ Reviewed by Michael Saboff.
+
+ Significantly large arity fixup reliably causes this crash.
+
+ * stress/arity-fixup-should-not-touch-stack-area-below-sp.js: Added.
+
2017-01-31 Filip Pizlo <fpi...@apple.com>
Move slow-running microbenchmarks out of JSTests/microbenchmarks
Added: trunk/JSTests/stress/arity-fixup-should-not-touch-stack-area-below-sp.js (0 => 211479)
--- trunk/JSTests/stress/arity-fixup-should-not-touch-stack-area-below-sp.js (rev 0)
+++ trunk/JSTests/stress/arity-fixup-should-not-touch-stack-area-below-sp.js 2017-02-01 11:29:25 UTC (rev 211479)
@@ -0,0 +1,5 @@
+var args = "y,".repeat(30000);
+var g = Function(args, "return 0");
+for (var i = 0; i < 1e3; ++i) {
+ g();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (211478 => 211479)
--- trunk/Source/_javascript_Core/ChangeLog 2017-02-01 10:59:17 UTC (rev 211478)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-02-01 11:29:25 UTC (rev 211479)
@@ -1,3 +1,24 @@
+2017-02-01 Yusuke Suzuki <utatane....@gmail.com>
+
+ ArityFixup should adjust SP first
+ https://bugs.webkit.org/show_bug.cgi?id=167239
+
+ Reviewed by Michael Saboff.
+
+ Arity fixup extends the stack and copy/fill the stack with
+ the values. At that time, we accidentally read/write stack
+ space below the stack pointer. As a result, we touch the area
+ of the stack space below the x64 red zone. These areas are unsafe.
+ OS may corrupt this space when constructing a signal stack.
+ The Linux kernel could not populate the pages for this space
+ and causes segmentation fault. This patch changes the stack
+ pointer before performing the arity fixup.
+
+ * jit/ThunkGenerators.cpp:
+ (JSC::arityFixupGenerator):
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+
2017-01-31 Filip Pizlo <fpi...@apple.com>
Make verifyEdge a RELEASE_ASSERT
Modified: trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp (211478 => 211479)
--- trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2017-02-01 10:59:17 UTC (rev 211478)
+++ trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2017-02-01 11:29:25 UTC (rev 211479)
@@ -440,6 +440,15 @@
jit.neg64(JSInterfaceJIT::argumentGPR0);
+ // Adjust call frame register and stack pointer to account for missing args.
+ // We need to change the stack pointer first before performing copy/fill loops.
+ // This stack space below the stack pointer is considered unsed by OS. Therefore,
+ // OS may corrupt this space when constructing a signal stack.
+ jit.move(JSInterfaceJIT::argumentGPR0, extraTemp);
+ jit.lshift64(JSInterfaceJIT::TrustedImm32(3), extraTemp);
+ jit.addPtr(extraTemp, JSInterfaceJIT::callFrameRegister);
+ jit.addPtr(extraTemp, JSInterfaceJIT::stackPointerRegister);
+
// Move current frame down argumentGPR0 number of slots
JSInterfaceJIT::Label copyLoop(jit.label());
jit.load64(JSInterfaceJIT::regT3, extraTemp);
@@ -455,12 +464,6 @@
jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::regT3);
jit.branchAdd32(MacroAssembler::NonZero, JSInterfaceJIT::TrustedImm32(1), JSInterfaceJIT::argumentGPR2).linkTo(fillUndefinedLoop, &jit);
- // Adjust call frame register and stack pointer to account for missing args
- jit.move(JSInterfaceJIT::argumentGPR0, extraTemp);
- jit.lshift64(JSInterfaceJIT::TrustedImm32(3), extraTemp);
- jit.addPtr(extraTemp, JSInterfaceJIT::callFrameRegister);
- jit.addPtr(extraTemp, JSInterfaceJIT::stackPointerRegister);
-
done.link(&jit);
# if CPU(X86_64)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (211478 => 211479)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2017-02-01 10:59:17 UTC (rev 211478)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2017-02-01 11:29:25 UTC (rev 211479)
@@ -613,6 +613,10 @@
// Move frame up t1 slots
negi t1
move cfr, t3
+ move t1, t0
+ lshiftp 3, t0
+ addp t0, cfr
+ addp t0, sp
.copyLoop:
loadi PayloadOffset[t3], t0
storei t0, PayloadOffset[t3, t1, 8]
@@ -631,9 +635,6 @@
addp 8, t3
baddinz 1, t2, .fillLoop
- lshiftp 3, t1
- addp t1, cfr
- addp t1, sp
.continue:
# Reload CodeBlock and PC, since the slow_path clobbered it.
loadp CodeBlock[cfr], t1
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (211478 => 211479)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2017-02-01 10:59:17 UTC (rev 211478)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2017-02-01 11:29:25 UTC (rev 211479)
@@ -525,6 +525,10 @@
move cfr, t3
subp CalleeSaveSpaceAsVirtualRegisters * 8, t3
addi CalleeSaveSpaceAsVirtualRegisters, t2
+ move t1, t0
+ lshiftp 3, t0
+ addp t0, cfr
+ addp t0, sp
.copyLoop:
loadq [t3], t0
storeq t0, [t3, t1, 8]
@@ -539,10 +543,6 @@
addp 8, t3
baddinz 1, t2, .fillLoop
- lshiftp 3, t1
- addp t1, cfr
- addp t1, sp
-
.continue:
# Reload CodeBlock and reset PC, since the slow_path clobbered them.
loadp CodeBlock[cfr], t1
