WebAssembly: improve stack trace
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 02:41:10 +0000 (02:41 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 02:41:10 +0000 (02:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179343

Reviewed by Saam Barati.

JSTests:

Update the tests to follow the new format. Notably, SHA1 module
hash is now included in traces, and stubs are properly identified.

* wasm/assert.js: Add an assertion which matches regular expressions.
* wasm/function-tests/nameSection.js:
* wasm/function-tests/stack-overflow.js:
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.assertOverflows):
(assertOverflows.assertThrows.wasm.1):
(assertOverflows.assertThrows.wasm.0):
(assertOverflows.assertThrows):
(assertOverflows):
* wasm/function-tests/stack-trace.js:
(import.Builder.from.string_appeared_here.assert): Deleted.
* wasm/function-tests/trap-after-cross-instance-call.js:
(wasmFrameCountFromError):
* wasm/function-tests/trap-load-2.js:
(wasmFrameCountFromError):
* wasm/function-tests/trap-load.js:
(wasmFrameCountFromError):

Source/JavaScriptCore:

Stack traces now include:

  - Module name, if provided by the name section.
  - Module SHA1 hash if no name was provided
  - Stub identification, to differentiate from user code
  - Slightly different naming to match design from:
      https://github.com/WebAssembly/design/blob/master/Web.md#developer-facing-display-conventions

* interpreter/StackVisitor.cpp:
(JSC::StackVisitor::Frame::functionName const):
* runtime/StackFrame.cpp:
(JSC::StackFrame::functionName const):
(JSC::StackFrame::visitChildren):
* wasm/WasmIndexOrName.cpp:
(JSC::Wasm::IndexOrName::IndexOrName):
(JSC::Wasm::makeString):
* wasm/WasmIndexOrName.h:
(JSC::Wasm::IndexOrName::nameSection const):
* wasm/WasmModuleInformation.cpp:
(JSC::Wasm::ModuleInformation::ModuleInformation):
* wasm/WasmModuleInformation.h:
* wasm/WasmNameSection.h:
(JSC::Wasm::NameSection::NameSection):
(JSC::Wasm::NameSection::get):
* wasm/WasmNameSectionParser.cpp:
(JSC::Wasm::NameSectionParser::parse):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225378 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
JSTests/ChangeLog
JSTests/wasm/assert.js
JSTests/wasm/function-tests/nameSection.js
JSTests/wasm/function-tests/stack-overflow.js
JSTests/wasm/function-tests/stack-trace.js
JSTests/wasm/function-tests/trap-after-cross-instance-call.js
JSTests/wasm/function-tests/trap-load-2.js
JSTests/wasm/function-tests/trap-load.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/interpreter/StackVisitor.cpp
Source/JavaScriptCore/runtime/StackFrame.cpp
Source/JavaScriptCore/wasm/WasmIndexOrName.cpp
Source/JavaScriptCore/wasm/WasmIndexOrName.h
Source/JavaScriptCore/wasm/WasmModuleInformation.cpp
Source/JavaScriptCore/wasm/WasmModuleInformation.h
Source/JavaScriptCore/wasm/WasmNameSection.h
Source/JavaScriptCore/wasm/WasmNameSectionParser.cpp

index 9d7155f..7f6b0f8 100644 (file)
@@ -1,3 +1,30 @@
+2017-11-30  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: improve stack trace
+        https://bugs.webkit.org/show_bug.cgi?id=179343
+
+        Reviewed by Saam Barati.
+
+        Update the tests to follow the new format. Notably, SHA1 module
+        hash is now included in traces, and stubs are properly identified.
+
+        * wasm/assert.js: Add an assertion which matches regular expressions.
+        * wasm/function-tests/nameSection.js:
+        * wasm/function-tests/stack-overflow.js:
+        (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.assertOverflows):
+        (assertOverflows.assertThrows.wasm.1):
+        (assertOverflows.assertThrows.wasm.0):
+        (assertOverflows.assertThrows):
+        (assertOverflows):
+        * wasm/function-tests/stack-trace.js:
+        (import.Builder.from.string_appeared_here.assert): Deleted.
+        * wasm/function-tests/trap-after-cross-instance-call.js:
+        (wasmFrameCountFromError):
+        * wasm/function-tests/trap-load-2.js:
+        (wasmFrameCountFromError):
+        * wasm/function-tests/trap-load.js:
+        (wasmFrameCountFromError):
+
 2017-11-30  Mark Lam  <mark.lam@apple.com>
 
         jsc shell's flashHeapAccess() should not do JS work after releasing access to the heap.
index 52b39bf..979e22d 100644 (file)
@@ -89,6 +89,13 @@ export const eq = (lhs, rhs, msg) => {
     }
 };
 
