<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 30, 2015 at 7:09 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dexonsmith<br>
Date: Mon Mar 30 21:09:55 2015<br>
New Revision: 233663<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233663&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233663&view=rev</a><br>
Log:<br>
Verifier: Move over DISubprogram::Verify()<br>
<br>
Move over the remaining (somewhat complicated) check from<br>
`DISubprogram::Verify()`.  I suspect this check could be optimized --<br>
e.g., it would be nice not to do another full traversal here -- but it's<br>
not exactly obvious how.</blockquote><div><br>"another" full traversal on top of which other one?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  For now, just bring it over as is.<br>
<br>
Once we have a better model for the "canonical" subprogram of a<br>
`Function`, we should enforce that all `!dbg` attachments lead to the<br>
canonical one.<br></blockquote><div><br>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).<br><br>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?<br><br>(& then possibly walk all the DISubprograms and ensure that only no llvm::Function is referenced from more than one DISubprogram) <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
    llvm/trunk/lib/IR/DebugInfo.cpp<br>
    llvm/trunk/lib/IR/Verifier.cpp<br>
    llvm/trunk/unittests/IR/IRBuilderTest.cpp<br>
<br>
Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=233663&r1=233662&r2=233663&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=233663&r1=233662&r2=233663&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
+++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Mar 30 21:09:55 2015<br>
@@ -256,39 +256,7 @@ bool DIType::Verify() const { return isT<br>
 bool DIBasicType::Verify() const { return isBasicType(); }<br>
 bool DIDerivedType::Verify() const { return isDerivedType(); }<br>
 bool DICompositeType::Verify() const { return isCompositeType(); }<br>
-<br>
-bool DISubprogram::Verify() const {<br>
-  auto *N = dyn_cast_or_null<MDSubprogram>(DbgNode);<br>
-  if (!N)<br>
-    return false;<br>
-<br>
-  // If a DISubprogram has an llvm::Function*, then scope chains from all<br>
-  // instructions within the function should lead to this DISubprogram.<br>
-  if (auto *F = getFunction()) {<br>
-    for (auto &BB : *F) {<br>
-      for (auto &I : BB) {<br>
-        MDLocation *DL = I.getDebugLoc();<br>
-        if (!DL)<br>
-          continue;<br>
-<br>
-        // walk the inlined-at scopes<br>
-        MDScope *Scope = DL->getInlinedAtScope();<br>
-        if (!Scope)<br>
-          return false;<br>
-        while (!isa<MDSubprogram>(Scope)) {<br>
-          Scope = cast<MDLexicalBlockBase>(Scope)->getScope();<br>
-          if (!Scope)<br>
-            return false;<br>
-        }<br>
-        if (!DISubprogram(Scope).describes(F))<br>
-          return false;<br>
-      }<br>
-    }<br>
-  }<br>
-<br>
-  return true;<br>
-}<br>
-<br>
+bool DISubprogram::Verify() const { return isSubprogram(); }<br>
 bool DIGlobalVariable::Verify() const { return isGlobalVariable(); }<br>
 bool DIVariable::Verify() const { return isVariable(); }<br>
<br>
<br>
Modified: llvm/trunk/lib/IR/Verifier.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=233663&r1=233662&r2=233663&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=233663&r1=233662&r2=233663&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/Verifier.cpp (original)<br>
+++ llvm/trunk/lib/IR/Verifier.cpp Mon Mar 30 21:09:55 2015<br>
@@ -921,6 +921,45 @@ void Verifier::visitMDSubprogram(const M<br>
   }<br>
   Assert(!hasConflictingReferenceFlags(N.getFlags()), "invalid reference flags",<br>
          &N);<br>
+<br>
+  if (!N.getFunction())<br>
+    return;<br>
+<br>
+  // FIXME: Should this be looking through bitcasts?<br>
+  auto *F = dyn_cast<Function>(N.getFunction()->getValue());<br>
+  if (!F)<br>
+    return;<br>
+<br>
+  // Check that all !dbg attachments lead to back to N (or, at least, another<br>
+  // subprogram that describes the same function).<br>
+  //<br>
+  // FIXME: Check this incrementally while visiting !dbg attachments.<br>
+  // FIXME: Only check when N is the canonical subprogram for F.<br>
+  SmallPtrSet<const MDNode *, 32> Seen;<br>
+  for (auto &BB : *F)<br>
+    for (auto &I : BB) {<br>
+      // Be careful about using MDLocation here since we might be dealing with<br>
+      // broken code (this is the Verifier after all).<br>
+      MDLocation *DL =<br>
+          dyn_cast_or_null<MDLocation>(I.getDebugLoc().getAsMDNode());<br>
+      if (!DL)<br>
+        continue;<br>
+      if (!Seen.insert(DL).second)<br>
+        continue;<br>
+<br>
+      MDLocalScope *Scope = DL->getInlinedAtScope();<br>
+      if (Scope && !Seen.insert(Scope).second)<br>
+        continue;<br>
+<br>
+      MDSubprogram *SP = Scope ? Scope->getSubprogram() : nullptr;<br>
+      if (SP && !Seen.insert(SP).second)<br>
+        continue;<br>
+<br>
+      // FIXME: Once N is canonical, check "SP == &N".<br>
+      Assert(DISubprogram(SP).describes(F),<br>
+             "!dbg attachment points at wrong subprogram for function", &N, F,<br>
+             &I, DL, Scope, SP);<br>
+    }<br>
 }<br>
<br>
 void Verifier::visitMDLexicalBlockBase(const MDLexicalBlockBase &N) {<br>
<br>
Modified: llvm/trunk/unittests/IR/IRBuilderTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/IRBuilderTest.cpp?rev=233663&r1=233662&r2=233663&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/IRBuilderTest.cpp?rev=233663&r1=233662&r2=233663&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/IR/IRBuilderTest.cpp (original)<br>
+++ llvm/trunk/unittests/IR/IRBuilderTest.cpp Mon Mar 30 21:09:55 2015<br>
@@ -17,6 +17,7 @@<br>
 #include "llvm/IR/MDBuilder.h"<br>
 #include "llvm/IR/Module.h"<br>
 #include "llvm/IR/NoFolder.h"<br>
+#include "llvm/IR/Verifier.h"<br>
 #include "gtest/gtest.h"<br>
<br>
 using namespace llvm;<br>
@@ -303,8 +304,8 @@ TEST_F(IRBuilderTest, DIBuilder) {<br>
                                   0, true, nullptr);<br>
   auto BadScope = DIB.createLexicalBlockFile(BarSP, File, 0);<br>
   I->setDebugLoc(DebugLoc::get(2, 0, BadScope));<br>
-  EXPECT_FALSE(SP.Verify());<br>
   DIB.finalize();<br>
+  EXPECT_TRUE(verifyModule(*M));<br>
 }<br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>