Skip to content
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

Fix a hash collision issue on the const_field query #61959

Merged
merged 8 commits into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,18 @@ impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::Allocation {
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>,
) {
self.bytes.hash_stable(hcx, hasher);
for reloc in self.relocations.iter() {
let mir::interpret::Allocation {
bytes, relocations, undef_mask, align, mutability,
extra: _,
} = self;
bytes.hash_stable(hcx, hasher);
relocations.len().hash_stable(hcx, hasher);
for reloc in relocations.iter() {
reloc.hash_stable(hcx, hasher);
}
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
self.undef_mask.hash_stable(hcx, hasher);
self.align.hash_stable(hcx, hasher);
self.mutability.hash_stable(hcx, hasher);
undef_mask.hash_stable(hcx, hasher);
align.hash_stable(hcx, hasher);
mutability.hash_stable(hcx, hasher);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/freshen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for TypeFreshener<'a, 'tcx> {
ConstValue::Param(_) |
ConstValue::Scalar(_) |
ConstValue::Slice { .. } |
ConstValue::ByRef(..) |
ConstValue::ByRef { .. } |
ConstValue::Unevaluated(..) => {}
}

Expand Down
12 changes: 7 additions & 5 deletions src/librustc/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`");
self.check_bounds(cx, ptr, size, CheckInAllocMsg::MemoryAccessTest)?;

self.mark_definedness(ptr, size, true)?;
self.mark_definedness(ptr, size, true);
self.clear_relocations(cx, ptr, size)?;

AllocationExtra::memory_written(self, ptr, size)?;
Expand Down Expand Up @@ -406,7 +406,10 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
{
let val = match val {
ScalarMaybeUndef::Scalar(scalar) => scalar,
ScalarMaybeUndef::Undef => return self.mark_definedness(ptr, type_size, false),
ScalarMaybeUndef::Undef => {
self.mark_definedness(ptr, type_size, false);
return Ok(());
},
};

let bytes = match val.to_bits_or_ptr(type_size, cx) {
Expand Down Expand Up @@ -550,16 +553,15 @@ impl<'tcx, Tag, Extra> Allocation<Tag, Extra> {
ptr: Pointer<Tag>,
size: Size,
new_state: bool,
) -> InterpResult<'tcx> {
) {
if size.bytes() == 0 {
return Ok(());
return;
}
self.undef_mask.set_range(
ptr.offset,
ptr.offset + size,
new_state,
);
Ok(())
}
}

Expand Down
25 changes: 16 additions & 9 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ pub enum ConstValue<'tcx> {
end: usize,
},

/// An allocation together with a pointer into the allocation.
/// Invariant: the pointer's `AllocId` resolves to the allocation.
/// The alignment exists to allow `const_field` to have `ByRef` access to nonprimitive fields
/// of `repr(packed)` structs. The alignment may be lower than the type of this constant.
/// This permits reads with lower alignment than what the type would normally require.
/// FIXME(RalfJ,oli-obk): The alignment checks are part of miri, but const eval doesn't really
/// need them. Disabling them may be too hard though.
ByRef(Pointer, Align, &'tcx Allocation),
/// A value not represented/representable by `Scalar` or `Slice`
ByRef {
/// The alignment exists to allow `const_field` to have `ByRef` access to nonprimitive
/// fields of `repr(packed)` structs. The alignment may be lower than the type of this
/// constant. This permits reads with lower alignment than what the type would normally
/// require.
/// FIXME(RalfJ,oli-obk): The alignment checks are part of miri, but const eval doesn't
/// really need them. Disabling them may be too hard though.
align: Align,
/// Offset into `alloc`
offset: Size,
/// The backing memory of the value, may contain more memory than needed for just the value
/// in order to share `Allocation`s between values
alloc: &'tcx Allocation,
},

/// Used in the HIR by using `Unevaluated` everywhere and later normalizing to one of the other
/// variants when the code is monomorphic enough for that.
Expand All @@ -67,7 +74,7 @@ impl<'tcx> ConstValue<'tcx> {
ConstValue::Param(_) |
ConstValue::Infer(_) |
ConstValue::Placeholder(_) |
ConstValue::ByRef(..) |
ConstValue::ByRef{ .. } |
ConstValue::Unevaluated(..) |
ConstValue::Slice { .. } => None,
ConstValue::Scalar(val) => Some(val),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/print/obsolete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl DefPathBasedNames<'tcx> {
// as well as the unprintable types of constants (see `push_type_name` for more details).
pub fn push_const_name(&self, c: &Const<'tcx>, output: &mut String, debug: bool) {
match c.val {
ConstValue::Scalar(..) | ConstValue::Slice { .. } | ConstValue::ByRef(..) => {
ConstValue::Scalar(..) | ConstValue::Slice { .. } | ConstValue::ByRef { .. } => {
// FIXME(const_generics): we could probably do a better job here.
write!(output, "{:?}", c).unwrap()
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>(
ty: a.ty,
}))
}
(ConstValue::ByRef(..), _) => {
(ConstValue::ByRef { .. }, _) => {
bug!(
"non-Scalar ConstValue encountered in super_relate_consts {:?} {:?}",
a,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,8 @@ impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::Const<'tcx> {
impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
match *self {
ConstValue::ByRef(ptr, align, alloc) => ConstValue::ByRef(ptr, align, alloc),
ConstValue::ByRef { offset, align, alloc } =>
ConstValue::ByRef { offset, align, alloc },
ConstValue::Infer(ic) => ConstValue::Infer(ic.fold_with(folder)),
ConstValue::Param(p) => ConstValue::Param(p.fold_with(folder)),
ConstValue::Placeholder(p) => ConstValue::Placeholder(p),
Expand All @@ -1348,7 +1349,7 @@ impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> {

fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
match *self {
ConstValue::ByRef(..) => false,
ConstValue::ByRef { .. } => false,
ConstValue::Infer(ic) => ic.visit_with(visitor),
ConstValue::Param(p) => p.visit_with(visitor),
ConstValue::Placeholder(_) => false,
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ pub fn codegen_static_initializer(
let static_ = cx.tcx.const_eval(param_env.and(cid))?;

let alloc = match static_.val {
ConstValue::ByRef(ptr, align, alloc) if ptr.offset.bytes() == 0 && align == alloc.align => {
ConstValue::ByRef {
offset, align, alloc,
} if offset.bytes() == 0 && align == alloc.align => {
alloc
},
_ => bug!("static const eval returned {:#?}", static_),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
let b_llval = bx.const_usize((end - start) as u64);
OperandValue::Pair(a_llval, b_llval)
},
ConstValue::ByRef(ptr, align, alloc) => {
return bx.load_operand(bx.from_const_alloc(layout, align, alloc, ptr.offset));
ConstValue::ByRef { offset, align, alloc } => {
return bx.load_operand(bx.from_const_alloc(layout, align, alloc, offset));
},
};

Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let layout = cx.layout_of(self.monomorphize(&ty));
match bx.tcx().const_eval(param_env.and(cid)) {
Ok(val) => match val.val {
mir::interpret::ConstValue::ByRef(ptr, align, alloc) => {
bx.cx().from_const_alloc(layout, align, alloc, ptr.offset)
mir::interpret::ConstValue::ByRef { offset, align, alloc } => {
bx.cx().from_const_alloc(layout, align, alloc, offset)
}
_ => bug!("promoteds should have an allocation: {:?}", val),
},
Expand Down
14 changes: 7 additions & 7 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn op_to_const<'tcx>(
Ok(mplace) => {
let ptr = mplace.ptr.to_ptr().unwrap();
let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
ConstValue::ByRef(ptr, mplace.align, alloc)
ConstValue::ByRef { offset: ptr.offset, align: mplace.align, alloc }
},
// see comment on `let try_as_immediate` above
Err(ImmTy { imm: Immediate::Scalar(x), .. }) => match x {
Expand All @@ -113,7 +113,7 @@ fn op_to_const<'tcx>(
let mplace = op.to_mem_place();
let ptr = mplace.ptr.to_ptr().unwrap();
let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
ConstValue::ByRef(ptr, mplace.align, alloc)
ConstValue::ByRef { offset: ptr.offset, align: mplace.align, alloc }
},
},
Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => {
Expand Down Expand Up @@ -541,11 +541,11 @@ fn validate_and_turn_into_const<'tcx>(
if tcx.is_static(def_id) || cid.promoted.is_some() {
let ptr = mplace.ptr.to_ptr()?;
Ok(tcx.mk_const(ty::Const {
val: ConstValue::ByRef(
ptr,
mplace.align,
ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),
),
val: ConstValue::ByRef {
offset: ptr.offset,
align: mplace.align,
alloc: ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),
},
ty: mplace.layout.ty,
}))
} else {
Expand Down
17 changes: 9 additions & 8 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ impl LiteralExpander<'tcx> {
// the easy case, deref a reference
(ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => {
let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id);
ConstValue::ByRef(
p,
ConstValue::ByRef {
offset: p.offset,
// FIXME(oli-obk): this should be the type's layout
alloc.align,
align: alloc.align,
alloc,
)
}
},
// unsize array to slice if pattern is array but match value or other patterns are slice
(ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => {
Expand Down Expand Up @@ -1436,9 +1436,10 @@ fn slice_pat_covered_by_const<'tcx>(
suffix: &[Pattern<'tcx>],
) -> Result<bool, ErrorReported> {
let data: &[u8] = match (const_val.val, &const_val.ty.sty) {
(ConstValue::ByRef(ptr, _, alloc), ty::Array(t, n)) => {
(ConstValue::ByRef { offset, alloc, .. }, ty::Array(t, n)) => {
assert_eq!(*t, tcx.types.u8);
let n = n.assert_usize(tcx).unwrap();
let ptr = Pointer::new(AllocId(0), offset);
alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap()
},
(ConstValue::Slice { data, start, end }, ty::Slice(t)) => {
Expand Down Expand Up @@ -1758,9 +1759,9 @@ fn specialize<'p, 'a: 'p, 'tcx>(
let (alloc, offset, n, ty) = match value.ty.sty {
ty::Array(t, n) => {
match value.val {
ConstValue::ByRef(ptr, _, alloc) => (
ConstValue::ByRef { offset, alloc, .. } => (
alloc,
ptr.offset,
offset,
n.unwrap_usize(cx.tcx),
t,
),
Expand All @@ -1778,7 +1779,7 @@ fn specialize<'p, 'a: 'p, 'tcx>(
(end - start) as u64,
t,
),
ConstValue::ByRef(..) => {
ConstValue::ByRef { .. } => {
// FIXME(oli-obk): implement `deref` for `ConstValue`
return None;
},
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> {
self.layout_of(self.monomorphize(val.ty)?)
})?;
let op = match val.val {
ConstValue::ByRef(ptr, align, _alloc) => {
ConstValue::ByRef { offset, align, alloc } => {
let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc);
// We rely on mutability being set correctly in that allocation to prevent writes
// where none should happen.
let ptr = self.tag_static_base_pointer(ptr);
let ptr = self.tag_static_base_pointer(Pointer::new(id, offset));
Operand::Indirect(MemPlace::from_ptr(ptr, align))
},
ConstValue::Scalar(x) =>
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ fn collect_const<'tcx>(
ConstValue::Scalar(Scalar::Ptr(ptr)) =>
collect_miri(tcx, ptr.alloc_id, output),
ConstValue::Slice { data: alloc, start: _, end: _ } |
ConstValue::ByRef(_, _, alloc) => {
ConstValue::ByRef { alloc, .. } => {
for &((), id) in alloc.relocations.values() {
collect_miri(tcx, id, output);
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,8 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt<'_>, id: DefId, span: Span)
};
let param_env = ty::ParamEnv::reveal_all();
if let Ok(static_) = tcx.const_eval(param_env.and(cid)) {
let alloc = if let ConstValue::ByRef(_, _, allocation) = static_.val {
allocation
let alloc = if let ConstValue::ByRef { alloc, .. } = static_.val {
alloc
} else {
bug!("Matching on non-ByRef static")
};
Expand Down
17 changes: 17 additions & 0 deletions src/test/incremental/issue-61530.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(repr_simd, platform_intrinsics)]

// revisions:rpass1 rpass2

#[repr(simd)]
struct I32x2(i32, i32);

extern "platform-intrinsic" {
fn simd_shuffle2<T, U>(x: T, y: T, idx: [u32; 2]) -> U;
}

fn main() {
unsafe {
let _: I32x2 = simd_shuffle2(I32x2(1, 2), I32x2(3, 4), [0, 0]);
let _: I32x2 = simd_shuffle2(I32x2(1, 2), I32x2(3, 4), [0, 0]);
}
}