Title: [217866] trunk
- Revision
- 217866
- Author
- [email protected]
- Date
- 2017-06-06 16:06:12 -0700 (Tue, 06 Jun 2017)
Log Message
Make sure we restore SP when doing calls that could be to JS
https://bugs.webkit.org/show_bug.cgi?id=172946
<rdar://problem/32579026>
Reviewed by JF Bastien.
JSTests:
* wasm/function-tests/many-args-tail-call-sp-restored.js: Added.
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.f1):
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.end):
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.f2):
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.let.instance.new.WebAssembly.Instance.new.WebAssembly.Module.builder.WebAssembly):
Source/_javascript_Core:
I was worried that there was a bug where we'd call JS, JS would tail call,
and we'd end up with a bogus SP. However, this bug does not exist since wasm
always calls to JS through a stub, and the stub treats SP as a callee save.
I wrote a test for this, and also made a note that this is the needed ABI.
* wasm/WasmBinding.cpp:
(JSC::Wasm::wasmToJs):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (217865 => 217866)
--- trunk/JSTests/ChangeLog 2017-06-06 23:04:48 UTC (rev 217865)
+++ trunk/JSTests/ChangeLog 2017-06-06 23:06:12 UTC (rev 217866)
@@ -1,3 +1,17 @@
+2017-06-06 Saam Barati <[email protected]>
+
+ Make sure we restore SP when doing calls that could be to JS
+ https://bugs.webkit.org/show_bug.cgi?id=172946
+ <rdar://problem/32579026>
+
+ Reviewed by JF Bastien.
+
+ * wasm/function-tests/many-args-tail-call-sp-restored.js: Added.
+ (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.f1):
+ (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.end):
+ (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.f2):
+ (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.let.instance.new.WebAssembly.Instance.new.WebAssembly.Module.builder.WebAssembly):
+
2017-06-06 Joseph Pecoraro <[email protected]>
Unreviewed rollout r217807. Caused a test to crash.
Added: trunk/JSTests/wasm/function-tests/many-args-tail-call-sp-restored.js (0 => 217866)
--- trunk/JSTests/wasm/function-tests/many-args-tail-call-sp-restored.js (rev 0)
+++ trunk/JSTests/wasm/function-tests/many-args-tail-call-sp-restored.js 2017-06-06 23:06:12 UTC (rev 217866)
@@ -0,0 +1,72 @@
+import Builder from '../Builder.js'
+import * as assert from '../assert.js'
+
+{
+ const count = 1000;
+ const signature = [];
+ for (let i = 0; i < count; ++i)
+ signature.push("i32");
+
+ let builder = new Builder()
+ .Type()
+ .End()
+ .Import()
+ .Function("imp", "f1", {params:signature, ret:"void"})
+ .Function("imp", "f2", {params:signature, ret:"void"})
+ .End()
+ .Function().End()
+ .Export()
+ .Function("foo")
+ .End()
+ .Code()
+ .Function("foo", {params: signature, ret: "void" });
+
+ for (let i = 0; i < count; ++i)
+ builder = builder.GetLocal(i);
+
+ builder = builder.Call(0);
+
+ for (let i = count; i--; )
+ builder = builder.GetLocal(i);
+
+ builder = builder.Call(1).Return().End().End();
+
+ let calledF1 = false;
+ let calledF2 = false;
+
+ function f1(...args) {
+ calledF1 = true;
+ let realArgs = [...args, ...args];
+ return end(...realArgs);
+ }
+ noInline(f1);
+
+ function end() {}
+ noInline(end);
+
+
+ function f2(...args) {
+ calledF2 = true;
+ let called = false;
+ assert.eq(args.length, count);
+ for (let i = 0; i < args.length; ++i) {
+ assert.eq(args[i], args.length - i - 1);
+ }
+ }
+ noInline(f2);
+
+ let instance = new WebAssembly.Instance(new WebAssembly.Module(builder.WebAssembly().get()), {imp: {f1, f2}});
+
+ const args = [];
+ for (let i = 0; i < count; ++i)
+ args.push(i);
+
+ for (let i = 0; i < 50; ++i) {
+ instance.exports.foo(...args);
+
+ assert.eq(calledF1, true);
+ assert.eq(calledF2, true);
+ calledF1 = false;
+ calledF2 = false;
+ }
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (217865 => 217866)
--- trunk/Source/_javascript_Core/ChangeLog 2017-06-06 23:04:48 UTC (rev 217865)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-06-06 23:06:12 UTC (rev 217866)
@@ -1,3 +1,20 @@
+2017-06-06 Saam Barati <[email protected]>
+
+ Make sure we restore SP when doing calls that could be to JS
+ https://bugs.webkit.org/show_bug.cgi?id=172946
+ <rdar://problem/32579026>
+
+ Reviewed by JF Bastien.
+
+ I was worried that there was a bug where we'd call JS, JS would tail call,
+ and we'd end up with a bogus SP. However, this bug does not exist since wasm
+ always calls to JS through a stub, and the stub treats SP as a callee save.
+
+ I wrote a test for this, and also made a note that this is the needed ABI.
+
+ * wasm/WasmBinding.cpp:
+ (JSC::Wasm::wasmToJs):
+
2017-06-06 Keith Miller <[email protected]>
OMG tier up checks should be a patchpoint
Modified: trunk/Source/_javascript_Core/wasm/WasmBinding.cpp (217865 => 217866)
--- trunk/Source/_javascript_Core/wasm/WasmBinding.cpp 2017-06-06 23:04:48 UTC (rev 217865)
+++ trunk/Source/_javascript_Core/wasm/WasmBinding.cpp 2017-06-06 23:06:12 UTC (rev 217866)
@@ -60,6 +60,9 @@
unsigned argCount = signature.argumentCount();
JIT jit;
+ // Note: WasmB3IRGenerator assumes that this stub treats SP as a callee save.
+ // If we ever change this, we will also need to change WasmB3IRGenerator.
+
// Below, we assume that the JS calling convention is always on the stack.
ASSERT(!jsCC.m_gprArgs.size());
ASSERT(!jsCC.m_fprArgs.size());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
