Ignore:
Timestamp:
Dec 15, 2016, 3:42:19 PM (9 years ago)
Author:
[email protected]
Message:

WebAssembly: improve compilation error messages
​https://bugs.webkit.org/show_bug.cgi?id=163919

Reviewed by Saam Barati.

JSTests:

Update error messages in these tests.
Use the assert.throws facility in many of them which weren't already.

  • wasm/js-api/element.js:

(assert.throws.new.WebAssembly.Module.builder.WebAssembly):
(assert.throws):

  • wasm/js-api/global-error.js:

(assert.throws.new.WebAssembly.Module.bin):
(assert.throws):
(new.Number):

  • wasm/js-api/table.js:

(assert.throws.new.WebAssembly.Module.builder.WebAssembly):
(assert.throws):
(assertBadTableImport):

  • wasm/js-api/test_Data.js:

(DataSectionWithoutMemory):

  • wasm/js-api/test_Start.js:

(InvalidStartFunctionIndex):

  • wasm/js-api/test_basic_api.js:

(const.c.in.constructorProperties.switch):

Source/JavaScriptCore:

The error handling messages were underwhelming because most
locations merely returned false on failure. This patch uses
std::expected to denote that failure isn't expected. Doing this
makes it almost impossible to mess up the code: either a function
returns a result (or a partial result for internal helpers) or an
error. We're not synchronizing the error string with the m_failed
bool anymore, and the caller will abort if they try to get a
result but the outcome was an error.

This also shortens the code significantly using macros, while also
judiciously preventing inlining of error handling code and biasing
towards success using UNLIKELY. This means that the generated code
should be more efficient (no string formatting on success, and
regalloc can avoid these unlikely paths).

The patch adds a few missing checks as well, especially related to
count limits and memory allocation failure.

As a follow-up I'd like to improve WTF::makeString further, so it
does coercions to string and understands ADL as I did in this
patch.

  • wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::fail):
(JSC::Wasm::parseAndCompile):

  • wasm/WasmB3IRGenerator.h:
  • wasm/WasmFormat.h:

(JSC::Wasm::isValidExternalKind):
(JSC::Wasm::makeString):

  • wasm/WasmFunctionParser.h:
  • wasm/WasmModuleParser.cpp:
  • wasm/WasmModuleParser.h:
  • wasm/WasmParser.h:

(JSC::Wasm::FailureHelper::makeString):
(JSC::Wasm::Parser::fail):
(JSC::Wasm::Parser<SuccessType>::Parser):
(JSC::Wasm::Parser<SuccessType>::consumeCharacter):
(JSC::Wasm::Parser<SuccessType>::consumeString):
(JSC::Wasm::Parser<SuccessType>::consumeUTF8String):
(JSC::Wasm::Parser<SuccessType>::parseVarUInt32):
(JSC::Wasm::Parser<SuccessType>::parseVarUInt64):
(JSC::Wasm::Parser<SuccessType>::parseVarInt32):
(JSC::Wasm::Parser<SuccessType>::parseVarInt64):
(JSC::Wasm::Parser<SuccessType>::parseUInt32):
(JSC::Wasm::Parser<SuccessType>::parseUInt64):
(JSC::Wasm::Parser<SuccessType>::parseUInt8):
(JSC::Wasm::Parser<SuccessType>::parseInt7):
(JSC::Wasm::Parser<SuccessType>::parseUInt7):
(JSC::Wasm::Parser<SuccessType>::parseVarUInt1):
(JSC::Wasm::Parser<SuccessType>::parseResultType):
(JSC::Wasm::Parser<SuccessType>::parseValueType):
(JSC::Wasm::Parser<SuccessType>::parseExternalKind):

  • wasm/WasmPlan.cpp:

(JSC::Wasm::Plan::run):

  • wasm/WasmSections.h:

(JSC::Wasm::isValidSection):
(JSC::Wasm::validateOrder):
(JSC::Wasm::makeString):

  • wasm/WasmValidate.cpp:

(JSC::Wasm::Validate::fail):
(JSC::Wasm::Validate::addUnreachable):
(JSC::Wasm::validateFunction):

  • wasm/WasmValidate.h:
  • wasm/generateWasmB3IRGeneratorInlinesHeader.py:
  • wasm/generateWasmOpsHeader.py:
  • wasm/generateWasmValidateInlinesHeader.py:

(loadMacro):
(storeMacro):

  • wasm/js/WebAssemblyInstanceConstructor.cpp:

(JSC::constructJSWebAssemblyInstance):

  • wasm/js/WebAssemblyModuleRecord.cpp:

