Add a unit test for DILocation::getMergedLocation()

[aprantl]

Summary

This is a unit test for DILocation::getMergedLocation() that exposes a bug in the implementation of r340583. (You'll need to build with assertions enabled to see it.)

[aprantl]

unittests/IR/MetadataTest.cpp

924 ↗

(On Diff #162492)

This triggers an assertion in DILocation::getScope() because the DIFile is not a DILocalScope.

Some ideas for fixing this:

• create a new dummy DISubprogram(name: "$llvm$merged$foo$bar")
• randomly pick one location's scope (it's line 0 after all)

[dblaikie]

Could you still come up with a practical test case for this? (doesn't
necessarily need to be checked in, to be honest - but something to validate
that this use case/support is needed & not working around a bug (& if it is
working around a bug, that we know about it so we can remove the workaround
eventually & not leave it in as folklore and cruft))

I can imagine that there could be cases where there would be no common
scope - due to inlining into a nodebug function (we have that classic
assertion to ensure all scope chains lead to the DISubprogram that contains
those locations - but that does not apply if there is no DISubprogram for a
function - if it is nodebug and has had debug info inlined into it (which
is still useful if, then, the nodebug function is inlined into somewhere
else)). So maybe you can search around there.

Though I also think probably the right answer here is not to pick one of
the two locations, but to drop the location entirely.

• Dave

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

while (S) {
     if (Locations.count(std::make_pair(S, L)))
       break;
     S = S->getScope().resolve();
     if (!S && L) {
       S = L->getScope();
       L = L->getInlinedAt();
     }
   }
+
+  // If the two locations are irreconsilable, just pick one. This is misleading,
+  // but on the other hand, it's a "line 0" location.
+  if (!S || !isa<DILocalScope>(S))
+    S = LocA->getScope();
   return DILocation::get(Result->getContext(), 0, 0, S, L);
 }
 
 DINode::DIFlags DINode::getFlag(StringRef Flag) {
   return StringSwitch<DIFlags>(Flag)
 #define HANDLE_DI_FLAG(ID, NAME) .Case("DIFlag" #NAME, Flag##NAME)
 #include "llvm/IR/DebugInfoFlags.def"
       .Default(DINode::FlagZero);

llvm/trunk/unittests/IR/MetadataTest.cpp

TEST_F(DILocationTest, Overflow) {
   }
   {
     DILocation *L = DILocation::get(Context, UINT32_MAX, U16 + 1, N);
     EXPECT_EQ(UINT32_MAX, L->getLine());
     EXPECT_EQ(0u, L->getColumn());
   }
 }
 
+TEST_F(DILocationTest, Merge) {
+  DISubprogram *N = getSubprogram();
+  DIScope *S = DILexicalBlock::get(Context, N, getFile(), 3, 4);
+
+  {
+    // Identical.
+    auto *A = DILocation::get(Context, 2, 7, N);
+    auto *B = DILocation::get(Context, 2, 7, N);
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(2u, M->getLine());
+    EXPECT_EQ(7u, M->getColumn());
+    EXPECT_EQ(N, M->getScope());
+  }
+
+  {
+    // Identical, different scopes.
+    auto *A = DILocation::get(Context, 2, 7, N);
+    auto *B = DILocation::get(Context, 2, 7, S);
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(0u, M->getLine()); // FIXME: Should this be 2?
+    EXPECT_EQ(0u, M->getColumn()); // FIXME: Should this be 7?
+    EXPECT_EQ(N, M->getScope());
+  }
+
+  {
+    // Different lines, same scopes.
+    auto *A = DILocation::get(Context, 1, 6, N);
+    auto *B = DILocation::get(Context, 2, 7, N);
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(0u, M->getLine());
+    EXPECT_EQ(0u, M->getColumn());
+    EXPECT_EQ(N, M->getScope());
+  }
+
+  {
+    // Twisty locations, all different, same function.
+    auto *A = DILocation::get(Context, 1, 6, N);
+    auto *B = DILocation::get(Context, 2, 7, S);
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(0u, M->getLine());
+    EXPECT_EQ(0u, M->getColumn());
+    EXPECT_EQ(N, M->getScope());
+  }
+
+  {
+    // Different function, same inlined-at.
+    auto *F = getFile();
+    auto *SP1 = DISubprogram::getDistinct(Context, F, "a", "a", F, 0, nullptr,
+                                          false, false, 0, nullptr, 0, 0, 0,
+                                          DINode::FlagZero, false, nullptr);
+    auto *SP2 = DISubprogram::getDistinct(Context, F, "b", "b", F, 0, nullptr,
+                                          false, false, 0, nullptr, 0, 0, 0,
+                                          DINode::FlagZero, false, nullptr);
+
+    auto *I = DILocation::get(Context, 2, 7, N);
+    auto *A = DILocation::get(Context, 1, 6, SP1, I);
+    auto *B = DILocation::get(Context, 2, 7, SP2, I);
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(0u, M->getLine());
+    EXPECT_EQ(0u, M->getColumn());
+    EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
+    EXPECT_EQ(I, M->getInlinedAt());
+  }
+
+   {
+    // Completely different.
+    auto *I = DILocation::get(Context, 2, 7, N);
+    auto *A = DILocation::get(Context, 1, 6, S, I);
+    auto *B = DILocation::get(Context, 2, 7, getSubprogram());
+    auto *M = DILocation::getMergedLocation(A, B);
+    EXPECT_EQ(0u, M->getLine());
+    EXPECT_EQ(0u, M->getColumn());
+    EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
+    EXPECT_EQ(S, M->getScope());
+    EXPECT_EQ(nullptr, M->getInlinedAt());
+  }
+}
+
 TEST_F(DILocationTest, getDistinct) {
   MDNode *N = getSubprogram();
   DILocation *L0 = DILocation::getDistinct(Context, 2, 7, N);
   EXPECT_TRUE(L0->isDistinct());
   DILocation *L1 = DILocation::get(Context, 2, 7, N);
   EXPECT_FALSE(L1->isDistinct());
   EXPECT_EQ(L1, DILocation::get(Context, 2, 7, N));
 }