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

Add a test for -znostart-stop-gc usage with LLD #137926

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Mar 3, 2025

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without -znostart-stop-gc the test fails.

r? @lqd

try-job: x86_64-gnu
try-job: x86_64-msvc-1

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs 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 Mar 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2025

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

@Kobzol Kobzol force-pushed the lld-no-start-stop-test branch 2 times, most recently from 9a407f7 to 85db3ab Compare March 3, 2025 09:43
@rust-log-analyzer

This comment has been minimized.

@lqd
Copy link
Member

lqd commented Mar 3, 2025

Thanks!

Let's simplify the test: we only need to link, not run it. We don't need dependencies. The test is very simple and doesn't need any rmake infrastructure. I don't think it needs "needs-lld" either, only to keep linking regardless of the linker.

This could be a single-file build-pass UI test, containing the DistributedSlice and the two statics, while making sure that the distributed slice static is referenced e.g. in main so that it's not garbage-collected.

@Kobzol Kobzol force-pushed the lld-no-start-stop-test branch 2 times, most recently from 395dd92 to 2b15250 Compare March 3, 2025 11:42
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 3, 2025

Thanks, simplified the test and converted it to an UI test.

@lqd
Copy link
Member

lqd commented Mar 3, 2025

Thanks, we just need to wait for the PR to land then.

@Kobzol Kobzol force-pushed the lld-no-start-stop-test branch from 2b15250 to 9b487e6 Compare March 3, 2025 12:02
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 3, 2025

I droped your commit so that we can just approve it once your PR merges.

@rustbot blocked #137685

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2025
@lqd
Copy link
Member

lqd commented Mar 3, 2025

Ah, there should be a better spot for this test though, not in the ui root.

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 3, 2025

Any suggestions? I was looking for e.g. a link directory, but there was only link-native-libs and linkage-attr.

@lqd
Copy link
Member

lqd commented Mar 3, 2025

There's a bunch of linking related tests in linkage-attr for some reason. Let's create a new "linking" folder and put the new test there. As long as we don't add new entries to tests/ui, that should be good enough.

@Kobzol Kobzol force-pushed the lld-no-start-stop-test branch from 9b487e6 to c771916 Compare March 3, 2025 16:15
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 3, 2025

Ok, done. Thanks!

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 8, 2025

#137685 has been merged, so this should be ready. I think that we need a tighter platform bound on the test though, otherwise this will also run on Windows. Let's check.

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2025
Add a test for `-znostart-stop-gc` usage with LLD

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without `-znostart-stop-gc` the test fails.

r? `@lqd`

try-job: x86_64-gnu
try-job: x86_64-msvc-1
@bors
Copy link
Contributor

bors commented Mar 8, 2025

⌛ Trying commit c771916 with merge 9473f18...

@lqd
Copy link
Member

lqd commented Mar 8, 2025

Let’s run the test only on x64 Linux, and it can be expanded in the future if we need it. The GC behavior is only changed on that target anyways.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Mar 8, 2025

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 8, 2025
@Kobzol Kobzol removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 8, 2025
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 9, 2025

Good, the test has failed. Now I'll try again with the flag being passed to LLD to see if it succeeds.

@bors
Copy link
Contributor

bors commented Mar 9, 2025

💔 Test failed - checks-actions

@Kobzol Kobzol force-pushed the lld-no-start-stop-test branch from 0d7a6bb to 7bf4e99 Compare March 9, 2025 10:37
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 9, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 9, 2025
Add a test for `-znostart-stop-gc` usage with LLD

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without `-znostart-stop-gc` the test fails.

r? `@lqd`

try-job: x86_64-gnu
try-job: x86_64-msvc-1
@bors
Copy link
Contributor

bors commented Mar 9, 2025

⌛ Trying commit 7bf4e99 with merge 0047668...

@Kobzol Kobzol marked this pull request as ready for review March 9, 2025 12:39
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 9, 2025

Ok so the test fails without the flag set and passes with the flag set, which is the behavior that we want.

@rustbot ready

@rustbot rustbot 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 Mar 9, 2025
@lqd
Copy link
Member

lqd commented Mar 9, 2025

the try build is still running so I won’t r+ now or it’ll magically land immediately, so r=me when it passes if you happen to see it sooner

@bors
Copy link
Contributor

bors commented Mar 9, 2025

☀️ Try build successful - checks-actions
Build commit: 0047668 (0047668201a539fb28f3aac968596b4df731fe71)

@lqd
Copy link
Member

lqd commented Mar 9, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2025

📌 Commit 7bf4e99 has been approved by lqd

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 Mar 9, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 10, 2025
Add a test for `-znostart-stop-gc` usage with LLD

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without `-znostart-stop-gc` the test fails.

r? `@lqd`

try-job: x86_64-gnu
try-job: x86_64-msvc-1
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#136395 (Update to rand 0.9.0)
 - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable)
 - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example)
 - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD)
 - rust-lang#138074 (Support `File::seek` for Hermit)
 - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs)
 - rust-lang#138270 (chore: Fix some comments)
 - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`)
 - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136395 (Update to rand 0.9.0)
 - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable)
 - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example)
 - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD)
 - rust-lang#138074 (Support `File::seek` for Hermit)
 - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs)
 - rust-lang#138270 (chore: Fix some comments)
 - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136395 (Update to rand 0.9.0)
 - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable)
 - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example)
 - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD)
 - rust-lang#138074 (Support `File::seek` for Hermit)
 - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs)
 - rust-lang#138270 (chore: Fix some comments)
 - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136395 (Update to rand 0.9.0)
 - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable)
 - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example)
 - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD)
 - rust-lang#138074 (Support `File::seek` for Hermit)
 - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs)
 - rust-lang#138270 (chore: Fix some comments)
 - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 10, 2025
Add a test for `-znostart-stop-gc` usage with LLD

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without `-znostart-stop-gc` the test fails.

r? ```@lqd```

try-job: x86_64-gnu
try-job: x86_64-msvc-1
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136395 (Update to rand 0.9.0)
 - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable)
 - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example)
 - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD)
 - rust-lang#138074 (Support `File::seek` for Hermit)
 - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs)
 - rust-lang#138270 (chore: Fix some comments)
 - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cbde8b9 into rust-lang:master Mar 11, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 11, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2025
Rollup merge of rust-lang#137926 - Kobzol:lld-no-start-stop-test, r=lqd

Add a test for `-znostart-stop-gc` usage with LLD

This test replicates the behavior of https://github.com/dtolnay/linkme, to test that it still works even with LLD. Without `-znostart-stop-gc` the test fails.

r? ``@lqd``

try-job: x86_64-gnu
try-job: x86_64-msvc-1
@Kobzol Kobzol deleted the lld-no-start-stop-test branch March 11, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc 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. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants