Switch Spicy over to use the new HILTI-side bitfield type.

Closes #1032. Using a typedef with bitfields works now. Closes #1484. `&convert` can now refer to individual bitfields.
zeek · Aug 14, 2023 · e55296b · e55296b
1 parent 0f6ac48
commit e55296b
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 53 deletions.
diff --git a/hilti/toolchain/include/ast/types/bitfield.h b/hilti/toolchain/include/ast/types/bitfield.h
@@ -58,8 +58,9 @@ class Bits : public hilti::NodeBase {
 
     bool operator==(const Bits& other) const {
         return id() == other.id() && _lower == other._lower && _upper == other._upper &&
-               _field_width == other._field_width && itemType() == other.itemType() &&
-               attributes() == other.attributes();
+               _field_width == other._field_width && itemType() == other.itemType();
+        // TODO: Attributes don't quite compare correctly, see e.g., spicy.types.bitfield.parse-enum failure
+        // && attributes() == other.attributes();
     }
 
 private:

diff --git a/spicy/toolchain/src/compiler/codegen/parsers/types.cc b/spicy/toolchain/src/compiler/codegen/parsers/types.cc
@@ -4,6 +4,7 @@
 
 #include <hilti/ast/builder/all.h>
 #include <hilti/ast/builder/expression.h>
+#include <hilti/ast/operators/tuple.h>
 #include <hilti/ast/types/struct.h>
 #include <hilti/base/logger.h>
 
@@ -49,8 +50,8 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
         return builder()->addTmp("x", t);
     }
 
-    Expression performUnpack(const Expression& target, const Type& t, int len, const std::vector<Expression>& unpack_args,
-                             const Meta& m, bool is_try) {
+    Expression performUnpack(const Expression& target, const Type& t, int len,
+                             const std::vector<Expression>& unpack_args, const Meta& m, bool is_try) {
         if ( ! is_try ) {
             auto error_msg = fmt("expecting %d bytes for unpacking value", len);
             pb->waitForInput(builder::integer(len), error_msg, m);
@@ -115,45 +116,30 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
                                  is_try);
     }
 
-    result_t operator()(const spicy::type::Bitfield& t) {
-        const auto& itype = t.integerType();
-        auto value = builder()->addTmp("bitfield", itype);
-        performUnpack(value, itype, t.width() / 8, {state().cur, fieldByteOrder()}, t.meta(), is_try);
+    result_t operator()(const hilti::type::Bitfield& t, position_t p) {
+        std::optional<Expression> bitorder = builder::id("spicy::BitOrder::LSB0");
 
-        builder()->addDebugMsg("spicy", fmt("%s = %%s", meta.field()->id()), {value});
-        builder()->addDebugIndent("spicy");
-
-        std::vector<Expression> extracted_bits;
+        if ( auto attrs = t.attributes() ) {
+            if ( auto a = AttributeSet::find(*attrs, "&bit-order") )
+                bitorder = *a->valueAsExpression();
+        }
 
-        for ( const auto& b : t.bits() ) {
-            auto bit_order = builder::id("spicy_rt::BitOrder::LSB0");
+        auto target = destination(t);
+        performUnpack(target, t, t.width() / 8, {state().cur, fieldByteOrder(), *bitorder}, t.meta(), is_try);
 
-            if ( const auto& a = AttributeSet::find(meta.field()->attributes(), "&bit-order") )
-                bit_order = *a->valueAsExpression();
-            else if ( const auto& p = state().unit.get().propertyItem("%bit-order") )
-                bit_order = *p->expression();
+        if ( pb->options().debug ) {
+            // 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()),
+                                   {builder::member(target, "__value__")});
 
-            auto x =
-                builder()->addTmp("bits", itype,
-                                  builder::call("hilti::extractBits", {value, builder::integer(b.lower()),
-                                                                          builder::integer(b.upper()), bit_order}));
+            builder()->addDebugIndent("spicy");
+            for ( const auto& bits : t.bits() )
+                builder()->addDebugMsg("spicy", fmt("%s = %%s", bits.id()), {builder::member(target, bits.id())});
 
-            if ( auto a = AttributeSet::find(b.attributes(), "&convert") ) {
-                auto converted = builder()->addTmp(ID("converted"), b.itemType());
-                auto block = builder()->addBlock();
-                block->addLocal(ID("__dd"), itype, x);
-                block->addAssign(converted, *a->valueAsExpression());
-                x = converted;
-            }
-
-            extracted_bits.push_back(x);
-            builder()->addDebugMsg("spicy", fmt("%s = %%s", b.id()), {x});
+            builder()->addDebugDedent("spicy");
         }
 
-        builder()->addDebugDedent("spicy");
-
-        auto target = destination(t.tupleType());
-        builder()->addAssign(target, builder::tuple(extracted_bits));
         return target;
     }
 
@@ -172,9 +158,7 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
         return performUnpack(destination(t), t, t.width() / 8, {state().cur, fieldByteOrder()}, t.meta(), is_try);
     }
 
-    result_t operator()(const hilti::type::Void& t) {
-        return hilti::expression::Void();
-    }
+    result_t operator()(const hilti::type::Void& t) { return hilti::expression::Void(); }
 
     result_t operator()(const hilti::type::Bytes& t) {
         auto chunked_attr = AttributeSet::find(meta.field()->attributes(), "&chunked");

diff --git a/spicy/toolchain/src/compiler/parser/parser.yy b/spicy/toolchain/src/compiler/parser/parser.yy
@@ -689,7 +689,7 @@ enum_label    : local_id                         { $$ = hilti::type::enum_::Labe
 bitfield_type : BITFIELD '(' CUINTEGER ')'
                                                  { _field_width = $3; }
                 '{' opt_bitfield_bits '}'
-                                                 { $$ = spicy::type::Bitfield($3, $7, __loc__); }
+                                                 { $$ = spicy::type::Bitfield($3, $7, {}, __loc__); }
 
 opt_bitfield_bits
               : bitfield_bits

diff --git a/spicy/toolchain/src/compiler/visitors/normalizer.cc b/spicy/toolchain/src/compiler/visitors/normalizer.cc
@@ -269,6 +269,30 @@ struct Visitor : public hilti::visitor::PostOrder<void, Visitor> {
             }
         }
     }
+
+    void operator()(const type::Bitfield& bf, position_t p) {
+        if ( auto field = p.parent().tryAs<type::unit::item::Field>() ) {
+            // Transfer any "&bitorder" attribute over to the type.
+            if ( auto a = AttributeSet::find(field->attributes(), "&bit-order");
+                 a && ! AttributeSet::find(bf.attributes(), "&bit-order") ) {
+                auto new_attrs = AttributeSet::add(bf.attributes(), *a);
+                logChange(p.node, "transfer &bitorder attribute");
+                p.node.as<type::Bitfield>().setAttributes(new_attrs);
+                modified = true;
+            }
+        }
+
+        if ( auto decl = p.parent().tryAs<hilti::declaration::Type>() ) {
+            // Transfer any "&bitorder" attribute over to the type.
+            if ( auto a = AttributeSet::find(decl->attributes(), "&bit-order");
+                 a && ! AttributeSet::find(bf.attributes(), "&bit-order") ) {
+                auto new_attrs = AttributeSet::add(bf.attributes(), *a);
+                logChange(p.node, "transfer &bitorder attribute");
+                p.node.as<type::Bitfield>().setAttributes(new_attrs);
+                modified = true;
+            }
+        }
+    }
 };
 
 } // anonymous namespace

