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

Always error on SizeOverflow during mir evaluation #63152

Merged
merged 13 commits into from
Aug 7, 2019
14 changes: 8 additions & 6 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,17 @@ impl<'tcx> ConstEvalErr<'tcx> {
message: &str,
lint_root: Option<hir::HirId>,
) -> Result<DiagnosticBuilder<'tcx>, ErrorHandled> {
match self.error {
let must_error = match self.error {
err_inval!(Layout(LayoutError::Unknown(_))) |
err_inval!(TooGeneric) =>
return Err(ErrorHandled::TooGeneric),
err_inval!(Layout(LayoutError::SizeOverflow(_))) |
err_inval!(TypeckError) =>
return Err(ErrorHandled::Reported),
_ => {},
}
err_inval!(Layout(LayoutError::SizeOverflow(_))) => true,
_ => false,
};
trace!("reporting const eval failure at {:?}", self.span);
let mut err = if let Some(lint_root) = lint_root {
let mut err = if let (Some(lint_root), false) = (lint_root, must_error) {
let hir_id = self.stacktrace
.iter()
.rev()
Expand All @@ -160,6 +160,8 @@ impl<'tcx> ConstEvalErr<'tcx> {
tcx.span,
message,
)
} else if must_error {
struct_error(tcx, &self.error.to_string())
} else {
struct_error(tcx, message)
};
Expand Down Expand Up @@ -335,7 +337,7 @@ impl fmt::Debug for InvalidProgramInfo<'tcx> {
TypeckError =>
write!(f, "encountered constants with type errors, stopping evaluation"),
Layout(ref err) =>
write!(f, "rustc layout computation failed: {:?}", err),
write!(f, "{}", err),
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/librustc_codegen_llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use std::iter;
use std::str;
use std::sync::Arc;
use syntax::symbol::LocalInternedString;
use syntax::source_map::Span;
use crate::abi::Abi;

/// There is one `CodegenCx` per compilation unit. Each one has its own LLVM
Expand Down Expand Up @@ -860,9 +861,16 @@ impl LayoutOf for CodegenCx<'ll, 'tcx> {
type TyLayout = TyLayout<'tcx>;

fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyLayout {
self.spanned_layout_of(ty, None)
}

fn spanned_layout_of(&self, ty: Ty<'tcx>, span: Option<Span>) -> Self::TyLayout {
self.tcx.layout_of(ty::ParamEnv::reveal_all().and(ty))
.unwrap_or_else(|e| if let LayoutError::SizeOverflow(_) = e {
self.sess().fatal(&e.to_string())
match span {
Some(span) => self.sess().span_fatal(span, &e.to_string()),
None => self.sess().fatal(&e.to_string()),
}
} else {
bug!("failed to get layout for `{}`: {}", ty, e)
})
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_codegen_ssa/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,19 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
rvalue: &mir::Rvalue<'tcx>,
location: Location) {
debug!("visit_assign(place={:?}, rvalue={:?})", place, rvalue);
let mut decl_span = None;
if let mir::PlaceBase::Local(local) = &place.base {
estebank marked this conversation as resolved.
Show resolved Hide resolved
if let Some(decl) = self.fx.mir.local_decls.get(*local) {
estebank marked this conversation as resolved.
Show resolved Hide resolved
decl_span = Some(decl.source_info.span);
}
}

if let mir::Place {
base: mir::PlaceBase::Local(index),
projection: None,
} = *place {
self.assign(index, location);
if !self.fx.rvalue_creates_operand(rvalue) {
if !self.fx.rvalue_creates_operand(rvalue, decl_span) {
self.not_ssa(index);
}
} else {
Expand Down
13 changes: 9 additions & 4 deletions src/librustc_codegen_ssa/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc::middle::lang_items::ExchangeMallocFnLangItem;
use rustc_apfloat::{ieee, Float, Status, Round};
use std::{u128, i128};
use syntax::symbol::sym;
use syntax::source_map::Span;

use crate::base;
use crate::MemFlags;
Expand Down Expand Up @@ -136,7 +137,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

_ => {
assert!(self.rvalue_creates_operand(rvalue));
assert!(self.rvalue_creates_operand(rvalue, None));
let (mut bx, temp) = self.codegen_rvalue_operand(bx, rvalue);
temp.val.store(&mut bx, dest);
bx
Expand Down Expand Up @@ -169,7 +170,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mut bx: Bx,
rvalue: &mir::Rvalue<'tcx>
) -> (Bx, OperandRef<'tcx, Bx::Value>) {
assert!(self.rvalue_creates_operand(rvalue), "cannot codegen {:?} to operand", rvalue);
assert!(
self.rvalue_creates_operand(rvalue, None),
"cannot codegen {:?} to operand",
rvalue,
);

match *rvalue {
mir::Rvalue::Cast(ref kind, ref source, mir_cast_ty) => {
Expand Down Expand Up @@ -691,7 +696,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>) -> bool {
pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>, span: Option<Span>) -> bool {
estebank marked this conversation as resolved.
Show resolved Hide resolved
match *rvalue {
mir::Rvalue::Ref(..) |
mir::Rvalue::Len(..) |
Expand All @@ -707,7 +712,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::Aggregate(..) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let ty = self.monomorphize(&ty);
self.cx.layout_of(ty).is_zst()
self.cx.spanned_layout_of(ty, span).is_zst()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn push_stack_frame(
&mut self,
instance: ty::Instance<'tcx>,
span: source_map::Span,
span: Span,
body: &'mir mir::Body<'tcx>,
return_place: Option<PlaceTy<'tcx, M::PointerTag>>,
return_to_block: StackPopCleanup,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_target/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::ops::{Add, Deref, Sub, Mul, AddAssign, Range, RangeInclusive};
use rustc_data_structures::newtype_index;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use syntax_pos::symbol::{sym, Symbol};
use syntax_pos::Span;

pub mod call;

Expand Down Expand Up @@ -1012,6 +1013,9 @@ pub trait LayoutOf {
type TyLayout;

fn layout_of(&self, ty: Self::Ty) -> Self::TyLayout;
fn spanned_layout_of(&self, ty: Self::Ty, _span: Option<Span>) -> Self::TyLayout {
self.layout_of(ty)
}
}

#[derive(Copy, Clone, PartialEq, Eq)]
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/consts/issue-55878.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// error-pattern: the type `[u8; 18446744073709551615]` is too big for the current architecture
fn main() {
println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());
}
14 changes: 14 additions & 0 deletions src/test/ui/consts/issue-55878.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0080]: the type `[u8; 18446744073709551615]` is too big for the current architecture
--> $SRC_DIR/libcore/mem/mod.rs:LL:COL
|
LL | intrinsics::size_of::<T>()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 18446744073709551615]` is too big for the current architecture
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine merging as is with a FIXME in place for adding a useful (not duplicated) note here. It's likely a bit involeved in the const eval engine, since the way I'm imagining it requires annotating every layout_of call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about that too and came to the same conclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have a way to make a separate label that won't be too hacky. What do you think a good wording for this error would be/what the end result should look like?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that there are many situations in const eval that compute layouts and could thus fail. I think we should label each with their own message. If we hardcode a message for just this case it will be nonsensical in all other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just removed the label altogether and left the main message only

|
::: $DIR/issue-55878.rs:3:26
|
LL | println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());
| ---------------------------------------------------

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
4 changes: 4 additions & 0 deletions src/test/ui/huge-array-simple.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: the type `[u8; N]` is too big for the current architecture
--> $DIR/huge-array-simple.rs:12:9
|
LL | let _fat : [u8; (1<<61)+(1<<31)] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That span isn't ideal, but it's hard to do better in the MIR, especially as there may be no HIR local in the case of temporaries. Not sure what to do about it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but at the same time it's a huge improvement over the current "good luck finding where the problem is" state of this error.

| ^^^^

error: aborting due to previous error

3 changes: 1 addition & 2 deletions src/test/ui/huge-array.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// error-pattern:; 1518600000

// FIXME https://github.com/rust-lang/rust/issues/59774
// normalize-stderr-test "thread.*panicked.*Metadata module not compiled.*\n" -> ""
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""

fn generic<T: Copy>(t: T) {
let s: [T; 1518600000] = [t; 1518600000];
//~^ ERROR the type `[[u8; 1518599999]; 1518600000]` is too big for the current architecture
}

fn main() {
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/huge-array.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: the type `[[u8; 1518599999]; 1518600000]` is too big for the current architecture
--> $DIR/huge-array.rs:6:9
|
LL | let s: [T; 1518600000] = [t; 1518600000];
| ^

error: aborting due to previous error

4 changes: 4 additions & 0 deletions src/test/ui/issues/issue-15919.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: the type `[usize; N]` is too big for the current architecture
--> $DIR/issue-15919.rs:15:9
|
LL | let x = [0usize; 0xffff_ffff_ffff_ffff];
| ^

error: aborting due to previous error

2 changes: 2 additions & 0 deletions src/test/ui/issues/issue-56762.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ impl TooBigArray {
}

static MY_TOO_BIG_ARRAY_1: TooBigArray = TooBigArray::new();
//~^ ERROR the type `[u8; 2305843009213693951]` is too big for the current architecture
static MY_TOO_BIG_ARRAY_2: [u8; HUGE_SIZE] = [0x00; HUGE_SIZE];
//~^ ERROR the type `[u8; 2305843009213693951]` is too big for the current architecture

fn main() { }
15 changes: 13 additions & 2 deletions src/test/ui/issues/issue-56762.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
error: the type `[u8; 2305843009213693951]` is too big for the current architecture
error[E0080]: the type `[u8; 2305843009213693951]` is too big for the current architecture
--> $DIR/issue-56762.rs:19:1
|
LL | static MY_TOO_BIG_ARRAY_1: TooBigArray = TooBigArray::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 2305843009213693951]` is too big for the current architecture

error: aborting due to previous error
error[E0080]: the type `[u8; 2305843009213693951]` is too big for the current architecture
--> $DIR/issue-56762.rs:21:1
|
LL | static MY_TOO_BIG_ARRAY_2: [u8; HUGE_SIZE] = [0x00; HUGE_SIZE];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 2305843009213693951]` is too big for the current architecture

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0080`.