Reapply "Verifier: Check for incompatible bit piece expressions"

This reverts commit r234717, reapplying r234698 (in spirit).

As described in r234717, the original `Verifier` check had a
use-after-free.  Instead of storing pointers to "interesting" debug info
intrinsics whose bit piece expressions should be verified once we have
typerefs, do a second traversal.  I've added a testcase to catch the
`llc` crasher.

Original commit message:

    Verifier: Check for incompatible bit piece expressions

    Convert an assertion into a `Verifier` check.  Bit piece expressions
    must fit inside the variable, and mustn't be the entire variable.
    Catching this in the verifier will help us find bugs sooner, and makes
    `DIVariable::getSizeInBits()` dead code.

llvm-svn: 234776

1 files changed, 92 insertions, 7 deletions

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 9fb6f9a1a7e..ba1a8e8b762 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -178,8 +178,8 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// \brief Keep track of the metadata nodes that have been checked already.
   SmallPtrSet<const Metadata *, 32> MDNodes;
 
-  /// \brief Track string-based type references.
-  SmallDenseMap<const MDString *, const MDNode *, 32> TypeRefs;
+  /// \brief Track unresolved string-based type references.
+  SmallDenseMap<const MDString *, const MDNode *, 32> UnresolvedTypeRefs;
 
   /// \brief The personality function referenced by the LandingPadInsts.
   /// All LandingPadInsts within the same function must use the same
@@ -407,6 +407,9 @@ private:
 
   // Module-level debug info verification...
   void verifyTypeRefs();
+  template <class MapTy>
+  void verifyBitPieceExpression(const DbgInfoIntrinsic &I,
+                                const MapTy &TypeRefs);
   void visitUnresolvedTypeRef(const MDString *S, const MDNode *N);
 };
 } // End anonymous namespace
@@ -702,7 +705,7 @@ bool Verifier::isValidUUID(const MDNode &N, const Metadata *MD) {
 
   // Keep track of names of types referenced via UUID so we can check that they
   // actually exist.
-  TypeRefs.insert(std::make_pair(S, &N));
+  UnresolvedTypeRefs.insert(std::make_pair(S, &N));
   return true;
 }
 
@@ -3386,6 +3389,71 @@ void Verifier::visitDbgIntrinsic(StringRef Kind, DbgIntrinsicTy &DII) {
          BB ? BB->getParent() : nullptr, Var, VarIA, Loc, LocIA);
 }
 
+template <class MapTy>
+static uint64_t getVariableSize(const MDLocalVariable &V, const MapTy &Map) {
+  // Be careful of broken types (checked elsewhere).
+  const Metadata *RawType = V.getRawType();
+  while (RawType) {
+    // Try to get the size directly.
+    if (auto *T = dyn_cast<MDType>(RawType))
+      if (uint64_t Size = T->getSizeInBits())
+        return Size;
+
+    if (auto *DT = dyn_cast<MDDerivedType>(RawType)) {
+      // Look at the base type.
+      RawType = DT->getRawBaseType();
+      continue;
+    }
+
+    if (auto *S = dyn_cast<MDString>(RawType)) {
+      // Don't error on missing types (checked elsewhere).
+      RawType = Map.lookup(S);
+      continue;
+    }
+
+    // Missing type or size.
+    break;
+  }
+
+  // Fail gracefully.
+  return 0;
+}
+
+template <class MapTy>
+void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I,
+                                        const MapTy &TypeRefs) {
+  MDLocalVariable *V;
+  MDExpression *E;
+  if (auto *DVI = dyn_cast<DbgValueInst>(&I)) {
+    V = dyn_cast_or_null<MDLocalVariable>(DVI->getRawVariable());
+    E = dyn_cast_or_null<MDExpression>(DVI->getRawExpression());
+  } else {
+    auto *DDI = cast<DbgDeclareInst>(&I);
+    V = dyn_cast_or_null<MDLocalVariable>(DDI->getRawVariable());
+    E = dyn_cast_or_null<MDExpression>(DDI->getRawExpression());
+  }
+
+  // We don't know whether this intrinsic verified correctly.
+  if (!V || !E || !E->isValid())
+    return;
+
+  // Nothing to do if this isn't a bit piece expression.
+  if (!E->isBitPiece())
+    return;
+
+  // If there's no size, the type is broken, but that should be checked
+  // elsewhere.
+  uint64_t VarSize = getVariableSize(*V, TypeRefs);
+  if (!VarSize)
+    return;
+
+  unsigned PieceSize = E->getBitPieceSize();
+  unsigned PieceOffset = E->getBitPieceOffset();
+  Assert(PieceSize + PieceOffset <= VarSize,
+         "piece is larger than or outside of variable", &I, V, E);
+  Assert(PieceSize != VarSize, "piece covers entire variable", &I, V, E);
+}
+
 void Verifier::visitUnresolvedTypeRef(const MDString *S, const MDNode *N) {
   // This is in its own function so we get an error for each bad type ref (not
   // just the first).
@@ -3397,18 +3465,35 @@ void Verifier::verifyTypeRefs() {
   if (!CUs)
     return;
 
-  // Visit all the compile units again to check the type references.
+  // Visit all the compile units again to map the type references.
+  SmallDenseMap<const MDString *, const MDType *, 32> TypeRefs;
   for (auto *CU : CUs->operands())
     if (auto Ts = cast<MDCompileUnit>(CU)->getRetainedTypes())
       for (MDType *Op : Ts)
         if (auto *T = dyn_cast<MDCompositeType>(Op))
-          TypeRefs.erase(T->getRawIdentifier());
-  if (TypeRefs.empty())
+          if (auto *S = T->getRawIdentifier()) {
+            UnresolvedTypeRefs.erase(S);
+            TypeRefs.insert(std::make_pair(S, T));
+          }
+
+  // Verify debug info intrinsic bit piece expressions.  This needs a second
+  // pass through the intructions, since we haven't built TypeRefs yet when
+  // verifying functions, and simply queuing the DbgInfoIntrinsics to evaluate
+  // later/now would queue up some that could be later deleted.
+  for (const Function &F : *M)
+    for (const BasicBlock &BB : F)
+      for (const Instruction &I : BB)
+        if (auto *DII = dyn_cast<DbgInfoIntrinsic>(&I))
+          verifyBitPieceExpression(*DII, TypeRefs);
+
+  // Return early if all typerefs were resolved.
+  if (UnresolvedTypeRefs.empty())
     return;
 
   // Sort the unresolved references by name so the output is deterministic.
   typedef std::pair<const MDString *, const MDNode *> TypeRef;
-  SmallVector<TypeRef, 32> Unresolved(TypeRefs.begin(), TypeRefs.end());
+  SmallVector<TypeRef, 32> Unresolved(UnresolvedTypeRefs.begin(),
+                                      UnresolvedTypeRefs.end());
   std::sort(Unresolved.begin(), Unresolved.end(),
             [](const TypeRef &LHS, const TypeRef &RHS) {
     return LHS.first->getString() < RHS.first->getString();