Title: [225378] trunk
Revision
225378
Author
jfbast...@apple.com
Date
2017-11-30 18:41:10 -0800 (Thu, 30 Nov 2017)

Log Message

WebAssembly: improve stack trace
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/_javascript_Core:

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):

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (225377 => 225378)


--- trunk/JSTests/ChangeLog	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/ChangeLog	2017-12-01 02:41:10 UTC (rev 225378)
@@ -1,3 +1,30 @@
+2017-11-30  JF Bastien  <jfbast...@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....@apple.com>
 
         jsc shell's flashHeapAccess() should not do JS work after releasing access to the heap.

Modified: trunk/JSTests/wasm/assert.js (225377 => 225378)


--- trunk/JSTests/wasm/assert.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/assert.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -89,6 +89,13 @@
     }
 };
 
+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;

Modified: trunk/JSTests/wasm/function-tests/nameSection.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/nameSection.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/nameSection.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -64,9 +64,9 @@
 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

Modified: trunk/JSTests/wasm/function-tests/stack-overflow.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/stack-overflow.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/stack-overflow.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -47,7 +47,7 @@
         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 @@
 
         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;
             }
         }

Modified: trunk/JSTests/wasm/function-tests/stack-trace.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/stack-trace.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/stack-trace.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -1,10 +1,6 @@
 import Builder from '../Builder.js'
+import * as assert from '../assert.js'
 
-function assert(b) {
-    if (!b)
-        throw new Error("Bad");
-}
-
 const builder = (new Builder())
     .Type().End()
     .Import()
@@ -40,18 +36,18 @@
 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;
 }

Modified: trunk/JSTests/wasm/function-tests/trap-after-cross-instance-call.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/trap-after-cross-instance-call.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/trap-after-cross-instance-call.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -52,7 +52,7 @@
 
 
 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;
 }
 

Modified: trunk/JSTests/wasm/function-tests/trap-load-2.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/trap-load-2.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/trap-load-2.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -20,7 +20,7 @@
     .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;
 }
 

Modified: trunk/JSTests/wasm/function-tests/trap-load.js (225377 => 225378)


--- trunk/JSTests/wasm/function-tests/trap-load.js	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/JSTests/wasm/function-tests/trap-load.js	2017-12-01 02:41:10 UTC (rev 225378)
@@ -24,7 +24,7 @@
 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;
 }
 

Modified: trunk/Source/_javascript_Core/ChangeLog (225377 => 225378)


--- trunk/Source/_javascript_Core/ChangeLog	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-12-01 02:41:10 UTC (rev 225378)
@@ -1,3 +1,37 @@
+2017-11-30  JF Bastien  <jfbast...@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.sz...@sony.com>
 
         Make LegacyCustomProtocolManager optional for network process

Modified: trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp (225377 => 225378)


--- trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp	2017-12-01 02:41:10 UTC (rev 225378)
@@ -279,10 +279,7 @@
 
     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");

Modified: trunk/Source/_javascript_Core/runtime/StackFrame.cpp (225377 => 225378)


--- trunk/Source/_javascript_Core/runtime/StackFrame.cpp	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/runtime/StackFrame.cpp	2017-12-01 02:41:10 UTC (rev 225378)
@@ -75,11 +75,8 @@
 
 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 @@
 
 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)

Modified: trunk/Source/_javascript_Core/wasm/WasmIndexOrName.cpp (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmIndexOrName.cpp	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmIndexOrName.cpp	2017-12-01 02:41:10 UTC (rev 225378)
@@ -39,17 +39,18 @@
             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

Modified: trunk/Source/_javascript_Core/wasm/WasmIndexOrName.h (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmIndexOrName.h	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmIndexOrName.h	2017-12-01 02:41:10 UTC (rev 225378)
@@ -43,6 +43,7 @@
     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&);
 

Modified: trunk/Source/_javascript_Core/wasm/WasmModuleInformation.cpp (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmModuleInformation.cpp	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmModuleInformation.cpp	2017-12-01 02:41:10 UTC (rev 225378)
@@ -29,12 +29,23 @@
 #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() { }

Modified: trunk/Source/_javascript_Core/wasm/WasmModuleInformation.h (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmModuleInformation.h	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmModuleInformation.h	2017-12-01 02:41:10 UTC (rev 225378)
@@ -57,6 +57,7 @@
     uint32_t internalFunctionCount() const { return internalFunctionSignatureIndices.size(); }
 
     const Vector<uint8_t> source;
+    const CString hash;
 
     Vector<Import> imports;
     Vector<SignatureIndex> importFunctionSignatureIndices;

Modified: trunk/Source/_javascript_Core/wasm/WasmNameSection.h (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmNameSection.h	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmNameSection.h	2017-12-01 02:41:10 UTC (rev 225378)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "WasmName.h"
+#include <wtf/text/CString.h>
 #include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/Vector.h>
 #include <utility>
@@ -33,11 +34,20 @@
 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;
 };
 

Modified: trunk/Source/_javascript_Core/wasm/WasmNameSectionParser.cpp (225377 => 225378)


--- trunk/Source/_javascript_Core/wasm/WasmNameSectionParser.cpp	2017-12-01 02:04:07 UTC (rev 225377)
+++ trunk/Source/_javascript_Core/wasm/WasmNameSectionParser.cpp	2017-12-01 02:41:10 UTC (rev 225378)
@@ -35,7 +35,7 @@
 
 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());
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to