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.

Sign up for GitHub

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

Jump to bottom

Dispose llvm::TargetMachines prior to llvm::Context being disposed #118464

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Dispose llvm::TargetMachines prior to llvm::Context being disposed #118464

merged 2 commits into from
Nov 30, 2023

Conversation

wesleywiser
Copy link
Member

If the TargetMachine is disposed after the Context is disposed, it can lead to use after frees in some cases.

I've observed this happening occasionally on code compiled for aarch64-pc-windows-msvc using -Zstack-protector=strong but other users have reported AVs from host aarch64-pc-windows-msvc compilers as well.

I was not able to extract a self-contained test case yet so there is no accompanying test.

Fixes #118462

@rustbot
Copy link
Collaborator

rustbot commented Nov 29, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added 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. labels Nov 29, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
wesleywiser and others added 2 commits November 29, 2023 18:12
@wesleywiser
Dispose llvm::TargetMachines prior to llvm::Context being disposed
3323e4d 
If the TargetMachine is disposed after the Context is disposed, it can
lead to use after frees in some cases.

I've observed this happening occasionally on code compiled for
aarch64-pc-windows-msvc using `-Zstack-protector=strong` but other users
have reported AVs from host aarch64-pc-windows-msvc compilers as well.
@wesleywiser @cuviper
Update compiler/rustc_codegen_llvm/src/lib.rs
1011078 
Co-authored-by: Josh Stone <[email protected]>
Noratrieb
Noratrieb approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably have a lifetime then, but let's merge this first, thanks for the quick fix!
I didn't spot this during review...

@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 30, 2023

📌 Commit 1011078 has been approved by Nilstrieb

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 30, 2023
@Noratrieb Noratrieb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 30, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 30, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2023
@bors
Auto merge of rust-lang#118473 - matthiaskrgr:rollup-q96bm3u, r=matth…
1670ff6 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#118452 (rustdoc-search: allow spaces around `::` in path query)
 - rust-lang#118453 (Tweak message on ADT with private fields building)
 - rust-lang#118456 (rustc_span: Remove unused symbols.)
 - rust-lang#118458 (rustdoc: remove small from  `small-section-header`)
 - rust-lang#118464 (Dispose llvm::TargetMachines prior to llvm::Context being disposed)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 640a431 into rust-lang:master Nov 30, 2023
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2023
@rust-timer
Unrolled build for rust-lang#118464
ef7a282 
Rollup merge of rust-lang#118464 - wesleywiser:fix_dispose_ordering, r=Nilstrieb

Dispose llvm::TargetMachines prior to llvm::Context being disposed

If the TargetMachine is disposed after the Context is disposed, it can lead to use after frees in some cases.

I've observed this happening occasionally on code compiled for aarch64-pc-windows-msvc using `-Zstack-protector=strong` but other users have reported AVs from host aarch64-pc-windows-msvc compilers as well.

I was not able to extract a self-contained test case yet so there is no accompanying test.

Fixes rust-lang#118462
@rustbot rustbot added this to the 1.76.0 milestone Nov 30, 2023
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 30, 2023
@wesleywiser wesleywiser added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Nov 30, 2023
@cuviper cuviper mentioned this pull request Dec 1, 2023
Merged
@cuviper cuviper modified the milestones: 1.76.0, 1.75.0 Dec 1, 2023
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 1, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2023
@bors
Auto merge of rust-lang#118492 - cuviper:beta-next, r=cuviper
1a06ac5 
[beta] backports

- Build pre-coroutine-transform coroutine body rust-lang#117686
- Fix coroutine validation for mixed panic strategy rust-lang#118422
- ConstProp: Remove const when rvalue check fails. rust-lang#118426
- Dispose llvm::TargetMachines prior to llvm::Context being disposed rust-lang#118464

r? ghost
@apiraino
Copy link
Contributor

apiraino commented Dec 4, 2023

I'll add for the record that the compiler team seems in favor of stable backport, too, though taken alone not worth a dot release (Zulip)

@rustbot +stable-accepted

@apiraino apiraino added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Dec 4, 2023
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Dec 4, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2023
@bors
Auto merge of rust-lang#118607 - Mark-Simulacrum:stable-next, r=Mark-…
a28077b 
…Simulacrum

[stable] 1.74.1 release

This includes backports of:

*  Dispose llvm::TargetMachines prior to llvm::Context being disposed rust-lang#118464
*  clarify fn discriminant guarantees: only free lifetimes may get erased rust-lang#118006
*  Move subtyper below reveal_all and change reveal_all rust-lang#116415
   *  Make subtyping explicit in MIR rust-lang#115025 (needed for above)

As well as infrastructure fix:

*  Don't ask for a specific branch in cargotest rust-lang#118597

r? `@Mark-Simulacrum`
@apiraino apiraino mentioned this pull request Dec 7, 2023
Closed
@mmastrac
Copy link

mmastrac commented Dec 7, 2023 •
edited
Loading

For future searchers, this may have resolved a bug we were having building deno_webgpu:

process didn't exit successfully: `/Users/username/.rustup/toolchains/1.74.0-aarch64-apple-darwin/bin/rustc 
--crate-name deno_webgpu --edition=2021 ext/webgpu/lib.rs --error-format=json 
--json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=169 --crate-type lib 
--emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C split-debuginfo=unpacked 
-C metadata=43e905977801a26c -C extra-filename=-43e905977801a26c 
--out-dir /Users/username/projects/deno/denoland/deno/target/debug/deps 
-C incremental=/Users/username/projects/deno/denoland/deno/target/debug/incremental 
-L dependency=/Users/username/projects/deno/denoland/deno/target/debug/deps 
--extern deno_core=/Users/username/projects/deno/denoland/deno/target/debug/deps/libdeno_core-9ad5927c8a9a916e.rmeta 
--extern serde=/Users/username/projects/deno/denoland/deno/target/debug/deps/libserde-df2c86549ae96031.rmeta 
--extern tokio=/Users/username/projects/deno/denoland/deno/target/debug/deps/libtokio-850e2048e0f3074d.rmeta 
--extern wgpu_core=/Users/username/projects/deno/denoland/deno/target/debug/deps/libwgpu_core-d3940c680f7fe049.rmeta 
--extern wgpu_types=/Users/username/projects/deno/denoland/deno/target/debug/deps/libwgpu_types-2dd3168ea63e6930.rmeta 
-C 'link-args=-fuse-ld=lld -weak_framework Metal -weak_framework MetalPerformanceShaders 
-weak_framework QuartzCore -weak_framework CoreGraphics' -D 'clippy::all' -D 'clippy::await_holding_refcell_ref' 
-D 'clippy::missing_safety_doc' -D 'clippy::undocumented_unsafe_blocks' --cfg tokio_unstable 
-L /Users/username/projects/deno/denoland/deno/target/debug/gn_out/obj 
-L native=/Users/username/projects/deno/denoland/deno/target/debug/build/objc_exception-65792dd8e535de72/out` 
(signal: 11, SIGSEGV: invalid memory reference)

@cuviper cuviper modified the milestones: 1.75.0, 1.74.1 Dec 8, 2023
@cuviper cuviper removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Dec 8, 2023
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 12, 2023
@he32
rust: Update to 1.74.1.
0154372 
pkgsrc changes:
 * remove NetBSD-8 support (embedded LLVM requires newer C++
   than what is in -8; it's conceivable that this could still
   build with an external LLVM)
 * undo powerpc 9.0 file naming tweak

Upstream changes:

Version 1.74.1 (2023-12-07)
===========================

- [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM]
  (rust-lang/rust#118464)
- [Clarify guarantees for std::mem::discriminant]
  (rust-lang/rust#118006)
- [Fix some subtyping-related regressions]
  (rust-lang/rust#116415)
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 8, 2024
@he32
rust: update to version 1.74.1.
1fca108 
Pkgsrc changes:

 * Remove NetBSD-8 support (embedded LLVm requires newer C++
   than what is in -8; it's conceivable that this could still
   build with an external LLVM)
 * undo powerpc 9.0 file naming tweak, since we no longer support -8.
 * Remove patch to LLVM for powerpc now included by upstream.
 * Minor adjustments, checksum changes etc.


Upstream changes:

Version 1.74.1 (2023-12-07)
===========================

- [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM]
  (rust-lang/rust#118464)
- [Clarify guarantees for std::mem::discriminant]
  (rust-lang/rust#118006)
- [Fix some subtyping-related regressions]
  (rust-lang/rust#116415)

Version 1.74.0 (2023-11-16)
==========================

Language
--------

- [Codify that `std::mem::Discriminant<T>` does not depend on any
  lifetimes in T]
  (rust-lang/rust#104299)
- [Replace `private_in_public` lint with `private_interfaces` and
  `private_bounds` per RFC 2145]
  (rust-lang/rust#113126)
  Read more in
  [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html).
- [Allow explicit `#[repr(Rust)]`]
  (rust-lang/rust#114201)
- [closure field capturing: don't depend on alignment of packed fields]
  (rust-lang/rust#115315)
- [Enable MIR-based drop-tracking for `async` blocks]
  (rust-lang/rust#107421)

Compiler
--------

- [stabilize combining +bundle and +whole-archive link modifiers]
  (rust-lang/rust#113301)
- [Stabilize `PATH` option for `--print KIND=PATH`]
  (rust-lang/rust#114183)
- [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`]
  (rust-lang/rust#115644)
- [Promote loongarch64-unknown-none* to Tier 2]
  (rust-lang/rust#115368)
- [Add `i686-pc-windows-gnullvm` as a tier 3 target]
  (rust-lang/rust#115687)

Libraries
---------

- [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err]
  (rust-lang/rust#98704)
- [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`]
  (rust-lang/rust#111278)
- [impl Step for IP addresses]
  (rust-lang/rust#113748)
- [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`]
  (rust-lang/rust#114041)
- [`impl TryFrom<char> for u16`]
  (rust-lang/rust#114065)
- [Stabilize `io_error_other` feature]
  (rust-lang/rust#115453)
- [Stabilize the `Saturating` type]
  (rust-lang/rust#115477)
- [Stabilize const_transmute_copy]
  (rust-lang/rust#115520)

Stabilized APIs
---------------

- [`core::num::Saturating`]
  (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html)
- [`impl From<io::Stdout> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio)
- [`impl From<io::Stderr> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`std::ffi::OsString::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsString::into_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes)
- [`std::ffi::OsStr::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsStr::as_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes)
- [`std::io::Error::other`]
  (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other)
- [`impl TryFrom<char> for u16`]
  (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16)
- [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E)

These APIs are now stable in const contexts:

- [`core::mem::transmute_copy`]
  (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html)
- [`str::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii)
- [`[u8]::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii)

Cargo
-----

- [fix: Set MSRV for internal packages]
  (rust-lang/cargo#12381)
- [config: merge lists in precedence order]
  (rust-lang/cargo#12515)
- [fix(update): Clarify meaning of --aggressive as --recursive]
  (rust-lang/cargo#12544)
- [fix(update): Make `-p` more convenient by being positional]
  (rust-lang/cargo#12545)
- [feat(help): Add styling to help output ]
  (rust-lang/cargo#12578)
- [feat(pkgid): Allow incomplete versions when unambigious]
  (rust-lang/cargo#12614)
- [feat: stabilize credential-process and registry-auth]
  (rust-lang/cargo#12649)
- [feat(cli): Add '-n' to dry-run]
  (rust-lang/cargo#12660)
- [Add support for `target.'cfg(..)'.linker`]
  (rust-lang/cargo#12535)
- [Stabilize `--keep-going`]
  (rust-lang/cargo#12568)
- [feat: Stabilize lints]
  (rust-lang/cargo#12648)

Rustdoc
-------

- [Add warning block support in rustdoc]
  (rust-lang/rust#106561)
- [Accept additional user-defined syntax classes in fenced code blocks]
  (rust-lang/rust#110800)
- [rustdoc-search: add support for type parameters]
  (rust-lang/rust#112725)
- [rustdoc: show inner enum and struct in type definition for concrete type]
  (rust-lang/rust#114855)

Compatibility Notes
-------------------

- [Raise minimum supported Apple OS versions]
  (rust-lang/rust#104385)
- [make Cell::swap panic if the Cells partially overlap]
  (rust-lang/rust#114795)
- [Reject invalid crate names in `--extern`]
  (rust-lang/rust#116001)
- [Don't resolve generic impls that may be shadowed by dyn built-in impls]
  (rust-lang/rust#114941)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

None this cycle.
@def- def- mentioned this pull request Feb 16, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuviper cuviper cuviper left review comments

@Noratrieb Noratrieb Noratrieb approved these changes

Assignees

@davidtwco davidtwco

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. stable-accepted Accepted for backporting to the compiler in the stable channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.74.1
Development

Successfully merging this pull request may close these issues.

Random STATUS_ACCESS_VIOLATION on aarch64-pc-windows-msvc
9 participants
@wesleywiser @rustbot @rust-log-analyzer @Noratrieb @bors @apiraino @mmastrac @cuviper @davidtwco