user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #1833 - hyd-dev:82261, r=RalfJung

Browse source 
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:🐛:opt_span_bug_fmt
   6: rustc_middle::util:🐛: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:🔙:symbol_export::symbol_name_for_instance_in_crate
  14: rustc_codegen_ssa:🔙:linker::exported_symbols
  15: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
  16: rustc_codegen_ssa:🔙:linker::LinkerInfo::new
  17: rustc_codegen_ssa:🔙: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:🔙:symbol_export::symbol_name_for_instance_in_crate` is called for for every `exported_symbols`: fb3ea63d9b/compiler/rustc_codegen_ssa/src/back/linker.rs (L1300-L1304)
- it calls `rustc_middle::middle::exported_symbols::ExportedSymbol::symbol_name_for_local_instance`: fb3ea63d9b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs (L412)
- which calls `rustc_symbol_mangling::symbol_name_provider`: fb3ea63d9b/compiler/rustc_middle/src/middle/exported_symbols.rs (L37-L44)
- which calls `item_name`: fb3ea63d9b/compiler/rustc_symbol_mangling/src/lib.rs (L216), which triggers the ICE

It might also be problematic for d39f0c64b8/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.
This commit is contained in:
bors 2021-06-15 10:07:24 +00:00
commit 35af23b6a9
8 changed files with 73 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

29
src/bin/miri.rs
View file

@ -19,7 +19,7 @@ use log::debug;
use rustc_driver::Compilation;
use rustc_errors::emitter::{ColorConfig, HumanReadableErrorType};
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_hir::{self as hir, def_id::LOCAL_CRATE, Node};
use rustc_interface::interface::Config;
use rustc_middle::{
middle::exported_symbols::{ExportedSymbol, SymbolExportLevel},
@ -109,12 +109,27 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L62-L63
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L174
tcx.reachable_set(()).iter().filter_map(|&local_def_id| {
tcx.codegen_fn_attrs(local_def_id)
.contains_extern_indicator()
.then_some((
ExportedSymbol::NonGeneric(local_def_id.to_def_id()),
SymbolExportLevel::C,
))
// Do the same filtering that rustc does:
// https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L84-L102
// Otherwise it may cause unexpected behaviours and ICEs
// (https://github.com/rust-lang/rust/issues/86261).
let is_reachable_non_generic = matches!(
tcx.hir().get(tcx.hir().local_def_id_to_hir_id(local_def_id)),
Node::Item(&hir::Item {
kind: hir::ItemKind::Static(..) | hir::ItemKind::Fn(..),
..
}) | Node::ImplItem(&hir::ImplItem {
kind: hir::ImplItemKind::Fn(..),
..
})
if !tcx.generics_of(local_def_id).requires_monomorphization(tcx)
);
(is_reachable_non_generic
&& tcx.codegen_fn_attrs(local_def_id).contains_extern_indicator())
.then_some((
ExportedSymbol::NonGeneric(local_def_id.to_def_id()),
SymbolExportLevel::C,
))
}),
)
}

2
src/shims/foreign_items.rs
View file

@ -176,7 +176,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
second_crate: tcx.crate_name(cnum),
});
}
if tcx.def_kind(def_id) != DefKind::Fn {
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"
);

5
test-cargo-miri/Cargo.lock generated
View file

@ -21,6 +21,7 @@ dependencies = [
"issue_1691",
"issue_1705",
"issue_1760",
"issue_rust_86261",
"rand",
"serde_derive",
]
@ -102,6 +103,10 @@ dependencies = [
name = "issue_1760"
version = "0.1.0"
[[package]]
name = "issue_rust_86261"
version = "0.1.0"
[[package]]
name = "libc"
version = "0.2.92"

1
test-cargo-miri/Cargo.toml
View file

@ -15,6 +15,7 @@ issue_1567 = { path = "issue-1567" }
issue_1691 = { path = "issue-1691" }
issue_1705 = { path = "issue-1705" }
issue_1760 = { path = "issue-1760" }
issue_rust_86261 = { path = "issue-rust-86261" }
[dev-dependencies]
rand = { version = "0.8", features = ["small_rng"] }

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

@ -2,3 +2,12 @@
fn exported_symbol() -> i32 {
123456
}
pub struct AssocFn;
impl AssocFn {
#[no_mangle]
pub fn assoc_fn_as_exported_symbol() -> i32 {
-123456
}
}

5
test-cargo-miri/issue-rust-86261/Cargo.toml Normal file
View file

@ -0,0 +1,5 @@
[package]
name = "issue_rust_86261"
version = "0.1.0"
authors = ["Miri Team"]
edition = "2018"

23
test-cargo-miri/issue-rust-86261/src/lib.rs Normal file
View file

@ -0,0 +1,23 @@
#![allow(unused_imports, unused_attributes, no_mangle_generic_items)]
// Regression test for https://github.com/rust-lang/rust/issues/86261:
// `#[no_mangle]` on a `use` item.
#[no_mangle]
use std::{thread,panic, io, boxed, any, string};
// `#[no_mangle]` on a struct has a similar problem.
#[no_mangle]
pub struct NoMangleStruct;
// If `#[no_mangle]` has effect on the `struct` above, calling `NoMangleStruct` will fail with
// "multiple definitions of symbol `NoMangleStruct`" error.
#[export_name = "NoMangleStruct"]
fn no_mangle_struct() {}
// `#[no_mangle]` on a generic function can also cause ICEs.
#[no_mangle]
fn no_mangle_generic<T>() {}
// Same as `no_mangle_struct()` but for the `no_mangle_generic()` generic function.
#[export_name = "no_mangle_generic"]
fn no_mangle_generic2() {}

7
test-cargo-miri/src/main.rs
View file

@ -62,15 +62,22 @@ mod test {
fn exported_symbol() {
extern crate cargo_miri_test;
extern crate exported_symbol;
extern crate issue_rust_86261;
// Test calling exported symbols in (transitive) dependencies.
// Repeat calls to make sure the `Instance` cache is not broken.
for _ in 0..3 {
extern "Rust" {
fn exported_symbol() -> i32;
fn assoc_fn_as_exported_symbol() -> i32;
fn make_true() -> bool;
fn NoMangleStruct();
fn no_mangle_generic();
}
assert_eq!(unsafe { exported_symbol() }, 123456);
assert_eq!(unsafe { assoc_fn_as_exported_symbol() }, -123456);
assert!(unsafe { make_true() });
unsafe { NoMangleStruct() }
unsafe { no_mangle_generic() }
}
}
}