Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 8 pull requests #104176

Closed
wants to merge 23 commits into from
Closed

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

chenyukang and others added 23 commits October 25, 2022 21:16
Users report an AV at runtime of the compiled binary when using lld and
ThinLTO on windows-msvc. The AV occurs when accessing a static value
which is defined in one crate but used in another. Based on the
disassembly of the cross-crate use, it appears that the use is not
correctly linked with the definition and is instead assigned a garbage
pointer value.

If we look at the symbol tables for each crates' obj file, we can see
what is happening:

*lib.obj*:

```
COFF SYMBOL TABLE
...
00E 00000000 SECT2  notype       External     | _ZN10reproducer7memrchr2FN17h612b61ca0e168901E
...
```

*bin.obj*:

```
COFF SYMBOL TABLE
...
010 00000000 UNDEF  notype       External     | __imp__ZN10reproducer7memrchr2FN17h612b61ca0e168901E
...
```

The use of the symbol has the "import" style symbol name but the
declaration doesn't generate any symbol with the same name. As a result,
linking the files generates a warning from lld:

> rust-lld: warning: bin.obj: locally defined symbol imported: reproducer::memrchr::FN::h612b61ca0e168901 (defined in lib.obj) [LNK4217]

and the symbol reference remains undefined at runtime leading to the AV.

To fix this, we just need to detect that we are performing ThinLTO (and
thus, static linking) and omit the `dllimport` attribute on the extern
item in LLVM IR.
This helps with `*-windows-gnullvm` targets
Apply suggestions from code review

Co-authored-by: David Wood <agile.lion3441@fuligin.ink>
…d, r=nikomatsakis

avoid making substs of type aliases late bound when used as fn args

fixes rust-lang#47511
fixes rust-lang#85533
(although I did not know theses issues existed when i was working on this 🙃)

currently `Alias<...>` is treated the same as `Struct<...>` when deciding if generics should be late bound or early bound but this is not correct as `Alias` might normalize to a projection which does not constrain the generics.

