fix nits and handling of extern static

Author: RalfJung

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(

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(())

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);
                     }

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
 

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