Merge remote-tracking branch 'origin/topic/awelzel/bitfield-examples'

CHANGES

@@ -1,3 +1,15 @@
+1.10.0-dev.133 | 2024-01-29 10:59:23 +0100
+
+  * GH-1489: Deprecate &bit-order on bit ranges. (Arne Welzel, Corelight)
+
+    This does not appear to have any effect and allowing it may be
+    confusing to users. Deprecate it with the idea of eventual
+    removal.
+
+  * Add extensive bitfield test including endianness behavior. (Arne Welzel, Corelight)
+
+  * Add bitfield examples. (Arne Welzel, Corelight)
+
 1.10.0-dev.129 | 2024-01-23 17:48:19 +0100
 
   * Adjust end of Bison-generated locations. (Robin Sommer, Corelight)

VERSION

@@ -1 +1 @@
-1.10.0-dev.129
+1.10.0-dev.133

doc/programming/parsing.rst

@@ -1009,17 +1009,113 @@ Generally, a field ``bitfield(N)`` field is parsed like an
 ``uint<N>``. The field then supports dereferencing individual bit
 ranges through their labels. The corresponding expressions
 (``self.x.<id>``) have the same ``uint<N>`` type as the parsed value
-itself, with the value shifted to the right so that the lowest
-extracted bit becomes bit 0 of the returned value. As you can see in
+itself, with the value shifted to the right so that the least significant
+extracted bit becomes the least significant bit of the returned value. As you can see in
 the example, the type of the field itself becomes a tuple composed of
 the values of the individual bit ranges.
 
 By default, a bitfield assumes the underlying integer comes in network
 byte order. You can specify a ``&byte-order`` attribute to change that
 (e.g., ``bitfield(32) { ... } &byte-order=spicy::ByteOrder::Little``).
-Furthermore, each bit range can also specify a ``&bit-order``
-attribute to specify the :ref:`ordering <spicy_bitorder>` for its
-bits; the default is ``spicy::BitOrder::LSB0``.
+
+When parsing a ``bitfield(16)`` in network byte order and with bit order
+``spicy::BitOrder::LSB0`` (default value of ``&bit-order``), bits are
+numbered 0 to 15 from right to left.
+
+.. code::
+
+    MSB                           LSB
+          <--   1         <--       0
+    6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+    +---------------+---------------+
+    |               |               |
+    +-------------------------------+
+
+
+This default bit numbering may be surprising given that some RFCs use the inverse
+as documented in `RFC 1700 <https://www.rfc-editor.org/rfc/rfc1700.html>`_.
+Here, the most significant bit is numbered 0 on the left with higher
+bit numbers representing less significant bits to the right.
+Concrete examples would be the `WebSocket framing <https://datatracker.ietf.org/doc/html/rfc6455#section-5.2>`_
+or `IPv4 header <https://datatracker.ietf.org/doc/html/rfc791#section-3.1>`_
+notations.
+
+.. code::
+
+    MSB                           LSB
+    0       -->         1    -->
+    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6
+    +-+-+-+-+-------+-+-------------+
+    |F|R|R|R| opcode|M| Payload len |
+    |I|S|S|S|  (4)  |A|     (7)     |
+    |N|V|V|V|       |S|             |
+    | |1|2|3|       |K|             |
+    +-+-+-+-+-------+-+-------------+
+
+To express such bitfields more naturally in Spicy, use ``&bit-order=spicy::BitOrder::MSB0``
+on the whole bitfield:
+
+.. spicy-code:: parse-websocket-bitfield.spicy
+
+    module WebSocket;
+
+    public type Header= unit {
+      : bitfield(32) {
+        fin: 0;
+        rsv: 1..3;
+        opcode: 4..7;
+        mask: 8;
+        payload_len: 9..15;
+    } &bit-order=spicy::BitOrder::MSB0
+
+The way to think about this is that the most significant bit of an integer in
+network byte order is always the most left bit and the least significant bit
+the most right one. Specifying the bit order as ``LSB0`` or ``MSB0`` essentially
+sets the bit numbering direction by specifying the location of bit 0.
+
+With little endian byte order, the bits are numbered zigzag-wise and
+``MSB0`` and ``LSB0`` can again be used to change the direction of the bit
+numbering. The following example uses ``spicy::ByteOrder::Little`` and
+the default ``LSB0`` bit order for ``bitfield(16)``. Notice how the most
+significant and least significant bit for a 2 byte little endian integer
+are next to each other.
+
+.. code::
+
+    f: bitfield(16) {
+
+      ...
+
+    } &byte-order=spicy::ByteOrder::Little;
+
+                  LSB MSB
+         <--        0     <--   1
+      7 6 5 4 3 2 1 0 5 4 3 2 1 0 9 8
+    +---------------+---------------+
+    |               |               |
+    +-------------------------------+
+
+With ``MSB0`` as bit order, the bit numbering direction is from left to right, instead:
+
+.. code::
+
+    f: bitfield(16) {
+
+      ...
+
+    } &byte-order=spicy::ByteOrder::Little &bit-order=spicy::BitOrder::MSB0;
+
+                LSB MSB
+        1  -->      0    -->
+    8 9 0 1 2 3 4 5 0 1 2 3 4 5 6 7
+    +---------------+---------------+
+    |               |               |
+    +-------------------------------+
+
+
+Bit numbering with larger sized bitfields in little endian gets only more
+confusing. Prefer network byte ordered bitfields unless it makes sense given
+the spec you're working with.
 
 The individual bit ranges support the ``&convert`` attribute and will
 adjust their types accordingly, just like a regular unit field (see

spicy/toolchain/src/compiler/visitors/validator.cc

@@ -752,6 +752,15 @@ struct VisitorPost : public hilti::visitor::PreOrder<void, VisitorPost>, public
                     error(fmt("'%s' can be used at most once", a), p);
             }
         }
