Skip to content

Commit 35af23b

Browse files
committed
Auto merge of #1833 - hyd-dev:82261, r=RalfJung
Filter out items other than non-generic functions and statics in our version of `exported_symbols` [`#[no_mangle]` on a `use` item](https://docs.rs/brotli-decompressor/2.3.1/src/brotli_decompressor/ffi/mod.rs.html#3-5) can make Miri ICE when compiling a dependency (rust-lang/rust#86261): ```rs #[no_mangle] use std::{thread,panic, io, boxed, any, string}; ``` <details> ``` error: internal compiler error: compiler/rustc_middle/src/ty/mod.rs:1650:13: item_name: no name for DefPath { data: [DisambiguatedDefPathData { data: Misc, disambiguator: 14 }], krate: crate0 } thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1007:9 stack backtrace: 0: std::panicking::begin_panic 1: std::panic::panic_any 2: rustc_errors::HandlerInner::bug 3: rustc_errors::Handler::bug 4: rustc_middle::ty::context::tls::with_opt 5: rustc_middle::util::bug::opt_span_bug_fmt 6: rustc_middle::util::bug::bug_fmt 7: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::item_name 8: rustc_symbol_mangling::symbol_name_provider 9: rustc_query_impl::<impl rustc_query_system::query::config::QueryAccessors<rustc_query_impl::plumbing::QueryCtxt> for rustc_query_impl::queries::symbol_name>::compute 10: rustc_query_system::query::plumbing::get_query_impl 11: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::symbol_name 12: rustc_middle::middle::exported_symbols::ExportedSymbol::symbol_name_for_local_instance 13: rustc_codegen_ssa::back::symbol_export::symbol_name_for_instance_in_crate 14: rustc_codegen_ssa::back::linker::exported_symbols 15: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold 16: rustc_codegen_ssa::back::linker::LinkerInfo::new 17: rustc_codegen_ssa::back::write::start_async_codegen 18: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate 19: rustc_interface::passes::QueryContext::enter 20: rustc_interface::queries::Queries::ongoing_codegen 21: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter 22: rustc_span::with_source_map 23: rustc_interface::interface::create_compiler_and_run 24: rustc_span::with_session_globals note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md note: rustc 1.54.0-nightly (a50d72158 2021-06-08) running on x86_64-unknown-linux-gnu note: compiler flags: -C embed-bitcode=no -C debuginfo=1 --crate-type lib note: some of the compiler flags provided by cargo are hidden query stack during panic: #0 [symbol_name] computing the symbol for `{misc#14}` end of query stack ``` </details> This might be because in #1776, we override the `exported_symbols` query, and our version of `exported_symbols` can return a `use` item which don't have a name if the `use` item is tagged with `#[no_mangle]`, and then: - `rustc_codegen_ssa::back::symbol_export::symbol_name_for_instance_in_crate` is called for for every `exported_symbols`: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_codegen_ssa/src/back/linker.rs#L1300-L1304 - it calls `rustc_middle::middle::exported_symbols::ExportedSymbol::symbol_name_for_local_instance`: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L412 - which calls `rustc_symbol_mangling::symbol_name_provider`: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_middle/src/middle/exported_symbols.rs#L37-L44 - which calls `item_name`: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_symbol_mangling/src/lib.rs#L216, which triggers the ICE It might also be problematic for https://github.com/rust-lang/miri/blob/d39f0c64b8b369188a73a655716ab56683a6537b/src/shims/foreign_items.rs#L165 which also uses `item_name`, but Miri cannot compile the dependency, so that code can't be reached. Therefore, this PR makes `exported_symbols` filter out all items that are not functions or statics, so all items returned will have a name, which avoids the ICE (I have tested it in the https://github.com/jorgecarleitao/arrow2 repository). (This PR also includes a commit that fixes a small (unrelated) bug for `#[no_mangle]` on associated functions -- I found that because I notice `#[no_mangle]` is supported on associated functions and they should not be filtered out in `exported_symbols`.) Fixes (when the submodule is bumped) rust-lang/rust#86261.
2 parents 486b5df + da2ed6f commit 35af23b

File tree

8 files changed

+73
-8
lines changed

8 files changed

+73
-8
lines changed

src/bin/miri.rs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use log::debug;
1919

2020
use rustc_driver::Compilation;
2121
use rustc_errors::emitter::{ColorConfig, HumanReadableErrorType};
22-
use rustc_hir::def_id::LOCAL_CRATE;
22+
use rustc_hir::{self as hir, def_id::LOCAL_CRATE, Node};
2323
use rustc_interface::interface::Config;
2424
use rustc_middle::{
2525
middle::exported_symbols::{ExportedSymbol, SymbolExportLevel},
@@ -109,12 +109,27 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
109109
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L62-L63
110110
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L174
111111
tcx.reachable_set(()).iter().filter_map(|&local_def_id| {
112-
tcx.codegen_fn_attrs(local_def_id)
113-
.contains_extern_indicator()
114-
.then_some((
115-
ExportedSymbol::NonGeneric(local_def_id.to_def_id()),
116-
SymbolExportLevel::C,
117-
))
112+
// Do the same filtering that rustc does:
113+
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L84-L102
114+
// Otherwise it may cause unexpected behaviours and ICEs
115+
// (https://github.com/rust-lang/rust/issues/86261).
116+
let is_reachable_non_generic = matches!(
117+
tcx.hir().get(tcx.hir().local_def_id_to_hir_id(local_def_id)),
118+
Node::Item(&hir::Item {
119+
kind: hir::ItemKind::Static(..) | hir::ItemKind::Fn(..),
120+
..
121+
}) | Node::ImplItem(&hir::ImplItem {
122+
kind: hir::ImplItemKind::Fn(..),
123+
..
124+
})
125+
if !tcx.generics_of(local_def_id).requires_monomorphization(tcx)
126+
);
127+
(is_reachable_non_generic
128+
&& tcx.codegen_fn_attrs(local_def_id).contains_extern_indicator())
129+
.then_some((
130+
ExportedSymbol::NonGeneric(local_def_id.to_def_id()),
131+
SymbolExportLevel::C,
132+
))
118133
}),
119134
)
120135
}

