Migrate `branch-protection-check-IBT` to rmake.rs by jieyouxu · Pull Request #134760 · rust-lang/rust

jieyouxu · 2024-12-25T12:34:37Z

The Makefile version never ran because of Makefile syntax confusion because ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64) compares x86 to x86_64, which always evaluates to false.
The test would've always failed because precompiled std is not built with -Z cf-protection=branch, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std.
Thus, the test input file is instead changed to a no_std program.
The test is currently limited to only x86_64-unknown-linux-gnu host, there are various other problems when the test is cross-compiled that I didn't want to fix atm, and is left as an exercise for the -Z cf-protection implementers.

The GNU property note was added by #110304 in order to address #103001.

Partially supersedes #129156.
The rmake.rs port was initially authored by @Rejyr in #126720.
This PR is co-authored with @Oneirical and @Rejyr.

r? @bjorn3 or reroll

try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
try-job: x86_64-msvc
try-job: x86_64-apple-1
try-job: x86_64-apple-2

…eck-IBT, r=<try> Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-msvc try-job: x86_64-apple

jieyouxu · 2024-12-25T12:39:47Z

@bors try

bors · 2024-12-25T12:40:56Z

⌛ Trying commit 5871cee with merge a90422e...

…eck-IBT, r=<try> Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-msvc try-job: x86_64-apple-1 try-job: x86_64-apple-2

bors · 2024-12-25T14:07:16Z

💔 Test failed - checks-actions

jieyouxu · 2024-12-25T14:09:38Z

... Right.

jieyouxu · 2024-12-26T10:56:08Z

@bors try

bors · 2024-12-26T10:57:19Z

⌛ Trying commit 5cd8653 with merge d4c0af3...

…eck-IBT, r=<try> Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. - Ignore cross-compile for now, because that will require us to build `core` for the cross-compile target (something like `minicore` will not suffice because we need to reach and go past the linking stage). The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-msvc try-job: x86_64-apple-1 try-job: x86_64-apple-2

bors · 2024-12-26T12:36:01Z

💔 Test failed - checks-actions

jieyouxu · 2024-12-26T13:21:24Z

Ok, what if I just try to produce a minimal executable...
@bors try

…eck-IBT, r=<try> Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. - Only specifically `x86_64-unknown-linux-gnu` for now. The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: x86_64-msvc try-job: x86_64-apple-1 try-job: x86_64-apple-2

bors · 2024-12-26T13:22:35Z

⌛ Trying commit 0734389 with merge 3d945a5...

bors · 2024-12-26T15:07:12Z

💔 Test failed - checks-actions

jieyouxu · 2024-12-27T16:41:38Z

Ok I gave up fighting for cross-compile scenarios, I made the test x86_64-unknown-linux-gnu host only for now.

@rustbot ready

lqd

If you're happy with the previous try builds,

with/without the couple nits below

tests/run-make/branch-protection-check-IBT/rmake.rs

