From b29b88ac31f552dae5b2976f39fb6f08f92b5649 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 12:10:38 -0700 Subject: harden s-expr parsing --- src/wasm/wasm-s-parser.cpp | 54 +++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 25 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 1842237a5..22dd36b13 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -37,7 +37,7 @@ int unhex(char c) { if (c >= '0' && c <= '9') return c - '0'; if (c >= 'a' && c <= 'f') return c - 'a' + 10; if (c >= 'A' && c <= 'F') return c - 'A' + 10; - abort(); + throw wasm::ParseException("invalid hexadecimal"); } } @@ -569,7 +569,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { } if (importModule.is()) { // this is an import, actually - assert(preParseImport); + if (!preParseImport) throw ParseException("!preParseImport in func"); std::unique_ptr im = make_unique(); im->name = name; im->module = importModule; @@ -582,7 +582,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { nameMapper.clear(); return; } - assert(!preParseImport); + if (preParseImport) throw ParseException("preParseImport in func"); if (brokeToAutoBlock) { ensureAutoBlock(); autoBlock->name = FAKE_RETURN; @@ -612,7 +612,7 @@ WasmType SExpressionWasmBuilder::stringToWasmType(const char* str, bool allowErr if (str[1] == '6' && str[2] == '4' && (prefix || str[3] == 0)) return f64; } if (allowError) return none; - abort(); + throw ParseException("invalid wasm type"); } Expression* SExpressionWasmBuilder::parseExpression(Element& s) { @@ -852,7 +852,7 @@ Expression* SExpressionWasmBuilder::makeExpression(Element& s) { default: abort_on(str); } } - abort(); + abort_on("unrecognized input string for parsing"); } Expression* SExpressionWasmBuilder::makeBinary(Element& s, BinaryOp op, WasmType type) { @@ -1112,11 +1112,11 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) { ret->bytes = 1; extra++; } else if (extra[0] == '1') { - assert(extra[1] == '6'); + if (extra[1] != '6') throw ParseException("expected load16"); ret->bytes = 2; extra += 2; } else if (extra[0] == '3') { - assert(extra[1] == '2'); + if (extra[1] != '2') throw ParseException("expected load32"); ret->bytes = 4; extra += 2; } @@ -1127,7 +1127,7 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) { while (!s[i]->isList()) { const char *str = s[i]->c_str(); const char *eq = strchr(str, '='); - assert(eq); + if (!eq) throw ParseException("no = in load attribute"); eq++; if (str[0] == 'a') { ret->align = atoi(eq); @@ -1152,11 +1152,11 @@ Expression* SExpressionWasmBuilder::makeStore(Element& s, WasmType type) { ret->bytes = 1; extra++; } else if (extra[0] == '1') { - assert(extra[1] == '6'); + if (extra[1] != '6') throw ParseException("expected store16"); ret->bytes = 2; extra += 2; } else if (extra[0] == '3') { - assert(extra[1] == '2'); + if (extra[1] != '2') throw ParseException("expected store32");; ret->bytes = 4; extra += 2; } @@ -1166,7 +1166,7 @@ Expression* SExpressionWasmBuilder::makeStore(Element& s, WasmType type) { while (!s[i]->isList()) { const char *str = s[i]->c_str(); const char *eq = strchr(str, '='); - assert(eq); + if (!eq) throw ParseException("missing = in store attribute");; eq++; if (str[0] == 'a') { ret->align = atoi(eq); @@ -1299,7 +1299,12 @@ Name SExpressionWasmBuilder::getLabel(Element& s) { return nameMapper.sourceToUnique(s.str()); } else { // offset, break to nth outside label - uint64_t offset = std::stoll(s.c_str(), nullptr, 0); + uint64_t offset; + try { + offset = std::stoll(s.c_str(), nullptr, 0); + } catch (std::invalid_argument) { + throw ParseException("invalid break offset"); + } if (offset > nameMapper.labelStack.size()) throw ParseException("invalid label", s.line, s.col); if (offset == nameMapper.labelStack.size()) { // a break to the function's scope. this means we need an automatic block, with a name @@ -1427,7 +1432,7 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { wasm.addImport(im.release()); i++; } else { - assert(inner.size() > 0 ? inner[0]->str() != IMPORT : true); + if (!(inner.size() > 0 ? inner[0]->str() != IMPORT : true)) throw ParseException("bad import ending"); // (memory (data ..)) format parseInnerData(*s[i]); wasm.memory.initial = wasm.memory.segments[0].data.size(); @@ -1507,7 +1512,7 @@ void SExpressionWasmBuilder::parseExport(Element& s) { ex->kind = ExternalKind::Global; if (wasm.getGlobalOrNull(ex->value) && wasm.getGlobal(ex->value)->mutable_) throw ParseException("cannot export a mutable global", s.line, s.col); } else { - WASM_UNREACHABLE(); + throw ParseException("invalid export"); } } else if (!s[2]->dollared() && !std::isdigit(s[2]->str()[0])) { ex->value = s[3]->str(); @@ -1519,7 +1524,7 @@ void SExpressionWasmBuilder::parseExport(Element& s) { } else if (s[2]->str() == GLOBAL) { ex->kind = ExternalKind::Global; } else { - WASM_UNREACHABLE(); + throw ParseException("invalid ext export"); } } else { // function @@ -1571,7 +1576,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } else if (im->kind == ExternalKind::Table) { im->name = Name("import$table$" + std::to_string(0)); } else { - WASM_UNREACHABLE(); + throw ParseException("invalid import"); } } if (!s[i]->quoted()) { @@ -1582,7 +1587,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } else if (s[i]->str() == GLOBAL) { im->kind = ExternalKind::Global; } else { - WASM_UNREACHABLE(); + throw ParseException("invalid ext import"); } i++; } else if (!newStyle) { @@ -1614,7 +1619,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } if (inner.size() > j+1) { Element& result = *inner[j+1]; - assert(result[0]->str() == RESULT); + if (result[0]->str() != RESULT) throw ParseException("expected result"); type->result = stringToWasmType(result[1]->str()); } } @@ -1624,7 +1629,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { im->globalType = stringToWasmType(inner[j]->str()); } else { auto& inner2 = *inner[j]; - assert(inner2[0]->str() == MUT); + if (inner2[0]->str() != MUT) throw ParseException("expected mut"); im->globalType = stringToWasmType(inner2[1]->str()); throw ParseException("cannot import a mutable global", s.line, s.col); } @@ -1692,7 +1697,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { } if (importModule.is()) { // this is an import, actually - assert(preParseImport); + if (!preParseImport) throw ParseException("!preParseImport in global"); if (mutable_) throw ParseException("cannot import a mutable global", s.line, s.col); std::unique_ptr im = make_unique(); im->name = global->name; @@ -1703,7 +1708,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { wasm.addImport(im.release()); return; } - assert(!preParseImport); + if (preParseImport) throw ParseException("preParseImport in global"); global->type = type; if (i < s.size()) { global->init = parseExpression(s[i++]); @@ -1711,7 +1716,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { throw ParseException("global without init", s.line, s.col); } global->mutable_ = mutable_; - assert(i == s.size()); + if (i != s.size()) throw ParseException("extra import elements"); wasm.addGlobal(global.release()); } @@ -1740,7 +1745,7 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) { } else if (inner[0]->str() == IMPORT) { importModule = inner[1]->str(); importBase = inner[2]->str(); - assert(preParseImport); + if (!preParseImport) throw ParseException("!preParseImport in table"); auto im = make_unique(); im->kind = ExternalKind::Table; im->module = importModule; @@ -1749,7 +1754,7 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) { wasm.addImport(im.release()); i++; } else { - WASM_UNREACHABLE(); + throw ParseException("invalid table"); } } if (i == s.size()) return; @@ -1815,7 +1820,6 @@ void SExpressionWasmBuilder::parseType(Element& s) { i++; } Element& func = *s[i]; - assert(func.isList()); for (size_t k = 1; k < func.size(); k++) { Element& curr = *func[k]; if (curr[0]->str() == PARAM) { -- cgit v1.2.3 From 18e83cb152447a07fc0b3fe9c16b2b8c60aab328 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 13:49:32 -0700 Subject: s-expr parsing: handle empty switch --- src/wasm/wasm-s-parser.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 22dd36b13..77ca8910d 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1340,6 +1340,7 @@ Expression* SExpressionWasmBuilder::makeBreakTable(Element& s) { while (!s[i]->isList()) { ret->targets.push_back(getLabel(*s[i++])); } + if (ret->targets.size() == 0) throw ParseException("switch with no targets"); ret->default_ = ret->targets.back(); ret->targets.pop_back(); ret->condition = parseExpression(s[i++]); -- cgit v1.2.3 From 7438fbd5cb4d882584f405ae37e726fd14928f4d Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 13:52:42 -0700 Subject: handle a parse error of a function declaration with mixed import inside --- src/wasm/wasm-s-parser.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 77ca8910d..614b30bdd 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -577,6 +577,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { im->kind = ExternalKind::Function; im->functionType = wasm.getFunctionType(type)->name; wasm.addImport(im.release()); + if (currFunction) throw ParseException("import module inside function dec"); assert(!currFunction); currLocalTypes.clear(); nameMapper.clear(); -- cgit v1.2.3 From 989c5295deebe71e4bf80e6cc5ae4f334134f9ea Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 13:56:25 -0700 Subject: handle duplicate imports and globals in s-expr parsing --- src/wasm/wasm-s-parser.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 614b30bdd..10d4c3f17 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -576,6 +576,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { im->base = importBase; im->kind = ExternalKind::Function; im->functionType = wasm.getFunctionType(type)->name; + if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); if (currFunction) throw ParseException("import module inside function dec"); assert(!currFunction); @@ -1431,6 +1432,7 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { im->module = importModule; im->base = importBase; im->name = importModule; + if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); i++; } else { @@ -1653,6 +1655,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { wasm.memory.max = atoi(inner[j++]->c_str()); } } + if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); } @@ -1707,6 +1710,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { im->base = importBase; im->kind = ExternalKind::Global; im->globalType = type; + if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); return; } @@ -1719,6 +1723,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { } global->mutable_ = mutable_; if (i != s.size()) throw ParseException("extra import elements"); + if (wasm.getGlobalOrNull(global->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addGlobal(global.release()); } @@ -1753,6 +1758,7 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) { im->module = importModule; im->base = importBase; im->name = importModule; + if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); i++; } else { -- cgit v1.2.3 From a13a4ea6d71d2bd06bc906ab5d26f3323c8d3cf6 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 14:11:22 -0700 Subject: handle duplicate function types in s-expr parsing --- src/wasm/wasm-s-parser.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 10d4c3f17..831ad41c3 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -409,6 +409,7 @@ void SExpressionWasmBuilder::preParseFunctionType(Element& s) { if (need) { functionType->name = Name::fromInt(wasm.functionTypes.size()); functionTypeNames.push_back(functionType->name); + if (wasm.getFunctionTypeOrNull(functionType->name)) throw ParseException("duplicate function type", s.line, s.col); wasm.addFunctionType(functionType.release()); } } @@ -1843,6 +1844,7 @@ void SExpressionWasmBuilder::parseType(Element& s) { type->name = Name::fromInt(wasm.functionTypes.size()); } functionTypeNames.push_back(type->name); + if (wasm.getFunctionTypeOrNull(type->name)) throw ParseException("duplicate function type", s.line, s.col); wasm.addFunctionType(type.release()); } -- cgit v1.2.3 From a2822077c5f92a11e02fba2aba83495254027a31 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 16:41:18 -0700 Subject: handle duplicate functions in s-expr parsing --- src/wasm/wasm-s-parser.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 831ad41c3..af77d75b6 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -600,6 +600,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { if (currFunction->result != result) throw ParseException("bad func declaration", s.line, s.col); currFunction->body = body; currFunction->type = type; + if (wasm.getFunctionOrNull(currFunction->name)) throw ParseException("duplicate function", s.line, s.col); wasm.addFunction(currFunction.release()); currLocalTypes.clear(); nameMapper.clear(); -- cgit v1.2.3 From 1a5dffbba9c247786ccfe4dd0a510d4e0f156595 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 17:01:02 -0700 Subject: host op parsing error handling --- src/wasm/wasm-s-parser.cpp | 9 +++++++++ src/wasm/wasm.cpp | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index af77d75b6..b2ee2a2f4 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -955,6 +955,15 @@ Expression* SExpressionWasmBuilder::makeHost(Element& s, HostOp op) { } else { parseCallOperands(s, 1, s.size(), ret); } + if (ret->op == HostOp::GrowMemory) { + if (ret->operands.size() != 1) { + throw ParseException("grow_memory needs one operand"); + } + } else { + if (ret->operands.size() != 0) { + throw ParseException("host needs zero operands"); + } + } ret->finalize(); return ret; } diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index d2bf4e75c..a879ebd05 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -74,7 +74,7 @@ Name GROW_WASM_MEMORY("__growWasmMemory"), const char* getExpressionName(Expression* curr) { switch (curr->_id) { - case Expression::Id::InvalidId: abort(); + case Expression::Id::InvalidId: WASM_UNREACHABLE(); case Expression::Id::BlockId: return "block"; case Expression::Id::IfId: return "if"; case Expression::Id::LoopId: return "loop"; @@ -500,7 +500,7 @@ void Host::finalize() { } break; } - default: abort(); + default: WASM_UNREACHABLE(); } } -- cgit v1.2.3 From 2ce31515cbb0953344dd5d67cfadb718a9abc8d8 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 29 May 2017 21:55:51 -0700 Subject: validate memory/table Address values in s-expr parsing --- src/wasm/wasm-s-parser.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index b2ee2a2f4..bff9232b8 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1650,20 +1650,36 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } } else if (im->kind == ExternalKind::Table) { if (j < inner.size() - 1) { - wasm.table.initial = atoi(inner[j++]->c_str()); + uint64_t num = atoi(inner[j++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive table size", s.line, s.col); + } + wasm.table.initial = num; } if (j < inner.size() - 1) { - wasm.table.max = atoi(inner[j++]->c_str()); + uint64_t num = atoi(inner[j++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive table size", s.line, s.col); + } + wasm.table.max = num; } else { wasm.table.max = Table::kMaxSize; } // ends with the table element type } else if (im->kind == ExternalKind::Memory) { if (j < inner.size()) { - wasm.memory.initial = atoi(inner[j++]->c_str()); + uint64_t num = atoi(inner[j++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive table size", s.line, s.col); + } + wasm.memory.initial = num; } if (j < inner.size()) { - wasm.memory.max = atoi(inner[j++]->c_str()); + uint64_t num = atoi(inner[j++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive table size", s.line, s.col); + } + wasm.memory.max = num; } } if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); -- cgit v1.2.3 From 08849db00cca62b3886458665b0bf2952f42a9d5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 30 May 2017 14:19:58 -0700 Subject: verify s-expr parsing of alignments --- src/wasm/wasm-s-parser.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index bff9232b8..60d424d7e 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1143,7 +1143,9 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) { if (!eq) throw ParseException("no = in load attribute"); eq++; if (str[0] == 'a') { - ret->align = atoi(eq); + uint64_t align = atoll(eq); + if (align > std::numeric_limits::max()) throw ParseException("bad align"); + ret->align = align; } else if (str[0] == 'o') { uint64_t offset = atoll(eq); if (offset > std::numeric_limits::max()) throw ParseException("bad offset"); @@ -1182,7 +1184,9 @@ Expression* SExpressionWasmBuilder::makeStore(Element& s, WasmType type) { if (!eq) throw ParseException("missing = in store attribute");; eq++; if (str[0] == 'a') { - ret->align = atoi(eq); + uint64_t align = atoll(eq); + if (align > std::numeric_limits::max()) throw ParseException("bad align"); + ret->align = align; } else if (str[0] == 'o') { ret->offset = atoi(eq); } else throw ParseException("bad store attribute"); -- cgit v1.2.3 From 1c7e82a4c6035d9ab52d892065bfff709a4424f8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 30 May 2017 14:20:57 -0700 Subject: handle out of range break offset parsing --- src/wasm/wasm-s-parser.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 60d424d7e..5ea8f2b0f 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1321,6 +1321,8 @@ Name SExpressionWasmBuilder::getLabel(Element& s) { offset = std::stoll(s.c_str(), nullptr, 0); } catch (std::invalid_argument) { throw ParseException("invalid break offset"); + } catch (std::out_of_range) { + throw ParseException("out of range break offset"); } if (offset > nameMapper.labelStack.size()) throw ParseException("invalid label", s.line, s.col); if (offset == nameMapper.labelStack.size()) { -- cgit v1.2.3 From fd71c67747c3a85db9670a0d558670c4cb124c91 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Tue, 30 May 2017 19:55:40 -0700 Subject: handle out of range memory size values in s-expr parsing --- src/wasm/wasm-s-parser.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 5ea8f2b0f..d3f411263 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1460,7 +1460,11 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { return; } } - wasm.memory.initial = atoi(s[i++]->c_str()); + uint64_t num = atoi(s[i++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive memory init", s.line, s.col); + } + wasm.memory.initial = num; if (i == s.size()) return; if (s[i]->isStr()) { uint64_t max = atoll(s[i]->c_str()); @@ -1475,7 +1479,11 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { if (curr[0]->str() == DATA) { offsetValue = 0; } else { - offsetValue = atoi(curr[j++]->c_str()); + uint64_t num = atoi(curr[j++]->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException("excessive memory offset", s.line, s.col); + } + offsetValue = num; } const char *input = curr[j]->c_str(); auto* offset = allocator.alloc(); -- cgit v1.2.3 From 0db29f6e05f03800b05b4778c118818149a193f9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 1 Jun 2017 17:23:01 -0700 Subject: refactor s-expr parsing code to remove duplication and unnecessary things --- src/wasm-s-parser.h | 12 +++++------ src/wasm/wasm-s-parser.cpp | 50 ++++++++++++++++------------------------------ 2 files changed, 23 insertions(+), 39 deletions(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h index b02b6523b..53594e7aa 100644 --- a/src/wasm-s-parser.h +++ b/src/wasm-s-parser.h @@ -53,10 +53,10 @@ class Element { public: Element(MixedArena& allocator) : isList_(true), list_(allocator), line(-1), col(-1), loc(nullptr) {} - bool isList() { return isList_; } - bool isStr() { return !isList_; } - bool dollared() { return isStr() && dollared_; } - bool quoted() { return isStr() && quoted_; } + bool isList() const { return isList_; } + bool isStr() const { return !isList_; } + bool dollared() const { return isStr() && dollared_; } + bool quoted() const { return isStr() && quoted_; } size_t line, col; SourceLocation* loc; @@ -69,8 +69,8 @@ public: } // string methods - cashew::IString str(); - const char* c_str(); + cashew::IString str() const; + const char* c_str() const; Element* setString(cashew::IString str__, bool dollared__, bool quoted__); Element* setMetadata(size_t line_, size_t col_, SourceLocation* loc_); diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index d3f411263..a9efae7a7 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -42,6 +42,15 @@ int unhex(char c) { } namespace wasm { + +static Address getCheckedAddress(const Element* s, const char* errorText) { + uint64_t num = atoi(s->c_str()); + if (num > std::numeric_limits::max()) { + throw ParseException(errorText, s->line, s->col); + } + return num; +} + Element::List& Element::list() { if (!isList()) throw ParseException("expected list", line, col); return list_; @@ -53,12 +62,12 @@ Element* Element::operator[](unsigned i) { return list()[i]; } -IString Element::str() { +IString Element::str() const { if (!isStr()) throw ParseException("expected string", line, col); return str_; } -const char* Element::c_str() { +const char* Element::c_str() const { if (!isStr()) throw ParseException("expected string", line, col); return str_.str; } @@ -580,7 +589,6 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addImport(im.release()); if (currFunction) throw ParseException("import module inside function dec"); - assert(!currFunction); currLocalTypes.clear(); nameMapper.clear(); return; @@ -1460,11 +1468,7 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { return; } } - uint64_t num = atoi(s[i++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive memory init", s.line, s.col); - } - wasm.memory.initial = num; + wasm.memory.initial = getCheckedAddress(s[i++], "excessive memory init"); if (i == s.size()) return; if (s[i]->isStr()) { uint64_t max = atoll(s[i]->c_str()); @@ -1479,11 +1483,7 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) { if (curr[0]->str() == DATA) { offsetValue = 0; } else { - uint64_t num = atoi(curr[j++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive memory offset", s.line, s.col); - } - offsetValue = num; + offsetValue = getCheckedAddress(curr[j++], "excessive memory offset"); } const char *input = curr[j]->c_str(); auto* offset = allocator.alloc(); @@ -1664,36 +1664,20 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } } else if (im->kind == ExternalKind::Table) { if (j < inner.size() - 1) { - uint64_t num = atoi(inner[j++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive table size", s.line, s.col); - } - wasm.table.initial = num; + wasm.table.initial = getCheckedAddress(inner[j++], "excessive table init size"); } if (j < inner.size() - 1) { - uint64_t num = atoi(inner[j++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive table size", s.line, s.col); - } - wasm.table.max = num; + wasm.table.max = getCheckedAddress(inner[j++], "excessive table max size"); } else { wasm.table.max = Table::kMaxSize; } // ends with the table element type } else if (im->kind == ExternalKind::Memory) { if (j < inner.size()) { - uint64_t num = atoi(inner[j++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive table size", s.line, s.col); - } - wasm.memory.initial = num; + wasm.memory.initial = getCheckedAddress(inner[j++], "excessive memory init size"); } if (j < inner.size()) { - uint64_t num = atoi(inner[j++]->c_str()); - if (num > std::numeric_limits::max()) { - throw ParseException("excessive table size", s.line, s.col); - } - wasm.memory.max = num; + wasm.memory.max = getCheckedAddress(inner[j++], "excessive memory max size"); } } if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); -- cgit v1.2.3 From 89cb24ae1f91132315479475c70a1224a224aab6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 1 Jun 2017 17:24:12 -0700 Subject: use atoll in getCheckedAddress string parsing --- src/wasm/wasm-s-parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index a9efae7a7..716355f46 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -44,7 +44,7 @@ int unhex(char c) { namespace wasm { static Address getCheckedAddress(const Element* s, const char* errorText) { - uint64_t num = atoi(s->c_str()); + uint64_t num = atoll(s->c_str()); if (num > std::numeric_limits::max()) { throw ParseException(errorText, s->line, s->col); } -- cgit v1.2.3 From bd001c187b90a570ce8babaad83af3b420f48eb5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 1 Jun 2017 17:24:57 -0700 Subject: clean up unnecessary spacing --- src/wasm/wasm-s-parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/wasm/wasm-s-parser.cpp') diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 716355f46..6536d78fb 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1648,7 +1648,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) { } if (inner.size() > j+1) { Element& result = *inner[j+1]; - if (result[0]->str() != RESULT) throw ParseException("expected result"); + if (result[0]->str() != RESULT) throw ParseException("expected result"); type->result = stringToWasmType(result[1]->str()); } } -- cgit v1.2.3