+export const matches = (lhs, rhs, msg) => {
+    if (typeof lhs !== "string" || !(rhs instanceof RegExp))
+        _fail(`Expected string and regex object, got ${typeof lhs} and ${typeof rhs}`, msg);
+    if (!rhs.test(lhs))
+        _fail(`"${msg}" does not match ${rhs}`, msg);
+};
+
 const canonicalizeI32 = (number) => {
     if (Math.round(number) === number && number >= 2 ** 31)
         number = number - 2 ** 32;
index ba5419e..f4d28a8 100644 (file)
@@ -64,9 +64,9 @@ assert.eq(result, 1 + 42);
 assert.truthy(stacktrace);
 stacktrace = stacktrace.split("\n");
 assert.falsy(stacktrace[0].indexOf("_silly") === -1);
-assert.eq(stacktrace[1], "wasm function@[wasm code]"); // the wasm->js stub
-assert.eq(stacktrace[2], "wasm function: _eggs@[wasm code]");
-assert.eq(stacktrace[3], "wasm function: _bacon@[wasm code]");
-assert.eq(stacktrace[4], "wasm function: _spam@[wasm code]");
-assert.eq(stacktrace[5], "wasm function: _parrot@[wasm code]");
-assert.eq(stacktrace[6], "wasm function@[wasm code]"); // wasm entry
+assert.eq(stacktrace[1], "wasm-stub@[wasm code]"); // the wasm->js stub
+assert.eq(stacktrace[2], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_eggs]@[wasm code]");
+assert.eq(stacktrace[3], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_bacon]@[wasm code]");
+assert.eq(stacktrace[4], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_spam]@[wasm code]");
+assert.eq(stacktrace[5], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_parrot]@[wasm code]");
+assert.eq(stacktrace[6], "wasm-stub@[wasm code]"); // wasm entry
index 3b439bd..2925f97 100644 (file)
@@ -47,7 +47,7 @@ import * as assert from '../assert.js'
         assert.truthy(stack.length > 50);
         for (let i = 0; i < 50; ++i) {
             let item = stack[stack.length - i - 1];
-            assert.eq(item, "wasm function: 0@[wasm code]");
+            assert.matches(item, /[A-F0-9]{40}\.wasm-function\[0\]@\[wasm code\]/);
         } 
     }
     assertOverflows(i1);
@@ -126,17 +126,17 @@ import * as assert from '../assert.js'
 
         stack = stack.split("\n");
         assert.truthy(stack.length > 50);
-        const oneString = "wasm function: 1@[wasm code]";
-        const zeroString = "wasm function: 0@[wasm code]";
-        let currentIndex = stack[stack.length - 1] === oneString ? 1 : 0;
+        const oneRe = /[A-F0-9]{40}\.wasm-function\[1\]@\[wasm code\]/;
+        const zeroRe = /[A-F0-9]{40}\.wasm-function\[0\]@\[wasm code\]/;
+        let currentIndex = oneRe.test(stack[stack.length - 1]) ? 1 : 0;
         for (let i = 0; i < 50; ++i) {
             let item = stack[stack.length - 1 - i];
             if (currentIndex === 1) {
-                assert.eq(item, oneString);
+                assert.matches(item, oneRe);
                 currentIndex = 0;
             } else {
                 assert.eq(currentIndex, 0);
-                assert.eq(item, zeroString);
+                assert.matches(item, zeroRe);
                 currentIndex = 1;
             }
         }
