Binpac generates a lot of switch statements with repeated blocks in
them (typically empty blocks). Running clang-tidy on the generated code
with bugprone-branch-clone generates a lot of warnings. Instead of
doing a ton of analysis in binpac to avoid generating the duplicates,
just mark any switch generated with an annotation to avoid reporting
them.
Elsewhere (zeek/zeek#2482), it was observed that when binpac encounters
include failures, it still exits with 0 indicating success. Subsequent
compilation of the produced .h and .cc files likely fails.
Exit with 1 on include errors to make pin pointing issues easier by
having make/ninja stop earlier.
CMake documentation says that CMAKE_CFG_INTDIR is no longer necessary to
find the right binary for the configuration and is in fact deprecated in
recent versions of CMake.
For example:
inum: uint32 = case (ed & 0x0f) of {
0x00 -> n_8; # n_8 is a uint8
0x01 -> n_16; # n_16 is a uint16
0x02 -> n_32; # n_32 is a uint32
default -> 0;
};
Previously, the temporary storage used for evaluating the
case-expression was based on whatever type the first case yields, which
is a uint8 in the above example. That behavior can lead to a narrowing
conversion whenever the 0x01 or 0x02 cases occur.
The new behavior is to base the temporary storage's type on the largest
numeric type that the case-expression can yield, which is uint32 in the
above example.
Incremental flowbuffer parsing sought to first parse the "minimum header
length" required to get the full frame length, possibly from a record
field, but generating the logic to parse that field could greedily
bundle in additional boundary-checks for all subsequent fields of
known-size.
E.g. for flowunit parsing of this:
type HDR = record {
version: uint8;
reserved: uint8;
len: uint16;
} &byteorder=bigendian;
type FOO_PDU(is_orig: bool) = record {
hdr: HDR;
plen: uint8;
ptype: uint8;
something: bytestring &restofdata;
} &byteorder=bigendian, &length=hdr.len;
The flowbuffer was correctly seeking to buffer 4 bytes and parse the
"hdr.len" field, but the generated parsing logic for "hdr.len" included
a boundary check all the way up to include "plen" and "ptype".
This causes out-of-bounds exceptions to be thrown for inputs that should
actually be possible to incrementally parse via flowbuffer.
OpenBSD shared library names are like "libfoo.so.major.minor" and
binpac was previously letting the post-release number into the name
like "libbinpac.so.0.54-7", which isn't compatible with that scheme.
Related to https://github.com/zeek/zeek/issues/649
Array lengths use signed integer storage, so multiplication of that
by the element size for purpose of bounds checking against available
data may produce a signed integer overlow, which is undefined behavior.
