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

Fix incorrect NDEBUG handling in LLVM bindings #127654

Merged
merged 2 commits into from
Jul 13, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jul 12, 2024

We currently compile our LLVM bindings using -DNDEBUG if debuginfo for LLVM is disabled. However, NDEBUG doesn't have any relation to debuginfo, it controls whether assertions are enabled.

Split the LLVM_NDEBUG environment variable into two, so that assertions and debuginfo are controlled independently.

After this change, LLVMRustDIBuilderInsertDeclareAtEnd triggers an assertion failure on LLVM 19 due to an incorrect cast. Fix it by removing the unused return value entirely.

r? @cuviper

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Jul 12, 2024
@nikic nikic mentioned this pull request Jul 12, 2024
cfg.define("NDEBUG", None);
}

if tracked_env_var_os("LLVM_NO_DEBUGINFO").is_some() {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use DEBUG here, as set by Cargo, so it's based on the crate debug level rather than the LLVM build. That will make a difference in distro builds that aren't even building LLVM at all.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, cc::Build should do that automatically, so I think we don't need any manual setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per https://docs.rs/cc/1.1.1/cc/struct.Build.html#method.debug this actually already happens automatically, so I think this code should just be removed.

Copy link
Contributor Author

@nikic nikic Jul 12, 2024

Choose a reason for hiding this comment

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

Oops, race condition :)

We currently compile our LLVM bindings using `-DNDEBUG` if
debuginfo for LLVM is disabled. However, `NDEBUG` doesn't have
any relation to debuginfo, it controls whether assertions are
enabled.

Rename the environment variable to `LLVM_ASSERTIONS` and drive
it using the `llvm_assertions` option. Also drop the explicit
`debug(false)` call, as cc already sets this up using the
cargo `DEBUG` environment variable.
The return value changed from an Instruction to a DbgRecord in
LLVM 19. As we don't actually use the result, drop the return
value entirely to support both.
@cuviper
Copy link
Member

cuviper commented Jul 12, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 12, 2024

📌 Commit 8dfd3a4 has been approved by cuviper

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 Jul 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 13, 2024
Rollup of 3 pull requests

Successful merges:

 - rust-lang#127654 (Fix incorrect NDEBUG handling in LLVM bindings)
 - rust-lang#127661 (Stabilize io_slice_advance)
 - rust-lang#127668 (Improved slice documentation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cb1ccc1 into rust-lang:master Jul 13, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 13, 2024
Rollup merge of rust-lang#127654 - nikic:llvm-ndebug-fix, r=cuviper

Fix incorrect NDEBUG handling in LLVM bindings

We currently compile our LLVM bindings using `-DNDEBUG` if debuginfo for LLVM is disabled. However, `NDEBUG` doesn't have any relation to debuginfo, it controls whether assertions are enabled.

Split the LLVM_NDEBUG environment variable into two, so that assertions and debuginfo are controlled independently.

After this change, `LLVMRustDIBuilderInsertDeclareAtEnd` triggers an assertion failure on LLVM 19 due to an incorrect cast. Fix it by removing the unused return value entirely.

r? `@cuviper`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Update to LLVM 19

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

try-job: dist-x86_64-apple
try-job: dist-apple-various
try-job: x86_64-apple-1
try-job: x86_64-apple-2
try-job: dist-aarch64-apple
try-job: aarch64-apple
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Update to LLVM 19

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

try-job: x86_64-msvc
try-job: i686-msvc
try-job: x86_64-msvc-ext
try-job: dist-x86_64-msvc
try-job: dist-i686-msvc
try-job: dist-aarch64-msvc
try-job: dist-x86_64-msvc-alt
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Update to LLVM 19

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

try-job: i686-mingw
try-job: x86_64-mingw
try-job: dist-i686-mingw
try-job: dist-x86_64-mingw
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2024
Update to LLVM 19

The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.

The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by rust-lang#126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.

Compatibility note for wasm:

> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

Fixes rust-lang#121444.
Fixes rust-lang#128212.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2024
Update to LLVM 19

The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.

The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by rust-lang#126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.

Compatibility note for wasm:

> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

Fixes rust-lang#121444.
Fixes rust-lang#128212.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2024
Update to LLVM 19

The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.

The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by rust-lang#126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.

Compatibility note for wasm:

> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.

Related changes:
 * rust-lang#127605
 * rust-lang#127613
 * rust-lang#127654
 * rust-lang#128141
 * llvm/llvm-project#98933

Fixes rust-lang#121444.
Fixes rust-lang#128212.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 2, 2024
Update to LLVM 19

The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.

The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by rust-lang/rust#126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.

Compatibility note for wasm:

> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.

Related changes:
 * rust-lang/rust#127605
 * rust-lang/rust#127613
 * rust-lang/rust#127654
 * rust-lang/rust#128141
 * llvm/llvm-project#98933

Fixes rust-lang/rust#121444.
Fixes rust-lang/rust#128212.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Aug 13, 2024
Update to LLVM 19

The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.

The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by rust-lang/rust#126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.

Compatibility note for wasm:

> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.

Related changes:
 * rust-lang/rust#127605
 * rust-lang/rust#127613
 * rust-lang/rust#127654
 * rust-lang/rust#128141
 * llvm/llvm-project#98933

Fixes rust-lang/rust#121444.
Fixes rust-lang/rust#128212.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants