bors [Tue, 15 Jun 2021 10:07:24 +0000 (10:07 +0000)]
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: 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.
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 https://github.com/rust-lang/miri/pull/1776#issuecomment-821343175 -- 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.
bors [Fri, 11 Jun 2021 07:32:23 +0000 (07:32 +0000)]
Auto merge of #1831 - hyd-dev:emulate-by-name-result, r=RalfJung
Fix the wrong `EmulateByNameResult::NotSupported` in `syscall` shim
Without the change, the newly added test will fail with:
```diff
-thread 'main' panicked at 'unsupported Miri functionality: can't execute syscall with ID 0', $DIR/unsupported_syscall.rs:10:9
+thread 'main' panicked at 'unsupported Miri functionality: can't call foreign function: syscall', $DIR/unsupported_syscall.rs:10:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
cc https://github.com/rust-lang/miri/pull/1818#discussion_r648868937
bors [Wed, 9 Jun 2021 18:09:06 +0000 (18:09 +0000)]
Auto merge of #1818 - landaire:feature/panic-on-unsupported-syscalls, r=RalfJung
Add support for panicking in the emulated application when unsupported functionality is encountered
This PR fixes #1807 and allows an optional flag to be specified to panic when an unsupported syscall is encountered. In essence, instead of bubbling up an error in the context of the Miri application Miri will panic within the context of the *emulated* application. This feature is desired to allow CI pipelines to determine if a Miri failure is unsupported functionality or actual UB. Please read [this comment](https://github.com/rust-lang/miri/issues/1807#issuecomment-845425076) for the rationale behind this change.
Note: this change does not cover all cases where unsupported functionality errors may be raised. If you search the repo for `throw_unsup_format!` there are many cases that I think are less likely to occur and may still be problematic for some folks.
TODO:
- [x] README documentation on this new flag
- [x] Add tests
bors [Wed, 9 Jun 2021 15:30:50 +0000 (15:30 +0000)]
Auto merge of #1797 - atsmtat:env-isolation, r=RalfJung
fix: avoid stopping machine upon running env operations in isolation
get and set current dir operations used to halt the machine by
throwing an exception in isolation mode. This change updates them to
return a dummy `NotFound` error instead, and keep the machine running.
I started with a custom error using `ErrorKind::Other`, but since it
can't be mapped to a raw OS error, I dropped it. `NotFound` kind of make
sense for get operations, but not much for set operations. But that's
the only error supported for windows currently.
Smit Soni [Fri, 14 May 2021 06:40:07 +0000 (23:40 -0700)]
Add a support to execute isolated op without halting
In user interface, added a new flag `-Zmiri-isolation-error` which
takes one of the four values -- hide, warn, warn-nobacktrace, and
abort. This option can be used to configure Miri to either abort or
return an error code upon executing isolated op. If not aborted, Miri
prints a warning, whose verbosity can be configured using this flag.
In implementation, added a new enum `IsolatedOp` to capture all the
settings related to ops requiring communication with the
host. Old `communicate` flag in both miri configs and machine
stats is replaced with a new helper function `communicate()` which
checks `isolated_op` internally.
Added a new helper function `reject_in_isolation` which can be called
by shims to reject ops according to the reject_with settings. Use miri
specific diagnostics function `report_msg` to print backtrace in the
warning. Update it to take an enum value instead of a bool, indicating
the level of diagnostics.
Updated shims related to current dir to use the new APIs. Added a new
test for current dir ops in isolation without halting machine.
bors [Mon, 7 Jun 2021 09:50:44 +0000 (09:50 +0000)]
Auto merge of #1826 - hyd-dev:rustup, r=RalfJung
Update `cargo-miri` tests
The test output has been changed recently (likely by rust-lang/rust#84863) and caused a test failure: https://github.com/rust-lang/miri/runs/2761591139#step:8:1228
bors [Sun, 6 Jun 2021 08:34:12 +0000 (08:34 +0000)]
Auto merge of #1823 - hyd-dev:extern-crate-std, r=RalfJung
Detect `std` by checking if the crate defines `#[lang = "start"]` rather than string comparison
I also considered to compare the crate name with `sym::std`, but it's easy to name any crate `std` by using `--crate-name std`, so I don't think that is robust enough.
Note that this only checks the crate, it does not check whether the call is in `sys::unix` or `sys::windows`, unlike the previous implementation, but I think it's already robust enough.
bors [Thu, 3 Jun 2021 15:42:41 +0000 (15:42 +0000)]
Auto merge of #1824 - Pointerbender:strings-test, r=RalfJung
added a strings.rs regression test case for potential future UB
This PR adds a regression test for the aliasing rules of a `Unique<T>` pointer.
At the time of writing this test case, Miri does not treat `Unique<T>`
pointers as a special case, these are treated like any other raw pointer.
However, there are existing Github issues which may lead to `Unique<T>`
becoming a special case through asserting unique ownership over the pointee:
- https://github.com/rust-lang/unsafe-code-guidelines/issues/258
- https://github.com/rust-lang/unsafe-code-guidelines/issues/262
In the new test case, the calls to `String::remove` and `String::insert[_str]` follow
code paths that would trigger undefined behavior in case `Unique<T>`
would ever assert semantic ownership over the pointee. Internally,
these methods call `self.vec.as_ptr()` and `self.vec.as_mut_ptr()` on
the vector of bytes that are backing the `String`. That `Vec<u8>` holds a
`Unique<u8>` internally. The second call to `Vec::as_mut_ptr(&mut self)`
would then invalidate the pointers derived from `Vec::as_ptr(&self)`.
Note that as long as `Unique<T>` is treated like any other raw pointer,
this test case should pass. It is merely here as a canary test for
potential future undefined behavior.
bors [Thu, 3 Jun 2021 08:52:57 +0000 (08:52 +0000)]
Auto merge of #1816 - Pointerbender:slices-tests, r=RalfJung
regression tests for pointer invalidation in core library slice methods
A fix for a pointer invalidation bug in `<[T]>::copy_within` has [landed](https://github.com/rust-lang/rust/pull/85610) on the Rust master branch. This PR updates the `rust-version` file to the latest master commit hash and adds extra tests to the Miri test suite to ensure that regressions of this type of bug can be detected for various slice methods with the `-Zmiri-track-raw-pointers` flag.
I took the liberty of adding 2 extra `#![feature]` attributes at the top of `slices.rs`, since there already was one unstable feature. I hope this is okay :smile:
One thing I noticed when running the entire Miri test suite with `MIRIFLAGS="-Zmiri-track-raw-pointers" ./miri test` is that there are currently failing tests on the master branch:
test result: FAILED. 199 passed; 6 failed; 1 ignored; 0 measured; 0 filtered out; finished in 12.95s
```
These failures were not fixed in this PR and already existed prior to this PR. I haven't investigated these yet, but am interested in helping out if possible!
bors [Sun, 30 May 2021 15:14:23 +0000 (15:14 +0000)]
Auto merge of #1791 - Aaron1011:measureme, r=RalfJung
Add `measureme` integration for profiling the interpreted program
This PR uses the `measureme` crate to profile the call stack of the
program being interpreted by Miri. This is accomplished by starting a
measureme 'event' when we enter a function call, and ending the event
when we exit the call. The `measureme` tooling can be used to produce a
call stack from the generated profile data.
Limitations:
* We currently record every single entry/exit. This might generate very
large profile outputs for programs with a large number of function
calls. In follow-up work, we might want to explore sampling (e.g. only
recording every N function calls).
* This does not integrate very well with Miri's concurrency support.
Each event we record starts when we push a frame, and ends when we pop
a frame. As a result, the timing recorded for a particular frame will include all of the work Miri does before that frame completes, including executing another thread.
The `measureme` integration is off by default, and must be enabled via
`-Zmiri-measureme=<output_name>`
Aaron Hill [Sat, 8 May 2021 16:20:51 +0000 (12:20 -0400)]
Add `measureme` integration for profiling the interpreted program
This PR uses the `measureme` crate to profile the call stack of the
program being interpreted by Miri. This is accomplished by starting a
measureme 'event' when we enter a function call, and ending the event
when we exit the call. The `measureme` tooling can be used to produce a
call stack from the generated profile data.
Limitations:
* We currently record every single entry/exit. This might generate very
large profile outputs for programs with a large number of function
calls. In follow-up work, we might want to explore sampling (e.g. only
recording every N function calls).
* This does not integrate very well with Miri's concurrency support.
Each event we record starts when we push a frame, and ends when we pop
a frame. As a result, switching between virtual threads will cause
events from different threads to be interleaved. Additionally, the
recorded for a particular frame will include all of the work Miri does
before that frame completes, including executing another thread.
The `measureme` integration is off by default, and must be enabled via
`-Zmiri-measureme=<output_name>`
bors [Sat, 22 May 2021 12:56:51 +0000 (12:56 +0000)]
Auto merge of #1811 - RalfJung:less-rc, r=RalfJung
get rid of some `Rc`
Now that the memory access hooks get references to `MemoryExtra`, we can avoid refcounting for the global state of Stacked Borrows and the data race detector.