Bug 1357911 - Baldr: update names section format (r=luke)

MozReview-Commit-ID: 5xIiHiB6ico

--HG--
extra : rebase_source : 23999118a544fbb2b54f346bd499a7c750ee566d

js/src/jit-test/lib/wasm-binary.js

@@ -28,6 +28,11 @@ const dataId           = 11;
 // User-defined section names
 const nameName         = "name";
 
+// Name section name types
+const nameTypeModule   = 0;
+const nameTypeFunction = 1;
+const nameTypeLocal    = 2;
+
 // Type codes
 const I32Code          = 0x7f;
 const I64Code          = 0x7e;

js/src/jit-test/tests/wasm/binary.js

@@ -232,20 +232,35 @@ function elemSection(elemArrays) {
     return { name: elemId, body };
 }
 
-function nameSection(elems) {
+function nameSection(moduleName, funcNames) {
     var body = [];
     body.push(...string(nameName));
-    body.push(...varU32(elems.length));
-    for (let fn of elems) {
-        body.push(...encodedString(fn.name, fn.nameLen));
-        if (!fn.locals) {
-           body.push(...varU32(0));
-           continue;
-        }
-        body.push(...varU32(fn.locals.length));
-        for (let local of fn.locals)
-            body.push(...encodedString(local.name, local.nameLen));
+
+    if (moduleName) {
+        body.push(...varU32(nameTypeModule));
+
+        var subsection = encodedString(moduleName);
+
+        body.push(...varU32(subsection.length));
+        body.push(...subsection);
     }
+
+    if (funcNames) {
+        body.push(...varU32(nameTypeFunction));
+
+        var subsection = varU32(funcNames.length);
+
+        var funcIndex = 0;
+        for (let f of funcNames) {
+            subsection.push(...varU32(f.index ? f.index : funcIndex));
+            subsection.push(...encodedString(f.name, f.nameLen));
+            funcIndex++;
+        }
+
+        body.push(...varU32(subsection.length));
+        body.push(...subsection);
+    }
+
     return { name: userDefinedId, body };
 }
 
@@ -380,7 +395,7 @@ wasmEval(moduleWithSections([tooBigNameSection]));
 var customDefSec = customSection("wee", 42, 13);
 var declSec = declSection([0]);
 var bodySec = bodySection([v2vBody]);
-var nameSec = nameSection([{name:'hi'}]);
+var nameSec = nameSection(null, [{name:'hi'}]);
 wasmEval(moduleWithSections([customDefSec, v2vSigSection, declSec, bodySec]));
 wasmEval(moduleWithSections([v2vSigSection, customDefSec, declSec, bodySec]));
 wasmEval(moduleWithSections([v2vSigSection, declSec, customDefSec, bodySec]));
@@ -429,7 +444,7 @@ for (var i = FirstInvalidOpcode; i <= 0xff; i++) {
 }
 
 // Checking stack trace.
-function runStackTraceTest(namesContent, expectedName) {
+function runStackTraceTest(moduleName, funcNames, expectedName) {
     var sections = [
         sigSection([v2vSig]),
         importSection([{sigIndex:0, module:"env", func:"callback"}]),
@@ -439,8 +454,8 @@ function runStackTraceTest(namesContent, expectedName) {
         customSection("whoa"),
         customSection("wee", 42),
     ];
-    if (namesContent)
-        sections.push(nameSection(namesContent));
+    if (moduleName || funcNames)
+        sections.push(nameSection(moduleName, funcNames));
     sections.push(customSection("yay", 13));
 
     var result = "";
@@ -452,16 +467,20 @@ function runStackTraceTest(namesContent, expectedName) {
     assertEq(result, expectedName);
 };
 
-runStackTraceTest(null, 'wasm-function[1]');
-runStackTraceTest([{name:'blah'}, {name: 'test'}], 'test');
-runStackTraceTest([{name:'blah'}, {name: 'test', locals: [{name: 'var1'}, {name: 'var2'}]}], 'test');
-runStackTraceTest([{name:'blah'}, {name: 'test', locals: [{name: 'var1'}, {name: 'var2'}]}], 'test');
-runStackTraceTest([{name:'blah'}, {name: 'test1'}, {name: 'test2'}], 'test1');
-runStackTraceTest([{name:'blah'}, {name: 'test☃'}], 'test☃');
-runStackTraceTest([{name:'blah'}, {name: 'te\xE0\xFF'}], 'te\xE0\xFF');
-runStackTraceTest([{name:'blah'}], 'wasm-function[1]');
-runStackTraceTest([], 'wasm-function[1]');
+runStackTraceTest(null, null, 'wasm-function[1]');
+runStackTraceTest(null, [{name:'blah'}, {name:'test'}], 'test');
+runStackTraceTest(null, [{name:'test', index:1}], 'test');
+runStackTraceTest(null, [{name:'blah'}, {name:'test', locals: [{name: 'var1'}, {name: 'var2'}]}], 'test');
+runStackTraceTest(null, [{name:'blah'}, {name:'test', locals: [{name: 'var1'}, {name: 'var2'}]}], 'test');
+runStackTraceTest(null, [{name:'blah'}, {name:'test1'}], 'test1');
+runStackTraceTest(null, [{name:'blah'}, {name:'test☃'}], 'test☃');
+runStackTraceTest(null, [{name:'blah'}, {name:'te\xE0\xFF'}], 'te\xE0\xFF');
+runStackTraceTest(null, [{name:'blah'}], 'wasm-function[1]');
+runStackTraceTest(null, [], 'wasm-function[1]');
+runStackTraceTest("", [{name:'blah'}, {name:'test'}], 'test');
+runStackTraceTest("a", [{name:'blah'}, {name:'test'}], 'test');
 // Notice that invalid names section content shall not fail the parsing
-runStackTraceTest([{name:'blah'}, {nameLen: 100, name: 'test'}], 'wasm-function[1]'); // invalid name size
-runStackTraceTest([{name:'blah'}, {name: 'test', locals: [{nameLen: 40, name: 'var1'}]}], 'wasm-function[1]'); // invalid variable name size
-runStackTraceTest([{name:'blah'}, {name: ''}], 'wasm-function[1]'); // empty name
+runStackTraceTest(null, [{name:'blah'}, {name:'test', index: 2}], 'wasm-function[1]'); // invalid index
+runStackTraceTest(null, [{name:'blah'}, {name:'test', index: 100000}], 'wasm-function[1]'); // invalid index
+runStackTraceTest(null, [{name:'blah'}, {name:'test', nameLen: 100}], 'wasm-function[1]'); // invalid name size
+runStackTraceTest(null, [{name:'blah'}, {name:''}], 'wasm-function[1]'); // empty name

js/src/wasm/WasmBinaryConstants.h

@@ -27,8 +27,6 @@ namespace wasm {
 static const uint32_t MagicNumber        = 0x6d736100; // "\0asm"
 static const uint32_t EncodingVersion    = 0x01;
 
-static const char NameSectionName[]      = "name";
-
 enum class SectionId
 {
     Custom                               = 0,
@@ -431,6 +429,15 @@ enum class Op
     Limit
 };
 
+static const char NameSectionName[]      = "name";
+
+enum class NameType
+{
+    Module                               = 0,
+    Function                             = 1,
+    Local                                = 2
+};
+
 // Telemetry sample values for the JS_AOT_USAGE key, indicating whether asm.js
 // or WebAssembly is used.
 

js/src/wasm/WasmCode.h

@@ -274,7 +274,9 @@ struct NameInBytecode
     uint32_t offset;
     uint32_t length;
 
-    NameInBytecode() = default;
+    NameInBytecode()
+      : offset(UINT32_MAX), length(0)
+    {}
     NameInBytecode(uint32_t offset, uint32_t length)
       : offset(offset), length(length)
     {}

js/src/wasm/WasmValidate.cpp

@@ -202,6 +202,36 @@ Decoder::skipCustomSection(ModuleEnvironment* env)
     return true;
 }
 
+bool
+Decoder::startNameSubsection(NameType nameType, uint32_t* endOffset)
+{
+    const uint8_t* initialPosition = cur_;
+
+    uint32_t nameTypeValue;
+    if (!readVarU32(&nameTypeValue))
+        return false;
+
+    if (nameTypeValue != uint8_t(nameType)) {
+        cur_ = initialPosition;
+        *endOffset = NotStarted;
+        return true;
+    }
+
+    uint32_t payloadLength;
+    if (!readVarU32(&payloadLength) || payloadLength > bytesRemain())
+        return false;
+
+    *endOffset = (cur_ - beg_) + payloadLength;
+    return true;
+}
+
+bool
+Decoder::finishNameSubsection(uint32_t endOffset)
+{
+    MOZ_ASSERT(endOffset != NotStarted);
+    return endOffset == uint32_t(cur_ - beg_);
+}
+
 // Misc helpers.
 
 bool
@@ -1560,60 +1590,81 @@ DecodeDataSection(Decoder& d, ModuleEnvironment* env)
     return true;
 }
 
-static void
-MaybeDecodeNameSectionBody(Decoder& d, ModuleEnvironment* env)
+static bool
+DecodeModuleNameSubsection(Decoder& d, ModuleEnvironment* env)
 {
-    // For simplicity, ignore all failures, even OOM. Failure will simply result
-    // in the names section not being included for this module.
+    uint32_t endOffset;
+    if (!d.startNameSubsection(NameType::Module, &endOffset))
+        return false;
+    if (endOffset == Decoder::NotStarted)
+        return true;
 
-    uint32_t numFuncNames;
-    if (!d.readVarU32(&numFuncNames))
-        return;
+    // Don't use NameInBytecode for module name; instead store a copy of the
+    // string. This way supplying a module name doesn't need to save the whole
+    // bytecode. While function names are likely to be stripped in practice,
+    // module names aren't necessarily.
 
-    if (numFuncNames > MaxFuncs)
-        return;
+    uint32_t nameLength;
+    if (!d.readVarU32(&nameLength))
+        return false;
+
+    const uint8_t* bytes;
+    if (!d.readBytes(nameLength, &bytes))
+        return false;
+
+    // Do nothing with module name for now; a future patch will incorporate the
+    // module name into the callstack format.
+
+    return d.finishNameSubsection(endOffset);
+}
+
+static bool
+DecodeFunctionNameSubsection(Decoder& d, ModuleEnvironment* env)
+{
+    uint32_t endOffset;
+    if (!d.startNameSubsection(NameType::Function, &endOffset))
+        return false;
+    if (endOffset == Decoder::NotStarted)
+        return true;
+
+    uint32_t nameCount = 0;
+    if (!d.readVarU32(&nameCount) || nameCount > MaxFuncs)
+        return false;
 
-    // Use a local vector (and not env->funcNames) since it could result in a
-    // partially initialized result in case of failure in the middle.
     NameInBytecodeVector funcNames;
-    if (!funcNames.resize(numFuncNames))
-        return;
 
-    for (uint32_t i = 0; i < numFuncNames; i++) {
-        uint32_t numBytes;
-        if (!d.readVarU32(&numBytes))
-            return;
-        if (numBytes > MaxStringLength)
-            return;
+    for (uint32_t i = 0; i < nameCount; ++i) {
+        uint32_t funcIndex = 0;
+        if (!d.readVarU32(&funcIndex))
+            return false;
 
-        NameInBytecode name;
-        name.offset = d.currentOffset();
-        name.length = numBytes;
-        funcNames[i] = name;
+        // Names must refer to real functions and be given in ascending order.
+        if (funcIndex >= env->numFuncs() || funcIndex < funcNames.length())
+            return false;
 
-        if (!d.readBytes(numBytes))
-            return;
+        if (!funcNames.resize(funcIndex + 1))
+            return false;
 
-        // Skip local names for a function.
-        uint32_t numLocals;
-        if (!d.readVarU32(&numLocals))
-            return;
-        if (numLocals > MaxLocals)
-            return;
+        uint32_t nameLength = 0;
+        if (!d.readVarU32(&nameLength) || nameLength > MaxStringLength)
+            return false;
 
-        for (uint32_t j = 0; j < numLocals; j++) {
-            uint32_t numBytes;
-            if (!d.readVarU32(&numBytes))
-                return;
-            if (numBytes > MaxStringLength)
-                return;
+        NameInBytecode func;
+        func.offset = d.currentOffset();
+        func.length = nameLength;
+        funcNames[funcIndex] = func;
 
-            if (!d.readBytes(numBytes))
-                return;
-        }
+        if (!d.readBytes(nameLength))
+            return false;
     }
 
+    if (!d.finishNameSubsection(endOffset))
+        return false;
+
+    // To encourage fully valid function names subsections; only save names if
+    // the entire subsection decoded correctly.
     env->funcNames = Move(funcNames);
+    return true;
 }
 
 static bool
@@ -1627,8 +1678,17 @@ DecodeNameSection(Decoder& d, ModuleEnvironment* env)
 
     // Once started, custom sections do not report validation errors.
 
-    MaybeDecodeNameSectionBody(d, env);
+    if (!DecodeModuleNameSubsection(d, env))
+        goto finish;
 
+    if (!DecodeFunctionNameSubsection(d, env))
+        goto finish;
+
+    // The names we care about have already been extracted into 'env' so don't
+    // bother decoding the rest of the name section. finishCustomSection() will
+    // skip to the end of the name section (as it would for any other error).
+
+  finish:
     d.finishCustomSection(sectionStart, sectionSize);
     return true;
 }

js/src/wasm/WasmValidate.h

@@ -555,6 +555,11 @@ class Decoder
     void finishCustomSection(uint32_t sectionStart, uint32_t sectionSize);
     MOZ_MUST_USE bool skipCustomSection(ModuleEnvironment* env);
 
+    // The Name section has its own subsections. Like startSection, NotStart is
+    // returned as the endOffset if the given name subsection wasn't present.
+
+    MOZ_MUST_USE bool startNameSubsection(NameType nameType, uint32_t* endOffset);
+    MOZ_MUST_USE bool finishNameSubsection(uint32_t endOffset);
 
     // The infallible "unchecked" decoding functions can be used when we are
     // sure that the bytes are well-formed (by construction or due to previous