index d70b3ea..2147a37 100644 (file)
@@ -1,9 +1,5 @@
 import Builder from '../Builder.js'
-
-function assert(b) {
-    if (!b)
-        throw new Error("Bad");
-}
+import * as assert from '../assert.js'
 
 const builder = (new Builder())
     .Type().End()
@@ -40,18 +36,18 @@ let imp = () => {
 const bin = builder.WebAssembly().get();
 const module = new WebAssembly.Module(bin);
 let instance = new WebAssembly.Instance(module, {imp: {f: imp}});
-assert(!stacktrace);
+assert.falsy(stacktrace);
 for (let i = 0; i < 10000; ++i) {
     instance.exports.entry();
-    assert(stacktrace);
+    assert.truthy(stacktrace);
     stacktrace = stacktrace.split("\n");
-    assert(stacktrace[0].indexOf("imp") !== -1); // the arrow function import named "imp".
-    assert(stacktrace[1] === "wasm function@[wasm code]"); // the wasm->js stub
-    assert(stacktrace[2] === "wasm function: 4@[wasm code]");
-    assert(stacktrace[3] === "wasm function: 2@[wasm code]");
-    assert(stacktrace[4] === "wasm function: 3@[wasm code]");
-    assert(stacktrace[5] === "wasm function: 1@[wasm code]");
-    assert(stacktrace[6] === "wasm function@[wasm code]"); // wasm entry
+    assert.truthy(stacktrace[0].indexOf("imp") !== -1); // the arrow function import named "imp".
+    assert.eq(stacktrace[1], "wasm-stub@[wasm code]"); // the wasm->js stub
+    assert.eq(stacktrace[2], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[4]@[wasm code]");
+    assert.eq(stacktrace[3], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[2]@[wasm code]");
+    assert.eq(stacktrace[4], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[3]@[wasm code]");
+    assert.eq(stacktrace[5], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[1]@[wasm code]");
+    assert.eq(stacktrace[6], "wasm-stub@[wasm code]"); // wasm entry
 
     stacktrace = null;
 }
index 0d86943..1963345 100644 (file)
@@ -52,7 +52,7 @@ let foo2 = new WebAssembly.Instance(module, importObject).exports.foo;
 
 
 function wasmFrameCountFromError(e) {
-    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm function") !== -1);
+    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm-") !== -1);
     return stackFrames.length;
 }
 
index f99b1ca..a6a8973 100644 (file)
@@ -20,7 +20,7 @@ const builder = (new Builder())
     .End();
 
 function wasmFrameCountFromError(e) {
-    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm function") !== -1);
+    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm-") !== -1);
     return stackFrames.length;
 }
 
index d98a567..7cf0e98 100644 (file)
@@ -24,7 +24,7 @@ const module = new WebAssembly.Module(bin);
 const foo = new WebAssembly.Instance(module, {a: {b: new WebAssembly.Memory({initial: numPages})}}).exports.foo;
 
 function wasmFrameCountFromError(e) {
-    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm function") !== -1);
+    let stackFrames = e.stack.split("\n").filter((s) => s.indexOf("wasm-") !== -1);
     return stackFrames.length;
 }
 
index dfea4ec..d46d540 100644 (file)
@@ -1,3 +1,37 @@
+2017-11-30  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: improve stack trace
+        https://bugs.webkit.org/show_bug.cgi?id=179343
+
+        Reviewed by Saam Barati.
+
+        Stack traces now include:
+
+          - Module name, if provided by the name section.
+          - Module SHA1 hash if no name was provided
+          - Stub identification, to differentiate from user code
+          - Slightly different naming to match design from:
+              https://github.com/WebAssembly/design/blob/master/Web.md#developer-facing-display-conventions
+
+        * interpreter/StackVisitor.cpp:
+        (JSC::StackVisitor::Frame::functionName const):
+        * runtime/StackFrame.cpp:
+        (JSC::StackFrame::functionName const):
+        (JSC::StackFrame::visitChildren):
+        * wasm/WasmIndexOrName.cpp:
+        (JSC::Wasm::IndexOrName::IndexOrName):
+        (JSC::Wasm::makeString):
+        * wasm/WasmIndexOrName.h:
+        (JSC::Wasm::IndexOrName::nameSection const):
+        * wasm/WasmModuleInformation.cpp:
+        (JSC::Wasm::ModuleInformation::ModuleInformation):
+        * wasm/WasmModuleInformation.h:
+        * wasm/WasmNameSection.h:
+        (JSC::Wasm::NameSection::NameSection):
+        (JSC::Wasm::NameSection::get):
+        * wasm/WasmNameSectionParser.cpp:
+        (JSC::Wasm::NameSectionParser::parse):
+
 2017-11-30  Stephan Szabo  <stephan.szabo@sony.com>
 
         Make LegacyCustomProtocolManager optional for network process
index d082b63..33bf995 100644 (file)
@@ -279,10 +279,7 @@ String StackVisitor::Frame::functionName() const
 
     switch (codeType()) {
     case CodeType::Wasm:
-        if (m_wasmFunctionIndexOrName.isEmpty())
-            traceLine = makeString("wasm function");
-        else
-            traceLine = makeString("wasm function: ", makeString(m_wasmFunctionIndexOrName));
+        traceLine = makeString(m_wasmFunctionIndexOrName);
         break;
     case CodeType::Eval:
         traceLine = ASCIILiteral("eval code");
index edb326a..9f2d60d 100644 (file)
@@ -75,11 +75,8 @@ String StackFrame::sourceURL() const
 
 String StackFrame::functionName(VM& vm) const
 {
-    if (m_isWasmFrame) {
-        if (m_wasmFunctionIndexOrName.isEmpty())
-            return ASCIILiteral("wasm function");
-        return makeString("wasm function: ", makeString(m_wasmFunctionIndexOrName));
-    }
+    if (m_isWasmFrame)
+        return makeString(m_wasmFunctionIndexOrName);
 
     if (m_codeBlock) {
         switch (m_codeBlock->codeType()) {
@@ -147,9 +144,6 @@ String StackFrame::toString(VM& vm) const
 
 void StackFrame::visitChildren(SlotVisitor& visitor)
 {
-    // FIXME: We should do something here about Wasm::IndexOrName.
-    // https://bugs.webkit.org/show_bug.cgi?id=176644
-    
     if (m_callee)
         visitor.append(m_callee);
     if (m_codeBlock)
index f3bef65..674a352 100644 (file)
@@ -39,17 +39,18 @@ IndexOrName::IndexOrName(Index index, std::pair<const Name*, RefPtr<NameSection>
             m_indexName.name = name.first;
         else
             m_indexName.index = indexTag | index;
-        m_nameSection = WTFMove(name.second);
     }
+    m_nameSection = WTFMove(name.second);
 }
 
 String makeString(const IndexOrName& ion)
 {
     if (ion.isEmpty())
-        return String();
+        return String("wasm-stub");
+    const String moduleName = ion.nameSection()->moduleName.size() ? String(ion.nameSection()->moduleName.data(), ion.nameSection()->moduleName.size()) : String(ion.nameSection()->moduleHash.data(), ion.nameSection()->moduleHash.size());
     if (ion.isIndex())
-        return String::number(ion.m_indexName.index & ~IndexOrName::indexTag);
-    return String(ion.m_indexName.name->data(), ion.m_indexName.name->size());
-};
+        return makeString(moduleName, ".wasm-function[", String::number(ion.m_indexName.index & ~IndexOrName::indexTag), "]");
+    return makeString(moduleName, ".wasm-function[", String(ion.m_indexName.name->data(), ion.m_indexName.name->size()), "]");
+}
 
 } } // namespace JSC::Wasm
index 70c7eb8..bea1f3c 100644 (file)
@@ -43,6 +43,7 @@ struct IndexOrName {
     bool isEmpty() const { return bitwise_cast<Index>(m_indexName) & emptyTag; }
     bool isIndex() const { return bitwise_cast<Index>(m_indexName) & indexTag; }
     bool isName() const { return !(isEmpty() || isName()); }
+    NameSection* nameSection() const { return m_nameSection ? m_nameSection.get() : nullptr; }
 
     friend String makeString(const IndexOrName&);
 
index 9da5cd0..0c80308 100644 (file)
 #if ENABLE(WEBASSEMBLY)
 
 #include "WasmNameSection.h"
+#include <wtf/SHA1.h>
+
+namespace {
+CString sha1(const Vector<uint8_t>& input)
+{
+    SHA1 hash;
+    hash.addBytes(input);
+    return hash.computeHexDigest();
+}
+}
 
 namespace JSC { namespace Wasm {
 
 ModuleInformation::ModuleInformation(Vector<uint8_t>&& sourceBytes)
     : source(WTFMove(sourceBytes))
-    , nameSection(new NameSection())
+    , hash(sha1(source))
+    , nameSection(new NameSection(hash))
 {
 }
 ModuleInformation::~ModuleInformation() { }
index 18239e3..c4f5ec8 100644 (file)
@@ -57,6 +57,7 @@ struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
     uint32_t internalFunctionCount() const { return internalFunctionSignatureIndices.size(); }
 
     const Vector<uint8_t> source;
+    const CString hash;
 
     Vector<Import> imports;
     Vector<SignatureIndex> importFunctionSignatureIndices;
index 8b1f009..2a9b0b8 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "WasmName.h"
+#include <wtf/text/CString.h>
 #include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/Vector.h>
 #include <utility>
 namespace JSC { namespace Wasm {
 
 struct NameSection : public ThreadSafeRefCounted<NameSection> {
+    NameSection(const CString &hash)
+        : moduleHash(hash.length())
+    {
+        for (size_t i = 0; i < hash.length(); ++i)
+            moduleHash[i] = static_cast<uint8_t>(*(hash.data() + i));
+    }
+    NameSection(const NameSection&) = delete;
+
     std::pair<const Name*, RefPtr<NameSection>> get(size_t functionIndexSpace)
     {
-        return functionIndexSpace < functionNames.size() ? std::make_pair(&functionNames[functionIndexSpace], RefPtr<NameSection>(this)) : std::pair<const Name*, RefPtr<NameSection>>(nullptr, nullptr);
+        return std::make_pair(functionIndexSpace < functionNames.size() ? &functionNames[functionIndexSpace] : nullptr, RefPtr<NameSection>(this));
     }
     Name moduleName;
+    Name moduleHash;
     Vector<Name> functionNames;
 };
 
index bf59e05..7339621 100644 (file)
@@ -35,7 +35,7 @@ namespace JSC { namespace Wasm {
 
 auto NameSectionParser::parse() -> Result
 {
-    RefPtr<NameSection> nameSection(adoptRef(*new NameSection()));
+    RefPtr<NameSection> nameSection(adoptRef(*new NameSection(m_info.hash)));
     WASM_PARSER_FAIL_IF(!nameSection->functionNames.tryReserveCapacity(m_info.functionIndexSpaceSize()), "can't allocate enough memory for function names");
     nameSection->functionNames.resize(m_info.functionIndexSpaceSize());