Skip to content

Conversation

@TyPR124
Copy link
Owner

@TyPR124 TyPR124 commented Jun 23, 2020

No description provided.

bors and others added 30 commits June 17, 2020 11:30
…uppe,Mark-Simulacrum

[RISC-V] Do not force frame pointers

We have been seeing some very inefficient code that went away when using
`-Cforce-frame-pointers=no`. For instance `core::ptr::drop_in_place` at
`-Oz` was compiled into a function which consisted entirely of saving
registers to the stack, then using the frame pointer to restore the same
registers (without any instructions between the prolog and epilog).

The RISC-V LLVM backend supports frame pointer elimination, so it makes
sense to allow this to happen when using Rust. It's not clear to me that
frame pointers have ever been required in the general case.

In #61675 it was pointed out that this made reassembling
stack traces easier, which is true, but there is a code generation
option for forcing frame pointers, and I feel the default should not be
to require frame pointers, given it demonstrably makes code size worse
(around 10% in some embedded applications).

The kinds of targets mentioned in #61675 are popular, but
should not dictate that code generation should be worse for all RISC-V
targets, especially as there is a way to use CFI information to
reconstruct the stack when the frame pointer is eliminated. It is also
a misconception that `fp` is always used for the frame pointer. `fp` is
an ABI name for `x8` (aka `s0`), and if no frame pointer is required,
`x8` may be used for other callee-saved values.

---

I am partly posting this to get feedback from @fintelia who introduced the change to require frame pointers, and @hanna-kruppe who had issues with the original PR. I would understand if we wanted to remove this setting on only a subset of RISC-V targets, but my preference would be to remove this setting everywhere.

There are more details on the code size savings seen in Tock here: tock/tock#1660
No one actually works with src/tools/librustdoc, it's almost empty.
The build is not actually needed often, and it can be added back on a
case-by-case basis if a specific PR needs access to it.
testing platform-specific changes
This is required to avoid cycles when evaluating auto trait
predicates.
We only need to cast the pointer once to change `Box<T>` to an array
`Box<[T; 1]>`, then we can let unsized coercion return `Box<[T]>`.
Fix link error with #[thread_local] introduced by #71192

r? @oli-obk
linker: Never pass `-no-pie` to non-gnu linkers

Fixes #73370
Insufficient sanitization of the x87 FPU tag word in the trusted enclave runtime allowed unprivileged adversaries in the containing host application to induce incoherent or unexpected results for ABI-compliant compiled enclave application code that uses the x87 FPU.

Vulnerability was disclosed to us by Fritz Alder, Jo Van Bulck, David Oswald and Frank Piessens
Previously, compile_fail and ignore code examples displayed a tooltip
indicating this in the documentation. This tooltip has now also been
added to should_panic examples.
bors and others added 28 commits June 21, 2020 02:20
Add a lint to catch clashing `extern` fn declarations.

Closes #69390.

Adds lint `clashing_extern_decl` to detect when, within a single crate, an extern function of the same name is declared with different types. Because two symbols of the same name cannot be resolved to two different functions at link time, and one function cannot possibly have two types, a clashing extern declaration is almost certainly a mistake.

This lint does not run between crates because a project may have dependencies which both rely on the same extern function, but declare it in a different (but valid) way. For example, they may both declare an opaque type for one or more of the arguments (which would end up distinct types), or use types that are valid conversions in the language the extern fn is defined in. In these cases, we can't say that the clashing declaration is incorrect.

r? @eddyb
Also hash predicates by address
Enable LLVM zlib

