Skip to content

Commit

Permalink
Fix extraction function for variable-length fields
Browse files Browse the repository at this point in the history
For VL fields, the bitwidth is dynamic and only known at extract
time. Which means that extract_VL is in charge of computing the Bignum
mask for the Field, which is then used to ensure that the field stays in
the correct range. However, we were using an incorrect expression to
compute the mask, which was causing it to be always 0 for bitwidths >=
32.

Fixes issue #538
  • Loading branch information
antoninbas committed Jan 26, 2018
1 parent 7c200ad commit f6f5de7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
3 changes: 1 addition & 2 deletions src/bm_sim/fields.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <bm/bm_sim/headers.h>

#include <algorithm> // for std::swap

#include "extract.h"

namespace bm {
Expand Down Expand Up @@ -81,7 +80,7 @@ int
Field::extract_VL(const char *data, int hdr_offset, int computed_nbits) {
nbits = computed_nbits;
nbytes = (nbits + 7) / 8;
mask = (1 << nbits); mask -= 1;
mask = 1; mask <<= nbits; mask -= 1;
bytes.resize(nbytes);
if (is_signed) {
assert(nbits > 1);
Expand Down
16 changes: 16 additions & 0 deletions tests/test_fields.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,19 @@ TEST_F(SaturatingFieldTest, Signed) {
f.sub(f, Data(1));
EXPECT_EQ(min, f.get<int>());
}


// This test was added for this issue:
// https://github.com/p4lang/behavioral-model/issues/538.
// Field::extract_VL used to compute the mask with "mask = (1 << nbits); mask -=
// 1;", which meant the mask was -1 (and not the desired 0xffff...) when nbits
// was >= 32.
TEST(FieldTest, ExportBytesVLLarge) {
Field f(0 /* nbits */, nullptr /* parent hdr */, true,
false /* is_signed */, false, true /* VL */, false);
std::string data(32, '\xab'); // 32 bytes
int computed_nbits = 24 * 8; // 24 bytes
f.extract_VL(data.data(), 0 /* hdr_offset */, computed_nbits);
f.export_bytes();
EXPECT_NE(0u, f.get<uint64_t>());
}

0 comments on commit f6f5de7

Please sign in to comment.