(JSC::WebAssemblyModuleRecord::link):

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/wasm/generateWasmValidateInlinesHeader.py

    r209630 r209880  
    6161    op = opcodes[name]
    6262    return """
    63 template<> bool Validate::addOp<OpType::""" + toCpp(name) + """>(ExpressionType value, ExpressionType& result)
     63template<> auto Validate::addOp<OpType::""" + toCpp(name) + """>(ExpressionType value, ExpressionType& result) -> Result
    6464{
    65     if (value != """ + cppType(op["parameter"][0]) + """) {
    66         m_errorMessage = makeString(\"""" + name + """ expects the value to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(value));
    67         return false;
    68     }
     65    if (UNLIKELY(value != """ + cppType(op["parameter"][0]) + """))
     66        return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ value type mismatch");
    6967
    7068    result = """ + cppType(op["return"][0]) + """;
    71     return true;
     69    return { };
    7270}
    7371"""
    … …  
    7775    op = opcodes[name]
    7876    return """
    79 template<> bool Validate::addOp<OpType::""" + toCpp(name) + """>(ExpressionType left, ExpressionType right, ExpressionType& result)
     77template<> auto Validate::addOp<OpType::""" + toCpp(name) + """>(ExpressionType left, ExpressionType right, ExpressionType& result) -> Result
    8078{
    81     if (left != """ + cppType(op["parameter"][0]) + """) {
    82         m_errorMessage = makeString(\"""" + name + """ expects the left value to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(left));
    83         return false;
    84     }
     79    if (UNLIKELY(left != """ + cppType(op["parameter"][0]) + """))
     80        return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ left value type mismatch");
    8581
    86     if (right != """ + cppType(op["parameter"][1]) + """) {
    87         m_errorMessage = makeString(\"""" + name + """ expects the right value to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(right));
    88         return false;
    89     }
     82    if (UNLIKELY(right != """ + cppType(op["parameter"][1]) + """))
     83        return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ right value type mismatch");
    9084
    9185    result = """ + cppType(op["return"][0]) + """;
    92     return true;
     86    return { };
    9387}
    9488"""
    … …  
    9892    return """
    9993    case LoadOpType::""" + toCpp(name) + """: {
    100         if (pointer != """ + cppType(op["parameter"][0]) + """) {
    101             m_errorMessage = makeString(\"""" + name + """ expects the pointer to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(pointer));
    102             return false;
    103         }
     94        if (UNLIKELY(pointer != """ + cppType(op["parameter"][0]) + """))
     95            return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ pointer type mismatch");
    10496
    10597        result = """ + cppType(op["return"][0]) + """;
    106         return true;
     98        return { };
    10799    }"""
    108100
    … …  
    112104    return """
    113105    case StoreOpType::""" + toCpp(name) + """: {
    114         if (pointer != """ + cppType(op["parameter"][0]) + """) {
    115             m_errorMessage = makeString(\"""" + name + """ expects the pointer to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(pointer));
    116             return false;
    117         }
     106        if (UNLIKELY(pointer != """ + cppType(op["parameter"][0]) + """))
     107            return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ pointer type mismatch");
    118108
    119         if (value != """ + cppType(op["parameter"][1]) + """) {
    120             m_errorMessage = makeString(\"""" + name + """ expects the value to be of type: ", toString(""" + cppType(op["parameter"][0]) + """), " but got a value with type: ", toString(value));
    121             return false;
    122         }
     109        if (UNLIKELY(value != """ + cppType(op["parameter"][1]) + """))
     110            return UnexpectedType<Result::ErrorType>("validation failed: """ + name + """ value type mismatch");
    123111
    124         return true;
     112        return { };
    125113    }"""
    126114
    … …  
    142130""" + unarySpecializations + binarySpecializations + """
    143131
    144 bool Validate::load(LoadOpType op, ExpressionType pointer, ExpressionType& result, uint32_t)
     132auto Validate::load(LoadOpType op, ExpressionType pointer, ExpressionType& result, uint32_t) -> Result
    145133{
    146     if (!hasMemory())
    147         return false;
     134    if (UNLIKELY(!hasMemory()))
     135        return UnexpectedType<Result::ErrorType>("validation failed: load instruction without memory");
    148136
    149137    switch (op) {
    … …  
    152140}
    153141
    154 bool Validate::store(StoreOpType op, ExpressionType pointer, ExpressionType value, uint32_t)
     142auto Validate::store(StoreOpType op, ExpressionType pointer, ExpressionType value, uint32_t) -> Result
    155143{
    156     if (!hasMemory())
    157         return false;
     144    if (UNLIKELY(!hasMemory()))
     145        return UnexpectedType<Result::ErrorType>("validation failed: store instruction without memory");
    158146
    159147    switch (op) {
Note: See TracChangeset for help on using the changeset viewer.