Support bitfield constants in Spicy for parsing.

One can now define bitfield "constants" for parsing by providing integer expressions with fields: type Foo = unit { x: bitfield(8) { a: 0..3 = 2; b: 4..7; c: 7 = 1; }; This will first parse the bitfield as usual and then enforce that the two bit ranges that are coming with expressions (i.e., `a` and `c`) indeed containing the expected values. If they don't, that's a parse error. We also support using such bitfield constants for look-ahead parsing: type Foo = unit { x: uint8[]; y: bitfield(8) { a: 0..3 = 4; b: 4..7; }; }; This will parse uint8s until a value is discovered that has its bits set as defined by the bitfield constant. (We use the term "constant" loosely here: only the bits with values are actually enforced to be constant, all others are parsed as usual.) Closes #1467.
zeek · Sep 1, 2023 · 22b31ed · 22b31ed
1 parent bbcd920
commit 22b31ed
Show file tree

Hide file tree

Showing 17 changed files with 220 additions and 23 deletions.
diff --git a/doc/examples/_skip.spicy b/doc/examples/_skip.spicy
@@ -0,0 +1,9 @@
+# Automatically generated; edit in Sphinx source code, not here.
+module Test;
+
+public type Foo = unit {
+    x: int8;
+     : skip bytes &size=5;
+    y: int8;
+    on %done { print self; }
+};
diff --git a/doc/programming/language/types.rst b/doc/programming/language/types.rst
@@ -40,6 +40,17 @@ parsed inside a unit.
 - Each ``RANGE`` has one of the forms ``LABEL: A`` or ``LABEL: A..B``
   where ``A`` and ``B`` are bit numbers.
 
+.. rubric:: Constants
+
+- ``bitfield(N) { RANGE_1 [= VALUE_1]; ...; RANGE_N [= VALUE_N] }``
+
+A bitfield constant represents expected values for all or some of the
+individual bitranges. They can be used only for parsing inside a unit
+field, not as values to otherwise operate with. To define such a
+constant with expected values, add ``= VALUE`` to the bitranges inside
+the type definition as suitable. See :ref:`parse_bitfield` for more
+information.
+
 .. include:: /autogen/types/bitfield.rst
 
 .. _type_bool:

diff --git a/doc/programming/parsing.rst b/doc/programming/parsing.rst
@@ -1037,6 +1037,25 @@ range to an enum, using ``$$`` to access the parsed value:
     :exec: printf '\x21' | spicy-driver %INPUT
     :show-with: foo.spicy
 
+When parsing a bitfield, you can enforce expected values for some
+or all of the bitranges through an assignment-style syntax:
+
+.. spicy-code::
+
+    type Foo = unit {
+        f: bitfield(8) {
+            x1: 0..3 = 2;
+            x2: 4..5;
+            x3: 6..7 = 3;
+        }
+    };
+
+Now parsing will fail if values of ``x1`` and ``x3`` aren't ``2`` and
+``3``, respectively. Internally, Spicy treats bitfields with such
+expected values similar to constants of other types, meaning they
+operate as valid look-ahead symbols as well (see
+:ref:`parse_lookahead`).
+
 .. _parse_bytes:
 
 Bytes

diff --git a/spicy/toolchain/src/ast/types.cc b/spicy/toolchain/src/ast/types.cc
@@ -2,11 +2,12 @@
 
 #include "spicy/ast/types.h"
 
+#include <hilti/ast/types/bitfield.h>
 #include <hilti/ast/types/bytes.h>
 #include <hilti/ast/types/integer.h>
 #include <hilti/ast/types/regexp.h>
 
 bool spicy::type::supportsLiterals(const hilti::Type& t) {
     return t.isA<hilti::type::Bytes>() || t.isA<hilti::type::RegExp>() || t.isA<hilti::type::SignedInteger>() ||
-           t.isA<hilti::type::UnsignedInteger>();
+           t.isA<hilti::type::UnsignedInteger>() || t.isA<hilti::type::Bitfield>();
 }
diff --git a/spicy/toolchain/src/compiler/codegen/parsers/literals.cc b/spicy/toolchain/src/compiler/codegen/parsers/literals.cc
@@ -263,6 +263,77 @@ struct Visitor : public hilti::visitor::PreOrder<std::optional<Expression>, Visi
     result_t operator()(const hilti::ctor::SignedInteger& c) {
         return parseInteger(c.type(), builder::expression(c), c.meta());
     }
+
+    result_t operator()(const hilti::ctor::Bitfield& c) {
+        auto offset = [](Expression view) { return builder::memberCall(std::move(view), "offset", {}); };
+
+        switch ( state().literal_mode ) {
+            case LiteralMode::Default:
+            case LiteralMode::Skip: {
+                auto [have_lah, no_lah] = builder()->addIfElse(state().lahead);
+
+                pushBuilder(have_lah);
+
+                pushBuilder(builder()->addIf(builder::unequal(state().lahead, builder::integer(production.tokenID()))));
+                pb->parseError("unexpected token to consume", c.meta());
+                popBuilder();
+
+                // Need to reparse the value to assign it to our destination.
+                auto value = pb->parseType(c.btype(), production.meta(), {});
+                builder()->addAssign(destination(c.btype()), value);
+
+                pb->consumeLookAhead();
+                popBuilder();
+
+                pushBuilder(no_lah);
+                auto old_cur = builder()->addTmp("ocur", state().cur);
+
+                value = pb->parseType(c.btype(), production.meta(), {});
+
+                // Check that the bit values match what we expect.
+                for ( const auto& b : c.bits() ) {
+                    auto error = builder()->addIf(builder::unequal(builder::member(value, b.id()), b.expression()));
+                    pushBuilder(error);
+                    builder()->addAssign(state().cur, old_cur);
+                    pb->parseError(fmt("unexpected value for bitfield element '%s'", b.id()), c.meta());
+                    popBuilder();
+                }
+
+                if ( state().literal_mode != LiteralMode::Skip )
+                    builder()->addAssign(destination(c.btype()), value);
+
+                popBuilder();
+
+                return value;
+            }
+
+            case LiteralMode::Search: // Handled in `parseLiteral`.
+            case LiteralMode::Try: {
+                auto old_cur = builder()->addTmp("ocur", state().cur);
+                auto bf = builder()->addTmp("bf", c.btype());
+                pb->parseTypeTry(c.btype(), production.meta(), bf);
+                auto new_cur = builder()->addTmp("ncur", state().cur);
+
+                auto match = builder()->addIf(builder::unequal(offset(old_cur), offset(new_cur)));
+                pushBuilder(match);
+                builder()->addAssign(state().cur, old_cur); // restore, because we must not move cur when in sync mode
+
+                // Check that the bit values match what we expect.
+                for ( const auto& b : c.bits() ) {
+                    auto error = builder()->addIf(builder::unequal(builder::member(bf, b.id()), b.expression()));
+                    pushBuilder(error);
+                    builder()->addAssign(new_cur, old_cur); // reset to old position
+                    popBuilder();
+                }
+
+                popBuilder();
+
+                return builder::begin(new_cur);
+            }
+        }
+
+        hilti::util::cannot_be_reached();
+    }
 };
 
 } // namespace

