IR: Allow metadata attachments on declarations, and fix lazy loaded metadata issue with globals.

[pcc]

Summary

This change is motivated by an upcoming change to the metadata representation
used for CFI. The indirect function call checker needs type information for
external function declarations in order to correctly generate jump table
entries for such declarations. We currently associate such type information
with declarations using a global metadata node, but I plan [1] to move all
such metadata to global object attachments.

In bitcode, metadata attachments for function declarations appear in the
global metadata block. This seems reasonable to me because I expect metadata
attachments on declarations to be uncommon. In the long term I'd also expect
this to be the case for CFI, because we'd want to use some specialized bitcode
format for this metadata that could be read as part of the ThinLTO thin-link
phase, which would mean that it would not appear in the global metadata block.

To solve the lazy loaded metadata issue I was seeing with D20147, I use the
same bitcode representation for metadata attachments for global variables as I
do for function declarations. Since there's a use case for metadata attachments
in the global metadata block, we might as well use that representation for
global variables as well, at least until we have a mechanism for lazy loading
global variables.

In the assembly format, the metadata attachments appear after the "declare"
keyword in order to avoid a parsing ambiguity. I could make the same change
for function definition attachments if that seems reasonable.

[1] http://lists.llvm.org/pipermail/llvm-dev/2016-June/100462.html

[pcc]

Ping. Note that D20147 is blocked on this.

[dexonsmith]

