Validate number of instructions in constant expression

circle.yml

@@ -252,8 +252,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5138
-          expected_failed: 294
+          expected_passed: 5140
+          expected_failed: 292
           expected_skipped: 6381
 
   sanitizers-macos:
@@ -270,8 +270,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5138
-          expected_failed: 294
+          expected_passed: 5140
+          expected_failed: 292
           expected_skipped: 6381
 
   benchmark:
@@ -381,8 +381,8 @@ jobs:
           expected_failed: 9
           expected_skipped: 7323
       - spectest:
-          expected_passed: 5138
-          expected_failed: 294
+          expected_passed: 5140
+          expected_failed: 292
           expected_skipped: 6381
 
 workflows:

lib/fizzy/parser.cpp

@@ -123,60 +123,62 @@ inline parser_result<ConstantExpression> parse_constant_expression(
 {
     ConstantExpression result;
 
-    Instr instr;
-    do
+    uint8_t opcode;
+    std::tie(opcode, pos) = parse_byte(pos, end);
+
+    const auto instr = static_cast<Instr>(opcode);
+    switch (instr)
     {
-        uint8_t opcode;
-        std::tie(opcode, pos) = parse_byte(pos, end);
+    default:
+        throw validation_error{
+            "unexpected instruction in the constant expression: " + std::to_string(*(pos - 1))};
 
-        instr = static_cast<Instr>(opcode);
-        switch (instr)
-        {
-        default:
-            throw validation_error{
-                "unexpected instruction in the constant expression: " + std::to_string(*(pos - 1))};
+    case Instr::end:
+        throw validation_error{"constant expression is empty"};
 
-        case Instr::end:
-            break;
+    case Instr::global_get:
+    {
+        result.kind = ConstantExpression::Kind::GlobalGet;
+        std::tie(result.value.global_index, pos) = leb128u_decode<uint32_t>(pos, end);
+        break;
+    }
 
-        case Instr::global_get:
-        {
-            result.kind = ConstantExpression::Kind::GlobalGet;
-            std::tie(result.value.global_index, pos) = leb128u_decode<uint32_t>(pos, end);
-            break;
-        }
+    case Instr::i32_const:
+    {
+        result.kind = ConstantExpression::Kind::Constant;
+        int32_t value;
+        std::tie(value, pos) = leb128s_decode<int32_t>(pos, end);
+        result.value.constant = static_cast<uint32_t>(value);
+        break;
+    }
 
-        case Instr::i32_const:
-        {
-            result.kind = ConstantExpression::Kind::Constant;
-            int32_t value;
-            std::tie(value, pos) = leb128s_decode<int32_t>(pos, end);
-            result.value.constant = static_cast<uint32_t>(value);
-            break;
-        }
+    case Instr::i64_const:
+    {
+        result.kind = ConstantExpression::Kind::Constant;
+        int64_t value;
+        std::tie(value, pos) = leb128s_decode<int64_t>(pos, end);
+        result.value.constant = static_cast<uint64_t>(value);
+        break;
+    }
+    case Instr::f32_const:
+        // TODO: support this once floating points are implemented
+        result.kind = ConstantExpression::Kind::Constant;
+        result.value.constant = 0;
+        pos = skip(4, pos, end);
+        break;
+    case Instr::f64_const:
+        // TODO: support this once floating points are implemented
+        result.kind = ConstantExpression::Kind::Constant;
+        result.value.constant = 0;
+        pos = skip(8, pos, end);
+        break;
+    }
 
-        case Instr::i64_const:
-        {
-            result.kind = ConstantExpression::Kind::Constant;
-            int64_t value;
-            std::tie(value, pos) = leb128s_decode<int64_t>(pos, end);
-            result.value.constant = static_cast<uint64_t>(value);
-            break;
-        }
-        case Instr::f32_const:
-            // TODO: support this once floating points are implemented
-            result.kind = ConstantExpression::Kind::Constant;
-            result.value.constant = 0;
-            pos = skip(4, pos, end);
-            break;
-        case Instr::f64_const:
-            // TODO: support this once floating points are implemented
-            result.kind = ConstantExpression::Kind::Constant;
-            result.value.constant = 0;
-            pos = skip(8, pos, end);
-            break;
-        }
-    } while (instr != Instr::end);
+    uint8_t end_opcode;
+    std::tie(end_opcode, pos) = parse_byte(pos, end);
+
+    if (static_cast<Instr>(end_opcode) != Instr::end)
+        throw validation_error{"constant expression has multiple instructions"};
 
     return {result, pos};
 }

test/unittests/parser_test.cpp

@@ -612,20 +612,6 @@ TEST(parser, global_multi_global_inited)
     EXPECT_EQ(module.globalsec[0].expression.value.global_index, 0x01);
 }
 
-TEST(parser, global_single_multi_instructions_inited)
-{
-    const auto section_contents = bytes{
-        0x01, 0x7f, 0x01, uint8_t(Instr::i32_const), 0x10, uint8_t(Instr::i64_const), 0x7f, 0x0b};
-    const auto bin = bytes{wasm_prefix} + make_section(6, section_contents);
-
-    const auto module = parse(bin);
-    ASSERT_EQ(module.globalsec.size(), 1);
-    EXPECT_TRUE(module.globalsec[0].type.is_mutable);
-    EXPECT_EQ(module.globalsec[0].type.value_type, ValType::i32);
-    EXPECT_EQ(module.globalsec[0].expression.kind, ConstantExpression::Kind::Constant);
-    EXPECT_EQ(module.globalsec[0].expression.value.constant, uint64_t(-1));
-}
-
 TEST(parser, global_multi_const_inited)
 {
     const auto section_contents =
@@ -916,7 +902,7 @@ TEST(parser, element_section_invalid_initializer)
 TEST(parser, element_section_no_table_section)
 {
     const auto wasm =
-        bytes{wasm_prefix} + make_section(9, make_vec({"000b"_bytes + make_vec({"00"_bytes})}));
+        bytes{wasm_prefix} + make_section(9, make_vec({"0041000b"_bytes + make_vec({"00"_bytes})}));
     EXPECT_THROW_MESSAGE(
         parse(wasm), validation_error, "element section encountered without a table section");
 }

test/unittests/validation_test.cpp

@@ -52,6 +52,19 @@ TEST(validation, globals_invalid_initializer)
     const auto bin_invalid3 = from_hex("0061736d01000000060b027f00412a0b7f0123000b");
     EXPECT_THROW_MESSAGE(parse(bin_invalid3), validation_error,
         "global can be initialized by another const global only if it's imported");
+
+    /* wat2wasm --no-check
+      (global (mut i32) (i32.const 16) (i32.const -1))
+    */
+    const auto bin_multi = from_hex("0061736d010000000608017f014110417f0b");
+    EXPECT_THROW_MESSAGE(
+        parse(bin_multi), validation_error, "constant expression has multiple instructions");
+
+    /* wat2wasm --no-check
+      (global (mut i64))
+    */
+    const auto bin_no_instr = from_hex("0061736d010000000604017e010b");
+    EXPECT_THROW_MESSAGE(parse(bin_no_instr), validation_error, "constant expression is empty");
 }
 
 TEST(validation, import_memories_multiple)