Skip to content

Commit 7ad9038

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

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 8 additions & 1 deletion
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(());
@@ -728,9 +729,15 @@ where
728729
// but that does not work: We could be a newtype around a pair, then the
729730
// fields do not match the `ScalarPair` components.
730731

732+
// In preparation, if we do *not* later reset the padding, we clear the entire
733+
// destination now to ensure that no stray pointer fragments are being
734+
// preserved (see <https://github.com/rust-lang/rust/issues/148470>).
735+
if !will_later_validate {
736+
alloc.write_uninit_full();
737+
}
738+
731739
alloc.write_scalar(alloc_range(Size::ZERO, a_val.size()), a_val)?;
732740
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.
734741
}
735742
Immediate::Uninit => alloc.write_uninit_full(),
736743
}

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)