[PATCH] D16167: [Verifier] Complain if a metadata attachment is null

[Duncan P. N. Exon Smith via llvm-commits]

LGTM.

> On 2016-Jan-13, at 17:23, Keno Fischer <kfischer at college.harvard.edu> wrote:
> 
> loladiro created this revision.
> loladiro added a reviewer: dexonsmith.
> loladiro added a subscriber: llvm-commits.
> loladiro set the repository for this revision to rL LLVM.
> 
> The bitcode writer asserts if a metadata attachment to an instruction is null, but the Verifier didn't catch it before. Additionally, also make the IR writer more accepting when it encounters null metadata attachments, since it is used by the Verifier to print its errors.
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D16167
> 
> Files:
>  lib/IR/AsmWriter.cpp
>  lib/IR/TypeFinder.cpp
>  lib/IR/Verifier.cpp
> 
> Index: lib/IR/Verifier.cpp
> ===================================================================
> --- lib/IR/Verifier.cpp
> +++ lib/IR/Verifier.cpp
> @@ -3462,6 +3462,12 @@
>     }
>   }
> 
> +  SmallVector<std::pair<unsigned, MDNode *>, 4> InstMD;
> +  I.getAllMetadata(InstMD);
> +  for (auto MD : InstMD) {
> +    Assert(MD.second, "Metadata attachment may not be null!", &I);
> +  }
> +
>   if (MDNode *MD = I.getMetadata(LLVMContext::MD_fpmath)) {
>     Assert(I.getType()->isFPOrFPVectorTy(),
>            "fpmath requires a floating point result!", &I);
> Index: lib/IR/TypeFinder.cpp
> ===================================================================
> --- lib/IR/TypeFinder.cpp
> +++ lib/IR/TypeFinder.cpp
> @@ -71,7 +71,8 @@
>         // Incorporate types hiding in metadata.
>         I.getAllMetadataOtherThanDebugLoc(MDForInst);
>         for (unsigned i = 0, e = MDForInst.size(); i != e; ++i)
> -          incorporateMDNode(MDForInst[i].second);
> +          if (MDForInst[i].second)
> +            incorporateMDNode(MDForInst[i].second);
> 
>         MDForInst.clear();
>       }
> Index: lib/IR/AsmWriter.cpp
> ===================================================================
> --- lib/IR/AsmWriter.cpp
> +++ lib/IR/AsmWriter.cpp
> @@ -872,7 +872,8 @@
>   SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
>   I.getAllMetadata(MDs);
>   for (auto &MD : MDs)
> -    CreateMetadataSlot(MD.second);
> +    if (MD.second)
> +      CreateMetadataSlot(MD.second);
> }
> 
> /// Clean up after incorporating a function. This is the only way to get out of
> @@ -1965,6 +1966,11 @@
>                                    TypePrinting *TypePrinter,
>                                    SlotTracker *Machine, const Module *Context,
>                                    bool FromValue) {
> +  if (!MD) {
> +    Out << "<null>";
> +    return;
> +  }
> +
>   if (const MDNode *N = dyn_cast<MDNode>(MD)) {
>     std::unique_ptr<SlotTracker> MachineStorage;
>     if (!Machine) {
> @@ -3120,7 +3126,9 @@
>   if (MDs.empty())
>     return;
> 
> -  if (MDNames.empty())
> +  // Second should not be null in well formed, IR, but this is also
> +  // called from the Verifier, so be permissive.
> +  if (MDNames.empty() && MDs[0].second)
>     MDs[0].second->getContext().getMDKindNames(MDNames);
> 
>   for (const auto &I : MDs) {
> 
> 
> <D16167.44818.patch>