Auto merge of #97550 - ojeda:comment-section, r=bjorn3

[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.

Author: bors

compiler/rustc_codegen_llvm/src/context.rs

@@ -33,6 +33,7 @@ use rustc_target::abi::{
 use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel};
 use smallvec::SmallVec;
 
+use libc::c_uint;
 use std::cell::{Cell, RefCell};
 use std::ffi::CStr;
 use std::str;
@@ -349,6 +350,23 @@ pub unsafe fn create_module<'ll>(
         );
     }
 
+    // Insert `llvm.ident` metadata.
+    //
+    // On the wasm targets it will get hooked up to the "producer" sections
+    // `processed-by` information.
+    let rustc_producer =
+        format!("rustc version {}", option_env!("CFG_VERSION").expect("CFG_VERSION"));
+    let name_metadata = llvm::LLVMMDStringInContext(
+        llcx,
+        rustc_producer.as_ptr().cast(),
+        rustc_producer.as_bytes().len() as c_uint,
+    );
+    llvm::LLVMAddNamedMetadataOperand(
+        llmod,
+        cstr!("llvm.ident").as_ptr(),
+        llvm::LLVMMDNodeInContext(llcx, &name_metadata, 1),
+    );
+
     llmod
 }
 

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -888,21 +888,6 @@ pub fn build_compile_unit_di_node<'ll, 'tcx>(
             llvm::LLVMAddNamedMetadataOperand(debug_context.llmod, llvm_gcov_ident.as_ptr(), val);
         }
 
-        // Insert `llvm.ident` metadata on the wasm targets since that will
-        // get hooked up to the "producer" sections `processed-by` information.
-        if tcx.sess.target.is_like_wasm {
-            let name_metadata = llvm::LLVMMDStringInContext(
-                debug_context.llcontext,
-                rustc_producer.as_ptr().cast(),
-                rustc_producer.as_bytes().len() as c_uint,
-            );
-            llvm::LLVMAddNamedMetadataOperand(
-                debug_context.llmod,
-                cstr!("llvm.ident").as_ptr(),
-                llvm::LLVMMDNodeInContext(debug_context.llcontext, &name_metadata, 1),
-            );
-        }
-
         return unit_metadata;
     };
 

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

@@ -1366,6 +1366,11 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M,
     if (WasmCustomSections)
       WasmCustomSections->eraseFromParent();
 
+    // `llvm.ident` named metadata also gets duplicated.
+    auto *llvmIdent = (*MOrErr)->getNamedMetadata("llvm.ident");
+    if (llvmIdent)
+      llvmIdent->eraseFromParent();
+
     return MOrErr;
   };
   bool ClearDSOLocal = clearDSOLocalOnDeclarations(Mod, Target);

tests/codegen/llvm-ident.rs

@@ -0,0 +1,15 @@
+// Verifies that the `!llvm.ident` named metadata is emitted.
+//
+// revisions: NONE OPT DEBUG
+//
+// [OPT] compile-flags: -Copt-level=2
+// [DEBUG] compile-flags: -Cdebuginfo=2
+
+// The named metadata should contain a single metadata node (see
+// `LLVMRustPrepareThinLTOImport` for details).
+// CHECK: !llvm.ident = !{![[ID:[0-9]+]]}
+
+// In addition, check that the metadata node has the expected content.
+// CHECK: ![[ID]] = !{!"rustc version 1.{{.*}}"}
+
+fn main() {}

tests/run-make/comment-section/Makefile

@@ -0,0 +1,15 @@
+include ../tools.mk
+
+# only-linux
+
+all:
+	echo 'fn main(){}' | $(RUSTC) - --emit=link,obj -Csave-temps --target=$(TARGET)
+
+	# Check linked output has a `.comment` section with the expected content.
+	readelf -p '.comment' $(TMPDIR)/rust_out | $(CGREP) -F 'rustc version 1.'
+
+	# Check all object files (including temporary outputs) have a `.comment`
+	# section with the expected content.
+	set -e; for f in $(TMPDIR)/*.o; do \
+		readelf -p '.comment' $$f | $(CGREP) -F 'rustc version 1.'; \
+	done

tests/run-make/llvm-ident/Makefile

@@ -0,0 +1,19 @@
+include ../tools.mk
+
+# only-linux
+
+all:
+	# `-Ccodegen-units=16 -Copt-level=2` is used here to trigger thin LTO
+	# across codegen units to test deduplication of the named metadata
+	# (see `LLVMRustPrepareThinLTOImport` for details).
+	echo 'fn main(){}' | $(RUSTC) - --emit=link,obj -Csave-temps -Ccodegen-units=16 -Copt-level=2 --target=$(TARGET)
+
+	# `llvm-dis` is used here since `--emit=llvm-ir` does not emit LLVM IR
+	# for temporary outputs.
+	"$(LLVM_BIN_DIR)"/llvm-dis $(TMPDIR)/*.bc
+
+	# Check LLVM IR files (including temporary outputs) have `!llvm.ident`
+	# named metadata, reusing the related codegen test.
+	set -e; for f in $(TMPDIR)/*.ll; do \
+		$(LLVM_FILECHECK) --input-file $$f ../../codegen/llvm-ident.rs; \
+	done