diff --git a/spicy/toolchain/src/compiler/visitors/resolver.cc b/spicy/toolchain/src/compiler/visitors/resolver.cc
@@ -58,20 +58,8 @@ enum class FieldType {
 // Visitor determining a unit field type.
 struct FieldTypeVisitor : public hilti::visitor::PreOrder<Type, FieldTypeVisitor> {
     explicit FieldTypeVisitor(FieldType ft) : ft(ft) {}
-
     FieldType ft;
 
-    result_t operator()(const type::Bitfield& t) {
-        switch ( ft ) {
-            case FieldType::DDType:
-            case FieldType::ItemType: return t.tupleType();
-
-            case FieldType::ParseType: return t;
-        };
-
-        hilti::util::cannot_be_reached();
-    }
-
     result_t operator()(const hilti::type::RegExp& /* t */) { return hilti::type::Bytes(); }
 };
 

diff --git a/tests/Baseline/spicy.types.bitfield.parse-and-convert/output b/tests/Baseline/spicy.types.bitfield.parse-and-convert/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.
+[$f=3]
diff --git a/tests/Baseline/spicy.types.bitfield.parse-bitorder/output b/tests/Baseline/spicy.types.bitfield.parse-bitorder/output
@@ -2,3 +2,4 @@
 0, 2
 1, 0
 1, 0
+0, 2
diff --git a/tests/Baseline/spicy.types.bitfield.typedef/output b/tests/Baseline/spicy.types.bitfield.typedef/output
@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+foo::Y {
+  x: (a: 31)
+}
diff --git a/tests/spicy/types/bitfield/parse-and-convert.spicy b/tests/spicy/types/bitfield/parse-and-convert.spicy
@@ -0,0 +1,14 @@
+# @TEST-EXEC:  printf '\377\377\377\377' | spicy-driver %INPUT >output
+# @TEST-EXEC:  btest-diff output
+
+module Mini;
+
+public type test = unit {
+     f: bitfield(32) {
+         x1: 0;
+         x2: 1..2;
+         x3: 3..4;
+     } &convert=$$.x2;
+
+     on %done { print self; }
+};
diff --git a/tests/spicy/types/bitfield/parse-bitorder.spicy b/tests/spicy/types/bitfield/parse-bitorder.spicy
@@ -1,10 +1,15 @@
-# @TEST-EXEC:  printf '\100\001\100\001\100\001' | spicy-driver %INPUT >output
+# @TEST-EXEC:  printf '\100\001\100\001\100\001\100\001' | spicy-driver %INPUT >output
 # @TEST-EXEC:  btest-diff output
 
 module Mini;
 
 import spicy;
 
+type F4 = bitfield(16) {
+    x1: 0;
+    x2: 1..2;
+} &bit-order=spicy::BitOrder::MSB0;
+
 public type test = unit {
     f1: bitfield(16) {
         x1: 0;
@@ -21,9 +26,12 @@ public type test = unit {
         x2: 1..2;
         };
 
+    f4: F4;
+
     on %done {
         print self.f1.x1, self.f1.x2;
         print self.f2.x1, self.f2.x2;
         print self.f3.x1, self.f3.x2;
+        print self.f4.x1, self.f4.x2;
         }
 };
diff --git a/tests/spicy/types/bitfield/typedef.spicy b/tests/spicy/types/bitfield/typedef.spicy
@@ -0,0 +1,12 @@
+# @TEST-EXEC:  printf '\377' | spicy-dump %INPUT >output
+# @TEST-EXEC:  btest-diff output
+
+module foo;
+
+type X = bitfield(8) {
+    a: 0..4;
+};
+
+public type Y = unit {
+	x: X;
+};