Auto merge of #2404 - RalfJung:mix, r=RalfJung

check for extern static size mismatches

Also handle get_alloc_extra the same throughout Stacked Borrows. I don't think this `unwrap` can actually cause an ICE since another part of SB will raise an error before, but still, seems strange to do this inconsistently in retagging vs expose_ptr.

Author: bors

src/concurrency/data_race.rs

@@ -1061,7 +1061,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
         if let Some(data_race) = &this.machine.data_race {
             if data_race.race_detecting() {
                 let size = place.layout.size;
-                let (alloc_id, base_offset, _tag) = this.ptr_get_alloc_id(place.ptr)?;
+                let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr)?;
                 // Load and log the atomic operation.
                 // Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option.
                 let alloc_meta = this.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();

src/intptrcast.rs

@@ -94,16 +94,21 @@ impl<'mir, 'tcx> GlobalStateInner {
         None
     }
 
-    pub fn expose_ptr(ecx: &mut MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId, sb: SbTag) {
+    pub fn expose_ptr(
+        ecx: &mut MiriEvalContext<'mir, 'tcx>,
+        alloc_id: AllocId,
+        sb: SbTag,
+    ) -> InterpResult<'tcx> {
         let global_state = ecx.machine.intptrcast.get_mut();
         // In strict mode, we don't need this, so we can save some cycles by not tracking it.
         if global_state.provenance_mode != ProvenanceMode::Strict {
             trace!("Exposing allocation id {alloc_id:?}");
             global_state.exposed.insert(alloc_id);
             if ecx.machine.stacked_borrows.is_some() {
-                ecx.expose_tag(alloc_id, sb);
+                ecx.expose_tag(alloc_id, sb)?;
             }
         }
+        Ok(())
     }
 
     pub fn ptr_from_addr_transmute(

src/machine.rs

@@ -638,12 +638,35 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
     ) -> InterpResult<'tcx, Pointer<Provenance>> {
         let link_name = ecx.item_link_name(def_id);
         if let Some(&ptr) = ecx.machine.extern_statics.get(&link_name) {
+            // Various parts of the engine rely on `get_alloc_info` for size and alignment
+            // information. That uses the type information of this static.
+            // Make sure it matches the Miri allocation for this.
+            let Provenance::Concrete { alloc_id, .. } = ptr.provenance else {
+                panic!("extern_statics cannot contain wildcards")
+            };
+            let (shim_size, shim_align, _kind) = ecx.get_alloc_info(alloc_id);
+            let extern_decl_layout =
+                ecx.tcx.layout_of(ty::ParamEnv::empty().and(ecx.tcx.type_of(def_id))).unwrap();
+            if extern_decl_layout.size != shim_size || extern_decl_layout.align.abi != shim_align {
+                throw_unsup_format!(
+                    "`extern` static `{name}` from crate `{krate}` has been declared \
+                    with a size of {decl_size} bytes and alignment of {decl_align} bytes, \
+                    but Miri emulates it via an extern static shim \
+                    with a size of {shim_size} bytes and alignment of {shim_align} bytes",
+                    name = ecx.tcx.def_path_str(def_id),
+                    krate = ecx.tcx.crate_name(def_id.krate),
+                    decl_size = extern_decl_layout.size.bytes(),
+                    decl_align = extern_decl_layout.align.abi.bytes(),
+                    shim_size = shim_size.bytes(),
+                    shim_align = shim_align.bytes(),
+                )
+            }
             Ok(ptr)
         } else {
             throw_unsup_format!(
-                "`extern` static `{}` from crate `{}` is not supported by Miri",
-                ecx.tcx.def_path_str(def_id),
-                ecx.tcx.crate_name(def_id.krate),
+                "`extern` static `{name}` from crate `{krate}` is not supported by Miri",
+                name = ecx.tcx.def_path_str(def_id),
+                krate = ecx.tcx.crate_name(def_id.krate),
             )
         }
     }
@@ -754,15 +777,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         ptr: Pointer<Self::Provenance>,
     ) -> InterpResult<'tcx> {
         match ptr.provenance {
-            Provenance::Concrete { alloc_id, sb } => {
-                intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb);
-            }
+            Provenance::Concrete { alloc_id, sb } =>
+                intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb),
             Provenance::Wildcard => {
                 // No need to do anything for wildcard pointers as
                 // their provenances have already been previously exposed.
+                Ok(())
             }
         }
