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

[beta] do not normalize non-scalar constants to a ConstValue::ScalarPair #54748

Closed
wants to merge 1 commit into from
Closed
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: 9 additions & 6 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,18 @@ pub enum ConstValue<'tcx> {
/// to allow HIR creation to happen for everything before needing to be able to run constant
/// evaluation
Unevaluated(DefId, &'tcx Substs<'tcx>),

/// Used only for types with layout::abi::Scalar ABI and ZSTs
///
/// Not using the enum `Value` to encode that this must not be `Undef`
Scalar(Scalar),
/// Used only for types with layout::abi::ScalarPair

/// Used only for *fat pointers* with layout::abi::ScalarPair
///
/// The second field may be undef in case of `Option<usize>::None`
ScalarPair(Scalar, ScalarMaybeUndef),
/// Used only for the remaining cases. An allocation + offset into the allocation.
/// Needed for pattern matching code related to slices and strings.
ScalarPair(Scalar, Scalar),

/// An allocation + offset into the allocation.
/// Invariant: The AllocId matches the allocation.
ByRef(AllocId, &'tcx Allocation, Size),
}
Expand Down Expand Up @@ -67,12 +70,12 @@ impl<'tcx> ConstValue<'tcx> {
ConstValue::ScalarPair(val, Scalar::Bits {
bits: len as u128,
size: cx.data_layout().pointer_size.bytes() as u8,
}.into())
})
}

