Skip to content

adjust for FnAbi changes #1928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
41c3017c82bbc16842cc3bc1afa904e6910e293c
59337cddd41880f8075b07860a99be4dc402ddb1
2 changes: 1 addition & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
link_name: Symbol,
) -> InterpResult<'tcx, ()> {
self.check_abi(abi, exp_abi)?;
if let Some(body) = self.eval_context_mut().lookup_exported_symbol(link_name)? {
if let Some((body, _)) = self.eval_context_mut().lookup_exported_symbol(link_name)? {
throw_machine_stop!(TerminationInfo::SymbolShimClashing {
link_name,
span: body.span.data(),
Expand Down
2 changes: 1 addition & 1 deletion src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
ecx.find_mir_or_eval_fn(instance, abi, args, ret, unwind)
}

Expand Down
154 changes: 84 additions & 70 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
collections::hash_map::Entry,
convert::{TryFrom, TryInto},
iter,
};
Expand Down Expand Up @@ -34,7 +35,7 @@ pub enum EmulateByNameResult<'mir, 'tcx> {
/// Jumping has already been taken care of.
AlreadyJumped,
/// A MIR body has been found for the function
MirBody(&'mir mir::Body<'tcx>),
MirBody(&'mir mir::Body<'tcx>, ty::Instance<'tcx>),
/// The item is not supported.
NotSupported,
}
Expand Down Expand Up @@ -135,81 +136,91 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
fn lookup_exported_symbol(
&mut self,
link_name: Symbol,
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
let tcx = this.tcx.tcx;

// If the result was cached, just return it.
if let Some(instance) = this.machine.exported_symbols_cache.get(&link_name) {
return instance.map(|instance| this.load_mir(instance.def, None)).transpose();
}

// Find it if it was not cached.
let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum)> = None;
// `dependency_formats` includes all the transitive informations needed to link a crate,
// which is what we need here since we need to dig out `exported_symbols` from all transitive
// dependencies.
let dependency_formats = tcx.dependency_formats(());
let dependency_format = dependency_formats
.iter()
.find(|(crate_type, _)| *crate_type == CrateType::Executable)
.expect("interpreting a non-executable crate");
for cnum in
iter::once(LOCAL_CRATE).chain(dependency_format.1.iter().enumerate().filter_map(
|(num, &linkage)| (linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1)),
))
{
// We can ignore `_export_level` here: we are a Rust crate, and everything is exported
// from a Rust crate.
for &(symbol, _export_level) in tcx.exported_symbols(cnum) {
if let ExportedSymbol::NonGeneric(def_id) = symbol {
let attrs = tcx.codegen_fn_attrs(def_id);
let symbol_name = if let Some(export_name) = attrs.export_name {
export_name
} else if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE) {
tcx.item_name(def_id)
} else {
// Skip over items without an explicitly defined symbol name.
continue;
};
if symbol_name == link_name {
if let Some((original_instance, original_cnum)) = instance_and_crate {
// Make sure we are consistent wrt what is 'first' and 'second'.
let original_span = tcx.def_span(original_instance.def_id()).data();
let span = tcx.def_span(def_id).data();
if original_span < span {
throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions {
link_name,
first: original_span,
first_crate: tcx.crate_name(original_cnum),
second: span,
second_crate: tcx.crate_name(cnum),
});
// (Cannot use `or_insert` since the code below might have to throw an error.)
let entry = this.machine.exported_symbols_cache.entry(link_name);
let instance = *match entry {
Entry::Occupied(e) => e.into_mut(),
Entry::Vacant(e) => {
// Find it if it was not cached.
let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum)> = None;
// `dependency_formats` includes all the transitive informations needed to link a crate,
// which is what we need here since we need to dig out `exported_symbols` from all transitive
// dependencies.
let dependency_formats = tcx.dependency_formats(());
let dependency_format = dependency_formats
.iter()
.find(|(crate_type, _)| *crate_type == CrateType::Executable)
.expect("interpreting a non-executable crate");
for cnum in iter::once(LOCAL_CRATE).chain(
dependency_format.1.iter().enumerate().filter_map(|(num, &linkage)| {
(linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1))
}),
) {
// We can ignore `_export_level` here: we are a Rust crate, and everything is exported
// from a Rust crate.
for &(symbol, _export_level) in tcx.exported_symbols(cnum) {
if let ExportedSymbol::NonGeneric(def_id) = symbol {
let attrs = tcx.codegen_fn_attrs(def_id);
let symbol_name = if let Some(export_name) = attrs.export_name {
export_name
} else if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE) {
tcx.item_name(def_id)
} else {
throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions {
link_name,
first: span,
first_crate: tcx.crate_name(cnum),
second: original_span,
second_crate: tcx.crate_name(original_cnum),
});
// Skip over items without an explicitly defined symbol name.
continue;
};
if symbol_name == link_name {
if let Some((original_instance, original_cnum)) = instance_and_crate
{
// Make sure we are consistent wrt what is 'first' and 'second'.
let original_span =
tcx.def_span(original_instance.def_id()).data();
let span = tcx.def_span(def_id).data();
if original_span < span {
throw_machine_stop!(
TerminationInfo::MultipleSymbolDefinitions {
link_name,
first: original_span,
first_crate: tcx.crate_name(original_cnum),
second: span,
second_crate: tcx.crate_name(cnum),
}
);
} else {
throw_machine_stop!(
TerminationInfo::MultipleSymbolDefinitions {
link_name,
first: span,
first_crate: tcx.crate_name(cnum),
second: original_span,
second_crate: tcx.crate_name(original_cnum),
}
);
}
}
if !matches!(tcx.def_kind(def_id), DefKind::Fn | DefKind::AssocFn) {
throw_ub_format!(
"attempt to call an exported symbol that is not defined as a function"
);
}
instance_and_crate = Some((ty::Instance::mono(tcx, def_id), cnum));
}
}
if !matches!(tcx.def_kind(def_id), DefKind::Fn | DefKind::AssocFn) {
throw_ub_format!(
"attempt to call an exported symbol that is not defined as a function"
);
}
instance_and_crate = Some((ty::Instance::mono(tcx, def_id), cnum));
}
}

e.insert(instance_and_crate.map(|ic| ic.0))
}
};
match instance {
None => Ok(None), // no symbol with this name
Some(instance) => Ok(Some((this.load_mir(instance.def, None)?, instance))),
}

let instance = instance_and_crate.map(|ic| ic.0);
// Cache it and load its MIR, if found.
this.machine.exported_symbols_cache.try_insert(link_name, instance).unwrap();
instance.map(|instance| this.load_mir(instance.def, None)).transpose()
}

/// Emulates calling a foreign item, failing if the item is not supported.
Expand All @@ -225,7 +236,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
let attrs = this.tcx.get_attrs(def_id);
let link_name = this
Expand Down Expand Up @@ -253,7 +264,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some(&*this.load_mir(panic_impl_instance.def, None)?));
return Ok(Some((
&*this.load_mir(panic_impl_instance.def, None)?,
panic_impl_instance,
)));
}
#[rustfmt::skip]
| "exit"
Expand Down Expand Up @@ -297,7 +311,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.go_to_block(ret);
}
EmulateByNameResult::AlreadyJumped => (),
EmulateByNameResult::MirBody(mir) => return Ok(Some(mir)),
EmulateByNameResult::MirBody(mir, instance) => return Ok(Some((mir, instance))),
EmulateByNameResult::NotSupported => {
if let Some(body) = this.lookup_exported_symbol(link_name)? {
return Ok(Some(body));
Expand Down Expand Up @@ -328,11 +342,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

match allocator_kind {
AllocatorKind::Global => {
let body = this
let (body, instance) = this
.lookup_exported_symbol(symbol)?
.expect("symbol should be present if there is a global allocator");

Ok(EmulateByNameResult::MirBody(body))
Ok(EmulateByNameResult::MirBody(body, instance))
}
AllocatorKind::Default => {
default(this)?;
Expand Down
4 changes: 2 additions & 2 deletions src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
trace!("eval_fn_call: {:#?}, {:?}", instance, ret.map(|p| p.0));

Expand All @@ -54,7 +54,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}

// Otherwise, load the MIR.
Ok(Some(&*this.load_mir(instance.def, None)?))
Ok(Some((&*this.load_mir(instance.def, None)?, instance)))
}

/// Returns `true` if the computation was performed, and `false` if we should just evaluate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn main() {

#[cfg(fn_ptr)]
unsafe { std::mem::transmute::<unsafe fn(), unsafe extern "C" fn()>(foo)() }
//[fn_ptr]~^ ERROR calling a function with ABI Rust using caller ABI C
//[fn_ptr]~^ ERROR calling a function with calling convention Rust using calling convention C

// `Instance` caching should not suppress ABI check.
#[cfg(cache)]
Expand All @@ -23,7 +23,7 @@ fn main() {
fn foo();
}
unsafe { foo() }
//[no_cache]~^ ERROR calling a function with ABI Rust using caller ABI C
//[cache]~^^ ERROR calling a function with ABI Rust using caller ABI C
//[no_cache]~^ ERROR calling a function with calling convention Rust using calling convention C
//[cache]~^^ ERROR calling a function with calling convention Rust using calling convention C
}
}
4 changes: 1 addition & 3 deletions tests/compile-fail/panic/bad_unwind.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// error-pattern: calling a function with ABI C-unwind using caller ABI C
// error-pattern: unwinding past a stack frame that does not allow unwinding
#![feature(c_unwind)]

//! Unwinding when the caller ABI is "C" (without "-unwind") is UB.
//! Currently we detect the ABI mismatch; we could probably allow such calls in principle one day
//! but then we have to detect the unexpected unwinding.

extern "C-unwind" fn unwind() {
panic!();
Expand Down