diff --git a/spicy/toolchain/src/compiler/codegen/parsers/types.cc b/spicy/toolchain/src/compiler/codegen/parsers/types.cc
@@ -67,7 +67,7 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
         else {
             auto has_data = pb->waitForInputOrEod(builder::integer(len));
 
-            auto result = builder()->addTmp("result", type::Result(t));
+            auto result = dst ? *dst : builder()->addTmp("result", type::Result(t));
 
             auto true_ = builder()->addIf(has_data);
             pushBuilder(true_);
@@ -128,6 +128,8 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
         performUnpack(target, t, t.width() / 8, {state().cur, fieldByteOrder(), *bitorder}, t.meta(), is_try);
 
         if ( pb->options().debug ) {
+            auto have_value = builder()->addIf(builder::hasMember(target, "__value__"));
+            pushBuilder(have_value);
             // Print all the bit ranges individually so that we can include
             // their IDs, which the standard tuple output wouldn't show.
             builder()->addDebugMsg("spicy", fmt("%s = %%s", meta.field()->id()),
@@ -138,6 +140,7 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
                 builder()->addDebugMsg("spicy", fmt("%s = %%s", bits.id()), {builder::member(target, bits.id())});
 
             builder()->addDebugDedent("spicy");
+            popBuilder();
         }
 
         return target;
@@ -303,7 +306,7 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
 
 Expression ParserBuilder::_parseType(const Type& t, const production::Meta& meta, const std::optional<Expression>& dst,
                                      bool is_try) {
-    assert(! is_try || (t.isA<type::SignedInteger>() || t.isA<type::UnsignedInteger>()));
+    assert(! is_try || (t.isA<type::SignedInteger>() || t.isA<type::UnsignedInteger>() || t.isA<type::Bitfield>()));
 
     if ( auto e = Visitor(this, meta, dst, is_try).dispatch(t) )
         return std::move(*e);
@@ -317,7 +320,7 @@ Expression ParserBuilder::parseType(const Type& t, const production::Meta& meta,
 
 Expression ParserBuilder::parseTypeTry(const Type& t, const production::Meta& meta,
                                        const std::optional<Expression>& dst) {
-    assert(t.isA<type::SignedInteger>() || t.isA<type::UnsignedInteger>());
+    assert(t.isA<type::SignedInteger>() || t.isA<type::UnsignedInteger>() || t.isA<type::Bitfield>());
 
     return _parseType(t, meta, dst, /*is_try =*/true);
 }
diff --git a/spicy/toolchain/src/compiler/parser/parser.yy b/spicy/toolchain/src/compiler/parser/parser.yy
@@ -258,7 +258,7 @@ static std::vector<hilti::DocString> _docs;
 %type <hilti::Expression>                            expr tuple_elem tuple_expr member_expr ctor_expr expr_0 expr_1 expr_2 expr_3 expr_4 expr_5 expr_6 expr_7 expr_8 expr_9 expr_a expr_b expr_c expr_d expr_e expr_f expr_g
 %type <std::vector<hilti::Expression>>               opt_tuple_elems1 opt_tuple_elems2 exprs opt_exprs opt_unit_field_args opt_unit_field_sinks
 %type <std::optional<hilti::Statement>>              opt_else_block
-%type <std::optional<hilti::Expression>>             opt_init_expression opt_unit_field_condition unit_field_repeat opt_unit_field_repeat opt_unit_switch_expr
+%type <std::optional<hilti::Expression>>             opt_init_expression opt_unit_field_condition unit_field_repeat opt_unit_field_repeat opt_unit_switch_expr opt_bitfield_bits_value
 %type <hilti::type::function::Parameter>             func_param
 %type <hilti::declaration::parameter::Kind>          opt_func_param_kind
 %type <hilti::type::function::Result>                func_result opt_func_result
@@ -702,10 +702,15 @@ bitfield_bits
               | bitfield_bits_spec               { $$ = std::vector<spicy::type::bitfield::Bits>(); $$.push_back(std::move($1)); }
 
 bitfield_bits_spec
-              : local_id ':' CUINTEGER DOTDOT CUINTEGER opt_attributes ';'
-                                                 { $$ = spicy::type::bitfield::Bits(std::move($1), $3, $5, _field_width, std::move($6), __loc__); }
-              | local_id ':' CUINTEGER opt_attributes ';'
-                                                 { $$ = spicy::type::bitfield::Bits(std::move($1), $3, $3, _field_width, std::move($4), __loc__); }
+              : local_id ':' CUINTEGER DOTDOT CUINTEGER opt_bitfield_bits_value opt_attributes ';'
+                                                 { $$ = spicy::type::bitfield::Bits(std::move($1), $3, $5, _field_width, std::move($7), std::move($6), __loc__); }
+              | local_id ':' CUINTEGER opt_bitfield_bits_value opt_attributes ';'
+                                                 { $$ = spicy::type::bitfield::Bits(std::move($1), $3, $3, _field_width, std::move($5), std::move($4), __loc__); }
+
+opt_bitfield_bits_value
+              : '=' expr                         { $$ = std::move($2); }
+              | /* empty */                      { $$ = {}; }
+              ;
 
 /* --- Begin of Spicy units --- */
 

diff --git a/spicy/toolchain/src/compiler/visitors/resolver.cc b/spicy/toolchain/src/compiler/visitors/resolver.cc
@@ -294,6 +294,16 @@ struct Visitor : public hilti::visitor::PreOrder<void, Visitor> {
             if ( ! type::isResolved(t) )
                 return;
 
+            if ( auto bf = t->tryAs<type::Bitfield>() ) {
+                // If a bitfield type comes with values for at least one of its
+                // items, it's actually a bitfield ctor. Replace the type with the
+                // ctor then.
+                if ( auto ctor = bf->ctorValue() ) {
+                    replaceField(&p, resolveField(u, *ctor));
+                    return;
+                }
+            }
+
             replaceField(&p, resolveField(u, *t));
         }
 

diff --git a/tests/Baseline/spicy.types.bitfield.ctor-fail/output b/tests/Baseline/spicy.types.bitfield.ctor-fail/output
@@ -0,0 +1,6 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[error] <...>/ctor-fail.spicy:10:5: value is outside of bitfield element's range
+[error] <...>/ctor-fail.spicy:11:5: value is outside of bitfield element's range
+[error] <...>/ctor-fail.spicy:13:5: cannot coerce expression '-1' of type 'int<64>' to type 'uint<8>'
+[error] <...>/ctor-fail.spicy:14:5: cannot coerce expression '"42"' of type 'string' to type 'uint<8>'
+[error] spicyc: aborting after errors
diff --git a/tests/Baseline/spicy.types.bitfield.ctor-look-ahead/output b/tests/Baseline/spicy.types.bitfield.ctor-look-ahead/output
@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[$x=[1, 2, 3], $y=(4, 8), $z=5]
diff --git a/tests/Baseline/spicy.types.bitfield.ctor/output b/tests/Baseline/spicy.types.bitfield.ctor/output
@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+(2, 8, 1)
+[error] terminating with uncaught exception of type spicy::rt::ParseError: unexpected value for bitfield element 'a' (<...>/ctor.spicy:10:6-14:4)
diff --git a/tests/Baseline/spicy.types.bitfield.width-fail/output b/tests/Baseline/spicy.types.bitfield.width-fail/output
@@ -1,4 +1,4 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[error] <...>/width-fail.spicy:9:8-12:6: upper limit is beyond the width of the bitfield
-[error] <...>/width-fail.spicy:9:8-12:6: lower limit needs to be lower than upper limit
+[error] <...>/width-fail.spicy:10:9: upper limit is beyond the width of the bitfield
+[error] <...>/width-fail.spicy:11:9: lower limit needs to be lower than upper limit
 [error] spicyc: aborting after errors
diff --git a/tests/Baseline/spicy.types.unit.synchronize-literals/output b/tests/Baseline/spicy.types.unit.synchronize-literals/output
@@ -1,7 +1,9 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[$a=b"Axy", $b=6, $c=b"Cxy", $d=b"Dxy"]
-[$a=(not set), $b=6, $c=b"Cxy", $d=b"Dxy"]
-[$a=b"Axy", $b=(not set), $c=b"Cxy", $d=b"Dxy"]
-[$a=b"Axy", $b=6, $c=(not set), $d=b"Dxy"]
-[$a=b"Axy", $b=6, $c=(not set), $d=b"Dxy"]
-[error] terminating with uncaught exception of type spicy::rt::ParseError: unexpected token to consume (<...>/synchronize-literals.spicy:23:8)
+[$a=b"Axy", $b=6, $c=b"Cxy", $d=b"Dxy", $e=(1)]
+[$a=(not set), $b=6, $c=b"Cxy", $d=b"Dxy", $e=(1)]
+[$a=b"Axy", $b=(not set), $c=b"Cxy", $d=b"Dxy", $e=(1)]
+[$a=b"Axy", $b=6, $c=(not set), $d=b"Dxy", $e=(1)]
+[$a=b"Axy", $b=6, $c=(not set), $d=b"Dxy", $e=(1)]
+[$a=b"Axy", $b=6, $c=b"Cxy", $d=(not set), $e=(1)]
+[error] terminating with uncaught exception of type spicy::rt::ParseError: failed to synchronize: expecting 'Cxy' (<...>/synchronize-literals.spicy:24:8)
+[error] terminating with uncaught exception of type spicy::rt::ParseError: failed to synchronize: failed to match regular expression (<...>/synchronize-literals.spicy:25:8)
diff --git a/tests/spicy/types/bitfield/ctor-fail.spicy b/tests/spicy/types/bitfield/ctor-fail.spicy
@@ -0,0 +1,16 @@
+# @TEST-EXEC-FAIL: spicyc -j %INPUT >>output 2>&1
+# @TEST-EXEC:      btest-diff output
+#
+# @TEST-DOC: Check that we catch invalid bitfield constants.
+
+module Test;
+
+public type Foo = unit {
+  x: bitfield(8) {
+    a: 1..3 = 255; # error
+    b: 1..3 = 8; # error
+    c: 1..3 = 7; # ok
+    d: 4..7 = -1;
+    e: 4..7 = "42";
+  };
+};
diff --git a/tests/spicy/types/bitfield/ctor-look-ahead.spicy b/tests/spicy/types/bitfield/ctor-look-ahead.spicy
@@ -0,0 +1,17 @@
+# @TEST-EXEC:  printf '\001\002\003\204\005' | spicy-driver %INPUT >>output
+# @TEST-EXEC:  btest-diff output
+#
+# @TEST-DOC: Check look-ahead vector parsing with bitfield constant as terminator.
+
+module Test;
+
+public type Foo = unit {
+  x: uint8[];
+  y: bitfield(8) {
+    a: 0..3 = 4;
+    b: 4..7;
+  };
+  z: uint8;
+
+  on %done { print self; }
+};
diff --git a/tests/spicy/types/bitfield/ctor.spicy b/tests/spicy/types/bitfield/ctor.spicy
@@ -0,0 +1,17 @@
+# @TEST-EXEC:       ${SCRIPTS}/printf '\x82' | spicy-driver %INPUT >>output 2>&1
+# @TEST-EXEC-FAIL:  ${SCRIPTS}/printf '\200' | spicy-driver %INPUT >>output 2>&1
+# @TEST-EXEC:  btest-diff output
+#
+# @TEST-DOC: Check bitfield constant parsing.
+
+module Test;
+
+public type Foo = unit {
+  x: bitfield(8) {
+    a: 0..3 = 2;
+    b: 4..7;
+    c: 7 = 1;
+  };
+
+  on %done { print self.x; }
+};
diff --git a/tests/spicy/types/unit/synchronize-literals.spicy b/tests/spicy/types/unit/synchronize-literals.spicy
@@ -2,15 +2,17 @@
 
 # @TEST-EXEC: spicyc -j -d %INPUT -o test.hlto
 #
-# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxyDxy' | spicy-driver -i 1 -d test.hlto >>output 2>&1
-# @TEST-EXEC: ${SCRIPTS}/printf '\x00\x00\x00\x06CxyDxy' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxyDxy\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf '\x00\x00\x00\x06CxyDxy\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
 #
-# @TEST-EXEC: ${SCRIPTS}/printf 'AxyCxyDxy' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf 'AxyCxyDxy\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
 #
-# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06Dxy' | spicy-driver -i 1 -d test.hlto >>output 2>&1
-# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxDxy' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06Dxy\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxDxy\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxyXXX\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
 
-# @TEST-EXEC-FAIL: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxD' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC-FAIL: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxD\x80' | spicy-driver -i 1 -d test.hlto >>output 2>&1
+# @TEST-EXEC-FAIL: ${SCRIPTS}/printf 'Axy\x00\x00\x00\x06CxyXXX\x7f' | spicy-driver -i 1 -d test.hlto >>output 2>&1
 #
 # @TEST-EXEC: btest-diff output
 
@@ -21,6 +23,9 @@ public type X = unit {
     b: uint32(6) &synchronize;
     c: b"Cxy" &synchronize;
     d: /Dxy/ &synchronize;
+    e: bitfield(8) {
+         msb: 7 = 1;
+    } &synchronize;
 
     on %synced { confirm; }
     on %done { print self; }