Compilers may generate ELF objects with compressed sections (although rustc currently doesn't do this). Currently, when linking these with `rust-lld`, you'll get this error:

`rust-lld: error: ...: contains a compressed section, but zlib is not available`

This enables zlib when building LLVM.
This fixes an issue with the following sample:

    mod foo {
	mod inaccessible {
	    pub struct X;
	}
	pub mod avail {
	    pub struct X;
	}
    }

    fn main() { X; }

Instead of suggesting both `use crate::foo::inaccessible::X;` and `use
crate::foo::avail::X;`, it should only suggest the latter.

It is done by trimming the list of suggestions from inaccessible paths
if accessible paths are present.

Visibility is checked with `is_accessible_from` now instead of being
hard-coded.

-

Some tests fixes are trivial, and others require a bit more explaining,
here are my comments:

src/test/ui/issues/issue-35675.stderr: Only needs to make the enum
public to have the suggestion make sense.

src/test/ui/issues/issue-42944.stderr: Importing the tuple struct won't
help because its constructor is not visible, so the attempted
constructor does not work. In that case, it's better not to suggest it.
The case where the constructor is public is covered in `issue-26545.rs`.
Upgrade Chalk

Things done in this PR:
- Upgrade Chalk to `0.11.0`
- Added compare-mode=chalk
- Bump rustc-hash in `librustc_data_structures` to `1.1.0` to match Chalk
- Removed `RustDefId` since the builtin type support is there
- Add a few more `FIXME(chalk)`s for problem spots I hit when running all tests with chalk
- Added some more implementation code for some newer builtin Chalk types (e.g. `FnDef`, `Array`)
- Lower `RegionOutlives` and `ObjectSafe` predicates
- Lower `Dyn` without the region
- Handle `Int`/`Float` `CanonicalVarKind`s
- Uncomment some Chalk tests that actually work now
- Remove the revisions in `src/test/ui/coherence/coherence-subtyping.rs` since they aren't doing anything different

r? @nikomatsakis
Cache flags and escaping vars for predicates

With predicates becoming interned (rust-lang/compiler-team#285) this is now possible and could be a perf win. It would become an even larger win once we have recursive predicates.

cc @lcnr @nikomatsakis

r? @ghost
Prefer accessible paths in 'use' suggestions

This PR addresses issue #26454, where `use` suggestions are made for paths that don't work. For example:

```rust
mod foo {
    mod bar {
        struct X;
    }
}

fn main() { X; } // suggests `use foo::bar::X;`
```
…manieu

deprecate wrapping_offset_from

As per #41079 (comment) which seems like a consensus.

r? @Amanieu
Miri: replace many bug! by span_bug!

r? @oli-obk
Do not send a notification for P-high stable regressions

This is kind of a hack to only match nightly and beta regressions, but not stable regressions. See my tests [on the playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6ff8a809162118aa2951f2ff12400067).

r? @spastorino cc @Mark-Simulacrum
Rollup of 6 pull requests

Successful merges:

 - #71660 (impl PartialEq<Vec<B>> for &[A], &mut [A])
 - #72623 (Prefer accessible paths in 'use' suggestions)
 - #73502 (Add E0765)
 - #73580 (deprecate wrapping_offset_from)
 - #73582 (Miri: replace many bug! by span_bug!)
 - #73585 (Do not send a notification for P-high stable regressions)

Failed merges:

 - #73581 (Create 0766 error code)

r? @ghost
Update cargo

3 commits in 79c769c3d7b4c2cf6a93781575b7f592ef974255..089cbb80b73ba242efdcf5430e89f63fa3b5328d
2020-06-11 22:13:37 +0000 to 2020-06-15 14:38:34 +0000
- Support linker with -Zdoctest-xcompile. (rust-lang/cargo#8359)
- Fix doctests not running with --target=HOST. (rust-lang/cargo#8358)
- Allow passing a registry index url directly to `cargo install` (rust-lang/cargo#8344)
…r=nikomatsakis"

This reverts commit 372cb9b, reversing
changes made to 5c61a8d.
None of the tools seem to need syn 0.15.35, so we can just build syn
1.0.

This was causing an issue with clippy's `compile-test` program: since
multiple versions of `syn` would exist in the build directory, we would
non-deterministically pick one based on filesystem iteration order. If
the pre-1.0 version of `syn` was picked, a strange build error would
occur (see
#73594 (comment))

To prevent this kind of issue from happening again, we now panic if we
find multiple versions of a crate in the build directly, instead of
silently picking the first version we find.
Revert PR #72389 - "Explain move errors that occur due to method calls involving `self"

r? @petrochenkov
…kler

impl ToSocketAddrs for (String, u16)

This adds a convenience impl of `ToSocketAddrs for (String, u16)`. When authoring HTTP services it's common to take command line options for `host` and `port` and parse them into `String` and `u16` respectively. Consider the following program:
```rust
#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
    // ...
}
```

Networking is a pretty common starting point for people new to Rust, and seeing `&*` in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that `String` can't be passed directly there. Instead with this patch we can omit the `&*` conversion and pass `host` directly:
```rust
#[derive(Debug, StructOpt)]
struct Config {
    host: String,
    port: u16,
}

async fn main() -> io::Result<()> {
    let config = Config::from_args();
    let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
    // ...
}
```

I think should be an easy and small ergonomics improvement for networking. Thanks!
@TyPR124 TyPR124 closed this Jun 23, 2020
TyPR124 pushed a commit that referenced this pull request Aug 30, 2021
Otherwise, we can get into a situation where you have
a subtype obligation `#1 <: #2` pending, #1 is constrained
by `check_casts`, but #2` is unaffected.

Co-authored-by: Niko Matsakis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.