Skip to content

Commit

Permalink
Infra: Disable buggy LTO and add smoke tests to CI (#320)
Browse files Browse the repository at this point in the history
Fixes marker_rustc_driver crashing on windows with `exit code: 0xc0000005, STATUS_ACCESS_VIOLATION`. Reported in  #318 (comment).

I disabled LTO, in release build and it fixes the problem.

I extended the CI to perform a really simple release smoke test both for the current release and for the previous release (in github action test CI job).

Here, for example, you may see that the bug reproduces in master on the changed CI
- [CI job for the current version of marker](https://github.com/rust-marker/marker/actions/runs/6985541482/job/19009920759?pr=320#step:4:93)
- [CI job for the released version of marker](https://github.com/rust-marker/marker/actions/runs/6985541482/job/19009920759?pr=320#step:4:93)


With the disabled LTO `release-smoke-test` [passes](https://github.com/rust-marker/marker/actions/runs/6985586586/job/19010007386?pr=320)


Btw. @xFrednet could you add the `release-smoke-test` jobs to required ones please?
  • Loading branch information
Veetaha committed Nov 25, 2023
1 parent 05727a8 commit bb0928e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 12 deletions.
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

0 comments on commit bb0928e

Please sign in to comment.