Using $next in conditional fields generates incorrect offsets
- Dominant language
- Python
- Stars
- 92
- Forks
- 30
- PR merge metrics
- No merged PRs in 30d
Description
I'm writing a definition for a protocol where the length field is dynamically sized. I've noted that all the fields after the conditional that pulls the extended length portion out are pointing at the wrong offset. It seems that $next is not doing what one would expect in the presence of a conditional.
Definition:
```
enum RfcommFrameType:
[maximum_bits: 8]
SET_ASYNC_BALANCED_MODE = 0x2f
UNNUMBERED_ACK = 0x63
DISCONNECT_MODE = 0x0f
DISCONNECT = 0xc3
UNNUMBERED_INFORMATION_WITH_HEADER_CHECK = 0xef
UNNUMBERED_INFORMATION_WITH_HEADER_CHECK_AND_POLL_FINAL = 0xff
-- RFCOMM extension. With Poll/Final bit set in UIH control byte, a credits
-- field is the first byte of the payload.
enum RfcommFixedChannel:
[maximum_bits: 8]
CONTROL = 0
enum RfcommCommandResponse:
-- Commands from the initiator and responses from the responder
-- have C/R = 0. Commands from the responder and responses from
-- the initiator have C/R = 1.
[maximum_bits: 1]
COMMAND = 0
RESPONSE = 1
enum RfcommDirection:
[maximum_bits: 1]
RESPONDER = 0
INITIATOR = 1
enum RfcommLengthExtended:
[maximum_bits: 1]
NORMAL = 1
EXTENDED = 0
struct RfcommFrame:
-- RFCOMM TS 07.10 frame
[requires: extended_address == true]
0 [+1] bits:
0 [+1] Flag extended_address
1 [+1] RfcommCommandResponse command_response
2 [+1] RfcommDirection direction
3 [+5] UInt channel
1 [+1] RfcommFrameType control
2 [+1] bits:
0 [+1] RfcommLengthExtended length_extended_flag
1 [+7] UInt length
if length_extended_flag == RfcommLengthExtended.EXTENDED:
$next [+1] UInt length_extended
if control == RfcommFrameType.UNNUMBERED_INFORMATION_WITH_HEADER_CHECK_AND_POLL_FINAL && channel != 0:
$next [+1] UInt credits
-- Credits field can appears as first byte of payload when Poll/Final bit is
-- set on UIH frames and channel is not control (0).
let combined_length = $present(length_extended) ? length * 256 + length_extended : length
# If present, credits is counted as part of payload.
let payload_length = $present(credits) ? combined_length - 1 : combined_length
$next [+payload_length] UInt:8[payload_length] payload
$next [+1] UInt fcs
-- Frame checksum. See: GSM 07.10 TS 101 369 V6.3.0
-- SABM, DISC, UA, DM frame types:
-- FCS should be calculated over address, control and length fields.
-- UIH frame type:
-- FCS should be calculated over address and control fields.
```
Everything after
```
if length_extended_flag == RfcommLengthExtended.EXTENDED:
$next [+1] UInt length_extended
```
is pointing one byte off, even if the `length_extended_flag != RfcommLengthExtended.EXTENDED`. I confirmed this with a `!has_length_extended()` check and looking at the `emboss_reserved_local_offset` variable in the generated code for each field.
For example one would expect `credits` to have offset `3` when `length_extended_flag != RfcommLengthExtended.EXTENDED`, but it actually has `4`.
On a related note: `IntrinsicSizeInBytes().Read()` reports one more byte than I would expect on this struct as well. Like it isn't taking into account the conditionals correctly.
There's some messy code that uses the generated stuff here for more context https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/246974
Contributor guide
Assessment
This issue has not been assessed yet.