Verifier: Add checks for associated metadata

[arsenm]

Summary

Also add missing assembler test for the valid cases.

[arsenm]

ping

[eugenis]

LGTM with a comment

llvm/lib/IR/Verifier.cpp

665

It looks like the !{null} case will print both "must have a global value" and "must be ValueAsMetadata". We probably want just the first one.

[arsenm]

llvm/lib/IR/Verifier.cpp

665

Looking at the test, it doesn't actually hit this one. I'm not actually sure how to construct a not-ValueAsMetadata that would reach here with text IR

[arsenm]

llvm/lib/IR/Verifier.cpp

665

Oh, I can just use a string here

[arsenm]

Before submitting I noticed the test added in 9aff829f78331ef3718bf73ea81d319c91730808 is failing on the reference to self. The test looks like a mistake to me. The commit message suggests this could have been anything. I also don't understand the test, since it's linking a single IR file (Is it missing an input?)

llvm/lib/IR/Verifier.cpp

665

I couldn't come up with a test where this happened. We also tend to emit every verifier error possible

[arsenm]

Fix linker test that had a global associate to itself, add more comprehensive link test

[eugenis]

LGTM, thank you!

[arsenm]

87f2e9448e82bbed4ac59bb61bea03256aa5f4de

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 }
 
 static const MCSymbolELF *getLinkedToSymbol(const GlobalObject *GO,
                                             const TargetMachine &TM) {
   MDNode *MD = GO->getMetadata(LLVMContext::MD_associated);
   if (!MD)
     return nullptr;
 
-  const MDOperand &Op = MD->getOperand(0);
-  if (!Op.get())
-    return nullptr;
-
-  auto *VM = dyn_cast<ValueAsMetadata>(Op);
-  if (!VM)
-    report_fatal_error("MD_associated operand is not ValueAsMetadata");
-
+  auto *VM = cast<ValueAsMetadata>(MD->getOperand(0).get());
   auto *OtherGV = dyn_cast<GlobalValue>(VM->getValue());
   return OtherGV ? dyn_cast<MCSymbolELF>(TM.getSymbol(OtherGV)) : nullptr;
 }
 
 static unsigned getEntrySizeForKind(SectionKind Kind) {
   if (Kind.isMergeable1ByteCString())
     return 1;
   else if (Kind.isMergeable2ByteCString())

llvm/lib/IR/Verifier.cpp

Check(!GV.isDeclaration() || GV.hasValidDeclarationLinkage(),
         "Global is external, but doesn't have external or weak linkage!", &GV);
 
   if (const GlobalObject *GO = dyn_cast<GlobalObject>(&GV)) {
 
     if (MaybeAlign A = GO->getAlign()) {
       Check(A->value() <= Value::MaximumAlignment,
             "huge alignment values are unsupported", GO);
     }
+
+    if (const MDNode *Associated =
+            GO->getMetadata(LLVMContext::MD_associated)) {
+      Check(Associated->getNumOperands() == 1,
+            "associated metadata must have one operand", &GV, Associated);
+      const Metadata *Op = Associated->getOperand(0).get();
+      Check(Op, "associated metadata must have a global value", GO, Associated);
+
+      const auto *VM = dyn_cast_or_null<ValueAsMetadata>(Op);
+      Check(VM, "associated metadata must be ValueAsMetadata", GO, Associated);
+      if (VM) {
+        Check(isa<PointerType>(VM->getValue()->getType()),
+              "associated value must be pointer typed", GV, Associated);
+
+        const Value *Stripped = VM->getValue()->stripPointerCastsAndAliases();
+        Check(isa<GlobalObject>(Stripped) || isa<Constant>(Stripped),
+              "associated metadata must point to a GlobalObject", GO, Stripped);
+        Check(Stripped != GO,
+              "global values should not associate to themselves", GO,
+              Associated);
+      }
+    }
   }
   Check(!GV.hasAppendingLinkage() || isa<GlobalVariable>(GV),
         "Only global variables can have appending linkage!", &GV);
 
   if (GV.hasAppendingLinkage()) {
     const GlobalVariable *GVar = dyn_cast<GlobalVariable>(&GV);
     Check(GVar && GVar->getValueType()->isArrayTy(),
           "Only global arrays can have appending linkage!", GVar);

llvm/test/Assembler/associated-metadata.ll

+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
[e-mail] = external constant [8 x i8]
[e-mail] = constant [8 x i8] zeroinitializer
+
[e-mail] = external addrspace(1) constant [8 x i8], !associated !0
[e-mail] = external addrspace(1) constant [8 x i8], !associated !1
+
[e-mail] = external addrspace(1) constant [8 x i8], !associated !2
[e-mail] = external addrspace(1) constant [8 x i8], !associated !3
+
+@alias = alias i32, ptr @gv.def
+
[e-mail] = external addrspace(1) constant [8 x i8], !associated !4
+
[e-mail] = external addrspace(1) constant [8 x i8], !associated !5
[e-mail] = alias i32, ptr addrspace(1) addrspacecast (ptr @gv.def to ptr addrspace(1))
+
+
[e-mail] = external addrspace(1) constant [8 x i8], !associated !6
+
+@ifunc = dso_local ifunc i32 (i32), ptr @ifunc_resolver
[e-mail] = external constant [8 x i8], !associated !7
+
[e-mail] = external constant [8 x i8], !associated !8
[e-mail] = external constant [8 x i8], !associated !9
[e-mail] = external constant [8 x i8], !associated !10
[e-mail] = external constant [8 x i8], !associated !11
[e-mail] = external addrspace(1) constant [8 x i8], !associated !12
+
+
+;.
+; CHECK: @[[GV_DECL:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8]
+; CHECK: @[[GV_DEF:[a-zA-Z0-9_$"\\.-]+]] = constant [8 x i8] zeroinitializer
+; CHECK: @[[GV_ASSOCIATED_FUNC_DECL:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !0
+; CHECK: @[[GV_ASSOCIATED_FUNC_DEF:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !1
+; CHECK: @[[GV_ASSOCIATED_GV_DECL:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !2
+; CHECK: @[[GV_ASSOCIATED_GV_DEF:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !3
+; CHECK: @[[GV_ASSOCIATED_ALIAS_GV_DEF:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !4
+; CHECK: @[[GV_ASSOCIATED_ALIAS_ADDRSPACECAST:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !5
+; CHECK: @[[GV_DEF_ASSOCIATED_ADDRSPACECAST:[a-zA-Z0-9_$"\\.-]+]] = external addrspace(1) constant [8 x i8], !associated !6
+; CHECK: @[[GV_ASSOCIATED_IFUNC:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8], !associated !7
+; CHECK: @[[GV_ASSOCIATED_NULL:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8], !associated !8
+; CHECK: @[[GV_ASSOCIATED_INTTOPTR:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8], !associated !9
+; CHECK: @[[GV_ASSOCIATED_POISON:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8], !associated !10
+; CHECK: @[[GV_ASSOCIATED_UNDEF:[a-zA-Z0-9_$"\\.-]+]] = external constant [8 x i8], !associated !11
+; CHECK: @[[ALIAS:[a-zA-Z0-9_$"\\.-]+]] = alias i32, ptr @gv.def
+; CHECK: @[[ALIAS_ADDRSPACECAST:[a-zA-Z0-9_$"\\.-]+]] = alias i32, addrspacecast (ptr @gv.def to ptr addrspace(1))
+; CHECK: @[[IFUNC:[a-zA-Z0-9_$"\\.-]+]] = dso_local ifunc i32 (i32), ptr @ifunc_resolver
+;.
+define ptr @ifunc_resolver() {
+; CHECK-LABEL: @ifunc_resolver(
+; CHECK-NEXT:    ret ptr null
+;
+  ret ptr null
+}
+
+
+declare void @func.decl()
+define void @func.def() {
+; CHECK-LABEL: @func.def(
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
+!0 = !{ ptr @func.decl }
+!1 = !{ ptr @func.def }
+!2 = !{ ptr @gv.decl }
+!3 = !{ ptr @gv.def }
+!4 = !{ ptr @alias }
+!5 = !{ ptr addrspace(1) @alias.addrspacecast }
+!6 = !{ ptr addrspace(1) addrspacecast (ptr @gv.def to ptr addrspace(1)) }
+!7 = !{ ptr @ifunc }
+!8 = !{ ptr null }
+!9 = !{ ptr inttoptr (i64 12345 to ptr) }
+!10 = !{ ptr poison }
+!11 = !{ ptr undef }
+!12 = !{ptr addrspace(1) addrspacecast (ptr null to ptr addrspace(1))}
+;.
+; CHECK: [[META0:![0-9]+]] = !{ptr @func.decl}
+; CHECK: [[META1:![0-9]+]] = !{ptr @func.def}
+; CHECK: [[META2:![0-9]+]] = !{ptr @gv.decl}
+; CHECK: [[META3:![0-9]+]] = !{ptr @gv.def}
+; CHECK: [[META4:![0-9]+]] = !{ptr @alias}
+; CHECK: [[META5:![0-9]+]] = !{ptr addrspace(1) @alias.addrspacecast}
+; CHECK: [[META6:![0-9]+]] = !{ptr addrspace(1) addrspacecast (ptr @gv.def to ptr addrspace(1))}
+; CHECK: [[META7:![0-9]+]] = !{ptr @ifunc}
+; CHECK: [[META8:![0-9]+]] = !{ptr null}
+; CHECK: [[META9:![0-9]+]] = !{ptr inttoptr (i64 12345 to ptr)}
+; CHECK: [[META10:![0-9]+]] = !{ptr poison}
+; CHECK: [[META11:![0-9]+]] = !{ptr undef}
+;.

llvm/test/CodeGen/X86/elf-associated.ll

 ; CHECK-DAG: .section bbb,"awo",@progbits,h,unique,2
 ; CHECK-DAG: .section .data.k,"awo",@progbits,h
 
 ; Non-GlobalValue metadata.
 @l = global i32 1, section "ccc", !associated !5
 !5 = !{ptr null}
 ; CHECK-DAG: .section ccc,"awo",@progbits,0,unique,3
 
-; Null metadata.
-@m = global i32 1, section "ddd", !associated !6
-!6 = distinct !{null}
-; CHECK-DAG: .section ddd,"awo",@progbits,0,unique,4
-
 ; Aliases are OK.
 @n = alias i32, ptr inttoptr (i64 add (i64 ptrtoint (ptr @a to i64), i64 1297036692682702848) to ptr)
 @o = global i32 1, section "eee", !associated !7
 !7 = !{ptr @n}
-; CHECK-DAG: .section eee,"awo",@progbits,n,unique,5
+; CHECK-DAG: .section eee,"awo",@progbits,n,unique,4

llvm/test/Linker/Inputs/associated-global.ll

+@a = global i32 0, !associated !0
+@b = external global i32, !associated !1
+@c = internal global i32 1, !associated !2
+@e = global i32 0, !associated !3
+
+!0 = !{ptr @b}
+!1 = !{ptr @a}
+!2 = !{ptr @e}
+!3 = !{ptr @c}

llvm/test/Linker/associated-global.ll

+; RUN: llvm-link -S %s %S/Inputs/associated-global.ll | FileCheck %s
+
+; CHECK: @c = internal global i32 1, !associated !0
+; CHECK: @d = global i32 0, !associated !1
+; CHECK: @a = global i32 0, !associated !2
+; CHECK: @b = global i32 0, !associated !3
+; CHECK: @c.3 = internal global i32 1, !associated !4
+; CHECK: @e = global i32 0, !associated !5
+
+; CHECK: !0 = !{ptr @d}
+; CHECK: !1 = !{ptr @c}
+; CHECK: !2 = !{ptr @b}
+; CHECK: !3 = !{ptr @a}
+; CHECK: !4 = !{ptr @e}
+; CHECK: !5 = !{ptr @c.3}
+
+
+@a = external global i32, !associated !0
+@b = global i32 0, !associated !1
+@c = internal global i32 1, !associated !2
+@d = global i32 0, !associated !3
+
+!0 = !{ptr @b}
+!1 = !{ptr @a}
+!2 = !{ptr @d}
+!3 = !{ptr @c}

llvm/test/Linker/metadata-global.ll

 ; RUN: llvm-link %s -S | FileCheck %s
 
 ; CHECK-DAG: @a = global i32 0
 ; CHECK-DAG: @b = global i32 0, !associated !0
 
-; CHECK-DAG: !0 = !{ptr @b}
+; CHECK-DAG: !0 = !{ptr @a}
 
 @a = global i32 0
 @b = global i32 0, !associated !0
 
-!0 = !{ptr @b}
+!0 = !{ptr @a}

llvm/test/Verifier/associated-metadata.ll

+; RUN: not llvm-as -disable-output < %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK: associated value must be pointer typed
+; CHECK-NEXT: ptr addrspace(1) @associated.int
+; CHECK-NEXT: !0 = !{i32 1}
[e-mail] = external addrspace(1) constant [8 x i8], !associated !0
+
+; CHECK: associated value must be pointer typed
+; CHECK-NEXT: ptr addrspace(1) @associated.float
+; CHECK-NEXT: !1 = !{float 1.000000e+00}
[e-mail] = external addrspace(1) constant [8 x i8], !associated !1
+
+; CHECK: associated metadata must have one operand
+; CHECK-NEXT: ptr addrspace(1) @associated.too.many.ops
+; CHECK-NEXT: !2 = !{ptr @gv.decl0, ptr @gv.decl1}
[e-mail] = external addrspace(1) constant [8 x i8], !associated !2
+
+; CHECK: associated metadata must have one operand
+; CHECK-NEXT: ptr addrspace(1) @associated.empty
+; CHECK-NEXT: !3 = !{}
[e-mail] = external addrspace(1) constant [8 x i8], !associated !3
+
+; CHECK: associated metadata must have a global value
+; CHECK-NEXT: ptr addrspace(1) @associated.null.metadata
+; CHECK-NEXT: !4 = !{null}
[e-mail] = external addrspace(1) constant [8 x i8], !associated !4
+
+; CHECK: global values should not associate to themselves
+; CHECK-NEXT: ptr @associated.self
+; CHECK-NEXT: !5 = !{ptr @associated.self}
[e-mail] = external constant [8 x i8], !associated !5
+
+; CHECK: associated metadata must be ValueAsMetadata
+; CHECK-NEXT: ptr @associated.string
+; CHECK-NEXT: !6 = !{!"string"}
[e-mail] = external constant [8 x i8], !associated !6
+
[e-mail]0 = external constant [8 x i8]
[e-mail]1 = external constant [8 x i8]
+
+!0 = !{i32 1}
+!1 = !{float 1.000000e+00}
+!2 = !{ptr @gv.decl0, ptr @gv.decl1}
+!3 = !{}
+!4 = !{null}
+!5 = !{ptr @associated.self}
+!6 = !{!"string"}