Sorry for the delay. At a high-level, here are my concerns (which you may have already addressed: I haven't looked at the patch yet):

• !dbg attachments on declarations should be disallowed (I suggest the verifier?).
• PGO attachments on declarations should probably also be disallowed?
• Along the same lines, I think having two !dbg (or PGO) attachments on the same function should be disallowed. I can imagine loosening this restriction in the future (with lots more work) but I think right now it would cause a bug.
• Lazy-loading needs to be fast.

I won't have time to review this in detail for at least a few days, but I'm also happy to defer to others.

[aprantl]

The code itself looks fine to me.

lib/Bitcode/Writer/ValueEnumerator.cpp

351

I'm just curious: Is this being addressed in one of the follow-up reviews? If not, can you please file a PR about it?

[pcc]

• Forbid !dbg and !prof attachments on declarations
• Add PR references

[pcc]

!dbg attachments on declarations should be disallowed (I suggest the verifier?).
PGO attachments on declarations should probably also be disallowed?

Done.

Along the same lines, I think having two !dbg (or PGO) attachments on the same function should be disallowed. I can imagine loosening this restriction in the future (with lots more work) but I think right now it would cause a bug.

Was already done for !dbg as part of r271358. I've committed a similar change for !prof in r272734.

Lazy-loading needs to be fast.

This change along with D20147 should not represent a performance regression in regard to global variable debug metadata. Global variable debug metadata will continue to be stored in the global metadata block and will continue to be materialized upon calling materializeMetadata.

lib/Bitcode/Writer/ValueEnumerator.cpp

351

Filed PR28134.

[pcc]

Ping.

[dexonsmith]

Not sure if this is waiting for me, but you answered all my concerns. LGTM.

docs/BitCodeFormat.rst

 ``[GCNAME, ...string...]``
 
 The ``GCNAME`` record (code 11) contains a variable number of values
 representing the bytes of a single garbage collector name string. There should
 be one ``GCNAME`` record for each garbage collector name referenced in function
 ``gc`` attributes within the module. These records can be referenced by 1-based
 index in the *gc* fields of ``FUNCTION`` records.
 
-MODULE_CODE_GLOBALVAR_ATTACHMENT Record
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-``[GLOBALVAR_ATTACHMENT, valueid, n x [id, mdnode]]``
-
-The ``GLOBALVAR_ATTACHMENT`` record (code 19) describes the metadata
-attachments for a global variable. The ``valueid`` is the value index for
-the global variable, and the remaining fields are pairs of metadata name
-indices and metadata node indices.
-
 .. _PARAMATTR_BLOCK:
 
 PARAMATTR_BLOCK Contents
 ------------------------
 
 The ``PARAMATTR_BLOCK`` block (id 9) contains a table of entries describing the
 attributes of function parameters. These entries are referenced by 1-based index
 in the *paramattr* field of module block `FUNCTION`_ records, or within the

include/llvm/Bitcode/LLVMBitCodes.h

enum ModuleCodes {
   // SOURCE_FILENAME: [namechar x N]
   MODULE_CODE_SOURCE_FILENAME = 16,
 
   // HASH: [5*i32]
   MODULE_CODE_HASH = 17,
 
   // IFUNC: [ifunc value type, addrspace, resolver val#, linkage, visibility]
   MODULE_CODE_IFUNC = 18,
-
-  // GLOBALVAR_ATTACHMENT: [valueid, n x [id, mdnode]]
-  MODULE_CODE_GLOBALVAR_ATTACHMENT = 19,
 };
 
 /// PARAMATTR blocks have code for defining a parameter attribute set.
 enum AttributeCodes {
   // FIXME: Remove `PARAMATTR_CODE_ENTRY_OLD' in 4.0
   PARAMATTR_CODE_ENTRY_OLD = 1, // ENTRY: [paramidx0, attr0,
                                 //         paramidx1, attr1...]
   PARAMATTR_CODE_ENTRY = 2,     // ENTRY: [paramidx0, attrgrp0,
enum MetadataCodes {
   METADATA_LOCAL_VAR = 28,          // [distinct, ...]
   METADATA_EXPRESSION = 29,         // [distinct, n x element]
   METADATA_OBJC_PROPERTY = 30,      // [distinct, name, file, line, ...]
   METADATA_IMPORTED_ENTITY = 31, // [distinct, tag, scope, entity, line, name]
   METADATA_MODULE = 32,          // [distinct, scope, name, ...]
   METADATA_MACRO = 33,           // [distinct, macinfo, line, name, value]
   METADATA_MACRO_FILE = 34,      // [distinct, macinfo, line, file, ...]
   METADATA_STRINGS = 35,         // [count, offset] blob([lengths][chars])
+  METADATA_GLOBAL_DECL_ATTACHMENT = 36, // [valueid, n x [id, mdnode]]
 };
 
 // The constants block (CONSTANTS_BLOCK_ID) describes emission for each
 // constant and maintains an implicit current type value.
 enum ConstantsCodes {
   CST_CODE_SETTYPE = 1,          // SETTYPE:       [typeid]
   CST_CODE_NULL = 2,             // NULL
   CST_CODE_UNDEF = 3,            // UNDEF

lib/AsmParser/LLParser.cpp

 
 
 /// toplevelentity
 ///   ::= 'declare' FunctionHeader
 bool LLParser::ParseDeclare() {
   assert(Lex.getKind() == lltok::kw_declare);
   Lex.Lex();
 
+  std::vector<std::pair<unsigned, MDNode *>> MDs;
+  while (Lex.getKind() == lltok::MetadataVar) {
+    unsigned MDK;
+    MDNode *N;
+    if (ParseMetadataAttachment(MDK, N))
+      return true;
+    MDs.push_back({MDK, N});
+  }
+
   Function *F;
-  return ParseFunctionHeader(F, false);
+  if (ParseFunctionHeader(F, false))
+    return true;
+  for (auto &MD  : MDs)
+    F->addMetadata(MD.first, *MD.second);
+  return false;
 }
 
 /// toplevelentity
 ///   ::= 'define' FunctionHeader (!dbg !56)* '{' ...
 bool LLParser::ParseDefine() {
   assert(Lex.getKind() == lltok::kw_define);
   Lex.Lex();
 

lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::METADATA_STRING_OLD: {
       MetadataList.assignValue(MD, NextMetadataNo++);
       break;
     }
     case bitc::METADATA_STRINGS:
       if (std::error_code EC =
               parseMetadataStrings(Record, Blob, NextMetadataNo))
         return EC;
       break;
+    case bitc::METADATA_GLOBAL_DECL_ATTACHMENT: {
+      if (Record.size() % 2 == 0)
+        return error("Invalid record");
+      unsigned ValueID = Record[0];
+      if (ValueID >= ValueList.size())
+        return error("Invalid record");
+      if (auto *GO = dyn_cast<GlobalObject>(ValueList[ValueID]))
+        parseGlobalObjectAttachment(*GO, ArrayRef<uint64_t>(Record).slice(1));
+      break;
+    }
     case bitc::METADATA_KIND: {
       // Support older bitcode files that had METADATA_KIND records in a
       // block with METADATA_BLOCK_ID.
       if (std::error_code EC = parseMetadataKindRecord(Record))
         return EC;
       break;
     }
     }
case bitc::MODULE_CODE_GLOBALVAR: {
             return error("Invalid global variable comdat ID");
           NewGV->setComdat(ComdatList[ComdatID - 1]);
         }
       } else if (hasImplicitComdat(RawLinkage)) {
         NewGV->setComdat(reinterpret_cast<Comdat *>(1));
       }
       break;
     }
-    case bitc::MODULE_CODE_GLOBALVAR_ATTACHMENT: {
-      if (Record.size() % 2 == 0)
-        return error("Invalid record");
-      unsigned ValueID = Record[0];
-      if (ValueID >= ValueList.size())
-        return error("Invalid record");
-      if (auto *GV = dyn_cast<GlobalVariable>(ValueList[ValueID]))
-        parseGlobalObjectAttachment(*GV, ArrayRef<uint64_t>(Record).slice(1));
-      break;
-    }
     // FUNCTION:  [type, callingconv, isproto, linkage, paramattr,
     //             alignment, section, visibility, gc, unnamed_addr,
     //             prologuedata, dllstorageclass, comdat, prefixdata]
     case bitc::MODULE_CODE_FUNCTION: {
       if (Record.size() < 8)
         return error("Invalid record");
       Type *Ty = getTypeByID(Record[0]);
       if (!Ty)

lib/Bitcode/Writer/BitcodeWriter.cpp

private:
   void writeMetadataRecords(ArrayRef<const Metadata *> MDs,
                             SmallVectorImpl<uint64_t> &Record);
   void writeModuleMetadata();
   void writeFunctionMetadata(const Function &F);
   void writeFunctionMetadataAttachment(const Function &F);
   void writeGlobalVariableMetadataAttachment(const GlobalVariable &GV);
   void pushGlobalMetadataAttachment(SmallVectorImpl<uint64_t> &Record,
                                     const GlobalObject &GO);
-  void writeModuleMetadataStore();
+  void writeModuleMetadataKinds();
   void writeOperandBundleTags();
   void writeConstants(unsigned FirstVal, unsigned LastVal, bool isGlobal);
   void writeModuleConstants();
   bool pushValueAndType(const Value *V, unsigned InstID,
                         SmallVectorImpl<unsigned> &Vals);
   void writeOperandBundles(ImmutableCallSite CS, unsigned InstID);
   void pushValue(const Value *V, unsigned InstID,
                  SmallVectorImpl<unsigned> &Vals);
void ModuleBitcodeWriter::writeModuleMetadata() {
   if (!VE.hasMDs() && M.named_metadata_empty())
     return;
 
   Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 3);
   SmallVector<uint64_t, 64> Record;
   writeMetadataStrings(VE.getMDStrings(), Record);
   writeMetadataRecords(VE.getNonMDStrings(), Record);
   writeNamedMetadata(Record);
+
+  auto AddDeclAttachedMetadata = [&](const GlobalObject &GO) {
+    SmallVector<uint64_t, 4> Record;
+    Record.push_back(VE.getValueID(&GO));
+    pushGlobalMetadataAttachment(Record, GO);
+    Stream.EmitRecord(bitc::METADATA_GLOBAL_DECL_ATTACHMENT, Record);
+  };
+  for (const Function &F : M)
+    if (F.isDeclaration() && F.hasMetadata())
+      AddDeclAttachedMetadata(F);
+  // FIXME: Only store metadata for declarations here, and move data for global
+  // variable definitions to a separate block.
+  for (const GlobalVariable &GV : M.globals())
+    if (GV.hasMetadata())
+      AddDeclAttachedMetadata(GV);
+
   Stream.ExitBlock();
 }
 
 void ModuleBitcodeWriter::writeFunctionMetadata(const Function &F) {
   if (!VE.hasMDs())
     return;
 
   Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 3);
for (const Instruction &I : BB) {
       }
       Stream.EmitRecord(bitc::METADATA_ATTACHMENT, Record, 0);
       Record.clear();
     }
 
   Stream.ExitBlock();
 }
 