#[inline]
pub fn new_dyn_trait(val: Scalar, vtable: Pointer) -> Self {
ConstValue::ScalarPair(val, Scalar::Ptr(vtable).into())
ConstValue::ScalarPair(val, Scalar::Ptr(vtable))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use hir::def::CtorKind;
use hir::def_id::DefId;
use hir::{self, HirId, InlineAsm};
use middle::region;
use mir::interpret::{ConstValue, EvalErrorKind, Scalar, ScalarMaybeUndef};
use mir::interpret::{ConstValue, EvalErrorKind, Scalar};
use mir::visit::MirVisitable;
use rustc_apfloat::ieee::{Double, Single};
use rustc_apfloat::Float;
Expand Down Expand Up @@ -2332,7 +2332,7 @@ pub fn fmt_const_val(f: &mut impl Write, const_val: &ty::Const) -> fmt::Result {
// print string literals
if let ConstValue::ScalarPair(ptr, len) = value {
if let Scalar::Ptr(ptr) = ptr {
if let ScalarMaybeUndef::Scalar(Scalar::Bits { bits: len, .. }) = len {
if let Scalar::Bits { bits: len, .. } = len {
if let Ref(_, &ty::TyS { sty: Str, .. }, _) = ty.sty {
return ty::tls::with(|tcx| {
let alloc = tcx.alloc_map.lock().get(ptr.alloc_id);
Expand Down
18 changes: 7 additions & 11 deletions src/librustc_codegen_llvm/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::mir::interpret::ConstEvalErr;
use rustc::mir::interpret::{ConstValue, ConstEvalErr};
use rustc::mir;
use rustc::mir::interpret::{ConstValue, ScalarMaybeUndef};
use rustc::ty;
use rustc::ty::layout::{self, Align, LayoutOf, TyLayout};
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -114,15 +113,12 @@ impl OperandRef<'ll, 'tcx> {
layout.scalar_pair_element_llvm_type(bx.cx, 0, true),
);
let b_layout = layout.scalar_pair_element_llvm_type(bx.cx, 1, true);
let b_llval = match b {
ScalarMaybeUndef::Scalar(b) => scalar_to_llvm(
bx.cx,
b,
b_scalar,
b_layout,
),
ScalarMaybeUndef::Undef => C_undef(b_layout),
};
let b_llval = scalar_to_llvm(
bx.cx,
b,
b_scalar,
b_layout,
);
OperandValue::Pair(a_llval, b_llval)
},
ConstValue::ByRef(_, alloc, offset) => {
Expand Down
16 changes: 13 additions & 3 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc::hir::{self, def_id::DefId};
use rustc::mir::interpret::ConstEvalErr;
use rustc::mir;
use rustc::ty::{self, TyCtxt, Instance, query::TyCtxtAt};
use rustc::ty::layout::{LayoutOf, TyLayout};
use rustc::ty::layout::{self, LayoutOf, TyLayout};
use rustc::ty::subst::Subst;
use rustc_data_structures::indexed_vec::IndexVec;

Expand Down Expand Up @@ -90,8 +90,18 @@ pub fn eval_promoted<'a, 'mir, 'tcx>(
pub fn op_to_const<'tcx>(
ecx: &EvalContext<'_, '_, 'tcx, CompileTimeEvaluator>,
op: OpTy<'tcx>,
normalize: bool,
may_normalize: bool,
) -> EvalResult<'tcx, &'tcx ty::Const<'tcx>> {
// We do not normalize just any data. Only scalar layout and fat pointers.
let normalize = may_normalize
&& match op.layout.abi {
layout::Abi::Scalar(..) => true,
layout::Abi::ScalarPair(..) => {
// Must be a fat pointer
op.layout.ty.builtin_deref(true).is_some()
},
_ => false,
};
let normalized_op = if normalize {
ecx.try_read_value(op)?
} else {
Expand All @@ -118,7 +128,7 @@ pub fn op_to_const<'tcx>(
Ok(Value::Scalar(x)) =>
ConstValue::Scalar(x.not_undef()?),
Ok(Value::ScalarPair(a, b)) =>
ConstValue::ScalarPair(a.not_undef()?, b),
ConstValue::ScalarPair(a.not_undef()?, b.not_undef()?),
};
Ok(ty::Const::from_const_value(ecx.tcx.tcx, val, op.layout.ty))
}
Expand Down
7 changes: 0 additions & 7 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,13 +1124,6 @@ pub fn compare_const_vals<'a, 'tcx>(
len_b,
),
) if ptr_a.offset.bytes() == 0 && ptr_b.offset.bytes() == 0 => {
let len_a = len_a.not_undef().ok();
let len_b = len_b.not_undef().ok();
if len_a.is_none() || len_b.is_none() {
tcx.sess.struct_err("str slice len is undef").delay_as_bug();
}
let len_a = len_a?;
let len_b = len_b?;
if let Ok(len_a) = len_a.to_bits(tcx.data_layout.pointer_size) {
if let Ok(len_b) = len_b.to_bits(tcx.data_layout.pointer_size) {
if len_a == len_b {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Ok(Operand::Indirect(MemPlace::from_ptr(Pointer::new(id, offset), alloc.align)))
},
ConstValue::ScalarPair(a, b) =>
Ok(Operand::Immediate(Value::ScalarPair(a.into(), b))),
Ok(Operand::Immediate(Value::ScalarPair(a.into(), b.into()))),
ConstValue::Scalar(x) =>
Ok(Operand::Immediate(Value::Scalar(x.into()))),
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ use rustc::hir::itemlikevisit::ItemLikeVisitor;

use rustc::hir::Node;
use rustc::hir::def_id::DefId;
use rustc::mir::interpret::{AllocId, ConstValue, ScalarMaybeUndef};
use rustc::mir::interpret::{AllocId, ConstValue};
use rustc::middle::lang_items::{ExchangeMallocFnLangItem, StartFnLangItem};
use rustc::ty::subst::Substs;
use rustc::ty::{self, TypeFoldable, Ty, TyCtxt, GenericParamDefKind};
Expand Down Expand Up @@ -1263,11 +1263,11 @@ fn collect_const<'a, 'tcx>(
};
match val {
ConstValue::Unevaluated(..) => bug!("const eval yielded unevaluated const"),
ConstValue::ScalarPair(Scalar::Ptr(a), ScalarMaybeUndef::Scalar(Scalar::Ptr(b))) => {
ConstValue::ScalarPair(Scalar::Ptr(a), Scalar::Ptr(b)) => {
collect_miri(tcx, a.alloc_id, output);
collect_miri(tcx, b.alloc_id, output);
}
ConstValue::ScalarPair(_, ScalarMaybeUndef::Scalar(Scalar::Ptr(ptr))) |
ConstValue::ScalarPair(_, Scalar::Ptr(ptr)) |
ConstValue::ScalarPair(Scalar::Ptr(ptr), _) |
ConstValue::Scalar(Scalar::Ptr(ptr)) =>
collect_miri(tcx, ptr.alloc_id, output),
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/const-eval/union-ice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const UNION: DummyUnion = DummyUnion { field1: 1065353216 };

const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR this constant cannot be used

const FIELD_PATH: Struct = Struct { //~ ERROR this constant cannot be used
const FIELD_PATH: Struct = Struct { //~ ERROR this constant likely exhibits undefined behavior
a: 42,
b: unsafe { UNION.field3 },
};
Expand Down
8 changes: 5 additions & 3 deletions src/test/ui/consts/const-eval/union-ice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ LL | const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR this constant can
|
= note: #[deny(const_err)] on by default

error: this constant cannot be used
error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/union-ice.rs:25:1
|
LL | / const FIELD_PATH: Struct = Struct { //~ ERROR this constant cannot be used
LL | / const FIELD_PATH: Struct = Struct { //~ ERROR this constant likely exhibits undefined behavior
LL | | a: 42,
LL | | b: unsafe { UNION.field3 },
LL | | };
| |__^ attempted to read undefined bytes
| |__^ type validation failed: encountered undefined bytes at .b
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/union-ice.rs:35:1
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/issues/issue-54387.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// compile-pass

pub struct GstRc {
_obj: *const (),
_borrowed: bool,
}

const FOO: Option<GstRc> = None;

fn main() {
let _meh = FOO;
}