Skip to content

Fix a hash collision issue on the const_field query #61959

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

Merged
merged 8 commits into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 9 additions & 5 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,17 @@ 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);
for reloc in relocations.iter() {
reloc.hash_stable(hcx, hasher);
}
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
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
5 changes: 2 additions & 3 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@ pub enum ConstValue<'tcx> {
end: usize,
},

/// An allocation together with a pointer into the allocation.
/// Invariant: the pointer's `AllocId` resolves to the allocation.
/// An allocation together with an offset into 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),
ByRef(Size, Align, &'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 Down
2 changes: 1 addition & 1 deletion src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ 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 Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ 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
6 changes: 3 additions & 3 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(ptr.offset, 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(ptr.offset, mplace.align, alloc)
},
},
Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => {
Expand Down Expand Up @@ -542,7 +542,7 @@ fn validate_and_turn_into_const<'tcx>(
let ptr = mplace.ptr.to_ptr()?;
Ok(tcx.mk_const(ty::Const {
val: ConstValue::ByRef(
ptr,
ptr.offset,
mplace.align,
ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),
),
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl LiteralExpander<'tcx> {
(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,
p.offset,
// FIXME(oli-obk): this should be the type's layout
alloc.align,
alloc,
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 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
17 changes: 17 additions & 0 deletions src/test/incremental/simd_intrinsic_ice.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]);
}
}