-void ModuleBitcodeWriter::writeModuleMetadataStore() {
+void ModuleBitcodeWriter::writeModuleMetadataKinds() {
   SmallVector<uint64_t, 64> Record;
 
   // Write metadata kinds
   // METADATA_KIND - [n x [id, name]]
   SmallVector<StringRef, 8> Names;
   M.getMDKindNames(Names);
 
   if (Names.empty()) return;
void ModuleBitcodeWriter::writeModule() {
 
   // Emit top-level description of module, including target triple, inline asm,
   // descriptors for global variables, and function prototype info.
   writeModuleInfo();
 
   // Emit constants.
   writeModuleConstants();
 
-  // Emit metadata.
-  writeModuleMetadata();
+  // Emit metadata kind names.
+  writeModuleMetadataKinds();
 
   // Emit metadata.
-  writeModuleMetadataStore();
+  writeModuleMetadata();
 
   // Emit module-level use-lists.
   if (VE.shouldPreserveUseListOrder())
     writeUseListBlock(nullptr);
 
   writeOperandBundleTags();
 
   // Emit function bodies.
   DenseMap<const Function *, uint64_t> FunctionToBitcodeIndex;
   for (Module::const_iterator F = M.begin(), E = M.end(); F != E; ++F)
     if (!F->isDeclaration())
       writeFunction(*F, FunctionToBitcodeIndex);
 
   // Need to write after the above call to WriteFunction which populates
   // the summary information in the index.
   if (Index)
     writePerModuleGlobalValueSummary();
 
   writeValueSymbolTable(M.getValueSymbolTable(),
                         /* IsModuleLevel */ true, &FunctionToBitcodeIndex);
 
-  for (const GlobalVariable &GV : M.globals())
-    if (GV.hasMetadata()) {
-      SmallVector<uint64_t, 4> Record;
-      Record.push_back(VE.getValueID(&GV));
-      pushGlobalMetadataAttachment(Record, GV);
-      Stream.EmitRecord(bitc::MODULE_CODE_GLOBALVAR_ATTACHMENT, Record);
-    }
-
   if (GenerateHash) {
     writeModuleHash(BlockStartPos);
   }
 
   Stream.ExitBlock();
 }
 
 static void writeInt32ToBuffer(uint32_t Value, SmallVectorImpl<char> &Buffer,

lib/Bitcode/Writer/ValueEnumerator.h

private:
   ///
   /// If \c MD has already been enumerated, check that \c F matches its
   /// function tag.  If not, call \a dropFunctionFromMetadata().
   ///
   /// Otherwise, mark \c MD as visited.  Assign it an ID, or just return it if
   /// it's an \a MDNode.
   const MDNode *enumerateMetadataImpl(unsigned F, const Metadata *MD);
 
-  unsigned getMetadataGlobalID(const GlobalObject *GO) const;
+  unsigned getMetadataFunctionID(const Function *F) const;
 
   /// Enumerate reachable metadata in (almost) post-order.
   ///
   /// Enumerate all the metadata reachable from MD.  We want to minimize the
   /// cost of reading bitcode records, and so the primary consideration is that
   /// operands of uniqued nodes are resolved before the nodes are read.  This
   /// avoids re-uniquing them on the context and factors away RAUW support.
   ///
   /// This algorithm guarantees that subgraphs of uniqued nodes are in
   /// post-order.  Distinct subgraphs reachable only from a single uniqued node
   /// will be in post-order.
   ///
   /// \note The relative order of a distinct and uniqued node is irrelevant.
   /// \a organizeMetadata() will later partition distinct nodes ahead of
   /// uniqued ones.
   ///{
-  void EnumerateMetadata(const GlobalObject *GO, const Metadata *MD);
+  void EnumerateMetadata(const Function *F, const Metadata *MD);
   void EnumerateMetadata(unsigned F, const Metadata *MD);
   ///}
 
   void EnumerateFunctionLocalMetadata(const Function &F,
                                       const LocalAsMetadata *Local);
   void EnumerateFunctionLocalMetadata(unsigned F, const LocalAsMetadata *Local);
   void EnumerateNamedMDNode(const NamedMDNode *NMD);
   void EnumerateValue(const Value *V);

lib/Bitcode/Writer/ValueEnumerator.cpp

ValueEnumerator::ValueEnumerator(const Module &M,
   EnumerateValueSymbolTable(M.getValueSymbolTable());
   EnumerateNamedMetadata(M);
 
   SmallVector<std::pair<unsigned, MDNode *>, 8> MDs;
   for (const GlobalVariable &GV : M.globals()) {
     MDs.clear();
     GV.getAllMetadata(MDs);
     for (const auto &I : MDs)
-      EnumerateMetadata(&GV, I.second);
+      // FIXME: Pass GV to EnumerateMetadata and arrange for the bitcode writer
+      // to write metadata to the global variable's own metadata block.
+      EnumerateMetadata(nullptr, I.second);
   }
 
   // Enumerate types used by function bodies and argument lists.
   for (const Function &F : M) {
     for (const Argument &A : F.args())
       EnumerateType(A.getType());
 
     // Enumerate metadata attached to this function.
     MDs.clear();
     F.getAllMetadata(MDs);
     for (const auto &I : MDs)
-      EnumerateMetadata(&F, I.second);
+      EnumerateMetadata(F.isDeclaration() ? nullptr : &F, I.second);
 
     for (const BasicBlock &BB : F)
       for (const Instruction &I : BB) {
         for (const Use &Op : I.operands()) {
           auto *MD = dyn_cast<MetadataAsValue>(&Op);
           if (!MD) {
             EnumerateOperandType(Op);
             continue;
for (const auto &I : M.named_metadata())
     EnumerateNamedMDNode(&I);
 }
 
 void ValueEnumerator::EnumerateNamedMDNode(const NamedMDNode *MD) {
   for (unsigned i = 0, e = MD->getNumOperands(); i != e; ++i)
     EnumerateMetadata(nullptr, MD->getOperand(i));
 }
 
-unsigned ValueEnumerator::getMetadataGlobalID(const GlobalObject *GO) const {
-  return GO ? getValueID(GO) + 1 : 0;
+unsigned ValueEnumerator::getMetadataFunctionID(const Function *F) const {
+  return F ? getValueID(F) + 1 : 0;
 }
 
-void ValueEnumerator::EnumerateMetadata(const GlobalObject *GO,
-                                        const Metadata *MD) {
-  EnumerateMetadata(getMetadataGlobalID(GO), MD);
+void ValueEnumerator::EnumerateMetadata(const Function *F, const Metadata *MD) {
+  EnumerateMetadata(getMetadataFunctionID(F), MD);
 }
 
 void ValueEnumerator::EnumerateFunctionLocalMetadata(
     const Function &F, const LocalAsMetadata *Local) {
-  EnumerateFunctionLocalMetadata(getMetadataGlobalID(&F), Local);
+  EnumerateFunctionLocalMetadata(getMetadataFunctionID(&F), Local);
 }
 
 void ValueEnumerator::dropFunctionFromMetadata(
     MetadataMapType::value_type &FirstMD) {
   SmallVector<const MDNode *, 64> Worklist;
   auto push = [this, &Worklist](MetadataMapType::value_type &MD) {
     auto &Entry = MD.second;
 

lib/IR/AsmWriter.cpp

for (AttributeSet::iterator I = AS.begin(Idx), E = AS.end(Idx);
         AttrStr += Attr.getAsString();
       }
     }
 
     if (!AttrStr.empty())
       Out << "; Function Attrs: " << AttrStr << '\n';
   }
 
-  if (F->isDeclaration())
-    Out << "declare ";
-  else
+  Machine.incorporateFunction(F);
+
+  if (F->isDeclaration()) {
+    Out << "declare";
+    SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
+    F->getAllMetadata(MDs);
+    printMetadataAttachments(MDs, " ");
+    Out << ' ';
+  } else
     Out << "define ";
 
   Out << getLinkagePrintName(F->getLinkage());
   PrintVisibility(F->getVisibility(), Out);
   PrintDLLStorageClass(F->getDLLStorageClass(), Out);
 
   // Print the calling convention.
   if (F->getCallingConv() != CallingConv::C) {
     PrintCallingConv(F->getCallingConv(), Out);
     Out << " ";
   }
 
   FunctionType *FT = F->getFunctionType();
   if (Attrs.hasAttributes(AttributeSet::ReturnIndex))
     Out <<  Attrs.getAsString(AttributeSet::ReturnIndex) << ' ';
   TypePrinter.print(F->getReturnType(), Out);
   Out << ' ';
   WriteAsOperandInternal(Out, F, &TypePrinter, &Machine, F->getParent());
   Out << '(';
-  Machine.incorporateFunction(F);
 
   // Loop over the arguments, printing them...
   if (F->isDeclaration() && !IsForDebug) {
     // We're only interested in the type here - don't print argument names.
     for (unsigned I = 0, E = FT->getNumParams(); I != E; ++I) {
       // Insert commas as we go... the first arg doesn't get a comma
       if (I)
         Out << ", ";
if (F->hasPrologueData()) {
     Out << " prologue ";
     writeOperand(F->getPrologueData(), true);
   }
   if (F->hasPersonalityFn()) {
     Out << " personality ";
     writeOperand(F->getPersonalityFn(), /*PrintType=*/true);
   }
 
+  if (F->isDeclaration()) {
+    Out << '\n';
+  } else {
     SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
     F->getAllMetadata(MDs);
     printMetadataAttachments(MDs, " ");
 
-  if (F->isDeclaration()) {
-    Out << '\n';
-  } else {
     Out << " {";
     // Output all of the function's basic blocks.
     for (Function::const_iterator I = F->begin(), E = F->end(); I != E; ++I)
       printBasicBlock(&*I);
 
     // Output the function's use-lists.
     printUseLists(F);
 

lib/IR/Verifier.cpp

if (Per)
              &F, F.getParent(), Per, Per->getParent());
   }
 
   if (F.isMaterializable()) {
     // Function has a body somewhere we can't see.
     Assert(MDs.empty(), "unmaterialized function cannot have metadata", &F,
            MDs.empty() ? nullptr : MDs.front().second);
   } else if (F.isDeclaration()) {
-    Assert(MDs.empty(), "function without a body cannot have metadata", &F,
-           MDs.empty() ? nullptr : MDs.front().second);
     Assert(!F.hasPersonalityFn(),
            "Function declaration shouldn't have a personality routine", &F);
   } else {
     // Verify that this function (which has a body) is not named "llvm.*".  It
     // is not legal to define intrinsics.
     Assert(!isLLVMdotName, "llvm intrinsics cannot be defined!", &F);
 
     // Check the entry node

test/Assembler/metadata-decl.ll

+; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
+; RUN: llvm-as < %s | llvm-dis -materialize-metadata | FileCheck %s
+
+; CHECK: @foo = external global i32, !foo !0
+@foo = external global i32, !foo !0
+
+; CHECK: declare !bar !1 void @bar()
+declare !bar !1 void @bar()
+
+!0 = distinct !{}
+!1 = distinct !{}

test/Assembler/metadata.ll

-; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
+; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck --check-prefix=CHECK --check-prefix=CHECK-UNMAT %s
+; RUN: llvm-as < %s | llvm-dis -materialize-metadata | FileCheck --check-prefix=CHECK-UNMAT %s
 ; RUN: verify-uselistorder %s
 
-; CHECK: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]]
+; CHECK-UNMAT: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]]
 @global = global i32 0, !foo !2, !foo !3, !baz !3
 
 ; CHECK-LABEL: @test
 ; CHECK: ret void, !foo [[M0:![0-9]+]], !bar [[M1:![0-9]+]]
 define void @test() !dbg !1 {
   add i32 2, 1, !bar !0
   add i32 1, 2, !foo !1
   call void @llvm.dbg.func.start(metadata !"foo")

 
 ; CHECK-LABEL: define void @test_attachment_name() {
 ; CHECK:   unreachable, !\342abc [[M4]]
 define void @test_attachment_name() {
   ;; Escape the first character when printing text IR, since it's a digit
   unreachable, !\34\32abc !4
 }
 
-; CHECK: [[M2]] = distinct !{}
-; CHECK: [[M3]] = distinct !{}
+; CHECK-UNMAT: [[M2]] = distinct !{}
+; CHECK-UNMAT: [[M3]] = distinct !{}
 ; CHECK: [[M0]] = !DILocation
 ; CHECK: [[M1]] = distinct !DISubprogram
 ; CHECK: [[M4]] = distinct !{}
 
 !llvm.module.flags = !{!7}
 !llvm.dbg.cu = !{!5}
 !0 = !DILocation(line: 662302, column: 26, scope: !1)
 !1 = distinct !DISubprogram(name: "foo", isDefinition: true, unit: !5)

tools/llvm-dis/llvm-dis.cpp

 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/DataStream.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include <system_error>

 ShowAnnotations("show-annotations",
                 cl::desc("Add informational comments to the .ll file"));
 
 static cl::opt<bool> PreserveAssemblyUseListOrder(
     "preserve-ll-uselistorder",
     cl::desc("Preserve use-list order when writing LLVM assembly."),
     cl::init(false), cl::Hidden);
 
+static cl::opt<bool>
+    MaterializeMetadata("materialize-metadata",
+                        cl::desc("Load module without materializing metadata, "
+                                 "then materialize only the metadata"));
+
 namespace {
 
 static void printDebugLoc(const DebugLoc &DL, formatted_raw_ostream &OS) {
   OS << DL.getLine() << ":" << DL.getCol();
   if (DILocation *IDL = DL.getInlinedAt()) {
     OS << "@";
     printDebugLoc(IDL, OS);
   }
static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
   DiagnosticPrinterRawOStream DP(OS);
   DI.print(DP);
   OS << '\n';
 
   if (DI.getSeverity() == DS_Error)
     exit(1);
 }
 
+static Expected<std::unique_ptr<Module>> openInputFile(LLVMContext &Context) {
+  if (MaterializeMetadata) {
+    ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
+        MemoryBuffer::getFileOrSTDIN(InputFilename);
+    if (!MBOrErr)
+      return errorCodeToError(MBOrErr.getError());
+    ErrorOr<std::unique_ptr<Module>> MOrErr =
+        getLazyBitcodeModule(std::move(*MBOrErr), Context,
+                             /*ShouldLazyLoadMetadata=*/true);
+    if (!MOrErr)
+      return errorCodeToError(MOrErr.getError());
+    (*MOrErr)->materializeMetadata();
+    return std::move(*MOrErr);
+  } else {
+    std::string ErrorMessage;
+    std::unique_ptr<DataStreamer> Streamer =
+        getDataFileStreamer(InputFilename, &ErrorMessage);
+    if (!Streamer)
+      return make_error<StringError>(ErrorMessage, inconvertibleErrorCode());
+    std::string DisplayFilename;
+    if (InputFilename == "-")
+      DisplayFilename = "<stdin>";
+    else
+      DisplayFilename = InputFilename;
+    ErrorOr<std::unique_ptr<Module>> MOrErr =
+        getStreamedBitcodeModule(DisplayFilename, std::move(Streamer), Context);
+    (*MOrErr)->materializeAll();
+    return std::move(*MOrErr);
+  }
+}
+
 int main(int argc, char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal();
   PrettyStackTraceProgram X(argc, argv);
 
   LLVMContext Context;
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   Context.setDiagnosticHandler(diagnosticHandler, argv[0]);
 
   cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n");
 
-  std::string ErrorMessage;
-  std::unique_ptr<Module> M;
-
-  // Use the bitcode streaming interface
-  std::unique_ptr<DataStreamer> Streamer =
-      getDataFileStreamer(InputFilename, &ErrorMessage);
-  if (Streamer) {
-    std::string DisplayFilename;
-    if (InputFilename == "-")
-      DisplayFilename = "<stdin>";
-    else
-      DisplayFilename = InputFilename;
-    ErrorOr<std::unique_ptr<Module>> MOrErr =
-        getStreamedBitcodeModule(DisplayFilename, std::move(Streamer), Context);
-    M = std::move(*MOrErr);
-    M->materializeAll();
-  } else {
-    errs() << argv[0] << ": " << ErrorMessage << '\n';
+  Expected<std::unique_ptr<Module>> MOrErr = openInputFile(Context);
+  if (!MOrErr) {
+    handleAllErrors(MOrErr.takeError(), [&](ErrorInfoBase &EIB) {
+      errs() << argv[0] << ": ";
+      EIB.log(errs());
+      errs() << '\n';
+    });
     return 1;
   }
+  std::unique_ptr<Module> M = std::move(*MOrErr);
 
   // Just use stdout.  We won't actually print anything on it.
   if (DontPrint)
     OutputFilename = "-";
 
   if (OutputFilename.empty()) { // Unspecified output, infer it.
     if (InputFilename == "-") {
       OutputFilename = "-";

unittests/IR/MetadataTest.cpp

TEST_F(FunctionAttachmentTest, getAll) {
   EXPECT_EQ(P, MDs[1].second);
   EXPECT_EQ(T1, MDs[2].second);
   EXPECT_EQ(T2, MDs[3].second);
 }
 
 TEST_F(FunctionAttachmentTest, Verifier) {
   Function *F = getFunction("foo");
   F->setMetadata("attach", getTuple());
+  F->setIsMaterializable(true);
 
-  // Confirm this has no body.
-  ASSERT_TRUE(F->empty());
-
-  // Functions without a body cannot have metadata attachments (they also can't
-  // be verified directly, so check that the module fails to verify).
-  EXPECT_TRUE(verifyModule(*F->getParent()));
+  // Confirm this is materializable.
+  ASSERT_TRUE(F->isMaterializable());
 
-  // Nor can materializable functions.
-  F->setIsMaterializable(true);
-  EXPECT_TRUE(verifyModule(*F->getParent()));
+  // Materializable functions cannot have metadata attachments.
+  EXPECT_TRUE(verifyFunction(*F));
 
-  // Functions with a body can.
+  // Function declarations can.
   F->setIsMaterializable(false);
+  EXPECT_FALSE(verifyModule(*F->getParent()));
+  EXPECT_FALSE(verifyFunction(*F));
+
+  // So can definitions.
   (void)new UnreachableInst(Context, BasicBlock::Create(Context, "bb", F));
   EXPECT_FALSE(verifyModule(*F->getParent()));
   EXPECT_FALSE(verifyFunction(*F));
 }
 
 TEST_F(FunctionAttachmentTest, EntryCount) {
   Function *F = getFunction("foo");
   EXPECT_FALSE(F->getEntryCount().hasValue());