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

Ignore branch-protection-check-IBT run-make test #126720

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

Rejyr
Copy link
Contributor

@Rejyr Rejyr commented Jun 20, 2024

The old Makefile implementation (#110304) had an improper comparison which caused the test to never run. However, both the updated Makefile implementation and the rmake implementation fail (missing .note.gnu.property). This could be a bug in the original implementation or test flakiness.

Edit: Manually recreating the test case shows that .note.gnu.property does not appear in nightly.

// main.rs
fn main() {
    println!("hello world");
}
$ rustc +nightly -V
rustc 1.81.0-nightly (c1b336cb6 2024-06-21)
$ rustc +stable -V
rustc 1.79.0 (129f3b996 2024-06-10)
$ rustc +nightly -Zcf-protection=branch -Clink-args=-nostartfiles -Csave-temps "-L$PWD" main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000008       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: bcae34e6431b2a37

Compiling without the other flags still does not show .note.gnu.property.

$ rustc +nightly main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.ABI-tag
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 4.4.0

Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000008       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: d60d5f108b63bf3a

Compiling on stable shows .note.gnu.property.

$ rustc +stable main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.gnu.property
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0 (property note)
    Properties:    x86 ISA needed: x86-64-baseline


Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 4a494eb578123314e6ff1caf1c8877e27004664f

Displaying notes found in: .note.ABI-tag
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 4.4.0

Part of #121876.

r? @jieyouxu

@rustbot rustbot added 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-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) labels Jun 20, 2024
# only-x86_64

all:
ifeq ($(filter x86,$(LLVM_COMPONENTS)),x86_64)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This compares x86 to x86_64, which always evaluates to false.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Rejyr
Copy link
Contributor Author

Rejyr commented Jun 22, 2024

@jieyouxu This seems to be an error in the implementation. Should this be marked with a FIXME with the test case ignored?

@jieyouxu
Copy link
Member

jieyouxu commented Jun 22, 2024

@Rejyr yes, can you drop the support library changes but keep both the Makefile and rmake.rs changes, and add an # ignore-test or //@ ignore-test directive on both (with a FIXME explaining the problem)? I'll take a closer look separately next week (i.e. let's merge this PR to ignore the not-doing-anything test first, then we can try to fix 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 Jun 22, 2024
@Rejyr Rejyr force-pushed the migrate-branch-protection-rmake branch from bc6f8fa to 1fa70ad Compare June 23, 2024 00:17
@Rejyr Rejyr force-pushed the migrate-branch-protection-rmake branch from 1fa70ad to 75a9379 Compare June 23, 2024 00:18
@jieyouxu
Copy link
Member

Thanks, I can r+ this once you are ready to take it out of draft.

@Rejyr Rejyr marked this pull request as ready for review June 23, 2024 00:37
@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2024

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

@jieyouxu
Copy link
Member

Thanks!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 23, 2024

📌 Commit 75a9379 has been approved by jieyouxu

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 23, 2024
@jieyouxu jieyouxu changed the title (wip) Migrate branch-protection-check-IBT to rmake Ignore branch-protection-check-IBT run-make test Jun 23, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 23, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#126720 (Ignore `branch-protection-check-IBT` run-make test)
 - rust-lang#126779 (Try to clarify the confusingly-named `RustDev` and `RustcDev` steps)
 - rust-lang#126782 (Support absolute source paths in bootstrap)
 - rust-lang#126783 (Fix issue number for the `tcplistener_into_incoming` feature)
 - rust-lang#126843 (Allow "C-unwind" fn to have C variadics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4a94e80 into rust-lang:master Jun 23, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 23, 2024
Rollup merge of rust-lang#126720 - Rejyr:migrate-branch-protection-rmake, r=jieyouxu

Ignore `branch-protection-check-IBT` run-make test

The old Makefile implementation (rust-lang#110304) had an improper comparison which caused the test to never run. However, both the updated Makefile implementation and the rmake implementation fail (missing `.note.gnu.property`). This could be a bug in the original implementation or test flakiness.

Edit: Manually recreating the test case shows that `.note.gnu.property` does not appear in nightly.
```rust
// main.rs
fn main() {
    println!("hello world");
}
```
```sh
$ rustc +nightly -V
rustc 1.81.0-nightly (c1b336c 2024-06-21)
$ rustc +stable -V
rustc 1.79.0 (129f3b9 2024-06-10)
```
```sh
$ rustc +nightly -Zcf-protection=branch -Clink-args=-nostartfiles -Csave-temps "-L$PWD" main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000008       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: bcae34e6431b2a37
```
Compiling without the other flags still does not show `.note.gnu.property`.
```sh
$ rustc +nightly main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.ABI-tag
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 4.4.0

Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000008       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: d60d5f108b63bf3a
```
Compiling on stable shows `.note.gnu.property`.
```sh
$ rustc +stable main.rs -o main
$ llvm-readobj --elf-output-style=GNU -nW main
Displaying notes found in: .note.gnu.property
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0 (property note)
    Properties:    x86 ISA needed: x86-64-baseline

Displaying notes found in: .note.gnu.build-id
  Owner                Data size        Description
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 4a494eb578123314e6ff1caf1c8877e27004664f

Displaying notes found in: .note.ABI-tag
  Owner                Data size        Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 4.4.0
```

Part of rust-lang#121876.

r? `@jieyouxu`
@Rejyr Rejyr deleted the migrate-branch-protection-rmake branch June 23, 2024 14:37
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
run-make: Delete `cat-and-grep-sanity-check` and restrict `branch-protection-check-IBT` to stable

Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

First, this PR deletes the now useless `cat-and-grep-sanity-check` test.

Second, it revisits the `branch-protection-check-IBT` test, which was disabled due to a nonsensical `llvm_components` check. rust-lang#126720 states that the test does work on stable rustc, so let's check this: added `//@ only-stable`.

If this works, some of the FIXME and commented-out lines will need cleanup before merging.

Please try:

try-job: x86_64-gnu-stable
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
run-make: Delete `cat-and-grep-sanity-check` and restrict `branch-protection-check-IBT` to stable

Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

First, this PR deletes the now useless `cat-and-grep-sanity-check` test.

Second, it revisits the `branch-protection-check-IBT` test, which was disabled due to a nonsensical `llvm_components` check. rust-lang#126720 states that the test does work on stable rustc, so let's check this: added `//@ only-stable`.

If this works, some of the FIXME and commented-out lines will need cleanup before merging.

Please try:

try-job: x86_64-gnu-stable
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)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants