[RFC] Support `.comment` section like GCC/Clang (`!llvm.ident`) by ojeda · Pull Request #97550 · rust-lang/rust

@rustbot rustbot added the T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

label

May 30, 2022

@JohnCSimon JohnCSimon added the needs-fcp

This change is insta-stable, or significant enough to need a team FCP to proceed.

label

Jul 3, 2022

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jan 20, 2023
[RFC] Support `.comment` section like GCC/Clang (`!llvm.ident`)

Both GCC and Clang write by default a `.comment` section with compiler information:

```txt
$ gcc -c -xc /dev/null && readelf -p '.comment' null.o

String dump of section '.comment':
  [     1]  GCC: (GNU) 11.2.0

$ clang -c -xc /dev/null && readelf -p '.comment' null.o

String dump of section '.comment':
  [     1]  clang version 14.0.1 (https://github.com/llvm/llvm-project.git c62053979489ccb002efe411c3af059addcb5d7d)
```

They also implement the `-Qn` flag to avoid doing so:

```txt
$ gcc -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!

$ clang -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!
```

So far, `rustc` only does it for WebAssembly targets and only when debug info is enabled:

```txt
$ echo 'fn main(){}' | rustc --target=wasm32-unknown-unknown --emit=llvm-ir -Cdebuginfo=2 - && grep llvm.ident rust_out.ll
!llvm.ident = !{!27}
```

The RFC part of this PR is about which behavior should `rustc` follow:
  - Always add it.
  - Add it by default, i.e. have an opt-out flag (GCC, Clang).
  - Have an opt-in flag.
  - Never add it (current).

There is also the question of whether debug info being enabled matters for that decision, given the current behavior of WebAssembly targets.

For instance, adding it by default gets us closer to other popular compilers, but that may surprise some users with an information leak. The most conservative option is to only do so opt-in, even if debug info is enabled (some users may be stripping debug info and not expecting something else to be leaked elsewhere).

Implementation-wise, this covers both `ModuleLlvm::new()` and `ModuleLlvm::new_metadata()` cases by moving the addition to `context::create_module` and adds a few test cases.

ThinLTO also sees the `llvm.ident` named metadata duplicated (in temporary outputs), so this deduplicates it like it is done for `wasm.custom_sections`. The tests also check this duplication does not take place.

@bors bors 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-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Jan 21, 2023

cuviper

@ojeda

Both GCC and Clang write by default a `.comment` section with compiler
information:

```txt
$ gcc -c -xc /dev/null && readelf -p '.comment' null.o

String dump of section '.comment':
  [     1]  GCC: (GNU) 11.2.0

$ clang -c -xc /dev/null && readelf -p '.comment' null.o

String dump of section '.comment':
  [     1]  clang version 14.0.1 (https://github.com/llvm/llvm-project.git c62053979489ccb002efe411c3af059addcb5d7d)
```

They also implement the `-Qn` flag to avoid doing so:

```txt
$ gcc -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!

$ clang -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!
```

So far, `rustc` only does it for WebAssembly targets and only
when debug info is enabled:

```txt
$ echo 'fn main(){}' | rustc --target=wasm32-unknown-unknown --emit=llvm-ir -Cdebuginfo=2 - && grep llvm.ident rust_out.ll
!llvm.ident = !{!27}
```

In the RFC part of this PR it was decided to always add
the information, which gets us closer to other popular compilers.
An opt-out flag like GCC and Clang may be added later on if deemed
necessary.

Implementation-wise, this covers both `ModuleLlvm::new()` and
`ModuleLlvm::new_metadata()` cases by moving the addition to
`context::create_module` and adds a few test cases.

ThinLTO also sees the `llvm.ident` named metadata duplicated (in
temporary outputs), so this deduplicates it like it is done for
`wasm.custom_sections`. The tests also check this duplication does
not take place.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

@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

Jul 21, 2023

@ojeda ojeda deleted the comment-section branch

July 21, 2023 23:48

@cuviper cuviper added the relnotes

Marks issues that should be documented in the release notes of the next release.

label

Jul 21, 2023

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Oct 6, 2023

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Nov 16, 2023

@ojeda ojeda mentioned this pull request

Jan 19, 2024

67 tasks