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.

Sign up for GitHub

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

Jump to bottom

Add GNU Property Note #110304

Merged
merged 4 commits into from
May 9, 2023
Merged

Add GNU Property Note #110304

merged 4 commits into from
May 9, 2023

Conversation

cchiw
Copy link

@cchiw cchiw commented Apr 14, 2023 •
edited
Loading

Fix #103001

Generates the missing property note:

Displaying notes found in: .note.gnu.property
  Owner                Data size 	Description
  GNU                  0x00000010	NT_GNU_PROPERTY_TYPE_0	      Properties: x86 feature: IBT

@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added 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 Apr 14, 2023
@cchiw cchiw mentioned this pull request Apr 14, 2023
Closed
davidtwco
davidtwco approved these changes Apr 17, 2023
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me, r=me after resolving my and @bjorn3's comments.

compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved
DanielKristofKiss
DanielKristofKiss reviewed Apr 18, 2023
View reviewed changes
Copy link

@DanielKristofKiss DanielKristofKiss left a comment

Choose a reason for hiding this comment

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

Hi all,
I'm from Arm and worked on PAC/BTI support in specs and LLVM.
Let some comment.

compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved
@cchiw cchiw mentioned this pull request Apr 20, 2023
Merged
@cchiw cchiw force-pushed the master branch 6 times, most recently from f5812f7 to 1195d22 Compare April 21, 2023 20:26
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member

jyn514 commented Apr 26, 2023

Can we test this somehow? Maybe a run-make test with // only-x86_64 which runs objdump on the generated binary to make sure it has this note?

@jyn514 jyn514 added A-linkage Area: linking into static, shared libraries and binaries A-security Area: Security related issues (example: address space layout randomization) labels Apr 26, 2023
@stephenhines
Copy link

Can we test this somehow? Maybe a run-make test with // only-x86_64 which runs objdump on the generated binary to make sure it has this note?

@cchiw is OOO this week, but I think that is a reasonable request. I'm assuming writing a test like rust/tests/run-make/cross-lang-lto-clang/ is what you intend.

@jyn514
Copy link
Member

jyn514 commented Apr 27, 2023

That sounds perfect, thank you!

@jyn514 jyn514 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 Apr 27, 2023
@fweimer-rh
Copy link

Can we test this somehow? Maybe a run-make test with // only-x86_64 which runs objdump on the generated binary to make sure it has this note?

llvm-readelf -n is what decodes these notes, not objdump.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cchiw
Copy link
Author

cchiw commented May 8, 2023 •
edited
Loading

Can we test this somehow? Maybe a run-make test with // only-x86_64 which runs objdump on the generated binary to make sure it has this note?

Great. This is addressed now. Thanks for the feedback.

@jyn514
Copy link
Member

jyn514 commented May 8, 2023

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented May 8, 2023

📌 Commit 37f3e2f has been approved by davidtwco

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 May 8, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request May 9, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 9, 2023
@bors
Auto merge of rust-lang#111380 - Dylan-DPC:rollup-xiptbhn, r=Dylan-DPC
ecd3dba 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#110304 (Add GNU Property Note)
 - rust-lang#110504 (Tweak borrow suggestion span)
 - rust-lang#110583 (tweak "make mut" spans when assigning to locals)
 - rust-lang#110694 (Implement builtin # syntax and use it for offset_of!(...))
 - rust-lang#111120 (Suggest let for possible binding with ty)
 - rust-lang#111252 (Min specialization improvements)
 - rust-lang#111361 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 02a85bd into rust-lang:master May 9, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 9, 2023
@cchiw
Copy link
Author

cchiw commented May 9, 2023

Hi, I wanted to learn generally about when crates get pulled into upstream. I guess more specifically when do you think the Object crate will be pulled in? The change I made for the Object crate will be included in their next release.

@cuviper
Copy link
Member

cuviper commented May 9, 2023

Dependency updates are ad hoc, as needed. (for better or worse)

You can open a PR to update that in the rust lockfiles once it's ready, with justification of course.

@Rejyr Rejyr mentioned this pull request Jun 20, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 23, 2024
@matthiaskrgr
Rollup merge of rust-lang#126720 - Rejyr:migrate-branch-protection-rm…
4a94e80 
…ake, 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`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 23, 2024
@rust-timer
Unrolled build for rust-lang#126720
f55382b 
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`
@jieyouxu jieyouxu mentioned this pull request Aug 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuviper cuviper cuviper left review comments

@lqd lqd lqd left review comments

@DanielKristofKiss DanielKristofKiss DanielKristofKiss left review comments

@bjorn3 bjorn3 bjorn3 left review comments

@stephenhines stephenhines stephenhines left review comments

@jyn514 jyn514 jyn514 left review comments

@aDotInTheVoid aDotInTheVoid aDotInTheVoid left review comments

@fweimer-rh fweimer-rh fweimer-rh left review comments

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
A-linkage Area: linking into static, shared libraries and binaries A-security Area: Security related issues (example: address space layout randomization) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

Synthetic object files disable control flow protection features