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

Disable buggy LTO #320

Merged
merged 11 commits into from
Nov 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
runs-on: ${{ matrix.os }}-latest

strategy:
fail-fast: false
matrix:
os: [ubuntu, windows, macos]

Expand All @@ -45,6 +46,7 @@ jobs:
runs-on: ${{ matrix.os }}-latest

strategy:
fail-fast: false
matrix:
os: [ubuntu, windows, macos]

Expand Down Expand Up @@ -100,6 +102,7 @@ jobs:
runs-on: ${{ matrix.os }}-latest

strategy:
fail-fast: false
matrix:
os: [ubuntu, windows, macos]

Expand Down Expand Up @@ -166,11 +169,30 @@ jobs:
# Check that the release automation works as expected
- run: scripts/release/test.sh

# This job performs a really simple smoke test that the release build of our binaries
# works well. We had to introduce this job after we saw that the release build on windows
# couldn't even work without crashing with exit code: 0xc0000005, STATUS_ACCESS_VIOLATION
# due to some bug in LTO.
release-smoke-test:
runs-on: ${{ matrix.os }}-latest

strategy:
fail-fast: false
matrix:
os: [ubuntu, windows, macos]

steps:
- uses: actions/checkout@v4

- run: MARKER_ALLOW_DRIVER_BUILD=1 cargo build -p cargo_marker -p marker_rustc_driver --release
- run: PATH="$PWD/target/release:$PATH" cargo marker

# Check that the Github Action works
github-action-test:
runs-on: ${{ matrix.os }}

strategy:
fail-fast: false
matrix:
# Make sure we cover all operating systems supported by Github Actions
os:
Expand Down Expand Up @@ -207,3 +229,8 @@ jobs:
# +stable is to force using the pre-installed `cargo` on the runner instead of
# what's specified in `rust-toolchain.toml`
- run: cargo +stable marker --version

# There may be bugs in the release build of our binaries. For example, in the past
# we saw that LTO could lead to exit code: 0xc0000005, STATUS_ACCESS_VIOLATION crash

- run: cargo marker -l 'marker_lints="*"'
35 changes: 24 additions & 11 deletions .github/workflows/release-on-tag.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ jobs:
env:
artifact: marker_rustc_driver-aarch64-unknown-linux-gnu

# We've seen this job hang for 6 hours with the logs ending with
# ```
# Compiling libloading v0.8.0
# Compiling bumpalo v3.14.0
# Error: The operation was canceled.
# ```
#
# No idea how that happens. Maybe that's a bug with the compiler hang in rustc,
# but this bug seems flaky, and reruning the job if it hangs should help
timeout-minutes: 60

steps:
- uses: actions/checkout@v4

Expand Down Expand Up @@ -175,17 +186,19 @@ jobs:
# For extra security we set the env variable with the token only
# for the steps that require them.
#
# There are two `cargo release` invocations here. One without the
# `cargo_marker` package, and one with it.
#
# Keep in mind that `cargo-marker` may install `marker_rustc_driver`
# from crates.io via `cargo install`.
# We used to have two `cargo release` invocations here. One without the
# `cargo_marker` package, and one with it. This was done because there is
# an implicit dependency between `cargo_marker` and `marker_rustc_driver`,
# since the former installs the latter. However, this separation somehow
# resulted in the second `cargo release` invocation to fail with the error
# ```
# the remote server responded with an error (status 403 Forbidden):
# must be logged in to perform that action
# ```
#
# That being said, we will be extra safe if `cargo_marker` is published
# after `marker_rustc_driver`.
- run: cargo release publish --exclude cargo_marker --execute --no-confirm --no-verify --allow-branch '*'
env:
CARGO_REGISTRY_TOKEN: ${{ secrets.CRATES_IO_TOKEN }}
- run: cargo release publish --package cargo_marker --execute --no-confirm --no-verify --allow-branch '*'
# No idea why this happens, but let's just keep a single cargo release
# to avoid this issue and to simplify the workflow, because the problem
# we tried to protect against is not that likely to happen anyway.
- run: cargo release publish --no-confirm --no-verify --no-push --no-tag --allow-branch '*'
env:
CARGO_REGISTRY_TOKEN: ${{ secrets.CRATES_IO_TOKEN }}
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ The following components are considered to be internal and they are excluded fro

## [Unreleased]

## [0.4.2] - 2023-11-25

[#320]: https://github.com/rust-marker/marker/pull/320

- [#320]: Disable LTO on release builds to fix the crash on Windows with `exit code: 0xc0000005, STATUS_ACCESS_VIOLATION`

## [0.4.1] - 2023-11-24

[#319]: https://github.com/rust-marker/marker/pull/319
Expand Down
4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ resolver = "2"

[profile.release]
codegen-units = 1
lto = true
strip = false

# ⚠️ Turns out there is some bug in LTO on Windows, because it leads to the
# `marker_rustc_driver` crashing with "exit code: 0xc0000005, STATUS_ACCESS_VIOLATION"
lto = false

[workspace.package]
edition = "2021"
Expand Down