I think this needs more tests before merging
more explanation of PR [here](https://hackmd.io/v44a-QVjTIqqhK9uretyQg?view)

Hackmd inline for future readers:
---

This assumes reader is familiar with the concept of early/late bound lifetimes. There's a section on rustc-dev-guide if not (although i think some details are a bit out of date)

## problem & background

Not all lifetimes on a fn can be late bound:
```rust
fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef {
    type Output = &'a (); // uh oh unconstrained lifetime
}
```
so we make make them early bound
```rust
fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef<'a> {// wow look at all that lifetimey
     type Output = &'a ();
}
```
(Closures have the same constraint however it is not enforced leading to soundness bugs, [rust-lang#84385](rust-lang#84385) implements this "downgrading late bound to early bound" for closures)

lifetimes on fn items are only late bound when they are "constrained" by the fn args:
```rust
fn foo<'a>(_: &'a ()) -> &'a ();
//               late bound, not present on `FooFnItem`
//               vv
impl<'a> Trait<(&'a (),)> for FooFnItem {
    type Output = &'a ();
}

// projections do not constrain inputs
fn bar<'a, T: Trait>(_: <T as Trait<'a>>::Assoc) -> &'a (); //  early bound
                                                            //  vv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for BarFnItem<'a, T> {
    type Output = &'a ();
}
```

current logic for determining if inputs "constrain" a lifetime works off of HIR so does not normalize aliases. It also assumes that any path with no self type constrains all its substs (i.e. `Foo<'a, u32>` has no self type but `T::Assoc` does). This falls apart for top level type aliases (see linked issues):

```rust
type Alias<'a, T> = <T as Trait<'a>>::Assoc;
//                      wow look its a path with no self type uwu
//                      i bet that constrains `'a` so it should be latebound
//                      vvvvvvvvvvv
fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a ();
//                     `Alias` normalized to make things clearer
//                     vvvvvvvvvvvvvvvvvvvvvvv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for FooFnDef<T> {
    type Output = &'a ();
    // oh no `'a` isnt constrained wah wah waaaah *trumbone noises*
    // i think, idk what musical instrument that is
}
```

## solution

The PR solves this by having the hir visitor that checks for lifetimes in constraining uses check if the path is a `DefKind::Alias`. If it is we ""normalize"" it by calling `type_of` and walking the returned type. This is a bit hacky as it requires a mapping between the substs on the path in hir, and the generics of the `type Alias<...>` which is on the ty layer.

Alternative solutions may involve calculating the "late boundness" of lifetimes after/during astconv rather than relying on hir at all. We already have code to determine whether a lifetime SHOULD be late bound or not as this is currently how the error for `fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a ();` gets emitted.

It is probably not possible to do this right now, late boundness is used by `generics_of` and `gather_explicit_predicates_of` as we currently do not put late bound lifetimes in `Generics`. Although this seems sus to me as the long term goal is to make all generics late bound which would result in `generics_of(function)` being empty? [rust-lang#103448](rust-lang#103448) places all lifetimes in `Generics` regardless of late boundness so that may be a good step towards making this possible.
…r=michaelwoerister

Fix Access Violation when using lld & ThinLTO on windows-msvc

Users report an AV at runtime of the compiled binary when using lld and ThinLTO on windows-msvc. The AV occurs when accessing a static value which is defined in one crate but used in another. Based on the disassembly of the cross-crate use, it appears that the use is not correctly linked with the definition and is instead assigned a garbage pointer value.

If we look at the symbol tables for each crates' obj file, we can see what is happening:

*lib.obj*:

```
COFF SYMBOL TABLE
...
00E 00000000 SECT2  notype       External     | _ZN10reproducer7memrchr2FN17h612b61ca0e168901E
...
```

*bin.obj*:

```
COFF SYMBOL TABLE
...
010 00000000 UNDEF  notype       External     | __imp__ZN10reproducer7memrchr2FN17h612b61ca0e168901E
...
```

The use of the symbol has the "import" style symbol name but the declaration doesn't generate any symbol with the same name. As a result, linking the files generates a warning from lld:

> rust-lld: warning: bin.obj: locally defined symbol imported: reproducer::memrchr::FN::h612b61ca0e168901 (defined in lib.obj) [LNK4217]

and the symbol reference remains undefined at runtime leading to the AV.

To fix this, we just need to detect that we are performing ThinLTO (and thus, static linking) and omit the `dllimport` attribute on the extern item in LLVM IR.

Fixes rust-lang#81408
…-hang, r=jackh726,wesleywiser

Avoid possible infinite  loop when next_point reaching the end of file

Fixes rust-lang#103451
If we return a span with `lo` = `hi`, `span_to_snippet` will always get `Ok("")`, which may introduce infinite loop if we don't care.

This PR make `find_width_of_character_at_span` return `width` with 1, so that `span_to_snippet` will get an `Err`.
first move on a nested span_label

trying not to be smart this time.
…if-let, r=jackh276,davidtwco

Recover from common if let syntax mistakes/typos

Fixes rust-lang#103587
…crum

Update several crates for improved support of the new targets

This helps with `*-windows-gnullvm` targets by reducing amount of patching.
…ce, r=Mark-Simulacrum

Don't intra linkcheck reference

This removes the reference from the intra-doc link checks. This causes problems if any of the reference content needs to change, it causes the linkchecker to break. The reference has its own broken link check (https://github.com/rust-lang/reference/tree/master/style-check) which uses pulldown-cmark on the source to find actual broken links (instead of false-positives like this regex does).

I think the intra-doc link check could potentially be removed completely, since I think rustdoc is now checking for them well enough. However, it may serve as a decent regression check.
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Nov 8, 2022
@Manishearth
Copy link
Member Author

@bors r+ p=5

mutually exclusive with #104168

@bors
Copy link
Contributor

bors commented Nov 8, 2022

📌 Commit 4cd7c67 has been approved by Manishearth

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2022
@notriddle
Copy link
Contributor

Failed in #103636

@notriddle
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 9, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between 85f4f41deb1557ca8ab228319d33003dd2f20f45 and 1c9d66a01a779d0e2b257ea7788af5df97d8f4a4
Rustdoc was updated
##[group]Run src/ci/scripts/verify-channel.sh
src/ci/scripts/verify-channel.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
---

---- compile_test stdout ----
diff of stderr:

 error[E0601]: `main` function not found in crate `ice_6250`
   --> $DIR/ice-6250.rs:16:2
 LL | }
 LL | }
    |  ^ consider adding a `main` function to `$DIR/ice-6250.rs`
 error[E0308]: mismatched types
   --> $DIR/ice-6250.rs:12:14
    |
    |
 LL |     for reference in vec![1, 2, 3] {
 ...
 ...
 LL |         Some(reference) = cache.data.get(key) {
    |              ^^^^^^^^^ expected integer, found `&i32`
 help: consider dereferencing the borrow
    |
    |
 LL |         Some(*reference) = cache.data.get(key) {
 
 error[E0308]: mismatched types
   --> $DIR/ice-6250.rs:12:9
    |
    |
 LL |         Some(reference) = cache.data.get(key) {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`
+help: consider adding `let`
+   |
+   |
+LL |         let Some(reference) = cache.data.get(key) {
+   |         +++
 
 error: aborting due to 3 previous errors
 
---
To only update this specific test, also pass `--test-args crashes/ice-6250.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/clippy-driver" "tests/ui/crashes/ice-6250.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui/crashes/ice-6250.stage-id" "-A" "unused" "--emit=metadata" "-Dwarnings" "-Zui-testing" "-L" "dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps" "-L" "dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps" "--extern" "clippy_utils=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libclippy_utils-5ac9fbce59ab185e.rlib" "--extern" "futures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libfutures-3ef490519219f110.rlib" "--extern" "rustc_semver=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/librustc_semver-963bbd3f89834643.rlib" "--extern" "quote=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libquote-b9c731f380466bd0.rlib" "--extern" "serde=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libserde-f474b2dfd384a595.rlib" "--extern" "syn=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libsyn-e95902f0f3ef680a.rlib" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libtokio-e0524b7e2611e851.rlib" "--extern" "serde_derive=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libserde_derive-4ace64e065702c69.so" "--extern" "if_chain=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libif_chain-03f75cdc6d4d3afc.rlib" "--extern" "itertools=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libitertools-b6f83e8bf7b1d2e3.rlib" "--extern" "regex=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libregex-619ac20e364f2b2c.rlib" "--extern" "derive_new=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libderive_new-97618c8d1e1f91be.so" "--extern" "parking_lot=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libparking_lot-f03750e49676bd17.rlib" "--extern" "clippy_lints=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libclippy_lints-f2da228ff241ec97.rlib" "--edition=2021" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui/crashes/ice-6250.stage-id.aux"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
{"message":"`main` function not found in crate `ice_6250`","code":{"code":"E0601","explanation":"No `main` function was found in a binary crate.\n\nTo fix this error, add a `main` function:\n\n```\nfn main() {\n    // Your program will start here.\n    println!(\"Hello world!\");\n}\n```\n\nIf you don't know the basics of Rust, you can look at the\n[Rust Book][rust-book] to get started.\n\n[rust-book]: https://doc.rust-lang.org/book/\n"},"level":"error","spans":[{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":317,"byte_end":317,"line_start":16,"line_end":16,"column_start":2,"column_end":2,"is_primary":true,"text":[{"text":"}","highlight_start":2,"highlight_end":2}],"label":"consider adding a `main` function to `tests/ui/crashes/ice-6250.rs`","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error[E0601]: `main` function not found in crate `ice_6250`\n  --> tests/ui/crashes/ice-6250.rs:16:2\n   |\nLL | }\n   |  ^ consider adding a `main` function to `tests/ui/crashes/ice-6250.rs`\n\n"}
{"message":"mismatched types","code":{"code":"E0308","explanation":"Expected type did not match the received type.\n\nErroneous code examples:\n\n```compile_fail,E0308\nfn plus_one(x: i32) -> i32 {\n    x + 1\n}\n\nplus_one(\"Not a number\");\n//       ^^^^^^^^^^^^^^ expected `i32`, found `&str`\n\nif \"Not a bool\" {\n// ^^^^^^^^^^^^ expected `bool`, found `&str`\n}\n\nlet x: f32 = \"Not a float\";\n//     ---   ^^^^^^^^^^^^^ expected `f32`, found `&str`\n//     |\n//     expected due to this\n```\n\nThis error occurs when an expression was used in a place where the compiler\nexpected an expression of a different type. It can occur in several cases, the\nmost common being when calling a function and passing an argument which has a\ndifferent type than the matching type in the function declaration.\n"},"level":"error","spans":[{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":236,"byte_end":245,"line_start":12,"line_end":12,"column_start":14,"column_end":23,"is_primary":true,"text":[{"text":"        Some(reference) = cache.data.get(key) {","highlight_start":14,"highlight_end":23}],"label":"expected integer, found `&i32`","suggested_replacement":null,"suggestion_applicability":null,"expansion":null},{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":165,"byte_end":174,"line_start":9,"line_end":9,"column_start":9,"column_end":18,"is_primary":false,"text":[{"text":"    for reference in vec![1, 2, 3] {","highlight_start":9,"highlight_end":18}],"label":"expected due to the type of this binding","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"consider dereferencing the borrow","code":null,"level":"help","spans":[{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":236,"byte_end":236,"line_start":12,"line_end":12,"column_start":14,"column_end":14,"is_primary":true,"text":[{"text":"        Some(reference) = cache.data.get(key) {","highlight_start":14,"highlight_end":14}],"label":null,"suggested_replacement":"*","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error[E0308]: mismatched types\n  --> tests/ui/crashes/ice-6250.rs:12:14\n   |\nLL |     for reference in vec![1, 2, 3] {\n   |         --------- expected due to the type of this binding\n...\nLL |         Some(reference) = cache.data.get(key) {\n   |              ^^^^^^^^^ expected integer, found `&i32`\n   |\nhelp: consider dereferencing the borrow\n   |\nLL |         Some(*reference) = cache.data.get(key) {\n   |              +\n\n"}
{"message":"mismatched types","code":{"code":"E0308","explanation":"Expected type did not match the received type.\n\nErroneous code examples:\n\n```compile_fail,E0308\nfn plus_one(x: i32) -> i32 {\n    x + 1\n}\n\nplus_one(\"Not a number\");\n//       ^^^^^^^^^^^^^^ expected `i32`, found `&str`\n\nif \"Not a bool\" {\n// ^^^^^^^^^^^^ expected `bool`, found `&str`\n}\n\nlet x: f32 = \"Not a float\";\n//     ---   ^^^^^^^^^^^^^ expected `f32`, found `&str`\n//     |\n//     expected due to this\n```\n\nThis error occurs when an expression was used in a place where the compiler\nexpected an expression of a different type. It can occur in several cases, the\nmost common being when calling a function and passing an argument which has a\ndifferent type than the matching type in the function declaration.\n"},"level":"error","spans":[{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":231,"byte_end":268,"line_start":12,"line_end":12,"column_start":9,"column_end":46,"is_primary":true,"text":[{"text":"        Some(reference) = cache.data.get(key) {","highlight_start":9,"highlight_end":46}],"label":"expected `bool`, found `()`","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"consider adding `let`","code":null,"level":"help","spans":[{"file_name":"tests/ui/crashes/ice-6250.rs","byte_start":231,"byte_end":231,"line_start":12,"line_end":12,"column_start":9,"column_end":9,"is_primary":true,"text":[{"text":"        Some(reference) = cache.data.get(key) {","highlight_start":9,"highlight_end":9}],"label":null,"suggested_replacement":"let ","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error[E0308]: mismatched types\n  --> tests/ui/crashes/ice-6250.rs:12:9\n   |\nLL |         Some(reference) = cache.data.get(key) {\n   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`\n   |\nhelp: consider adding `let`\n   |\nLL |         let Some(reference) = cache.data.get(key) {\n   |         +++\n\n"}
{"message":"Some errors have detailed explanations: E0308, E0601.","code":null,"level":"failure-note","spans":[],"children":[],"rendered":"Some errors have detailed explanations: E0308, E0601.\n"}
{"message":"For more information about an error, try `rustc --explain E0308`.","code":null,"level":"failure-note","spans":[],"children":[],"rendered":"For more information about an error, try `rustc --explain E0308`.\n"}

------------------------------------------

@Manishearth
Copy link
Member Author

@notriddle oh I thought that had been kicked out of the queue already (when r-ing rollups please also r- the failing pr)

@chenyukang
Copy link
Member

@matthiaskrgr
I need to fix this issue, How to run this single test case in local?
Seems this command not working:

x test ./src/tools/clippy/tests/ui/crashes/ice-6250.rs --bless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.