- The Makefile version *never* ran because of Makefile syntax confusion. - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` + `no_core` program. Co-authored-by: Jerry Wang <jerrylwang123@gmail.com> Co-authored-by: Oneirical <manchot@videotron.ca>

jieyouxu · 2024-12-28T04:03:34Z

The current formulation of the test only runs on x86_64-unknown-linux-gnu host. However, the original test simply never ran due to the Makefile being broken. So this is still a net gain in overall test coverage (lol). As for how to expand test coverage of this test, that is left to -Z cf-protection=branch / Intel CET (cf. #93754) implementors as an exercise.

@bors r=@lqd rollup

bors · 2024-12-28T04:03:37Z

📌 Commit b32591e has been approved by lqd

It is now in the queue for this repository.

Rollup of 5 pull requests Successful merges: - rust-lang#134737 (Implement `default_overrides_default_fields` lint) - rust-lang#134760 (Migrate `branch-protection-check-IBT` to rmake.rs) - rust-lang#134829 (Migrate `libs-through-symlink` to rmake.rs) - rust-lang#134832 (Update `compiler-builtins` to 0.1.140) - rust-lang#134840 (compiletest: Only pass the post-colon value to `parse_normalize_rule`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134760 - jieyouxu:enable-branch-protection-check-IBT, r=lqd Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. - The test is currently limited to only `x86_64-unknown-linux-gnu` host, there are various other problems when the test is cross-compiled that I didn't want to fix atm, and is left as an exercise for the `-Z cf-protection` implementers. The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: x86_64-msvc try-job: x86_64-apple-1 try-job: x86_64-apple-2

…check-IBT, r=lqd Migrate `branch-protection-check-IBT` to rmake.rs - The Makefile version *never* ran because of Makefile syntax confusion because `ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)` [compares `x86` to `x86_64`, which always evaluates to false](rust-lang#126720 (comment)). - The test would've always failed because precompiled std is not built with `-Z cf-protection=branch`, but linkers require all input object files to indicate IBT support in order to enable IBT for the executable, which is not the case for std. - Thus, the test input file is instead changed to a `no_std` program. - The test is currently limited to only `x86_64-unknown-linux-gnu` host, there are various other problems when the test is cross-compiled that I didn't want to fix atm, and is left as an exercise for the `-Z cf-protection` implementers. The GNU property note was added by rust-lang#110304 in order to address rust-lang#103001. Partially supersedes rust-lang#129156. The rmake.rs port was initially authored by `@Rejyr` in rust-lang#126720. This PR is co-authored with `@Oneirical` and `@Rejyr.` r? `@bjorn3` or reroll try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: x86_64-msvc try-job: x86_64-apple-1 try-job: x86_64-apple-2

Rollup of 5 pull requests Successful merges: - rust-lang#134737 (Implement `default_overrides_default_fields` lint) - rust-lang#134760 (Migrate `branch-protection-check-IBT` to rmake.rs) - rust-lang#134829 (Migrate `libs-through-symlink` to rmake.rs) - rust-lang#134832 (Update `compiler-builtins` to 0.1.140) - rust-lang#134840 (compiletest: Only pass the post-colon value to `parse_normalize_rule`) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned bjorn3 Dec 25, 2024

jieyouxu 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 Dec 25, 2024

This comment was marked as outdated.

Sign in to view

This comment was marked as off-topic.

Sign in to view

jieyouxu mentioned this pull request Dec 25, 2024

Tracking Issue for porting run-make tests to use Rust #121876

Closed

This comment has been minimized.

Sign in to view

jieyouxu force-pushed the enable-branch-protection-check-IBT branch from 5871cee to 5cd8653 Compare December 26, 2024 10:55

This comment has been minimized.

Sign in to view

jieyouxu force-pushed the enable-branch-protection-check-IBT branch from 5cd8653 to 0734389 Compare December 26, 2024 13:13

This comment has been minimized.

Sign in to view

jieyouxu force-pushed the enable-branch-protection-check-IBT branch from 0734389 to 4b4bc52 Compare December 27, 2024 16:37

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 Dec 27, 2024

lqd approved these changes Dec 27, 2024

View reviewed changes

tests/run-make/branch-protection-check-IBT/rmake.rs Outdated Show resolved Hide resolved

tests/run-make/branch-protection-check-IBT/rmake.rs Show resolved Hide resolved

tests/run-make/branch-protection-check-IBT/rmake.rs Outdated Show resolved Hide resolved

jieyouxu assigned lqd and unassigned bjorn3 Dec 28, 2024

jieyouxu force-pushed the enable-branch-protection-check-IBT branch from 4b4bc52 to b32591e Compare December 28, 2024 03:58

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 Dec 28, 2024

Zalathar mentioned this pull request Dec 28, 2024

Rollup of 5 pull requests #134844

Merged

bors merged commit dd03fba into rust-lang:master Dec 28, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 28, 2024

jieyouxu deleted the enable-branch-protection-check-IBT branch December 28, 2024 11:17

jieyouxu mentioned this pull request Jan 21, 2025

run-make: Delete cat-and-grep-sanity-check and restrict branch-protection-check-IBT to stable #129156

Closed

Uh oh!

Conversation

jieyouxu commented Dec 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as off-topic.

jieyouxu commented Dec 25, 2024

Uh oh!

bors commented Dec 25, 2024

Uh oh!

This comment has been minimized.

bors commented Dec 25, 2024

Uh oh!

jieyouxu commented Dec 25, 2024

Uh oh!

jieyouxu commented Dec 26, 2024

Uh oh!

bors commented Dec 26, 2024

Uh oh!

This comment has been minimized.

bors commented Dec 26, 2024

Uh oh!

jieyouxu commented Dec 26, 2024

Uh oh!

bors commented Dec 26, 2024

Uh oh!

This comment has been minimized.

bors commented Dec 26, 2024

Uh oh!

jieyouxu commented Dec 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lqd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jieyouxu commented Dec 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Dec 28, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jieyouxu commented Dec 25, 2024 •

edited

Loading

jieyouxu commented Dec 27, 2024 •

edited

Loading

jieyouxu commented Dec 28, 2024 •

edited

Loading