Skip to content

Commit

Permalink
Auto merge of #1832 - hyd-dev:1776-follow-up, r=RalfJung
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Jun 15, 2021
2 parents 042db78 + aaaa142 commit 486b5df
Show file tree
Hide file tree
Showing 13 changed files with 350 additions and 371 deletions.
13 changes: 12 additions & 1 deletion src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ pub enum TerminationInfo {
second: SpanData,
second_crate: Symbol,
},
SymbolShimClashing {
link_name: Symbol,
span: SpanData,
},
}

impl fmt::Display for TerminationInfo {
Expand All @@ -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
),
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions src/shims/backtrace.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 486b5df

Please sign in to comment.