[llvm][llvm-dis] Fix 'llvm-dis' with '--materialize-metadata --show-annotations' crashes

[t-baydyusenov]

Added handling the case of a non-materialized module, also don't call printInfoComment for immaterializable values

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Timur Baydyusenov (t-baydyusenov)

Changes

Added handling the case of a non-materialized module

---

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

3 Files Affected:

• (modified) llvm/lib/IR/AsmWriter.cpp (+6-7)
• (modified) llvm/test/Assembler/metadata-annotations.ll (+18-4)
• (modified) llvm/tools/llvm-dis/llvm-dis.cpp (+15)

diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 0c8565c927a24..4d4ffe93a8067 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2931,7 +2931,7 @@ class AssemblyWriter {
 
   // printInfoComment - Print a little comment after the instruction indicating
   // which slot it occupies.
-  void printInfoComment(const Value &V);
+  void printInfoComment(const Value &V, bool isMaterializable = false);
 
   // printGCRelocateComment - print comment after call to the gc.relocate
   // intrinsic indicating base and derived pointer names.
@@ -3963,7 +3963,7 @@ void AssemblyWriter::printGlobal(const GlobalVariable *GV) {
   if (Attrs.hasAttributes())
     Out << " #" << Machine.getAttributeGroupSlot(Attrs);
 
-  printInfoComment(*GV);
+  printInfoComment(*GV, GV->isMaterializable());
 }
 
 void AssemblyWriter::printAlias(const GlobalAlias *GA) {
@@ -4001,7 +4001,7 @@ void AssemblyWriter::printAlias(const GlobalAlias *GA) {
     Out << '"';
   }
 
-  printInfoComment(*GA);
+  printInfoComment(*GA, GA->isMaterializable());
   Out << '\n';
 }
 
@@ -4040,7 +4040,7 @@ void AssemblyWriter::printIFunc(const GlobalIFunc *GI) {
     printMetadataAttachments(MDs, ", ");
   }
 
-  printInfoComment(*GI);
+  printInfoComment(*GI, GI->isMaterializable());
   Out << '\n';
 }
 
@@ -4319,13 +4319,12 @@ void AssemblyWriter::printGCRelocateComment(const GCRelocateInst &Relocate) {
 
 /// printInfoComment - Print a little comment after the instruction indicating
 /// which slot it occupies.
-void AssemblyWriter::printInfoComment(const Value &V) {
+void AssemblyWriter::printInfoComment(const Value &V, bool isMaterializable) {
   if (const auto *Relocate = dyn_cast<GCRelocateInst>(&V))
     printGCRelocateComment(*Relocate);
 
-  if (AnnotationWriter) {
+  if (AnnotationWriter && !isMaterializable)
     AnnotationWriter->printInfoComment(V, Out);
-  }
 
   if (PrintInstDebugLocs) {
     if (auto *I = dyn_cast<Instruction>(&V)) {
diff --git a/llvm/test/Assembler/metadata-annotations.ll b/llvm/test/Assembler/metadata-annotations.ll
index 4fd471338cd0a..2a08a17849dbd 100644
--- a/llvm/test/Assembler/metadata-annotations.ll
+++ b/llvm/test/Assembler/metadata-annotations.ll
@@ -1,9 +1,23 @@
 ; RUN: llvm-as < %s | llvm-dis --materialize-metadata --show-annotations | FileCheck %s
 
+; CHECK: @global_var = global i32 1
+; CHECK: @alias = alias i32, ptr @global_var
+; CHECK: @ifunc = ifunc i32 (), ptr @ifunc_resolver
+@global_var = global i32 1
+@alias = alias i32, ptr @global_var
+@ifunc = ifunc i32 (), ptr @ifunc_resolver
+
+; CHECK: ; Materializable
+; CHECK-NEXT: define ptr @ifunc_resolver() {}
+define ptr @ifunc_resolver() {
+  ret ptr @defined_function
+}
+
 ; CHECK: ; Materializable
-; CHECK-NEXT: define dso_local i32 @test() {}
-define dso_local i32 @test() {
-entry:
-  ret i32 0
+; CHECK-NEXT: define void @defined_function() {}
+define void @defined_function() {
+  ret void
 }
 
+; CHECK: declare void @declared_function()
+declare void @declared_function()
diff --git a/llvm/tools/llvm-dis/llvm-dis.cpp b/llvm/tools/llvm-dis/llvm-dis.cpp
index 35c540963a487..d6ef11b5c8ad9 100644
--- a/llvm/tools/llvm-dis/llvm-dis.cpp
+++ b/llvm/tools/llvm-dis/llvm-dis.cpp
@@ -101,13 +101,28 @@ static void printDebugLoc(const DebugLoc &DL, formatted_raw_ostream &OS) {
   }
 }
 class CommentWriter : public AssemblyAnnotationWriter {
+private:
+  bool canSafelyAccessUses(const Value &V) {
+    // Can't safely access uses, if module not materialized.
+    if (!isa<GlobalValue>(&V))
+      return true;
+    const GlobalValue *GV = cast<GlobalValue>(&V);
+    return GV->getParent() && GV->getParent()->isMaterialized();
+  }
+
 public:
   void emitFunctionAnnot(const Function *F,
                          formatted_raw_ostream &OS) override {
+    if (!canSafelyAccessUses(*F))
+      return;
+
     OS << "; [#uses=" << F->getNumUses() << ']';  // Output # uses
     OS << '\n';
   }
   void printInfoComment(const Value &V, formatted_raw_ostream &OS) override {
+    if (!canSafelyAccessUses(V))
+      return;
+
     bool Padded = false;
     if (!V.getType()->isVoidTy()) {
       OS.PadToColumn(50);

[arsenm]

Suggested change

	if (!isa<GlobalValue>(&V))
	return true;
	const GlobalValue *GV = cast<GlobalValue>(&V);
	return GV->getParent() && GV->getParent()->isMaterialized();
	const GlobalValue *GV = dyn_cast<GlobalValue>(&V);
	return !GV || (GV->getParent() && GV->getParent()->isMaterialized());

[t-baydyusenov]

Addressed