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
18 changes: 11 additions & 7 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,10 +160,14 @@ impl<'tcx> ConstEvalErr<'tcx> {
tcx.span,
message,
)
} else if must_error {
struct_error(tcx, &self.error.to_string())
} else {
struct_error(tcx, message)
};
err.span_label(self.span, self.error.to_string());
if !must_error {
err.span_label(self.span, self.error.to_string());
}
// Skip the last, which is just the environment of the constant. The stacktrace
// is sometimes empty because we create "fake" eval contexts in CTFE to do work
// on constant values.
Expand Down Expand Up @@ -335,7 +339,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
7 changes: 6 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::{DUMMY_SP, 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,13 @@ impl LayoutOf for CodegenCx<'ll, 'tcx> {
type TyLayout = TyLayout<'tcx>;

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

fn spanned_layout_of(&self, ty: Ty<'tcx>, span: 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())
self.sess().span_fatal(span, &e.to_string())
} else {
bug!("failed to get layout for `{}`: {}", ty, e)
})
Expand Down
30 changes: 21 additions & 9 deletions src/librustc_codegen_ssa/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc::mir::visit::{Visitor, PlaceContext, MutatingUseContext, NonMutatingUs
use rustc::mir::traversal;
use rustc::ty;
use rustc::ty::layout::{LayoutOf, HasTyCtxt};
use syntax_pos::DUMMY_SP;
use super::FunctionCx;
use crate::traits::*;

Expand All @@ -20,10 +21,13 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

analyzer.visit_body(mir);

for (index, ty) in mir.local_decls.iter().map(|l| l.ty).enumerate() {
for (index, (ty, span)) in mir.local_decls.iter()
.map(|l| (l.ty, l.source_info.span))
.enumerate()
{
let ty = fx.monomorphize(&ty);
debug!("local {} has type {:?}", index, ty);
let layout = fx.cx.layout_of(ty);
let layout = fx.cx.spanned_layout_of(ty, span);
if fx.cx.is_backend_immediate(layout) {
// These sorts of types are immediates that we can store
// in an Value without an alloca.
Expand Down Expand Up @@ -93,10 +97,12 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
}
}

fn process_place(&mut self,
place_ref: &mir::PlaceRef<'_, 'tcx>,
context: PlaceContext,
location: Location) {
fn process_place(
&mut self,
place_ref: &mir::PlaceRef<'_, 'tcx>,
context: PlaceContext,
location: Location,
) {
let cx = self.fx.cx;

if let Some(proj) = place_ref.projection {
Expand All @@ -116,12 +122,17 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
.projection_ty(cx.tcx(), &proj.elem)
.ty;
let elem_ty = self.fx.monomorphize(&elem_ty);
if cx.layout_of(elem_ty).is_zst() {
let span = if let mir::PlaceBase::Local(index) = place_ref.base {
self.fx.mir.local_decls[*index].source_info.span
} else {
DUMMY_SP
};
if cx.spanned_layout_of(elem_ty, span).is_zst() {
return;
}

if let mir::ProjectionElem::Field(..) = proj.elem {
let layout = cx.layout_of(base_ty.ty);
let layout = cx.spanned_layout_of(base_ty.ty, span);
if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) {
// Recurse with the same context, instead of `Projection`,
// potentially stopping at non-operand projections,
Expand Down Expand Up @@ -188,7 +199,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
projection: None,
} = *place {
self.assign(index, location);
if !self.fx.rvalue_creates_operand(rvalue) {
let decl_span = self.fx.mir.local_decls[index].source_info.span;
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::{DUMMY_SP, 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, DUMMY_SP));
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, DUMMY_SP),
"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: Span) -> bool {
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: Span) -> Self::TyLayout {
self.layout_of(ty)
}
}

#[derive(Copy, Clone, PartialEq, Eq)]
Expand Down
7 changes: 7 additions & 0 deletions src/test/compile-fail/consts/issue-55878.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// normalize-stderr-64bit "18446744073709551615" -> "SIZE"
// normalize-stderr-32bit "4294967295" -> "SIZE"

// error-pattern: is too big for the current architecture
fn main() {
println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the error here for 32bit vs 64bit, that this cannot be handled by normalization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing a span from (at least) one specific target that is present in others:

error[E0080]: the type `[u8; SIZE]` is too big for the current architecture	
  --> $SRC_DIR/libcore/mem/mod.rs:LL:COL	
   |	
LL |     intrinsics::size_of::<T>()	
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^	
   | 	
  ::: $DIR/issue-55878.rs:6:26	
   |	
LL |     println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());	
   |                          ---------------------------------------------------	

vs

error[E0080]: the type `[u8; SIZE]` is too big for the current architecture	
   | 	
  ::: $DIR/issue-55878.rs:6:26	
   |	
LL |     println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());	
   |                          ---------------------------------------------------	

Copy link
Member

Choose a reason for hiding this comment

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

That's extremely odd, isn't it?

Normalization could still handle this -- or you use the two-test-cases approach with ignore-32bit and ignore-64bit? AFAIK compile-fail is on the way out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The divergence is not due to 32bit vs 64bit, it's because the dist-i586-gnu-i586-i686-musl rustc seems to be packaged slightly different than the other targets, so we miss core and std information to point spans to. Adding this info was a relatively recent improvement from the last year.

I agree that compile-fail should be discouraged, but short of waiting for us to fix the packaging issue I don't see a good solution to this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see.

Is it possible to ignore musl targets, or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is but I have no way of knowing a priori wether this is a musl-only discrepancy or not.

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 not super happy about adding more compile-fail tests, but oh well... :)

11 changes: 11 additions & 0 deletions src/test/ui/huge-array-simple-32.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// ignore-64bit

// 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" -> ""
#![allow(exceeding_bitshifts)]

fn main() {
let _fat: [u8; (1<<31)+(1<<15)] = //~ ERROR too big for the current architecture
[0; (1u32<<31) as usize +(1u32<<15) as usize];
}
8 changes: 8 additions & 0 deletions src/test/ui/huge-array-simple-32.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: the type `[u8; 2147516416]` is too big for the current architecture
--> $DIR/huge-array-simple-32.rs:9:9
|
LL | let _fat: [u8; (1<<31)+(1<<15)] =
| ^^^^

error: aborting due to previous error

11 changes: 11 additions & 0 deletions src/test/ui/huge-array-simple-64.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// ignore-32bit

// 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" -> ""
#![allow(exceeding_bitshifts)]

fn main() {
let _fat: [u8; (1<<61)+(1<<31)] = //~ ERROR too big for the current architecture
[0; (1u64<<61) as usize +(1u64<<31) as usize];
}
8 changes: 8 additions & 0 deletions src/test/ui/huge-array-simple-64.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: the type `[u8; 2305843011361177600]` is too big for the current architecture
--> $DIR/huge-array-simple-64.rs:9:9
|
LL | let _fat: [u8; (1<<61)+(1<<31)] =
| ^^^^

error: aborting due to previous error

20 changes: 0 additions & 20 deletions src/test/ui/huge-array-simple.rs

This file was deleted.

4 changes: 0 additions & 4 deletions src/test/ui/huge-array-simple.stderr

This file was deleted.

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

9 changes: 5 additions & 4 deletions src/test/ui/huge-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""

#[cfg(target_pointer_width = "32")]
fn main() {
let big: Option<[u32; (1<<29)-1]> = None;
}
type BIG = Option<[u32; (1<<29)-1]>;

#[cfg(target_pointer_width = "64")]
type BIG = Option<[u32; (1<<45)-1]>;

fn main() {
let big: Option<[u32; (1<<45)-1]> = None;
let big: BIG = None;
//~^ ERROR is too big for the current architecture
}
4 changes: 4 additions & 0 deletions src/test/ui/huge-enum.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: the type `TYPE` is too big for the current architecture
--> $DIR/huge-enum.rs:15:9
|
LL | let big: BIG = None;
| ^^^

error: aborting due to previous error

2 changes: 2 additions & 0 deletions src/test/ui/huge-struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,6 @@ struct S1M<T> { val: S1k<S1k<T>> }

fn main() {
let fat: Option<S1M<S1M<S1M<u32>>>> = None;
//~^ ERROR the type `S32<S1M<S1M<u32>>>` is too big for the current architecture

}
4 changes: 4 additions & 0 deletions src/test/ui/huge-struct.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: the type `SXX<SXX<SXX<u32>>>` is too big for the current architecture
--> $DIR/huge-struct.rs:49:9
|
LL | let fat: Option<SXX<SXX<SXX<u32>>>> = None;
| ^^^

error: aborting due to previous error

12 changes: 12 additions & 0 deletions src/test/ui/issues/issue-15919-32.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ignore-64bit

// 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 main() {
let x = [0usize; 0xffff_ffff]; //~ ERROR too big
}

// These two tests need to have different literals, as we can't rely on conditional compilation
estebank marked this conversation as resolved.
Show resolved Hide resolved
// for them while retaining the same spans/lines.
8 changes: 8 additions & 0 deletions src/test/ui/issues/issue-15919-32.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: the type `[usize; 4294967295]` is too big for the current architecture
--> $DIR/issue-15919-32.rs:8:9
|
LL | let x = [0usize; 0xffff_ffff];
| ^

error: aborting due to previous error

12 changes: 12 additions & 0 deletions src/test/ui/issues/issue-15919-64.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ignore-32bit

// 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 main() {
let x = [0usize; 0xffff_ffff_ffff_ffff]; //~ ERROR too big
}

// These two tests need to have different literals, as we can't rely on conditional compilation
estebank marked this conversation as resolved.
Show resolved Hide resolved
// for them while retaining the same spans/lines.
Loading