-        Ok(())
     }
 
     /// Convert a pointer with provenance into an allocation-offset pair,

src/shims/backtrace.rs

@@ -123,7 +123,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         let ptr = this.read_pointer(ptr)?;
         // Take apart the pointer, we need its pieces.
-        let (alloc_id, offset, _tag) = this.ptr_get_alloc_id(ptr)?;
+        let (alloc_id, offset, _prov) = this.ptr_get_alloc_id(ptr)?;
 
         let fn_instance =
             if let Some(GlobalAlloc::Function(instance)) = this.tcx.get_global_alloc(alloc_id) {

src/stacked_borrows/mod.rs

@@ -777,20 +777,31 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 return Ok(())
             };
 
-            let extra = this.get_alloc_extra(alloc_id)?;
-            let mut stacked_borrows = extra
-                .stacked_borrows
-                .as_ref()
-                .expect("we should have Stacked Borrows data")
-                .borrow_mut();
-            stacked_borrows.history.log_creation(
-                Some(orig_tag),
-                new_tag,
-                alloc_range(base_offset, size),
-                current_span,
-            );
-            if protect {
-                stacked_borrows.history.log_protector(orig_tag, new_tag, current_span);
+            let (_size, _align, kind) = this.get_alloc_info(alloc_id);
+            match kind {
+                AllocKind::LiveData => {
+                    // This should have alloc_extra data, but `get_alloc_extra` can still fail
+                    // if converting this alloc_id from a global to a local one
+                    // uncovers a non-supported `extern static`.
+                    let extra = this.get_alloc_extra(alloc_id)?;
+                    let mut stacked_borrows = extra
+                        .stacked_borrows
+                        .as_ref()
+                        .expect("we should have Stacked Borrows data")
+                        .borrow_mut();
+                    stacked_borrows.history.log_creation(
+                        Some(orig_tag),
+                        new_tag,
+                        alloc_range(base_offset, size),
+                        current_span,
+                    );
+                    if protect {
+                        stacked_borrows.history.log_protector(orig_tag, new_tag, current_span);
+                    }
+                }
+                AllocKind::Function | AllocKind::Dead => {
+                    // No stacked borrows on these allocations.
+                }
             }
             Ok(())
         };
@@ -1116,7 +1127,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     }
 
     /// Mark the given tag as exposed. It was found on a pointer with the given AllocId.
-    fn expose_tag(&mut self, alloc_id: AllocId, tag: SbTag) {
+    fn expose_tag(&mut self, alloc_id: AllocId, tag: SbTag) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
 
         // Function pointers and dead objects don't have an alloc_extra so we ignore them.
@@ -1125,14 +1136,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let (_size, _align, kind) = this.get_alloc_info(alloc_id);
         match kind {
             AllocKind::LiveData => {
-                // This should have alloc_extra data.
-                let alloc_extra = this.get_alloc_extra(alloc_id).unwrap();
+                // This should have alloc_extra data, but `get_alloc_extra` can still fail
+                // if converting this alloc_id from a global to a local one
+                // uncovers a non-supported `extern static`.
+                let alloc_extra = this.get_alloc_extra(alloc_id)?;
                 trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id:?}");
                 alloc_extra.stacked_borrows.as_ref().unwrap().borrow_mut().exposed_tags.insert(tag);
             }
             AllocKind::Function | AllocKind::Dead => {
                 // No stacked borrows on these allocations.
             }
         }
+        Ok(())
     }
 }

tests/fail/extern_static_wrong_size.rs

@@ -0,0 +1,10 @@
+//@ only-target-linux: we need a specific extern supported on this target
+//@normalize-stderr-test: "[48] bytes" -> "N bytes"
+
+extern "C" {
+    static mut environ: i8;
+}
+
+fn main() {
+    let _val = unsafe { environ }; //~ ERROR: /has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of [48] bytes and alignment of [48] bytes/
+}

tests/fail/extern_static_wrong_size.stderr

@@ -0,0 +1,14 @@
+error: unsupported operation: `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes
+  --> $DIR/extern_static_wrong_size.rs:LL:CC
+   |
+LL |     let _val = unsafe { environ };
+   |                         ^^^^^^^ `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes
+   |
+   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
+   = note: backtrace:
+   = note: inside `main` at $DIR/extern_static_wrong_size.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+