src/shims/foreign_items.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
176176
second_crate: tcx.crate_name(cnum),
177177
});
178178
}
179-
if tcx.def_kind(def_id) != DefKind::Fn {
179+
if !matches!(tcx.def_kind(def_id), DefKind::Fn | DefKind::AssocFn) {
180180
throw_ub_format!(
181181
"attempt to call an exported symbol that is not defined as a function"
182182
);

test-cargo-miri/Cargo.lock

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test-cargo-miri/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ issue_1567 = { path = "issue-1567" }
1515
issue_1691 = { path = "issue-1691" }
1616
issue_1705 = { path = "issue-1705" }
1717
issue_1760 = { path = "issue-1760" }
18+
issue_rust_86261 = { path = "issue-rust-86261" }
1819

1920
[dev-dependencies]
2021
rand = { version = "0.8", features = ["small_rng"] }

test-cargo-miri/exported-symbol-dep/src/lib.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,12 @@
22
fn exported_symbol() -> i32 {
33
123456
44
}
5+
6+
pub struct AssocFn;
7+
8+
impl AssocFn {
9+
#[no_mangle]
10+
pub fn assoc_fn_as_exported_symbol() -> i32 {
11+
-123456
12+
}
13+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[package]
2+
name = "issue_rust_86261"
3+
version = "0.1.0"
4+
authors = ["Miri Team"]
5+
edition = "2018"
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#![allow(unused_imports, unused_attributes, no_mangle_generic_items)]
2+
3+
// Regression test for https://github.com/rust-lang/rust/issues/86261:
4+
// `#[no_mangle]` on a `use` item.
5+
#[no_mangle]
6+
use std::{thread,panic, io, boxed, any, string};
7+
8+
// `#[no_mangle]` on a struct has a similar problem.
9+
#[no_mangle]
10+
pub struct NoMangleStruct;
11+
12+
// If `#[no_mangle]` has effect on the `struct` above, calling `NoMangleStruct` will fail with
13+
// "multiple definitions of symbol `NoMangleStruct`" error.
14+
#[export_name = "NoMangleStruct"]
15+
fn no_mangle_struct() {}
16+
17+
// `#[no_mangle]` on a generic function can also cause ICEs.
18+
#[no_mangle]
19+
fn no_mangle_generic<T>() {}
20+
21+
// Same as `no_mangle_struct()` but for the `no_mangle_generic()` generic function.
22+
#[export_name = "no_mangle_generic"]
23+
fn no_mangle_generic2() {}

test-cargo-miri/src/main.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,22 @@ mod test {
6262
fn exported_symbol() {
6363
extern crate cargo_miri_test;
6464
extern crate exported_symbol;
65+
extern crate issue_rust_86261;
6566
// Test calling exported symbols in (transitive) dependencies.
6667
// Repeat calls to make sure the `Instance` cache is not broken.
6768
for _ in 0..3 {
6869
extern "Rust" {
6970
fn exported_symbol() -> i32;
71+
fn assoc_fn_as_exported_symbol() -> i32;
7072
fn make_true() -> bool;
73+
fn NoMangleStruct();
74+
fn no_mangle_generic();
7175
}
7276
assert_eq!(unsafe { exported_symbol() }, 123456);
77+
assert_eq!(unsafe { assoc_fn_as_exported_symbol() }, -123456);
7378
assert!(unsafe { make_true() });
79+
unsafe { NoMangleStruct() }
80+
unsafe { no_mangle_generic() }
7481
}
7582
}
7683
}

0 commit comments

Comments
 (0)