Skip to content

Commit 9a04be9

Browse files
oli-obkoli-obk
authored
Merge pull request rust-lang#232 from oli-obk/master
Only check pointers when dereferencing
2 parents 10ec543 + 823b952 commit 9a04be9

File tree

6 files changed

+104
-106
lines changed

6 files changed

+104
-106
lines changed

‎src/eval_context.rs

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
351351
let global_value = self.globals.get_mut(&id)
352352
.expect("global should have been cached (static)");
353353
match global_value.value {
354-
Value::ByRef(ptr) => self.memory.mark_static_initalized(ptr.alloc_id, mutable)?,
354+
// FIXME: to_ptr()? might be too extreme here, static zsts might reach this under certain conditions
355+
Value::ByRef(ptr) => self.memory.mark_static_initalized(ptr.to_ptr()?.alloc_id, mutable)?,
355356
Value::ByVal(val) => if let PrimVal::Ptr(ptr) = val {
356357
self.memory.mark_inner_allocation(ptr.alloc_id, mutable)?;
357358
},
@@ -409,6 +410,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
409410
pub fn deallocate_local(&mut self, local: Option<Value>) -> EvalResult<'tcx> {
410411
if let Some(Value::ByRef(ptr)) = local {
411412
trace!("deallocating local");
413+
let ptr = ptr.to_ptr()?;
412414
self.memory.dump_alloc(ptr.alloc_id);
413415
match self.memory.deallocate(ptr) {
414416
// We could alternatively check whether the alloc_id is static before calling
@@ -693,26 +695,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
693695

694696
// Check alignment and non-NULLness.
695697
let (_, align) = self.size_and_align_of_dst(ty, val)?;
696-
match ptr {
697-
PrimVal::Ptr(ptr) => {
698-
self.memory.check_align(ptr, align, 0)?;
699-
}
700-
PrimVal::Bytes(bytes) => {
701-
let v = ((bytes as u128) % (1 << self.memory.pointer_size())) as u64;
702-
if v == 0 {
703-
return Err(EvalError::InvalidNullPointerUsage);
704-
}
705-
if v % align != 0 {
706-
return Err(EvalError::AlignmentCheckFailed {
707-
has: v % align,
708-
required: align,
709-
});
710-
}
711-
}
712-
PrimVal::Undef => {
713-
return Err(EvalError::ReadUndefBytes);
714-
}
715-
}
698+
self.memory.check_align(ptr, align, 0)?;
716699

717700
self.write_value(val, dest, dest_ty)?;
718701
}
@@ -1036,14 +1019,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
10361019
match self.stack[frame].locals[local.index() - 1] {
10371020
None => return Err(EvalError::DeadLocal),
10381021
Some(Value::ByRef(ptr)) => {
1039-
Lvalue::from_ptr(ptr)
1022+
Lvalue::from_primval_ptr(ptr)
10401023
},
10411024
Some(val) => {
10421025
let ty = self.stack[frame].mir.local_decls[local].ty;
10431026
let ty = self.monomorphize(ty, self.stack[frame].instance.substs);
10441027
let substs = self.stack[frame].instance.substs;
10451028
let ptr = self.alloc_ptr_with_substs(ty, substs)?;
1046-
self.stack[frame].locals[local.index() - 1] = Some(Value::ByRef(ptr)); // it stays live
1029+
self.stack[frame].locals[local.index() - 1] = Some(Value::ByRef(PrimVal::Ptr(ptr))); // it stays live
10471030
self.write_value_to_ptr(val, PrimVal::Ptr(ptr), ty)?;
10481031
Lvalue::from_ptr(ptr)
10491032
}
@@ -1053,7 +1036,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
10531036
Lvalue::Global(cid) => {
10541037
let global_val = *self.globals.get(&cid).expect("global not cached");
10551038
match global_val.value {
1056-
Value::ByRef(ptr) => Lvalue::from_ptr(ptr),
1039+
Value::ByRef(ptr) => Lvalue::from_primval_ptr(ptr),
10571040
_ => {
10581041
let ptr = self.alloc_ptr_with_substs(global_val.ty, cid.instance.substs)?;
10591042
self.memory.mark_static(ptr.alloc_id);
@@ -1064,7 +1047,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
10641047
}
10651048
let lval = self.globals.get_mut(&cid).expect("already checked");
10661049
*lval = Global {
1067-
value: Value::ByRef(ptr),
1050+
value: Value::ByRef(PrimVal::Ptr(ptr)),
10681051
.. global_val
10691052
};
10701053
Lvalue::from_ptr(ptr)
@@ -1164,7 +1147,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
11641147
//
11651148
// Thus, it would be an error to replace the `ByRef` with a `ByVal`, unless we
11661149
// knew for certain that there were no outstanding pointers to this allocation.
1167-
self.write_value_to_ptr(src_val, PrimVal::Ptr(dest_ptr), dest_ty)?;
1150+
self.write_value_to_ptr(src_val, dest_ptr, dest_ty)?;
11681151

11691152
} else if let Value::ByRef(src_ptr) = src_val {
11701153
// If the value is not `ByRef`, then we know there are no pointers to it
@@ -1182,8 +1165,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
11821165
write_dest(self, src_val)?;
11831166
} else {
11841167
let dest_ptr = self.alloc_ptr(dest_ty)?;
1185-
self.copy(PrimVal::Ptr(src_ptr), PrimVal::Ptr(dest_ptr), dest_ty)?;
1186-
write_dest(self, Value::ByRef(dest_ptr))?;
1168+
self.copy(src_ptr, PrimVal::Ptr(dest_ptr), dest_ty)?;
1169+
write_dest(self, Value::ByRef(PrimVal::Ptr(dest_ptr)))?;
11871170
}
11881171

11891172
} else {
@@ -1201,7 +1184,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
12011184
dest_ty: Ty<'tcx>,
12021185
) -> EvalResult<'tcx> {
12031186
match value {
1204-
Value::ByRef(ptr) => self.copy(PrimVal::Ptr(ptr), dest, dest_ty),
1187+
Value::ByRef(ptr) => self.copy(ptr, dest, dest_ty),
12051188
Value::ByVal(primval) => {
12061189
let size = self.type_size(dest_ty)?.expect("dest type must be sized");
12071190
self.memory.write_primval(dest, primval, size)
@@ -1331,7 +1314,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
13311314
}
13321315
}
13331316

1334-
pub(super) fn read_value(&mut self, ptr: Pointer, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
1317+
pub(super) fn read_value(&mut self, ptr: PrimVal, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
13351318
if let Some(val) = self.try_read_value(ptr, ty)? {
13361319
Ok(val)
13371320
} else {
@@ -1356,13 +1339,13 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
13561339
}
13571340
}
13581341

1359-
fn try_read_value(&mut self, ptr: Pointer, ty: Ty<'tcx>) -> EvalResult<'tcx, Option<Value>> {
1342+
fn try_read_value(&mut self, ptr: PrimVal, ty: Ty<'tcx>) -> EvalResult<'tcx, Option<Value>> {
13601343
use syntax::ast::FloatTy;
13611344

13621345
let val = match ty.sty {
1363-
ty::TyBool => PrimVal::from_bool(self.memory.read_bool(ptr)?),
1346+
ty::TyBool => PrimVal::from_bool(self.memory.read_bool(ptr.to_ptr()?)?),
13641347
ty::TyChar => {
1365-
let c = self.memory.read_uint(ptr, 4)? as u32;
1348+
let c = self.memory.read_uint(ptr.to_ptr()?, 4)? as u32;
13661349
match ::std::char::from_u32(c) {
13671350
Some(ch) => PrimVal::from_char(ch),
13681351
None => return Err(EvalError::InvalidChar(c as u128)),
@@ -1381,8 +1364,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
13811364
};
13821365
// if we transmute a ptr to an isize, reading it back into a primval shouldn't panic
13831366
// Due to read_ptr ignoring the sign, we need to jump around some hoops
1384-
match self.memory.read_int(ptr, size) {
1385-
Err(EvalError::ReadPointerAsBytes) if size == self.memory.pointer_size() => self.memory.read_ptr(ptr)?,
1367+
match self.memory.read_int(ptr.to_ptr()?, size) {
1368+
Err(EvalError::ReadPointerAsBytes) if size == self.memory.pointer_size() => self.memory.read_ptr(ptr.to_ptr()?)?,
13861369
other => PrimVal::from_i128(other?),
13871370
}
13881371
}
@@ -1399,30 +1382,30 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
13991382
};
14001383
if size == self.memory.pointer_size() {
14011384
// if we transmute a ptr to an usize, reading it back into a primval shouldn't panic
1402-
self.memory.read_ptr(ptr)?
1385+
self.memory.read_ptr(ptr.to_ptr()?)?
14031386
} else {
1404-
PrimVal::from_u128(self.memory.read_uint(ptr, size)?)
1387+
PrimVal::from_u128(self.memory.read_uint(ptr.to_ptr()?, size)?)
14051388
}
14061389
}
14071390

1408-
ty::TyFloat(FloatTy::F32) => PrimVal::from_f32(self.memory.read_f32(ptr)?),
1409-
ty::TyFloat(FloatTy::F64) => PrimVal::from_f64(self.memory.read_f64(ptr)?),
1391+
ty::TyFloat(FloatTy::F32) => PrimVal::from_f32(self.memory.read_f32(ptr.to_ptr()?)?),
1392+
ty::TyFloat(FloatTy::F64) => PrimVal::from_f64(self.memory.read_f64(ptr.to_ptr()?)?),
14101393

1411-
ty::TyFnPtr(_) => self.memory.read_ptr(ptr)?,
1394+
ty::TyFnPtr(_) => self.memory.read_ptr(ptr.to_ptr()?)?,
14121395
ty::TyRef(_, ref tam) |
1413-
ty::TyRawPtr(ref tam) => return self.read_ptr(ptr, tam.ty).map(Some),
1396+
ty::TyRawPtr(ref tam) => return self.read_ptr(ptr.to_ptr()?, tam.ty).map(Some),
14141397

14151398
ty::TyAdt(def, _) => {
14161399
if def.is_box() {
1417-
return self.read_ptr(ptr, ty.boxed_ty()).map(Some);
1400+
return self.read_ptr(ptr.to_ptr()?, ty.boxed_ty()).map(Some);
14181401
}
14191402
use rustc::ty::layout::Layout::*;
14201403
if let CEnum { discr, signed, .. } = *self.type_layout(ty)? {
14211404
let size = discr.size().bytes();
14221405
if signed {
1423-
PrimVal::from_i128(self.memory.read_int(ptr, size)?)
1406+
PrimVal::from_i128(self.memory.read_int(ptr.to_ptr()?, size)?)
14241407
} else {
1425-
PrimVal::from_u128(self.memory.read_uint(ptr, size)?)
1408+
PrimVal::from_u128(self.memory.read_uint(ptr.to_ptr()?, size)?)
14261409
}
14271410
} else {
14281411
return Ok(None);
@@ -1541,7 +1524,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
15411524
let src_f_ptr = src_ptr.offset(src_field_offset, self.memory.layout)?;
15421525
let dst_f_ptr = dest.offset(dst_field_offset, self.memory.layout)?;
15431526
if src_fty == dst_fty {
1544-
self.copy(PrimVal::Ptr(src_f_ptr), PrimVal::Ptr(dst_f_ptr), src_fty)?;
1527+
self.copy(src_f_ptr, PrimVal::Ptr(dst_f_ptr), src_fty)?;
15451528
} else {
15461529
self.unsize_into(Value::ByRef(src_f_ptr), src_fty, Lvalue::from_ptr(dst_f_ptr), dst_fty)?;
15471530
}
@@ -1570,10 +1553,13 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
15701553
Err(err) => {
15711554
panic!("Failed to access local: {:?}", err);
15721555
}
1573-
Ok(Value::ByRef(ptr)) => {
1556+
Ok(Value::ByRef(PrimVal::Ptr(ptr))) => {
15741557
write!(msg, " by ref:").unwrap();
15751558
allocs.push(ptr.alloc_id);
15761559
}
1560+
Ok(Value::ByRef(ptr)) => {
1561+
write!(msg, " integral by ref: {:?}", ptr).unwrap();
1562+
}
15771563
Ok(Value::ByVal(val)) => {
15781564
write!(msg, " {:?}", val).unwrap();
15791565
if let PrimVal::Ptr(ptr) = val { allocs.push(ptr.alloc_id); }

‎src/lvalue.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ impl<'tcx> Lvalue<'tcx> {
6767
Self::from_primval_ptr(PrimVal::Undef)
6868
}
6969

70-
fn from_primval_ptr(ptr: PrimVal) -> Self {
70+
pub(crate) fn from_primval_ptr(ptr: PrimVal) -> Self {
7171
Lvalue::Ptr { ptr, extra: LvalueExtra::None }
7272
}
7373

74-
pub fn from_ptr(ptr: Pointer) -> Self {
74+
pub(crate) fn from_ptr(ptr: Pointer) -> Self {
7575
Self::from_primval_ptr(PrimVal::Ptr(ptr))
7676
}
7777

@@ -192,7 +192,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
192192
match lvalue {
193193
Lvalue::Ptr { ptr, extra } => {
194194
assert_eq!(extra, LvalueExtra::None);
195-
Ok(Value::ByRef(ptr.to_ptr()?))
195+
Ok(Value::ByRef(ptr))
196196
}
197197
Lvalue::Local { frame, local } => {
198198
self.stack[frame].get_local(local)

‎src/memory.rs

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
189189
}
190190

191191
let ptr = self.allocate(bytes.len() as u64, 1)?;
192-
self.write_bytes(ptr, bytes)?;
192+
self.write_bytes(PrimVal::Ptr(ptr), bytes)?;
193193
self.mark_static_initalized(ptr.alloc_id, false)?;
194194
self.literal_alloc_cache.insert(bytes.to_vec(), ptr.alloc_id);
195195
Ok(ptr)
@@ -288,39 +288,52 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
288288
self.layout.endian
289289
}
290290

291-
pub fn check_align(&self, ptr: Pointer, align: u64, len: u64) -> EvalResult<'tcx> {
292-
let alloc = self.get(ptr.alloc_id)?;
293-
// check whether the memory was marked as packed
294-
// we select all elements that have the correct alloc_id and are within
295-
// the range given by the offset into the allocation and the length
296-
let start = Entry {
297-
alloc_id: ptr.alloc_id,
298-
packed_start: 0,
299-
packed_end: ptr.offset + len,
300-
};
301-
let end = Entry {
302-
alloc_id: ptr.alloc_id,
303-
packed_start: ptr.offset + len,
304-
packed_end: 0,
291+
pub fn check_align(&self, ptr: PrimVal, align: u64, len: u64) -> EvalResult<'tcx> {
292+
let offset = match ptr {
293+
PrimVal::Ptr(ptr) => {
294+
let alloc = self.get(ptr.alloc_id)?;
295+
// check whether the memory was marked as packed
296+
// we select all elements that have the correct alloc_id and are within
297+
// the range given by the offset into the allocation and the length
298+
let start = Entry {
299+
alloc_id: ptr.alloc_id,
300+
packed_start: 0,
301+
packed_end: ptr.offset + len,
302+
};
303+
let end = Entry {
304+
alloc_id: ptr.alloc_id,
305+
packed_start: ptr.offset + len,
306+
packed_end: 0,
307+
};
308+
for &Entry { packed_start, packed_end, .. } in self.packed.range(start..end) {
309+
// if the region we are checking is covered by a region in `packed`
310+
// ignore the actual alignment
311+
if packed_start <= ptr.offset && (ptr.offset + len) <= packed_end {
312+
return Ok(());
313+
}
314+
}
315+
if alloc.align < align {
316+
return Err(EvalError::AlignmentCheckFailed {
317+
has: alloc.align,
318+
required: align,
319+
});
320+
}
321+
ptr.offset
322+
},
323+
PrimVal::Bytes(bytes) => {
324+
let v = ((bytes as u128) % (1 << self.pointer_size())) as u64;
325+
if v == 0 {
326+
return Err(EvalError::InvalidNullPointerUsage);
327+
}
328+
v
329+
},
330+
PrimVal::Undef => return Err(EvalError::ReadUndefBytes),
305331
};
306-
for &Entry { packed_start, packed_end, .. } in self.packed.range(start..end) {
307-
// if the region we are checking is covered by a region in `packed`
308-
// ignore the actual alignment
309-
if packed_start <= ptr.offset && (ptr.offset + len) <= packed_end {
310-
return Ok(());
311-
}
312-
}
313-
if alloc.align < align {
314-
return Err(EvalError::AlignmentCheckFailed {
315-
has: alloc.align,
316-
required: align,
317-
});
318-
}
319-
if ptr.offset % align == 0 {
332+
if offset % align == 0 {
320333
Ok(())
321334
} else {
322335
Err(EvalError::AlignmentCheckFailed {
323-
has: ptr.offset % align,
336+
has: offset % align,
324337
required: align,
325338
})
326339
}
@@ -572,7 +585,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
572585
if size == 0 {
573586
return Ok(&[]);
574587
}
575-
self.check_align(ptr, align, size)?;
588+
self.check_align(PrimVal::Ptr(ptr), align, size)?;
576589
self.check_bounds(ptr.offset(size, self.layout)?, true)?; // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow)
577590
let alloc = self.get(ptr.alloc_id)?;
578591
assert_eq!(ptr.offset as usize as u64, ptr.offset);
@@ -585,7 +598,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
585598
if size == 0 {
586599
return Ok(&mut []);
587600
}
588-
self.check_align(ptr, align, size)?;
601+
self.check_align(PrimVal::Ptr(ptr), align, size)?;
589602
self.check_bounds(ptr.offset(size, self.layout)?, true)?; // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow)
590603
let alloc = self.get_mut(ptr.alloc_id)?;
591604
assert_eq!(ptr.offset as usize as u64, ptr.offset);
@@ -716,20 +729,20 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
716729
self.get_bytes(ptr.to_ptr()?, size, 1)
717730
}
718731

719-
pub fn write_bytes(&mut self, ptr: Pointer, src: &[u8]) -> EvalResult<'tcx> {
732+
pub fn write_bytes(&mut self, ptr: PrimVal, src: &[u8]) -> EvalResult<'tcx> {
720733
if src.is_empty() {
721734
return Ok(());
722735
}
723-
let bytes = self.get_bytes_mut(ptr, src.len() as u64, 1)?;
736+
let bytes = self.get_bytes_mut(ptr.to_ptr()?, src.len() as u64, 1)?;
724737
bytes.clone_from_slice(src);
725738
Ok(())
726739
}
727740

728-
pub fn write_repeat(&mut self, ptr: Pointer, val: u8, count: u64) -> EvalResult<'tcx> {
741+
pub fn write_repeat(&mut self, ptr: PrimVal, val: u8, count: u64) -> EvalResult<'tcx> {
729742
if count == 0 {
730743
return Ok(());
731744
}
732-
let bytes = self.get_bytes_mut(ptr, count, 1)?;
745+
let bytes = self.get_bytes_mut(ptr.to_ptr()?, count, 1)?;
733746
for b in bytes { *b = val; }
734747
Ok(())
735748
}

0 commit comments

Comments
 (0)