[ThinLTOBitcodeWriter] Do not crash on a typed declaration

[igorkudrin]

This fixes a crash when splitAndWriteThinLTOBitcode() hits a declaration with type metadata. For example, such declarations can be generated by the EliminateAvailableExternally pass.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-lto

Author: Igor Kudrin (igorkudrin)

Changes

This fixes a crash when splitAndWriteThinLTOBitcode() hits a declaration with type metadata. For example, such declarations can be generated by the EliminateAvailableExternally pass.

---

Full diff: https://github.com/llvm/llvm-project/pull/69564.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp (+1-1)
• (added) llvm/test/Transforms/ThinLTOBitcodeWriter/split-typed-decl.ll (+12)

diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index fc1e70b1b3d3d81..98ef2065d6d9492 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -329,7 +329,7 @@ void splitAndWriteThinLTOBitcode(
   // comdat in MergedM to keep the comdat together.
   DenseSet<const Comdat *> MergedMComdats;
   for (GlobalVariable &GV : M.globals())
-    if (HasTypeMetadata(&GV)) {
+    if (!GV.isDeclaration() && HasTypeMetadata(&GV)) {
       if (const auto *C = GV.getComdat())
         MergedMComdats.insert(C);
       forEachVirtualFunction(GV.getInitializer(), [&](Function *F) {
diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-typed-decl.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-typed-decl.ll
new file mode 100644
index 000000000000000..033d2d0cbd50b39
--- /dev/null
+++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-typed-decl.ll
@@ -0,0 +1,12 @@
+;; Generating bitcode files with split LTO modules should not crash if there are
+;; typed declarations in sources.
+
+; RUN: opt -passes=elim-avail-extern --thinlto-bc --thinlto-split-lto-unit -o - %s
+
+@_ZTV3Foo = available_externally constant { [3 x ptr] } zeroinitializer, align 8, !type !0
+
+define void @Bar() {
+  ret void
+}
+
+!0 = !{i64 16, !"_ZTS3Foo"}

[teresajohnson]

This fixes a crash when splitAndWriteThinLTOBitcode() hits a declaration with type metadata. For example, such declarations can be generated by the EliminateAvailableExternally pass.

Normally we explicitly disable the EliminateAvailableExternally pass before writing ThinLTO bitcode as we do not want these eliminated before LTO (see

llvm-project/llvm/lib/Passes/PassBuilderPipelines.cpp

Lines 1304 to 1314 in 6b8a142

	// Remove avail extern fns and globals definitions since we aren't compiling
	// an object file for later LTO. For LTO we want to preserve these so they
	// are eligible for inlining at link-time. Note if they are unreferenced they
	// will be removed by GlobalDCE later, so this only impacts referenced
	// available externally globals. Eventually they will be suppressed during
	// codegen, but eliminating here enables more opportunity for GlobalDCE as it
	// may make globals referenced by available external functions dead and saves
	// running remaining passes on the eliminated functions. These should be
	// preserved during prelinking for link-time inlining decisions.
	if (!LTOPreLink)
	MPM.addPass(EliminateAvailableExternallyPass());

).

What configuration provoked this failure?

[igorkudrin]

What configuration provoked this failure?

The problem is probably not something that can be seen in the standard usage. We came across it while working on a downstream tool that runs some optimizations, including the LTO pipeline, and saves bitcode output files for later use. In our case, we can avoid the issue by excluding the EliminateAvailableExternally from the pipeline, but I think that the issue is worth fixing anyway, since the crash is triggered by a correct input IR.

[teresajohnson]

lgtm but a suggestion for simplifying the test