[llvm] r233663 - Verifier: Move over DISubprogram::Verify()
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Mar 30 20:59:46 PDT 2015
> On 2015-Mar-30, at 20:05, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> 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?
On top of the traversal the Verifier does itself.
>
> 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).
Yeah, I definitely want to keep this assertion, it's pretty valuable :).
BTW, I've effectively removed the assertion from the backend with this
commit, trusting the `Verifier` to check the IR instead.
>
> 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)
Yeah, that's basically what I'm thinking. I'd like to somehow share
the traversal that the Verifier does already (incrementally build a
map from MDLocation/MDLexicalBlock => (Function, MDSubprogram)) and
check that they're all in agreement.
>
>
> 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
>
More information about the llvm-commits
mailing list