Merge remote-tracking branch 'origin/topic/robin/gh-3298-bitfields'

* origin/topic/robin/gh-3298-bitfields:
  Spicy: Fix support for exporting bitfields.
  Bump Spicy.

CHANGES

@@ -1,3 +1,12 @@
+6.1.0-dev.419 | 2023-09-21 09:55:51 +0200
+
+  * GH-3298: Spicy: Fix support for exporting bitfields. (Robin Sommer, Corelight)
+
+    Anonymous bitfields now have their fields lifted into the surrounding
+    record.
+
+  * Bump Spicy. (Robin Sommer, Corelight)
+
 6.1.0-dev.416 | 2023-09-20 13:04:21 +0200
 
   * Correctly compute name for spicyz export with debug log. (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-6.1.0-dev.416
+6.1.0-dev.419

auxil/spicy

@@ -1 +1 @@
-Subproject commit 7e074675b994d59050b4468dec145526daa22760
+Subproject commit 56f84b8f328c81387803714af3d26431b20baf7c

src/spicy/runtime-support.h

@@ -435,6 +435,8 @@ hilti::rt::Time network_time();
 // Forward-declare to_val() functions.
 template<typename T, typename std::enable_if_t<hilti::rt::is_tuple<T>::value>* = nullptr>
 ValPtr to_val(const T& t, TypePtr target);
+template<typename... Ts>
+inline ValPtr to_val(const hilti::rt::Bitfield<Ts...>& v, TypePtr target);
 template<typename T, typename std::enable_if_t<std::is_base_of<::hilti::rt::trait::isStruct, T>::value>* = nullptr>
 ValPtr to_val(const T& t, TypePtr target);
 template<typename T, typename std::enable_if_t<std::is_enum<typename T::Value>::value>* = nullptr>
@@ -812,12 +814,45 @@ inline ValPtr to_val(const T& t, TypePtr target) {
         throw TypeMismatch("tuple", target);
 
     auto rval = make_intrusive<RecordVal>(rtype);
-    int idx = 0;
+    size_t idx = 0;
     hilti::rt::tuple_for_each(t, [&](const auto& x) { set_record_field(rval.get(), rtype, idx++, x); });
 
     return rval;
 }
 
+/**
+ * Converts a Spicy-side bitfield to a Zeek record value. The result is returned
+ * with ref count +1.
+ */
+template<typename... Ts>
+inline ValPtr to_val(const hilti::rt::Bitfield<Ts...>& v, TypePtr target) {
+    using Bitfield = hilti::rt::Bitfield<Ts...>;
+
+    if ( target->Tag() != TYPE_RECORD )
+        throw TypeMismatch("bitfield", target);
+
+    auto rtype = cast_intrusive<RecordType>(target);
+
+    if ( sizeof...(Ts) - 1 != rtype->NumFields() )
+        throw TypeMismatch("bitfield", target);
+
+    auto rval = make_intrusive<RecordVal>(rtype);
+    int idx = 0;
+    hilti::rt::tuple_for_each(v.value, [&](const auto& x) {
+        if ( idx < sizeof...(Ts) - 1 ) // last element is original integer value, with no record equivalent
+            set_record_field(rval.get(), rtype, idx++, x);
+    });
+
+    return rval;
+}
+
+template<typename>
+constexpr bool is_optional_impl = false;
+template<typename T>
+constexpr bool is_optional_impl<std::optional<T>> = true;
+template<typename T>
+constexpr bool is_optional = is_optional_impl<std::remove_cv_t<std::remove_reference_t<T>>>;
+
 /**
  * Converts Spicy-side struct to a Zeek record value. The result is returned
  * with a ref count +1.
@@ -838,13 +873,35 @@ inline ValPtr to_val(const T& t, TypePtr target) {
         if ( idx >= num_fields )
             throw TypeMismatch(hilti::rt::fmt("no matching record field for field '%s'", name));
 
-        auto field = rtype->GetFieldType(idx);
+        // Special-case: Lift up anonymous bitfields (which always come as std::optionals).
-        std::string field_name = rtype->FieldName(idx);
+        if ( name == "<anon>" ) {
+            using X = typename std::decay<decltype(val)>::type;
+            if constexpr ( is_optional<X> ) {
+                if constexpr ( std::is_base_of<::hilti::rt::trait::isBitfield, typename X::value_type>::value ) {
+                    size_t j = 0;
+                    hilti::rt::tuple_for_each(val->value, [&](const auto& x) {
+                        if ( j++ < std::tuple_size<decltype(val->value)>() -
+                                       1 ) // last element is original integer value, with no record equivalent
+                            set_record_field(rval.get(), rtype, idx++, x);
+                    });
+                    return;
+                }
+            }
 
-        if ( field_name != name )
+            // There can't be any other anonymous fields.
-            throw TypeMismatch(hilti::rt::fmt("mismatch in field name: expected '%s', found '%s'", name, field_name));
+            auto msg = hilti::rt::fmt("unexpected anonymous field: %s", name);
+            reporter->InternalError("%s", msg.c_str());
+        }
+        else {
+            auto field = rtype->GetFieldType(idx);
+            std::string field_name = rtype->FieldName(idx);
 
-        set_record_field(rval.get(), rtype, idx++, val);
+            if ( field_name != name )
+                throw TypeMismatch(
+                    hilti::rt::fmt("mismatch in field name: expected '%s', found '%s'", name, field_name));
+
+            set_record_field(rval.get(), rtype, idx++, val);
+        }
     });
 
     // We already check above that all Spicy-side fields are mapped so we

src/spicy/spicyz/glue-compiler.cc

@@ -1366,6 +1366,38 @@ struct VisitorZeekType : hilti::visitor::PreOrder<hilti::Result<hilti::Expressio
     }
 
     result_t operator()(const hilti::type::Address& t) { return base_type("zeek_rt::ZeekTypeTag::Addr"); }
+
+    result_t operator()(const hilti::type::Bitfield& t) {
+        std::vector<hilti::Expression> fields;
+        for ( const auto& b : t.bits() ) {
+            auto ztype = createZeekType(b.itemType());
+            if ( ! ztype )
+                return ztype.error();
+
+            fields.emplace_back(create_record_field(b.id(), *ztype, false, false));
+        }
+
+        hilti::ID local;
+        hilti::ID ns;
+
+        if ( auto id_ = id() ) {
+            local = id_->local();
+            ns = id_->namespace_();
+        }
+        else {
+            // Invent a (hopefully unique) name for the Zeek-side record type
+            // so that we can handle anonymous tuple types.
+            static uint64_t i = 0;
+            local = hilti::util::
+                fmt("__spicy_bitfield_%u"
+                    "_%" PRIu64,
+                    static_cast<unsigned int>(getpid()), ++i);
+            ns = namespace_();
+        }
+
+        return create_record_type(ns, local, fields);
+    }
+
     result_t operator()(const hilti::type::Bool& t) { return base_type("zeek_rt::ZeekTypeTag::Bool"); }
     result_t operator()(const hilti::type::Bytes& t) { return base_type("zeek_rt::ZeekTypeTag::String"); }
     result_t operator()(const hilti::type::Interval& t) { return base_type("zeek_rt::ZeekTypeTag::Interval"); }
@@ -1468,11 +1500,23 @@ struct VisitorZeekType : hilti::visitor::PreOrder<hilti::Result<hilti::Expressio
             if ( export_.skip )
                 continue;
 
-            auto ztype = createZeekType(f.type);
+            // Special-case: Lift up elements of anonymous bitfields.
-            if ( ! ztype )
+            if ( auto bf = f.type.tryAs<hilti::type::Bitfield>(); bf && f.is_anonymous ) {
-                return ztype.error();
+                for ( const auto& b : bf->bits() ) {
+                    auto ztype = createZeekType(b.itemType());
+                    if ( ! ztype )
+                        return ztype.error();
 
-            fields.emplace_back(create_record_field(f.id, *ztype, f.is_optional, export_.log));
+                    fields.emplace_back(create_record_field(b.id(), *ztype, f.is_optional, export_.log));
+                }
+            }
+            else if ( ! f.is_anonymous ) {
+                auto ztype = createZeekType(f.type);
+                if ( ! ztype )
+                    return ztype.error();
+
+                fields.emplace_back(create_record_field(f.id, *ztype, f.is_optional, export_.log));
+            }
         }
 
         return create_record_type(id()->namespace_(), id()->local(), fields);
@@ -1499,15 +1543,21 @@ struct VisitorUnitFields : hilti::visitor::PreOrder<void, VisitorUnitFields> {
     std::vector<GlueCompiler::RecordField> fields;
 
     void operator()(const ::spicy::type::unit::item::Field& f, position_t p) {
-        if ( f.isTransient() || f.parseType().isA<hilti::type::Void>() )
+        if ( (f.isTransient() && ! f.isAnonymous()) || f.parseType().isA<hilti::type::Void>() )
             return;
 
-        auto field = GlueCompiler::RecordField{.id = f.id(), .type = f.itemType(), .is_optional = true};
+        auto field = GlueCompiler::RecordField{.id = f.id(),
+                                               .type = f.itemType(),
+                                               .is_optional = true,
+                                               .is_anonymous = f.isAnonymous()};
         fields.emplace_back(std::move(field));
     }
 
     void operator()(const ::spicy::type::unit::item::Variable& f, const position_t p) {
-        auto field = GlueCompiler::RecordField{.id = f.id(), .type = f.itemType(), .is_optional = f.isOptional()};
+        auto field = GlueCompiler::RecordField{.id = f.id(),
+                                               .type = f.itemType(),
+                                               .is_optional = f.isOptional(),
+                                               .is_anonymous = false};
         fields.emplace_back(std::move(field));
     }
 

src/spicy/spicyz/glue-compiler.h

@@ -203,9 +203,10 @@ public:
 
     /** Return type for `recordField()`. */
     struct RecordField {
-        hilti::ID id;     /**< name of record field */
+        hilti::ID id;      /**< name of record field */
-        hilti::Type type; /**< Spicy-side type object */
+        hilti::Type type;  /**< Spicy-side type object */
-        bool is_optional; /**< true if field is optional */
+        bool is_optional;  /**< true if field is optional */
+        bool is_anonymous; /**< true if field is annymous */
     };
 
     /**

testing/btest/Baseline/spicy.export-bitfield/output

@@ -0,0 +1,15 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+{
+[a] = [type_name=count, log=F, value=80, default_val=<uninitialized>, optional=T],
+[bf] = [type_name=record , log=F, value=[x1=79, y1=15, z1=4], default_val=<uninitialized>, optional=T],
+[y2] = [type_name=count, log=F, value=3, default_val=<uninitialized>, optional=T],
+[z2] = [type_name=count, log=F, value=5, default_val=<uninitialized>, optional=T],
+[x2] = [type_name=count, log=F, value=83, default_val=<uninitialized>, optional=T],
+[b] = [type_name=count, log=F, value=84, default_val=<uninitialized>, optional=T]
+}
+{
+[z1] = [type_name=count, log=F, value=4, default_val=<uninitialized>, optional=F],
+[y1] = [type_name=count, log=F, value=15, default_val=<uninitialized>, optional=F],
+[x1] = [type_name=count, log=F, value=79, default_val=<uninitialized>, optional=F]
+}
+[a=80, bf=[x1=79, y1=15, z1=4], x2=83, y2=3, z2=5, b=84]

testing/btest/spicy/export-bitfield.zeek

@@ -0,0 +1,49 @@
+# @TEST-REQUIRES: have-spicy
+#
+# @TEST-EXEC: spicyz -do export.hlto export.spicy export.evt
+# @TEST-EXEC: zeek -Cr $TRACES/http/post.trace export.hlto %INPUT >>output
+# @TEST-EXEC: test '!' -e reporter.log
+# @TEST-EXEC: btest-diff output
+#
+# @TEST-DOC: Tests that named and anonymous bitfields are exported as expected.
+
+# @TEST-START-FILE export.spicy
+module foo;
+
+public type X = unit {
+    a: uint8;
+
+    bf: bitfield(8) {
+	 x1: 0..7;
+	 y1: 0..3;
+	 z1: 4..7;
+     };
+
+     : bitfield(8) {
+	 x2: 0..7;
+	 y2: 0..3;
+	 z2: 4..7;
+     };
+
+    b: uint8;
+};
+# @TEST-END-FILE
+
+# @TEST-START-FILE export.evt
+import foo;
+
+protocol analyzer FOO over TCP:
+    parse originator with foo::X,
+    port 80/tcp;
+
+export foo::X;
+
+on foo::X -> event foo::hello(self);
+# @TEST-END-FILE
+
+event foo::hello(x: foo::X)
+	{
+	print record_fields(x);
+	print record_fields(x$bf);
+	print x;
+	}