From e09e3c898cbc371e8c3f9e6f1be5417b30daa24a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Oct 2018 12:34:10 +0200 Subject: [PATCH] fix nits and handling of extern static --- src/librustc_mir/interpret/memory.rs | 37 +++++++++++++++--------- src/librustc_mir/interpret/terminator.rs | 2 +- src/librustc_mir/interpret/validity.rs | 29 +++++++++---------- src/test/ui/issues/issue-14227.rs | 4 +-- src/test/ui/issues/issue-14227.stderr | 6 ++-- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 1b75982a83a..5437c8ababc 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -19,7 +19,7 @@ use std::collections::VecDeque; use std::ptr; -use rustc::ty::{self, Instance, query::TyCtxtAt}; +use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; use rustc::ty::layout::{self, Align, TargetDataLayout, Size, HasDataLayout}; use rustc::mir::interpret::{Pointer, AllocId, Allocation, ConstValue, GlobalId, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, @@ -235,7 +235,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // Check non-NULL/Undef, extract offset let (offset, alloc_align) = match ptr { Scalar::Ptr(ptr) => { - let (size, align) = self.get_size_and_align(ptr.alloc_id)?; + let (size, align) = self.get_size_and_align(ptr.alloc_id); // check this is not NULL -- which we can ensure only if this is in-bounds // of some (potentially dead) allocation. if ptr.offset > size { @@ -359,19 +359,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } - pub fn get_size_and_align(&self, id: AllocId) -> EvalResult<'tcx, (Size, Align)> { - Ok(match self.get(id) { - Ok(alloc) => (Size::from_bytes(alloc.bytes.len() as u64), alloc.align), - Err(err) => match err.kind { - EvalErrorKind::DanglingPointerDeref => - // This should be in the dead allocation map - *self.dead_alloc_map.get(&id).expect( - "allocation missing in dead_alloc_map" - ), - // E.g. a function ptr allocation - _ => return Err(err) + pub fn get_size_and_align(&self, id: AllocId) -> (Size, Align) { + if let Ok(alloc) = self.get(id) { + return (Size::from_bytes(alloc.bytes.len() as u64), alloc.align); + } + // Could also be a fn ptr or extern static + match self.tcx.alloc_map.lock().get(id) { + Some(AllocType::Function(..)) => (Size::ZERO, Align::from_bytes(1, 1).unwrap()), + Some(AllocType::Static(did)) => { + // The only way `get` couldnÄt have worked here is if this is an extern static + assert!(self.tcx.is_foreign_item(did)); + // Use size and align of the type + let ty = self.tcx.type_of(did); + let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); + (layout.size, layout.align) } - }) + _ => { + // Must be a deallocated pointer + *self.dead_alloc_map.get(&id).expect( + "allocation missing in dead_alloc_map" + ) + } + } } pub fn get_mut( diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index e82d60af639..862f61df227 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -384,7 +384,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } else { // FIXME: The caller thinks this function cannot return. How do // we verify that the callee agrees? - // On the plus side, the the callee every writes to its return place, + // On the plus side, the the callee ever writes to its return place, // that will be detected as UB (because we set that to NULL above). } Ok(()) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 2aa6b07df90..f0a51000cd8 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -209,22 +209,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // for safe ptrs, recursively check if let ty::Ref(..) = value.layout.ty.sty { - if const_mode { - // Skip validation entirely for some external statics - if let Scalar::Ptr(ptr) = place.ptr { - let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id); - if let Some(AllocType::Static(did)) = alloc_kind { - // `extern static` cannot be validated as they have no body. - // They are not even properly aligned. - // Statics from other crates are already checked. - // They might be checked at a different type, but for now we want - // to avoid recursing too deeply. This is not sound! - if !did.is_local() || self.tcx.is_foreign_item(did) { - return Ok(()); - } - } - } - } // Make sure this is non-NULL and aligned let (size, align) = self.size_and_align_of(place.extra, place.layout)?; match self.memory.check_align(place.ptr, align) { @@ -244,6 +228,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> if !place.layout.is_zst() { let ptr = try_validation!(place.ptr.to_ptr(), "integer pointer in non-ZST reference", path); + if const_mode { + // Skip validation entirely for some external statics + let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id); + if let Some(AllocType::Static(did)) = alloc_kind { + // `extern static` cannot be validated as they have no body. + // FIXME: Statics from other crates are also skipped. + // They might be checked at a different type, but for now we + // want to avoid recursing too deeply. This is not sound! + if !did.is_local() || self.tcx.is_foreign_item(did) { + return Ok(()); + } + } + } try_validation!(self.memory.check_bounds(ptr, size, false), "dangling (not entirely in bounds) reference", path); } diff --git a/src/test/ui/issues/issue-14227.rs b/src/test/ui/issues/issue-14227.rs index 250e78ce246..857db50edbb 100644 --- a/src/test/ui/issues/issue-14227.rs +++ b/src/test/ui/issues/issue-14227.rs @@ -11,9 +11,9 @@ #![allow(safe_extern_statics, warnings)] extern { - pub static symbol: (); + pub static symbol: u32; } -static CRASH: () = symbol; +static CRASH: u32 = symbol; //~^ ERROR could not evaluate static initializer //~| tried to read from foreign (extern) static diff --git a/src/test/ui/issues/issue-14227.stderr b/src/test/ui/issues/issue-14227.stderr index f5f39465b18..dc6c72d8a72 100644 --- a/src/test/ui/issues/issue-14227.stderr +++ b/src/test/ui/issues/issue-14227.stderr @@ -1,8 +1,8 @@ error[E0080]: could not evaluate static initializer - --> $DIR/issue-14227.rs:16:20 + --> $DIR/issue-14227.rs:16:21 | -LL | static CRASH: () = symbol; - | ^^^^^^ tried to read from foreign (extern) static +LL | static CRASH: u32 = symbol; + | ^^^^^^ tried to read from foreign (extern) static error: aborting due to previous error