[llvm] r233663 - Verifier: Move over DISubprogram::Verify()

[David Blaikie]

On Mon, Mar 30, 2015 at 7:09 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> Author: dexonsmith
> Date: Mon Mar 30 21:09:55 2015
> New Revision: 233663
>
> URL: http://llvm.org/viewvc/llvm-project?rev=233663&view=rev
> Log:
> Verifier: Move over DISubprogram::Verify()
>
> Move over the remaining (somewhat complicated) check from
> `DISubprogram::Verify()`.  I suspect this check could be optimized --
> e.g., it would be nice not to do another full traversal here -- but it's
> not exactly obvious how.


"another" full traversal on top of which other one?


>   For now, just bring it over as is.
>
> Once we have a better model for the "canonical" subprogram of a
> `Function`, we should enforce that all `!dbg` attachments lead to the
> canonical one.
>

This checking was stuff I added during that infamous assertion's addition
(violating the precondition that all scope chains lead to the same
DISubprogram, and that DISubprogram is the one that references this
Function).

We could, for each Function, just walk all the instructions, walk the
chains, ensure they all reach the same DISubprogram and that DISubprogram
references this Function? Is that a better pivot?

(& then possibly walk all the DISubprograms and ensure that only no
llvm::Function is referenced from more than one DISubprogram)


>
> Modified:
>     llvm/trunk/lib/IR/DebugInfo.cpp
>     llvm/trunk/lib/IR/Verifier.cpp
>     llvm/trunk/unittests/IR/IRBuilderTest.cpp
>
> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=233663&r1=233662&r2=233663&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
> +++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Mar 30 21:09:55 2015
> @@ -256,39 +256,7 @@ bool DIType::Verify() const { return isT
>  bool DIBasicType::Verify() const { return isBasicType(); }
>  bool DIDerivedType::Verify() const { return isDerivedType(); }
>  bool DICompositeType::Verify() const { return isCompositeType(); }
> -
> -bool DISubprogram::Verify() const {
> -  auto *N = dyn_cast_or_null<MDSubprogram>(DbgNode);
> -  if (!N)
> -    return false;
> -
> -  // If a DISubprogram has an llvm::Function*, then scope chains from all
> -  // instructions within the function should lead to this DISubprogram.
> -  if (auto *F = getFunction()) {
> -    for (auto &BB : *F) {
> -      for (auto &I : BB) {
> -        MDLocation *DL = I.getDebugLoc();
> -        if (!DL)
> -          continue;
> -
> -        // walk the inlined-at scopes
> -        MDScope *Scope = DL->getInlinedAtScope();
> -        if (!Scope)
> -          return false;
> -        while (!isa<MDSubprogram>(Scope)) {
> -          Scope = cast<MDLexicalBlockBase>(Scope)->getScope();
> -          if (!Scope)
> -            return false;
> -        }
> -        if (!DISubprogram(Scope).describes(F))
> -          return false;
> -      }
> -    }
> -  }
> -
> -  return true;
> -}
> -
> +bool DISubprogram::Verify() const { return isSubprogram(); }
>  bool DIGlobalVariable::Verify() const { return isGlobalVariable(); }
>  bool DIVariable::Verify() const { return isVariable(); }
>
>
> Modified: llvm/trunk/lib/IR/Verifier.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=233663&r1=233662&r2=233663&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/IR/Verifier.cpp (original)
> +++ llvm/trunk/lib/IR/Verifier.cpp Mon Mar 30 21:09:55 2015
> @@ -921,6 +921,45 @@ void Verifier::visitMDSubprogram(const M
>    }
>    Assert(!hasConflictingReferenceFlags(N.getFlags()), "invalid reference
> flags",
>           &N);
> +
> +  if (!N.getFunction())
> +    return;
> +
> +  // FIXME: Should this be looking through bitcasts?
> +  auto *F = dyn_cast<Function>(N.getFunction()->getValue());
> +  if (!F)
> +    return;
> +
> +  // Check that all !dbg attachments lead to back to N (or, at least,
> another
> +  // subprogram that describes the same function).
> +  //
> +  // FIXME: Check this incrementally while visiting !dbg attachments.
> +  // FIXME: Only check when N is the canonical subprogram for F.
> +  SmallPtrSet<const MDNode *, 32> Seen;
> +  for (auto &BB : *F)
> +    for (auto &I : BB) {
> +      // Be careful about using MDLocation here since we might be dealing
> with
> +      // broken code (this is the Verifier after all).
> +      MDLocation *DL =
> +          dyn_cast_or_null<MDLocation>(I.getDebugLoc().getAsMDNode());
> +      if (!DL)
> +        continue;
> +      if (!Seen.insert(DL).second)
> +        continue;
> +
> +      MDLocalScope *Scope = DL->getInlinedAtScope();
> +      if (Scope && !Seen.insert(Scope).second)
> +        continue;
> +
> +      MDSubprogram *SP = Scope ? Scope->getSubprogram() : nullptr;
> +      if (SP && !Seen.insert(SP).second)
> +        continue;
> +
> +      // FIXME: Once N is canonical, check "SP == &N".
> +      Assert(DISubprogram(SP).describes(F),
> +             "!dbg attachment points at wrong subprogram for function",
> &N, F,
> +             &I, DL, Scope, SP);
> +    }
>  }
>
>  void Verifier::visitMDLexicalBlockBase(const MDLexicalBlockBase &N) {
>
> Modified: llvm/trunk/unittests/IR/IRBuilderTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/IRBuilderTest.cpp?rev=233663&r1=233662&r2=233663&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/IR/IRBuilderTest.cpp (original)
> +++ llvm/trunk/unittests/IR/IRBuilderTest.cpp Mon Mar 30 21:09:55 2015
> @@ -17,6 +17,7 @@
>  #include "llvm/IR/MDBuilder.h"
>  #include "llvm/IR/Module.h"
>  #include "llvm/IR/NoFolder.h"
> +#include "llvm/IR/Verifier.h"
>  #include "gtest/gtest.h"
>
>  using namespace llvm;
> @@ -303,8 +304,8 @@ TEST_F(IRBuilderTest, DIBuilder) {
>                                    0, true, nullptr);
>    auto BadScope = DIB.createLexicalBlockFile(BarSP, File, 0);
>    I->setDebugLoc(DebugLoc::get(2, 0, BadScope));
> -  EXPECT_FALSE(SP.Verify());
>    DIB.finalize();
> +  EXPECT_TRUE(verifyModule(*M));
>  }
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150330/9c239982/attachment.html>