Skip to content

Commit 5d5d229

Browse files
committed
when writing a scalar pair, always reset the entire destination range
1 parent 42455aa commit 5d5d229

File tree

3 files changed

+41
-5
lines changed

3 files changed

+41
-5
lines changed

compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,7 @@ where
704704
// wrong type.
705705

706706
let tcx = *self.tcx;
707+
let will_later_validate = M::enforce_validity(self, layout);
707708
let Some(mut alloc) = self.get_place_alloc_mut(&MPlaceTy { mplace: dest, layout })? else {
708709
// zero-sized access
709710
return interp_ok(());
@@ -714,23 +715,31 @@ where
714715
alloc.write_scalar(alloc_range(Size::ZERO, scalar.size()), scalar)?;
715716
}
716717
Immediate::ScalarPair(a_val, b_val) => {
717-
let BackendRepr::ScalarPair(a, b) = layout.backend_repr else {
718+
let BackendRepr::ScalarPair(_a, b) = layout.backend_repr else {
718719
span_bug!(
719720
self.cur_span(),
720721
"write_immediate_to_mplace: invalid ScalarPair layout: {:#?}",
721722
layout
722723
)
723724
};
724-
let b_offset = a.size(&tcx).align_to(b.align(&tcx).abi);
725+
let a_size = a_val.size();
726+
let b_offset = a_size.align_to(b.align(&tcx).abi);
725727
assert!(b_offset.bytes() > 0); // in `operand_field` we use the offset to tell apart the fields
726728

727729
// It is tempting to verify `b_offset` against `layout.fields.offset(1)`,
728730
// but that does not work: We could be a newtype around a pair, then the
729731
// fields do not match the `ScalarPair` components.
730732

731-
alloc.write_scalar(alloc_range(Size::ZERO, a_val.size()), a_val)?;
733+
// In preparation, if we do *not* later reset the padding, we clear the entire
734+
// destination now to ensure that no stray pointer fragments are being
735+
// preserved (see <https://github.com/rust-lang/rust/issues/148470>).
736+
// We can skip this if there is no padding (e.g. for wide pointers).
737+
if !will_later_validate && a_size + b_val.size() != layout.size {
738+
alloc.write_uninit_full();
739+
}
740+
741+
alloc.write_scalar(alloc_range(Size::ZERO, a_size), a_val)?;
732742
alloc.write_scalar(alloc_range(b_offset, b_val.size()), b_val)?;
733-
// We don't have to reset padding here, `write_immediate` will anyway do a validation run.
734743
}
735744
Immediate::Uninit => alloc.write_uninit_full(),
736745
}

compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,12 @@ impl<Prov: Provenance> ProvenanceMap<Prov> {
290290
}
291291

292292
/// Removes all provenance inside the given range.
293-
/// If there is provenance overlapping with the edges, might result in an error.
294293
#[allow(irrefutable_let_patterns)] // these actually make the code more clear
295294
pub fn clear(&mut self, range: AllocRange, data_bytes: &[u8], cx: &impl HasDataLayout) {
295+
if range.size == Size::ZERO {
296+
return;
297+
}
298+
296299
let start = range.start;
297300
let end = range.end();
298301
// Clear the bytewise part -- this is easy.

tests/ui/consts/const-eval/ptr_fragments.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,30 @@ const _PARTIAL_OVERWRITE: () = {
6767
}
6868
};
6969

70+
#[allow(dead_code)]
71+
fn fragment_in_dst_padding_gets_overwritten() {
72+
#[repr(C)]
73+
struct Pair {
74+
x: u128,
75+
// at offset 16
76+
y: u64,
77+
}
78+
79+
const C: MaybeUninit<Pair> = unsafe {
80+
let mut m = MaybeUninit::<Pair>::uninit();
81+
// Store pointer half-way into trailing padding.
82+
m.as_mut_ptr().byte_add(20).cast::<&i32>().write_unaligned(&0);
83+
// Overwrite `m`.
84+
let val = Pair { x: 0, y: 0 };
85+
*m.as_mut_ptr() = val;
86+
// If the assignment of `val` above only copied the field and left the rest of `m`
87+
// unchanged, there would be pointer fragments left in the padding which would be carried
88+
// all the way to the final value, causing compilation failure.
89+
// We prevent this by having the copy of `val` overwrite the entire destination.
90+
m
91+
};
92+
}
93+
7094
fn main() {
7195
assert_eq!(unsafe { MEMCPY_RET.assume_init().read() }, 42);
7296
}

0 commit comments

Comments
 (0)