Skip to content

Commit 501b34c

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 d8a22a7 commit 501b34c

2 files changed

Lines changed: 94 additions & 6 deletions

File tree

bindgen/codegen/bitfield_unit.rs

Lines changed: 24 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,12 @@ 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 =
426+
if BIT_WIDTH as usize + bit_shift >= usize::BITS as usize {
427+
!0usize << bit_shift
428+
} else {
429+
((1usize << BIT_WIDTH) - 1) << bit_shift
430+
};
422431

423432
let mut i = 0;
424433
while i < bytes_needed {
@@ -519,7 +528,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
519528
}
520529

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

524535
if cfg!(target_endian = "big") {
525536
val = val.reverse_bits() >>
@@ -588,7 +599,9 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
588599
// Use usize for fields that fit, u64 only when necessary.
589600
if BIT_WIDTH as usize + bit_shift <= usize::BITS as usize {
590601
let mut val = val as usize;
591-
val &= (1usize << BIT_WIDTH) - 1;
602+
if (BIT_WIDTH as u32) < usize::BITS {
603+
val &= (1usize << BIT_WIDTH) - 1;
604+
}
592605

593606
if cfg!(target_endian = "big") {
594607
val = val.reverse_bits() >>
@@ -597,7 +610,12 @@ impl<const N: usize> __BindgenBitfieldUnit<[u8; N]> {
597610

598611
val <<= bit_shift;
599612

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

602620
let mut i = 0;
603621
while i < bytes_needed {

bindgen/codegen/bitfield_unit_tests.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,73 @@ 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 =
411+
__BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
412+
let mut unit_shifted_runtime =
413+
__BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
414+
unit_shifted_const.set_const::<1, 63>(value & ((1u64 << 63) - 1));
415+
unit_shifted_runtime.set(1, 63, value & ((1u64 << 63) - 1));
416+
assert_eq!(
417+
unit_shifted_const.get(0, 64),
418+
unit_shifted_runtime.get(0, 64),
419+
);
420+
assert_eq!(
421+
unit_shifted_const.get_const::<1, 63>(),
422+
unit_shifted_runtime.get(1, 63),
423+
);
424+
425+
let mut unit_shifted_raw = __BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
426+
let mut unit_shifted_raw_runtime =
427+
__BindgenBitfieldUnit::<[u8; 9]>::new([0xAA; 9]);
428+
unsafe {
429+
__BindgenBitfieldUnit::raw_set_const::<1, 63>(
430+
&mut unit_shifted_raw,
431+
value & ((1u64 << 63) - 1),
432+
);
433+
}
434+
unit_shifted_raw_runtime.set(1, 63, value & ((1u64 << 63) - 1));
435+
assert_eq!(
436+
unit_shifted_raw.get(0, 64),
437+
unit_shifted_raw_runtime.get(0, 64),
438+
);
439+
}

0 commit comments

Comments
 (0)