Rollup of 8 pull requests #104175
Closed
Rollup of 8 pull requests #104175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…th 1 when reaching end
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 <[email protected]>
…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.
…h-div, r=notriddle Migrate crate-search element to CSS variables There should no UI changes. r? `@notriddle`
rustbot
added
T-bootstrap
bors
added
S-waiting-on-bors
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup