Skip to content

Restrict LLVM inline asm location cookie usage. Fixes #150451 - #160197

Open
susitsm wants to merge 3 commits into
rust-lang:mainfrom
susitsm:restrict-inline-asm-cookies
Open

Restrict LLVM inline asm location cookie usage. Fixes #150451#160197
susitsm wants to merge 3 commits into
rust-lang:mainfrom
susitsm:restrict-inline-asm-cookies

Conversation

@susitsm

@susitsm susitsm commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The parallel frontend makes the cookies nondeterministic in their current form, resulting in nondeterministic outputs when bitcode is emitted or LTO is used.

Causes minor diagnostic regression for inline asm in release builds. See #150451 for details.

Fixes #150451

r? bjorn3

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 Jul 30, 2026
@rustbot

This comment has been minimized.

Sign in to view
@susitsm
susitsm force-pushed the restrict-inline-asm-cookies branch 2 times, most recently from 1238b04 to e840de9 Compare July 30, 2026 06:49
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@susitsm
susitsm force-pushed the restrict-inline-asm-cookies branch from e840de9 to 4326f37 Compare July 30, 2026 07:21
@rustbot

rustbot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

@susitsm susitsm mentioned this pull request Aug 10, 2026
Open
@petrochenkov petrochenkov self-assigned this Aug 27, 2026
This was referenced Sep 1, 2026
Open
Open
petrochenkov
petrochenkov reviewed Sep 3, 2026
View reviewed changes
Comment thread compiler/rustc_codegen_ssa/src/traits/backend.rs Outdated
@petrochenkov

petrochenkov commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Is it possible to add a test case for this?
tests\run-make\parallel-reproducible-build\rmake.rs has some infra for it.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@susitsm
susitsm force-pushed the restrict-inline-asm-cookies branch from 4326f37 to dbef79f Compare September 3, 2026 14:46
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@susitsm
susitsm force-pushed the restrict-inline-asm-cookies branch 2 times, most recently from a22e364 to 6d6d87d Compare September 4, 2026 07:02
@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2026
petrochenkov
petrochenkov reviewed Sep 4, 2026
View reviewed changes
Comment thread tests/ui/asm/inline-syntax.rs
// Global assembly errors don't have line numbers, so no error on ARM.

//[arm]~? ERROR unknown directive
//[arm]~? ERROR unknown directive

@petrochenkov petrochenkov Sep 4, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change (and its equivalent in tests/ui/asm/inline-syntax.arm.stderr) should be reverted, since the flags were added above?

View changes since the review

@petrochenkov petrochenkov Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Testing in #160197 (comment).

@petrochenkov petrochenkov Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting, the tests pass.
Why did these diagnostics change?

@susitsm susitsm Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't request a review because I am still trying to get to the bottom of this. I don't know why there were 2 of those in the first place. Both of them came from the global_asm! in the test.

@susitsm susitsm Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Setting -Cembed-bitcode=false before my patches also removes the copy.

@susitsm susitsm Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have moved the last commit (one changing the asm ui test flags) to be the first. This already requires changing the expected stderr to not have duplicate errors. The later change to loc cookie encoding has no effect on the output, the ui tests pass without more changes.

As for why it was duplicated when embedding bitcode, I have no idea.

@petrochenkov

petrochenkov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@bors try jobs=dist-armv7-linux

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Sep 4, 2026
@bors
Auto merge of #160197 - susitsm:restrict-inline-asm-cookies, r=<try>
8ae3936 
Restrict LLVM inline asm location cookie usage. Fixes #150451


try-job: dist-armv7-linux
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2026
@petrochenkov petrochenkov mentioned this pull request Sep 4, 2026
Open
@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 8ae3936 (8ae3936981b03d57fde220a0672fb50e3c8169e7)
Base parent: b924f94 (b924f94129bce80f36dd9f08bb78dd5393fe196e)

@susitsm
change compile-flags for asm ui tests to not embed bitcode or use LTO
0238ef9
@susitsm
add test for inline asm cookie reproducibility
fa5601d
@susitsm
Disable inline asm line info cookies when llvm bitcode is saved or LT…
970cb98 
…O is enabled

The parallel frontend makes the cookies nondeterministic in their
current form, resulting in nondeterministic outputs when bitcode is
emitted or LTO is used.

Causes minor diagnostic regression for inline asm in release builds.
@susitsm
susitsm force-pushed the restrict-inline-asm-cookies branch from 6d6d87d to 970cb98 Compare September 5, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@petrochenkov petrochenkov

@bjorn3 bjorn3

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

parallel compiler: threads::spawnning loop not reproducible

5 participants

@susitsm @rustbot @rust-log-analyzer @petrochenkov @bjorn3