Filter out items other than non-generic functions and statics in our version of `exported_symbols` #1833

ghost · 2021-06-13T17:33:34Z

#[no_mangle] on a use item can make Miri ICE when compiling a dependency (rust-lang/rust#86261):

#[no_mangle]
use std::{thread,panic, io, boxed, any, string};

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

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

miri/src/shims/foreign_items.rs

Line 165 in d39f0c6

tcx.item_name(def_id)

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.

RalfJung · 2021-06-13T18:52:19Z

#[no_mangle] on a use item

Does this even make any sense? Looks like something rustc should reject...

ghost · 2021-06-14T06:26:17Z

Does this even make any sense? Looks like something rustc should reject...

Probably doesn't. rustc emits unused_attributes for that: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_passes/src/check_attr.rs#L1153-L1154

That crate also puts #[no_mangle] on struct, enum and type... They have names so they can't cause this ICE, but I just found that they can cause another problem (which this PR also fixes):

// In a dependency.
#[no_mangle]
#[allow(unused_attributes)]
pub struct Foo;

error: Undefined Behavior: attempt to call an exported symbol that is not defined as a function
 --> src/main.rs:7:14
  |
7 |     unsafe { Foo() }
  |              ^^^^^ attempt to call an exported symbol that is not defined as a function

I believe the error should be error: unsupported operation: can't call foreign function: Foo instead, because I got the undefined reference to `Foo' linker error on cargo run.

I experimented with it further and found an ICE that this PR ~~doesn't fix~~ (EDIT: fixed!) when compiling a dependency:

// In a dependency.
#[no_mangle]
#[allow(no_mangle_generic_items)]
fn foo<T>() {}

error: internal compiler error: compiler/rustc_middle/src/ty/instance.rs:298:17: Instance::mono: DefId(0:3 ~ miri_playground[b134]::foo) has type parameters

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::subst::<impl rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg>>::fill_item
   8: rustc_middle::ty::subst::<impl rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg>>::for_item
   9: rustc_middle::ty::instance::Instance::mono
  10: rustc_middle::middle::exported_symbols::ExportedSymbol::symbol_name_for_local_instance
  11: rustc_codegen_ssa::back::symbol_export::symbol_name_for_instance_in_crate
  12: rustc_codegen_ssa::back::linker::exported_symbols
  13: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
  14: rustc_codegen_ssa::back::linker::LinkerInfo::new
  15: rustc_codegen_ssa::back::write::start_async_codegen
  16: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate
  17: rustc_interface::passes::QueryContext::enter
  18: rustc_interface::queries::Queries::ongoing_codegen
  19: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  20: rustc_span::with_source_map
  21: rustc_interface::interface::create_compiler_and_run
  22: 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:
end of query stack

I believe it should also cause error: unsupported operation: can't call foreign function: foo when interpreting the program, since cargo run fails with undefined reference to `foo' linker error.

RalfJung · 2021-06-14T07:21:56Z

If we start ignoring some no_mangle items (instead of relying on rustc to reject them), I'd like to be sure that we do this consistently with rustc itself.
Can you figure out how rustc ignores use and struct no_mangle when applied to non-function/static items?

bjorn3 · 2021-06-14T07:23:35Z

Can you figure out how rustc ignores use and struct no_mangle when applied to non-function/static items?

It probably simple never looks up the #[no_mangle] attribute due to them not being codegenable items.

ghost · 2021-06-14T07:26:51Z

Can you figure out how rustc ignores use and struct no_mangle when applied to non-function/static items?

It probably simple never looks up the #[no_mangle] attribute due to them not being codegenable items.

I think the corresponding source code is https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L84-L102.

…symbols`

ghost · 2021-06-14T13:17:24Z

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

+    #[no_mangle]
+    pub fn assoc_fn_as_exported_symbol() -> i32 {


(This is testing #[no_mangle] on Node::ImplItem(&hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }), which is included in exported_symbols (but previously broken in Miri: da2ed6f -- I fix that together in this PR in order to test this).)

I can't test this in tests/run-pass, because #[no_mangle] only seems to work on associated functions that are public in a library crate...
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=f1244afcdd26e2a28445f6e82ca46b50

If the associated function is used directly, it can works with rustc when compiling as a binrary, but it still doesn't work in Miri:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=46fc298b46ff44093c45cab9fddb6650

So you are saying even with this patch, https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=46fc298b46ff44093c45cab9fddb6650 fails? Could you open an issue for that?

RalfJung · 2021-06-15T09:59:15Z

This looks reasonable, thanks. :)
@bors r+

bors · 2021-06-15T09:59:17Z

📌 Commit da2ed6f has been approved by RalfJung

bors · 2021-06-15T10:07:28Z

⌛ Testing commit da2ed6f with merge 35af23b...

bors · 2021-06-15T10:24:38Z

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 35af23b to master...

ghost changed the title ~~Filter out items other than functions and statics in our version of exported_symbols~~ Filter out items other than non-generic functions and statics in our version of exported_symbols Jun 14, 2021

hyd-dev added 2 commits June 14, 2021 20:55

Do not return DefId that doesn't have exported symbol in `exported_…

9946734

…symbols`

Don't report UB for #[no_mangle] on associated functions

da2ed6f

ghost commented Jun 14, 2021

View reviewed changes

ghost mentioned this pull request Jun 15, 2021

Update Miri rust-lang/rust#86323

Merged

bors merged commit 35af23b into rust-lang:master Jun 15, 2021

ghost mentioned this pull request Jun 15, 2021

#[no_mangle] on associated functions #1837

Closed

ghost deleted the 82261 branch June 15, 2021 10:27

ghost mentioned this pull request Jun 20, 2021

Associated functions that contain extern indicator or have #[rustc_std_internal_symbol] are reachable rust-lang/rust#86492

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter out items other than non-generic functions and statics in our version of `exported_symbols` #1833

Filter out items other than non-generic functions and statics in our version of `exported_symbols` #1833

ghost commented Jun 13, 2021 •

edited by ghost

Loading

RalfJung commented Jun 13, 2021

ghost commented Jun 14, 2021 •

edited by ghost

Loading

RalfJung commented Jun 14, 2021

bjorn3 commented Jun 14, 2021

ghost commented Jun 14, 2021

ghost Jun 14, 2021 •

edited by ghost

Loading

RalfJung Jun 15, 2021

RalfJung commented Jun 15, 2021

bors commented Jun 15, 2021

bors commented Jun 15, 2021

bors commented Jun 15, 2021

Filter out items other than non-generic functions and statics in our version of exported_symbols #1833

Filter out items other than non-generic functions and statics in our version of exported_symbols #1833

Conversation

ghost commented Jun 13, 2021 • edited by ghost Loading

RalfJung commented Jun 13, 2021

ghost commented Jun 14, 2021 • edited by ghost Loading

RalfJung commented Jun 14, 2021

bjorn3 commented Jun 14, 2021

ghost commented Jun 14, 2021

ghost Jun 14, 2021 • edited by ghost Loading

Choose a reason for hiding this comment

RalfJung Jun 15, 2021

Choose a reason for hiding this comment

RalfJung commented Jun 15, 2021

bors commented Jun 15, 2021

bors commented Jun 15, 2021

bors commented Jun 15, 2021

Filter out items other than non-generic functions and statics in our version of `exported_symbols` #1833

Filter out items other than non-generic functions and statics in our version of `exported_symbols` #1833

ghost commented Jun 13, 2021 •

edited by ghost

Loading

ghost commented Jun 14, 2021 •

edited by ghost

Loading

ghost Jun 14, 2021 •

edited by ghost

Loading