Auto merge of #1832 - hyd-dev:1776-follow-up, r=RalfJung

Report an error if a `#[no_mangle]`/`#[export_name = ...]` function has the same symbol name as a built-in shim

Implements #1776 (comment).

The error looks like this:
```
error: found `malloc` symbol definition that clashes with a built-in shim
  --> tests/compile-fail/function_calls/exported_symbol_shim_clashing.rs:12:9
   |
12 |         malloc(0);
   |         ^^^^^^^^^ found `malloc` symbol definition that clashes with a built-in shim
   |
help: the `malloc` symbol is defined here

  --> tests/compile-fail/function_calls/exported_symbol_shim_clashing.rs:2:1
   |
2  | / extern "C" fn malloc(_: usize) -> *mut std::ffi::c_void {
3  | |     //~^ HELP the `malloc` symbol is defined here
4  | |     unreachable!()
5  | | }
   | |_^
   = note: inside `main` at tests/compile-fail/function_calls/exported_symbol_shim_clashing.rs:12:9
```

This does not implement "better error messages than we do currently for arg/ABI mismatches" in #1776 (comment) -- I failed to remove all `check_arg_count()` and `check_abi()` (they are still used in `src/shims/intrinsics.rs` and `call_dlsym()`) and they don't receive the name of the shim.

Author: bors

src/diagnostics.rs

@@ -26,6 +26,10 @@ pub enum TerminationInfo {
         second: SpanData,
         second_crate: Symbol,
     },
+    SymbolShimClashing {
+        link_name: Symbol,
+        span: SpanData,
+    },
 }
 
 impl fmt::Display for TerminationInfo {
@@ -39,6 +43,11 @@ impl fmt::Display for TerminationInfo {
             Deadlock => write!(f, "the evaluated program deadlocked"),
             MultipleSymbolDefinitions { link_name, .. } =>
                 write!(f, "multiple definitions of symbol `{}`", link_name),
+            SymbolShimClashing { link_name, .. } => write!(
+                f,
+                "found `{}` symbol definition that clashes with a built-in shim",
+                link_name
+            ),
         }
     }
 }
@@ -79,7 +88,7 @@ pub fn report_error<'tcx, 'mir>(
                 UnsupportedInIsolation(_) => Some("unsupported operation"),
                 ExperimentalUb { .. } => Some("Undefined Behavior"),
                 Deadlock => Some("deadlock"),
-                MultipleSymbolDefinitions { .. } => None,
+                MultipleSymbolDefinitions { .. } | SymbolShimClashing { .. } => None,
             };
             #[rustfmt::skip]
             let helps = match info {
@@ -98,6 +107,8 @@ pub fn report_error<'tcx, 'mir>(
                         (Some(*first), format!("it's first defined here, in crate `{}`", first_crate)),
                         (Some(*second), format!("then it's defined here again, in crate `{}`", second_crate)),
                     ],
+                SymbolShimClashing { link_name, span } =>
+                    vec![(Some(*span), format!("the `{}` symbol is defined here", link_name))],
                 _ => vec![],
             };
             (title, helps)

src/helpers.rs

@@ -8,6 +8,7 @@ use log::trace;
 use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_middle::mir;
 use rustc_middle::ty::{self, layout::TyAndLayout, List, TyCtxt};
+use rustc_span::Symbol;
 use rustc_target::abi::{Align, FieldsShape, LayoutOf, Size, Variants};
 use rustc_target::spec::abi::Abi;
 
@@ -677,6 +678,36 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("{}", error_msg.as_ref());
         }
     }
+
+    fn check_abi_and_shim_symbol_clash(
+        &mut self,
+        abi: Abi,
+        exp_abi: Abi,
+        link_name: Symbol,
+    ) -> InterpResult<'tcx, ()> {
+        self.check_abi(abi, exp_abi)?;
+        if let Some(body) = self.eval_context_mut().lookup_exported_symbol(link_name)? {
+            throw_machine_stop!(TerminationInfo::SymbolShimClashing {
+                link_name,
+                span: body.span.data(),
+            })
+        }
+        Ok(())
+    }
+
+    fn check_shim<'a, const N: usize>(
+        &mut self,
+        abi: Abi,
+        exp_abi: Abi,
+        link_name: Symbol,
+        args: &'a [OpTy<'tcx, Tag>],
+    ) -> InterpResult<'tcx, &'a [OpTy<'tcx, Tag>; N]>
+    where
+        &'a [OpTy<'tcx, Tag>; N]: TryFrom<&'a [OpTy<'tcx, Tag>]>,
+    {
+        self.check_abi_and_shim_symbol_clash(abi, exp_abi, link_name)?;
+        check_arg_count(args)
+    }
 }
 
 /// Check that the number of args is what we expect.

src/machine.rs

@@ -297,7 +297,8 @@ pub struct Evaluator<'mir, 'tcx> {
     string_cache: FxHashMap<String, measureme::StringId>,
 
     /// Cache of `Instance` exported under the given `Symbol` name.
-    pub(crate) exported_symbols_cache: FxHashMap<Symbol, Instance<'tcx>>,
+    /// `None` means no `Instance` exported under the given name is found.
+    pub(crate) exported_symbols_cache: FxHashMap<Symbol, Option<Instance<'tcx>>>,
 
     /// Whether to raise a panic in the context of the evaluated process when unsupported
     /// functionality is encountered. If `false`, an error is propagated in the Miri application context

src/shims/backtrace.rs

@@ -1,22 +1,23 @@
 use crate::rustc_target::abi::LayoutOf as _;
 use crate::*;
-use helpers::check_arg_count;
 use rustc_ast::ast::Mutability;
 use rustc_middle::ty::{self, TypeAndMut};
-use rustc_span::BytePos;
-use rustc_target::abi::Size;
+use rustc_span::{BytePos, Symbol};
+use rustc_target::{abi::Size, spec::abi::Abi};
 use std::convert::TryInto as _;
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn handle_miri_get_backtrace(
         &mut self,
+        abi: Abi,
+        link_name: Symbol,
         args: &[OpTy<'tcx, Tag>],
         dest: &PlaceTy<'tcx, Tag>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         let tcx = this.tcx;
-        let &[ref flags] = check_arg_count(args)?;
+        let &[ref flags] = this.check_shim(abi, Abi::Rust, link_name, args)?;
 
         let flags = this.read_scalar(flags)?.to_u64()?;
         if flags != 0 {
@@ -71,12 +72,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     fn handle_miri_resolve_frame(
         &mut self,
+        abi: Abi,
+        link_name: Symbol,
         args: &[OpTy<'tcx, Tag>],
         dest: &PlaceTy<'tcx, Tag>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         let tcx = this.tcx;
-        let &[ref ptr, ref flags] = check_arg_count(args)?;
+        let &[ref ptr, ref flags] = this.check_shim(abi, Abi::Rust, link_name, args)?;
 
         let flags = this.read_scalar(flags)?.to_u64()?;
         if flags != 0 {