Tools which rely on DWARF for generating code coverage report, don't generate accurate numbers on test builds. For instance, [this sample main](757bdbf388/src/main.rs) returns [100% coverage](https://coveralls.io/builds/4940156/source?filename=main.rs) when [kcov](https://github.com/SimonKagstrom/kcov/) runs.
With @pnkfelix 's great help, we could narrow down the issue: The linker strips unused function during phase 6. Here's a patch which stops stripping when someone calls `rustc --test $ARGS`. @pnkfelix wasn't sure if we should add a new flag, or just use --test. What do you think @alexcrichton ?
Also, I'm not too sure: where is the best place to add a test for this addition?
Thanks for the help!
LLVM's memory dependence analysis doesn't properly account for calls
that could unwind and thus effectively act as a branching point. This
can lead to stores that are only visible when the call unwinds being
removed, possibly leading to calls to drop() functions with b0rked
memory contents.
As there is no fix for this in LLVM yet and we want to keep
compatibility to current LLVM versions anyways, we have to workaround
this bug by omitting the noalias attribute on &mut function arguments.
Benchmarks suggest that the performance loss by this change is very
small.
Thanks to @RalfJung for pushing me towards not removing too many
noalias annotations and @alexcrichton for helping out with the test for
this bug.
Fixes#29485
Turning gc-sections off improves code coverage based for tools which
use DWARF debugging information (like kcov). Otherwise dead code is
stripped and kcov returns a coverage percentage that doesn't reflect
reality.
This PR adds to `NameBinding` so it can more fully represent bindings from imports as well from items, refactors away `Target`, generalizes `ImportResolution` to a simpler type `NameResolution`, and uses a single `NameResolution`-valued map in place the existing maps `children` and `import_resolutions` (of `NameBinding`s and `ImportResolution`s, respectively), simplifying duplicate checking and name resolution.
It also unifies the `resolve_name_in_module` in `lib.rs` with its namesake in `resolve_imports.rs`, clarifying and improving the core logic (fixes#31403 and fixes#31404) while maintaining clear future-comparability with shadowable globs (i.e., never reporting that a resolution is a `Success` or is `Failing` unless this would also be knowable with shadowable globs).
Since it fixes#31403, this is technically a [breaking-change], but it is exceedingly unlikely to cause breakage in practice. The following is an example of code that would break:
```rust
mod foo {
pub mod bar {} // This defines bar in the type namespace
pub use alpha::bar; // This defines bar in the value namespace
// This should define baz in both namespaces, but it only defines baz in the type namespace.
pub use self::bar as baz;
pub fn baz() {} // This should collide with baz, but now it does not.
}
pub fn f() {}
mod alpha {
pub use self::f as bar; // Changing this to `pub fn bar() {}` causes the collision right now.
pub use super::*;
}
```
r? @nrc
These commits are an implementation of https://github.com/rust-lang/rfcs/pull/1359 which is tracked via https://github.com/rust-lang/rust/issues/31398. The `before_exec` implementation fit easily with the current process spawning framework we have, but unfortunately the `exec` implementation required a bit of a larger refactoring. The stdio handles were all largely managed as implementation details of `std::process` and the `exec` function lived in `std::sys`, so the two didn't have access to one another.
I took this as a sign that a deeper refactoring was necessary, and I personally feel that the end result is cleaner for both Windows and Unix. The commits should be separated nicely for reviewing (or all at once if you're feeling ambitious), but the changes made here were:
* The process spawning on Unix was refactored in to a pre-exec and post-exec function. The post-exec function isn't allowed to do any allocations of any form, and management of transmitting errors back to the parent is managed by the pre-exec function (as it's the one that actually forks).
* Some management of the exit status was pushed into platform-specific modules. On Unix we must cache the return value of `wait` as the pid is consumed after we wait on it, but on Windows we can just keep querying the system because the handle stays valid.
* The `Stdio::None` variant was renamed to `Stdio::Null` to better reflect what it's doing.
* The global lock on `CreateProcess` is now correctly positioned to avoid unintended inheritance of pipe handles that other threads are sending to their child processes. After a more careful reading of the article referenced the race is not in `CreateProcess` itself, but rather the property that handles are unintentionally shared.
* All stdio management now happens in platform-specific modules. This provides a cleaner implementation/interpretation for `FromFraw{Fd,Handle}` for each platform as well as a cleaner transition from a configuration to what-to-do once we actually need to do the spawn.
With these refactorings in place, implementing `before_exec` and `exec` ended up both being pretty trivial! (each in their own commit)
LLVM's memory dependence analysis doesn't properly account for calls
that could unwind and thus effectively act as a branching point. This
can lead to stores that are only visible when the call unwinds being
removed, possibly leading to calls to drop() functions with b0rked
memory contents.
As there is no fix for this in LLVM yet and we want to keep
compatibility to current LLVM versions anyways, we have to workaround
this bug by omitting the noalias attribute on &mut function arguments.
Benchmarks suggest that the performance loss by this change is very
small.
Thanks to @RalfJung for pushing me towards not removing too many
noalias annotations and @alexcrichton for helping out with the test for
this bug.
Fixes#29485
Previously when breaking tokens into smaller pieces, the replace_token
function have been used. It replaced current token and updated span
information, but it did not clear the list of expected tokens, neither
did it update remaining info about last token. This could lead to
incorrect error message, like one described in the issue #24780:
expected one of ... `>` ... found `>`
This commit implements the `exec` function proposed in [RFC 1359][rfc] which is
a function on the `CommandExt` trait to execute all parts of a `Command::spawn`
without the `fork` on Unix. More details on the function itself can be found in
the comments in the commit.
[rfc]: https://github.com/rust-lang/rfcs/pull/1359
cc #31398
This is a Unix-specific function which adds the ability to register a closure to
run pre-exec to configure the child process as required (note that these
closures are run post-fork).
cc #31398
A spec like `#[cfg(foo(bar))]` is not allowed as an attribute. This
makes the same spec be rejected by the compiler if passed in as a
`--cfg` argument.
Fixes#31495
This commit is an implementation of the new compiler flags required by [RFC
1361][rfc]. This specifically adds a new `cfg` option to the `--print` flag to
the compiler. This new directive will print the defined `#[cfg]` directives by
the compiler for the target in question.
[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1361-cargo-cfg-dependencies.md
A spec like `#[cfg(foo(bar))]` is not allowed as an attribute. This
makes the same spec be rejected by the compiler if passed in as a
`--cfg` argument.
Fixes#31495
Previously when breaking tokens into smaller pieces, the replace_token
function have been used. It replaced current token and updated span
information, but it did not clear the list of expected tokens, neither
did it update remaining info about last token. This could lead to
incorrect error message, like one described in the issue #24780:
expected one of ... `>` ... found `>`
Refactor away resolve_name_in_module in resolve_imports.rs
Rewrite and improve the core name resolution procedure in NameResolution::result and Module::resolve_name
Refactor the duplicate checking code into NameResolution::try_define
These commits finish up closing out https://github.com/rust-lang/rust/issues/24237 by filling out all locations we create new file descriptors with variants that atomically create the file descriptor and set CLOEXEC where possible. Previous support for doing this in `File::open` was added in #27971 and support for `try_clone` was added in #27980. This commit fills out:
* `Socket::new` now passes `SOCK_CLOEXEC`
* `Socket::accept` now uses `accept4`
* `pipe2` is used instead of `pipe`
Unfortunately most of this support is Linux-specific, and most of it is post-2.6.18 (our oldest supported version), so all of the detection here is done dynamically. It looks like OSX does not have equivalent variants for these functions, so there's nothing more we can do there. Support for BSDs can be added over time if they also have these functions.
Closes#24237
Issue #31109 uncovered two semi-related problems:
* A panic in `str::parse::<f64>`
* A panic in `rustc::middle::const_eval::lit_to_const` where the result of float parsing was unwrapped.
This series of commits fixes both issues and also drive-by-fixes some things I noticed while tracking down the parsing panic.
Abort on stack overflow instead of re-raising SIGSEGV
We use guard pages that cause the process to abort to protect against
undefined behavior in the event of stack overflow. We have a handler
that catches segfaults, prints out an error message if the segfault was
due to a stack overflow, then unregisters itself and returns to allow
the signal to be re-raised and kill the process.
This caused some confusion, as it was unexpected that safe code would be
able to cause a segfault, while it's easy to overflow the stack in safe
code. To avoid this confusion, when we detect a segfault in the guard
page, abort instead of the previous behavior of re-raising SIGSEGV.
To test this, we need to adapt the tests for segfault to actually check
the exit status. Doing so revealed that the existing test for segfault
behavior was actually invalid; LLVM optimizes the explicit null pointer
reference down to an illegal instruction, so the program aborts with
SIGILL instead of SIGSEGV and the test didn't actually trigger the
signal handler at all. Use a C helper function to get a null pointer
that LLVM can't optimize away, so we get our segfault instead.
This is a [breaking-change] if anyone is relying on the exact signal
raised to kill a process on stack overflow.
Closes#31273
We use guard pages that cause the process to abort to protect against
undefined behavior in the event of stack overflow. We have a handler
that catches segfaults, prints out an error message if the segfault was
due to a stack overflow, then unregisters itself and returns to allow
the signal to be re-raised and kill the process.
This caused some confusion, as it was unexpected that safe code would be
able to cause a segfault, while it's easy to overflow the stack in safe
code. To avoid this confusion, when we detect a segfault in the guard
page, abort instead of the previous behavior of re-raising the SIGSEGV.
To test this, we need to adapt the tests for segfault to actually check
the exit status. Doing so revealed that the existing test for segfault
behavior was actually invalid; LLVM optimizes the explicit null pointer
reference down to an illegal instruction, so the program aborts with
SIGILL instead of SIGSEGV and the test didn't actually trigger the
signal handler at all. Use a C helper function to get a null pointer
that LLVM can't optimize away, so we get our segfault instead.
This is a [breaking-change] if anyone is relying on the exact signal
raised to kill a process on stack overflow.
Closes#31273
The scope of these refactorings is a little bit bigger than the title implies. See each commit for details.
I’m submitting this for nitpicking now (the first 4 commits), because I feel the basic idea/implementation is sound and should work. I will eventually expand this PR to cover the translator changes necessary for all this to work (+ tests), ~~and perhaps implement a dynamic dropping scheme while I’m at it as well.~~
r? @nikomatsakis
This commit attempts to use the `pipe2` syscall on Linux to atomically set the
CLOEXEC flag for pipes created. Unfortunately this was added in 2.6.27 so we
have to dynamically determine whether we can use it or not.
This commit also updates the `fds-are-cloexec.rs` test to test stdio handles for
spawned processes as well.
If a new cleanup is added to a cleanup scope, the cached exits for that
scope are cleared, so all previous cleanups have to be translated
again. In the worst case this means that we get N distinct landing pads
where the last one has N cleanups, then N-1 and so on.
As new cleanups are to be executed before older ones, we can instead
cache the number of already translated cleanups in addition to the
block that contains them, and then only translate new ones, if any and
then jump to the cached ones, getting away with linear growth instead.
For the crate in #31381 this reduces the compile time for an optimized
build from >20 minutes (I cancelled the build at that point) to about 11
seconds. Testing a few crates that come with rustc show compile time
improvements somewhere between 1 and 8%. The "big" winner being
rustc_platform_intrinsics which features code similar to that in #31381.
Fixes#31381
The first commit improves detection of unused imports -- it should have been part of #30325. Right now, the unused import in the changed test would not be reported.
The rest of the commits are miscellaneous, independent clean-ups in resolve that I didn't think warranted individual PRs.
r? @nrc
After the truly incredible and embarrassing mess I managed to make in my last pull request, this should be a bit less messy.
Fixes#31267 - with this change, the code mentioned in the issue compiles.
Found and fixed another issue as well - constants of zero-size types, when used in ExprRepeats inside associated constants, were causing the compiler to crash at the same place as #31267. An example of this:
```
struct Bar;
const BAZ: Bar = Bar;
struct Foo([Bar; 1]);
struct Biz;
impl Biz {
const BAZ: Foo = Foo([BAZ; 1]);
}
fn main() {
let foo = Biz::BAZ;
println!("{:?}", foo);
}
```
However, I'm fairly certain that my fix for this is not as elegant as it could be. The problem seems to occur only with an associated constant of a tuple struct containing a fixed size array which is initialized using a repeat expression, and when the element to be repeated provided to the repeat expression is another constant which is of a zero-sized type. The fix works by looking for constants and associated constants which are zero-width and consequently contain no data, but for which rustc is still attempting to emit an LLVM value; it simply stops rustc from attempting to emit anything. By my logic, this should work fine since the only values that are emitted in this case (according to the comments) are for closures with side effects, and constants will never have side effects, so it's fine to simply get rid of them. It fixes the error and things compile fine with it, but I have a sneaking suspicion that it could be done in a far better manner.
r? @nikomatsakis
