Byte-update lowering: handle sub-byte sized bit fields #5390
Conversation
tautschnig
requested review from
chrisr-diffblue,
kroening,
peterschrammel and
smowton
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #5390 +/- ##
========================================
Coverage 68.52% 68.52%
========================================
Files 1187 1187
Lines 98265 98271 +6
========================================
+ Hits 67339 67345 +6
Misses 30926 30926
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -2136,10 +2136,30 @@ static exprt lower_byte_update( | |||
| instantiate_byte_array(value_as_byte_array, 0, (type_bits + 7) / 8, ns); | |||
| } | |||
|
|
|||
| const std::size_t update_size = update_bytes.size(); | |||
There was a problem hiding this comment.
While we're here: recommend always using unit suffixes in this code (update_size_bytes, width_bits)
| @@ -0,0 +1,8 @@ | |||
| CORE | |||
There was a problem hiding this comment.
Tests: can we test behaviour in the other endianness somehow?
There was a problem hiding this comment.
Setting the --arch flags appropriately should do it.
There was a problem hiding this comment.
I don't think endianness actually matters in this test?
| if(b.dummy <= sizeof(struct blob)) | ||
| memset(&b, 0, b.dummy); | ||
|
|
||
| assert(b1.dummy != sizeof(struct blob) || b.bit == 0); |
There was a problem hiding this comment.
Brutal! If you are need to change things in this PR, please could you add a comment or two here to explain what you are testing because I fear that people without your knowledge of C struct packing rules and how they relate to bit-fields might miss some of the subtlety here.
| @@ -0,0 +1,8 @@ | |||
| CORE | |||
There was a problem hiding this comment.
Setting the --arch flags appropriately should do it.
|
@tautschnig can we merge this PR and close #5389? |
Extend the value to be updated to the update size immediately to avoid extractbits operations that attempt to extract bits that do not exist. Fixes: diffblue#5389
Use variable names that immediately clarify that widths are measured in bits.
tautschnig
force-pushed
the
fix-bitfield-byte-update
branch
from
54411e0 to
e16b7db
Compare
Extend the value to be updated to the update size immediately to avoid
extractbits operations that attempt to extract bits that do not exist.
Fixes: #5389