-
Notifications
You must be signed in to change notification settings - Fork 353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ensure that integers cast to pointers will never point at a valid alloc, not even the zst alloc #81
Changes from 1 commit
921f5af
b2d476e
1c40fb0
d42a7d0
75f56eb
f71c31c
2c34d65
511fa40
4a39c22
14ff641
13f22f8
f77a0ab
e2091ff
1549c2d
4748587
5ee75c0
1c5c6cd
64155ff
fd68670
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,10 +203,6 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
PrimVal::from_uint_with_size(n, self.memory.pointer_size()) | ||
} | ||
|
||
fn isize_primval(&self, n: i64) -> PrimVal { | ||
PrimVal::from_int_with_size(n, self.memory.pointer_size()) | ||
} | ||
|
||
fn str_to_value(&mut self, s: &str) -> EvalResult<'tcx, Value> { | ||
// FIXME: cache these allocs | ||
let ptr = self.memory.allocate(s.len(), 1)?; | ||
|
@@ -523,7 +519,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
self.write_value(value, dest, value_ty)?; | ||
} else { | ||
assert_eq!(operands.len(), 0); | ||
let zero = self.isize_primval(0); | ||
let value_size = self.type_size(dest_ty).expect("pointer types are sized"); | ||
let zero = PrimVal::from_int_with_size(0, value_size); | ||
self.write_primval(dest, zero)?; | ||
} | ||
} else { | ||
|
@@ -541,13 +538,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
let operand_ty = self.operand_ty(operand); | ||
assert_eq!(self.type_size(operand_ty), Some(0)); | ||
} | ||
let offset = self.nonnull_offset(dest_ty, nndiscr, discrfield)?; | ||
let (offset, ty) = self.nonnull_offset_and_ty(dest_ty, nndiscr, discrfield)?; | ||
|
||
// FIXME(solson) | ||
let dest = self.force_allocation(dest)?.to_ptr(); | ||
|
||
let dest = dest.offset(offset.bytes() as isize); | ||
try!(self.memory.write_isize(dest, 0)); | ||
let dest_size = self.type_size(ty).unwrap_or(self.memory.pointer_size()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In what cases can this default to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the unsized case. Since any unsized value is a pointer to the actual sized value and this optimization only cares about the pointer part, not the extra part There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What unsized case? EDIT: Nevermind, I now see the conversation lower down; let's continue there. |
||
try!(self.memory.write_int(dest, 0, dest_size)); | ||
} | ||
} else { | ||
bug!("tried to assign {:?} to Layout::RawNullablePointer", kind); | ||
|
@@ -694,7 +692,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
} | ||
} | ||
|
||
fn nonnull_offset(&self, ty: Ty<'tcx>, nndiscr: u64, discrfield: &[u32]) -> EvalResult<'tcx, Size> { | ||
fn nonnull_offset_and_ty(&self, ty: Ty<'tcx>, nndiscr: u64, discrfield: &[u32]) -> EvalResult<'tcx, (Size, Ty<'tcx>)> { | ||
// Skip the constant 0 at the start meant for LLVM GEP. | ||
let mut path = discrfield.iter().skip(1).map(|&i| i as usize); | ||
|
||
|
@@ -709,10 +707,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
_ => bug!("non-enum for StructWrappedNullablePointer: {}", ty), | ||
}; | ||
|
||
self.field_path_offset(inner_ty, path) | ||
self.field_path_offset_and_ty(inner_ty, path) | ||
} | ||
|
||
fn field_path_offset<I: Iterator<Item = usize>>(&self, mut ty: Ty<'tcx>, path: I) -> EvalResult<'tcx, Size> { | ||
fn field_path_offset_and_ty<I: Iterator<Item = usize>>(&self, mut ty: Ty<'tcx>, path: I) -> EvalResult<'tcx, (Size, Ty<'tcx>)> { | ||
let mut offset = Size::from_bytes(0); | ||
|
||
// Skip the initial 0 intended for LLVM GEP. | ||
|
@@ -722,7 +720,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
offset = offset.checked_add(field_offset, &self.tcx.data_layout).unwrap(); | ||
} | ||
|
||
Ok(offset) | ||
Ok((offset, ty)) | ||
} | ||
|
||
fn get_field_ty(&self, ty: Ty<'tcx>, field_index: usize) -> EvalResult<'tcx, Ty<'tcx>> { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -263,14 +263,17 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
self.memory.read_int(adt_ptr, discr_size as usize)? as u64 | ||
} | ||
|
||
RawNullablePointer { nndiscr, .. } => { | ||
self.read_nonnull_discriminant_value(adt_ptr, nndiscr)? | ||
RawNullablePointer { nndiscr, value } => { | ||
let discr_size = value.size(&self.tcx.data_layout).bytes() as usize; | ||
self.read_nonnull_discriminant_value(adt_ptr, nndiscr, discr_size)? | ||
} | ||
|
||
StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => { | ||
let offset = self.nonnull_offset(adt_ty, nndiscr, discrfield)?; | ||
let (offset, ty) = self.nonnull_offset_and_ty(adt_ty, nndiscr, discrfield)?; | ||
let nonnull = adt_ptr.offset(offset.bytes() as isize); | ||
self.read_nonnull_discriminant_value(nonnull, nndiscr)? | ||
// only the pointer part of a fat pointer is used for this space optimization | ||
let discr_size = self.type_size(ty).unwrap_or(self.memory.pointer_size()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto here. When can the nonnull field be a unsized? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can't be, but the MIR ends up giving us an index path that will give us the first field of the fat pointer, which ends up being te unsized value. Not sure if the code giving us the type path is wrong or if this is intended. I'll check and get back to you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first field of the fat pointer is the thin data pointer. This is a GEP field path, it will not deref nested pointers, just offset into a flat structure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. It sounds like we'd need to adjust the type, then, to match @eddyb's point. Alternately, maybe it can return the size instead of the type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
self.read_nonnull_discriminant_value(nonnull, nndiscr, discr_size)? | ||
} | ||
|
||
// The discriminant_value intrinsic returns 0 for non-sum types. | ||
|
@@ -281,8 +284,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { | |
Ok(discr_val) | ||
} | ||
|
||
fn read_nonnull_discriminant_value(&self, ptr: Pointer, nndiscr: u64) -> EvalResult<'tcx, u64> { | ||
let not_null = match self.memory.read_usize(ptr) { | ||
fn read_nonnull_discriminant_value(&self, ptr: Pointer, nndiscr: u64, discr_size: usize) -> EvalResult<'tcx, u64> { | ||
let not_null = match self.memory.read_uint(ptr, discr_size) { | ||
Ok(0) => false, | ||
Ok(_) | Err(EvalError::ReadPointerAsBytes) => true, | ||
Err(e) => return Err(e), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#![allow(dead_code)] | ||
|
||
enum E { | ||
A = 1, | ||
B = 2, | ||
C = 3, | ||
} | ||
|
||
fn main() { | ||
let enone = None::<E>; | ||
if let Some(..) = enone { | ||
panic!(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? I think we can assume
isize
is pointer sized, andisize_primval
does mean the targetisize
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want an isize here. The rawnullablepointer opt also happens for nonpointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, right, for NonZero things that aren't pointer-sized? I hate the
RawNullablePointer
names in layout...