+
+        if ( auto t = f.itemType().tryAs<type::Bitfield>() ) {
+            for ( const auto& b : t->bits() ) {
+                if ( AttributeSet::has(b.attributes(), "&bit-order") )
+                    hilti::logger().deprecated(fmt("&bit-order on bitfield item '%s' has no effect and is deprecated",
+                                                   b.id()),
+                                               b.meta().location());
+            }
+        }
     }
 
     void operator()(const spicy::type::unit::item::UnresolvedField& u, position_t p) {

tests/Baseline/spicy.types.bitfield.bit-bitorder/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.
+[warning] <...>/bit-bitorder.spicy:14:5-14:56: &bit-order on bitfield item 'x1015_msb0' has no effect and is deprecated
+[warning] <...>/bit-bitorder.spicy:15:5-15:56: &bit-order on bitfield item 'x1015_lsb0' has no effect and is deprecated

tests/Baseline/spicy.types.bitfield.parse-bitorder-endianness/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.
+be_lsb0, (1, 2, 0, 4, 32, 32, 32)
+le_lsb0, (0, 1, 0, 3, 1, 1, 1)
+le_msb0, (0, 0, 1, 6, 2, 2, 2)
+le32_lsb0, (0, 0, 7, 5, 32, 0, 7, 8, 8, 8, 1)
+be32_msb0, (1, 3, 6, 2, 2, 5, 0, 15, 15, 15, 1)

tests/spicy/types/bitfield/bit-bitorder.spicy

@@ -0,0 +1,17 @@
+# @TEST-EXEC: spicyc -p -o /dev/null %INPUT 2>output
+# @TEST-EXEC: btest-diff output
+#
+# @TEST-DOC: Test deprecated "&bit-order" attribute on bits emits deprecation warning.
+
+module Mini;
+
+import spicy;
+
+public type X = unit {
+  be16_lsb0: bitfield(16) {
+    x0: 0;
+    x1015: 10..15;
+    x1015_msb0: 10..15 &bit-order=spicy::BitOrder::MSB0;
+    x1015_lsb0: 10..15 &bit-order=spicy::BitOrder::LSB0;
+  };
+};

tests/spicy/types/bitfield/parse-bitorder-endianness.spicy

@@ -0,0 +1,143 @@
+# @TEST-EXEC: echo "" | spicy-driver %INPUT >output
+# @TEST-EXEC: btest-diff output
+
+module Mini;
+
+import spicy;
+
+public type X = unit {
+
+  be16_lsb0: bitfield(16) {
+    x0: 0;                # 1
+    x12: 1..2;            # 2 10
+    x35: 3..5;            # 0
+    x79: 7..9;            # 4  100 )
+    x1015: 10..15;        # 32 100000
+    x1015_msb0: 10..15 &bit-order=spicy::BitOrder::MSB0;  # still 32
+    x1015_lsb0: 10..15 &bit-order=spicy::BitOrder::LSB0;  # still 32
+  } &parse-from=b"\x82\x05" {
+    # input         10000010 00000101   \x82\x05
+    #                    1          0
+    # big-lsb0-bits 54321098 76543210
+    assert $$.x0 == 1;
+    assert $$.x12 == 2;
+    assert $$.x35 == 0;
+    assert $$.x79 == 4;
+    assert $$.x1015 == 32;
+    assert $$.x1015_msb0 == 32;
+    assert $$.x1015_lsb0 == 32;
+  }
+
+  le16_lsb0: bitfield(16) {
+    x0: 0;            # 0
+    x12: 1..2;        # 1
+    x35: 3..5;        # 0
+    x79: 7..9;        # 3 011
+    x1015: 10..15;    # 1
+    x1015_msb0: 10..15 &bit-order=spicy::BitOrder::MSB0;  # still 1
+    x1015_lsb0: 10..15 &bit-order=spicy::BitOrder::LSB0;  # still 1
+  } &byte-order=spicy::ByteOrder::Little
+    &parse-from=b"\x82\x05" {
+    # input            10000010 00000101   \x82\x05
+    #                    1           1
+    # little-lsb0-bits 76543210 54321098
+    assert $$.x0 == 0;
+    assert $$.x12 == 1;
+    assert $$.x35 == 0;
+    assert $$.x79 == 3;
+    assert $$.x1015 == 1;
+    assert $$.x1015_msb0 == 1;
+    assert $$.x1015_lsb0 == 1;
+  }
+
+  le16_msb0: bitfield(16) {
+    x0: 0;             # 0
+    x12: 1..2;         # 0
+    x35: 3..5;         # 1 001
+    x79: 7..9;         # 6 110
+    x1015: 10..15;     # 2 000010
+    x1015_msb0: 10..15 &bit-order=spicy::BitOrder::MSB0;  # still 2
+    x1015_lsb0: 10..15 &bit-order=spicy::BitOrder::LSB0;  # still 2
+  } &byte-order=spicy::ByteOrder::Little
+    &bit-order=spicy::BitOrder::MSB0
+    &parse-from=b"\x82\x05" {
+    # input:           10000010 00000101   \x82\x05
+    #                    1      0
+    # little-msb0-bits 89012345 01234567
+    assert $$.x0 == 0;
+    assert $$.x12 == 0;
+    assert $$.x35 == 1;
+    assert $$.x79 == 6;
+    assert $$.x1015_msb0 == 2;
+    assert $$.x1015_lsb0 == 2;
+  }
+
+  le32_lsb0: bitfield(32) {
+    x0: 0;
+    x12: 1..2;
+    x35: 3..5;  # 7 111
+    x79: 7..9;  # 5 101
+    x1015: 10..15; # 32 10000
+    x1923: 19..23;
+    x2527: 25..27; # 7 111
+    x2831: 28..31; # 8 1000
+    x2831_msb0: 28..31 &bit-order=spicy::BitOrder::MSB0;  # still 8
+    x2831_lsb0: 28..31 &bit-order=spicy::BitOrder::LSB0;  # still 8
+    x31: 31; # 1
+  } &byte-order=spicy::ByteOrder::Little
+    &parse-from=b"\xf8\x82\x05\x8f" {
+    # LE is just confusing.
+    # input:           11111000 10000010 00000101 10001111   \xf8\x82\x05\x8f
+    #                                1      2      3
+    # little-lsb0-bits 76543210 54321098 32109876 10987654
+    assert $$.x0 == 0;
+    assert $$.x12 == 0;
+    assert $$.x35 == 7;
+    assert $$.x79 == 5;
+    assert $$.x1015 == 32;
+    assert $$.x1923 == 0;
+    assert $$.x2527 == 7;
+    assert $$.x2831 == 8;
+    assert $$.x2831_msb0 == 8;
+    assert $$.x2831_lsb0 == 8;
+    assert $$.x31 == 1;
+  }
+
+  be32_msb0: bitfield(32) {
+    x0: 0;
+    x12: 1..2;
+    x35: 3..5;
+    x79: 7..9;
+    x1015: 10..15;
+    x1923: 19..23;
+    x2527: 25..27;
+    x2831: 28..31;
+    x2831_msb0: 28..31 &bit-order=spicy::BitOrder::MSB0;
+    x2831_lsb0: 28..31 &bit-order=spicy::BitOrder::LSB0;
+    x31: 31; # 1
+  } &bit-order=spicy::BitOrder::MSB0
+    &parse-from=b"\xf8\x82\x05\x8f" {
+    # input:        11111000 10000010 00000101 10001111   \xf8\x82\x05\x8f
+    #                          1          2          3
+    # big-msb0-bits 01234567 89012345 67890123 45678901
+    assert $$.x0 == 1;
+    assert $$.x12 == 3;
+    assert $$.x35 == 6;
+    assert $$.x79 == 2;
+    assert $$.x1015 == 2;
+    assert $$.x1923 == 5;
+    assert $$.x2527 == 0;
+    assert $$.x2831 == 15;
+    assert $$.x2831_msb0 == 15;
+    assert $$.x2831_lsb0 == 15;
+    assert $$.x31 == 1;
+  }
+
+  on %done {
+    print "be_lsb0", self.be16_lsb0;
+    print "le_lsb0", self.le16_lsb0;
+    print "le_msb0", self.le16_msb0;
+    print "le32_lsb0", self.le32_lsb0;
+    print "be32_msb0", self.be32_msb0;
+  }
+};