Skip to content

Commit cff5004

Browse files
dimblebyCopilot
andcommitted
Fix shift-overflow in const-generic bitfield accessors
When BIT_WIDTH equals the native word size (e.g. a 64-bit field on a 64-bit target), `(1usize << BIT_WIDTH) - 1` shifted by the full width of usize, panicking in debug builds and producing a wrong mask in release. Affected `get_const`, `set_const`, `raw_get_const` and `raw_set_const`. Guard the value mask with `BIT_WIDTH < usize::BITS` and compute the runtime `set`. Add a regression test exercising width = 64. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 937b8d0 commit cff5004

2 files changed

Lines changed: 95 additions & 6 deletions

File tree

bindgen/codegen/bitfield_unit.rs

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
346346
}
347347

348348
val >>= bit_shift;
349-
val &= (1usize << BIT_WIDTH) - 1;
349+
if (BIT_WIDTH as u32) < usize::BITS {
350+
val &= (1usize << BIT_WIDTH) - 1;
351+
}
350352

351353
if cfg!(target_endian = "big") {
352354
val = val.reverse_bits() >>
@@ -409,7 +411,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
409411
// The compiler eliminates the unused branch since BIT_WIDTH is const.
410412
if BIT_WIDTH as usize + bit_shift <= usize::BITS as usize {
411413
let mut val = val as usize;
412-
val &= (1usize << BIT_WIDTH) - 1;
414+
if (BIT_WIDTH as u32) < usize::BITS {
415+
val &= (1usize << BIT_WIDTH) - 1;
416+
}
413417

414418
if cfg!(target_endian = "big") {
415419
val = val.reverse_bits() >>
@@ -418,7 +422,13 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
418422

419423
val <<= bit_shift;
420424

421-
let field_mask = ((1usize << BIT_WIDTH) - 1) << bit_shift;
425+
let field_mask = if BIT_WIDTH as usize + bit_shift >=
426+
usize::BITS as usize
427+
{
428+
!0usize << bit_shift
429+
} else {
430+
((1usize << BIT_WIDTH) - 1) << bit_shift
431+
};
422432

423433
let mut i = 0;
424434
while i < bytes_needed {
@@ -519,7 +529,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
519529
}
520530

521531
val >>= bit_shift;
522-
val &= (1usize << BIT_WIDTH) - 1;
532+
if (BIT_WIDTH as u32) < usize::BITS {
533+
val &= (1usize << BIT_WIDTH) - 1;
534+
}
523535

524536
if cfg!(target_endian = "big") {
525537
val = val.reverse_bits() >>
@@ -588,7 +600,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
588600
// Use usize for fields that fit, u64 only when necessary.
589601
if BIT_WIDTH as usize + bit_shift <= usize::BITS as usize {
590602
let mut val = val as usize;
591-
val &= (1usize << BIT_WIDTH) - 1;
603+
if (BIT_WIDTH as u32) < usize::BITS {
604+
val &= (1usize << BIT_WIDTH) - 1;
605+
}
592606

593607
if cfg!(target_endian = "big") {
594608
val = val.reverse_bits() >>
@@ -597,7 +611,13 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
597611

598612
val <<= bit_shift;
599613

600-
let field_mask = ((1usize << BIT_WIDTH) - 1) << bit_shift;
614+
let field_mask = if BIT_WIDTH as usize + bit_shift >=
615+
usize::BITS as usize
616+
{
617+
!0usize << bit_shift
618+
} else {
619+
((1usize << BIT_WIDTH) - 1) << bit_shift
620+
};
601621

602622
let mut i = 0;
603623
while i < bytes_needed {

bindgen/codegen/bitfield_unit_tests.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,72 @@ fn bitfield_unit_raw_const_methods() {
367367
// Compare by reading back
368368
assert_eq!(unit_const.get(0, 16), unit_runtime.get(0, 16));
369369
}
370+
371+
// Regression: const-generic accessors must not shift-overflow when
372+
// BIT_WIDTH equals the native word size (usize::BITS). Previously
373+
// `(1usize << BIT_WIDTH) - 1` panicked in debug builds for 64-bit
374+
// fields on 64-bit targets (and 32-bit fields on 32-bit targets).
375+
#[test]
376+
fn bitfield_unit_const_full_word_width() {
377+
let storage = [0x12u8, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xF0];
378+
let unit = __BindgenBitfieldUnit::<[u8; 8]>::new(storage);
379+
380+
// Width == 64 on 64-bit hosts, and width == 32 exercises the
381+
// boundary on 32-bit hosts (still safe on 64-bit, just routes
382+
// through a different branch).
383+
assert_eq!(unit.get_const::<0, 64>(), unit.get(0, 64));
384+
assert_eq!(unit.get_const::<0, 32>(), unit.get(0, 32));
385+
386+
unsafe {
387+
assert_eq!(
388+
__BindgenBitfieldUnit::raw_get_const::<0, 64>(&unit),
389+
unit.get(0, 64),
390+
);
391+
}
392+
393+
let mut unit_const = __BindgenBitfieldUnit::<[u8; 8]>::new([0; 8]);
394+
let mut unit_runtime = __BindgenBitfieldUnit::<[u8; 8]>::new([0; 8]);
395+
let value = 0xDEAD_BEEF_CAFE_BABE_u64;
396+
397+
unit_const.set_const::<0, 64>(value);
398+
unit_runtime.set(0, 64, value);
399+
assert_eq!(unit_const.get(0, 64), unit_runtime.get(0, 64));
400+
401+
let mut unit_raw = __BindgenBitfieldUnit::<[u8; 8]>::new([0; 8]);
402+
unsafe {
403+
__BindgenBitfieldUnit::raw_set_const::<0, 64>(&mut unit_raw, value);
404+
}
405+
assert_eq!(unit_raw.get(0, 64), value);
406+
407+
// Exercise the new `field_mask = !0 << bit_shift` branch with a
408+
// non-zero bit_shift (BIT_WIDTH + bit_shift == usize::BITS but
409+
// BIT_WIDTH < usize::BITS, so the value-mask still runs).
410+
let mut unit_shifted_const = __BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
411+
let mut unit_shifted_runtime =
412+
__BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
413+
unit_shifted_const.set_const::<1, 63>(value & ((1u64 << 63) - 1));
414+
unit_shifted_runtime.set(1, 63, value & ((1u64 << 63) - 1));
415+
assert_eq!(
416+
unit_shifted_const.get(0, 64),
417+
unit_shifted_runtime.get(0, 64),
418+
);
419+
assert_eq!(
420+
unit_shifted_const.get_const::<1, 63>(),
421+
unit_shifted_runtime.get(1, 63),
422+
);
423+
424+
let mut unit_shifted_raw = __BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
425+
let mut unit_shifted_raw_runtime =
426+
__BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
427+
unsafe {
428+
__BindgenBitfieldUnit::raw_set_const::<1, 63>(
429+
&mut unit_shifted_raw,
430+
value & ((1u64 << 63) - 1),
431+
);
432+
}
433+
unit_shifted_raw_runtime.set(1, 63, value & ((1u64 << 63) - 1));
434+
assert_eq!(
435+
unit_shifted_raw.get(0, 64),
436+
unit_shifted_raw_runtime.get(0, 64),
437+
);
438+
}

0 commit comments

Comments
 (0)