DebugInfo: Reapply r209984 (reverted in r210143), asserting that abstract DbgVariable...
authorDavid Blaikie <dblaikie@gmail.com>
Wed, 4 Jun 2014 23:50:52 +0000 (23:50 +0000)
committerDavid Blaikie <dblaikie@gmail.com>
Wed, 4 Jun 2014 23:50:52 +0000 (23:50 +0000)
Abstract variables within abstract scopes that are entirely optimized
away in their first inlining are omitted because their scope is not
present so the variable is never created. Instead, we should ensure the
scope is created so the variable can be added, even if it's been
optimized away in its first inlining.

This fixes the incorrect debug info in missing-abstract-variable.ll
(added in r210143) and passes an asserts self-hosting build, so
hopefully there's not more of these issues left behind... *fingers
crossed*.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@210221 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/LexicalScopes.h
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
lib/CodeGen/AsmPrinter/DwarfDebug.h
lib/CodeGen/AsmPrinter/DwarfUnit.cpp
test/DebugInfo/missing-abstract-variable.ll

index 31d687267416ceaa8711705350633fdbf4511f06..036aea30a5102b48d06b885f706f8393f1a95a52 100644 (file)
@@ -197,6 +197,9 @@ public:
   /// dump - Print data structures to dbgs().
   void dump();
 
+  /// getOrCreateAbstractScope - Find or create an abstract lexical scope.
+  LexicalScope *getOrCreateAbstractScope(const MDNode *N);
+
 private:
   /// getOrCreateLexicalScope - Find lexical scope for the given DebugLoc. If
   /// not available then create new lexical scope.
@@ -208,9 +211,6 @@ private:
   /// getOrCreateInlinedScope - Find or create an inlined lexical scope.
   LexicalScope *getOrCreateInlinedScope(MDNode *Scope, MDNode *InlinedAt);
 
-  /// getOrCreateAbstractScope - Find or create an abstract lexical scope.
-  LexicalScope *getOrCreateAbstractScope(const MDNode *N);
-
   /// extractLexicalScopes - Extract instruction ranges for each lexical scopes
   /// for the given machine function.
   void extractLexicalScopes(SmallVectorImpl<InsnRange> &MIRanges,
index 73496b0c38232c906f350b1ee85c9ab6d747fb55..4dd36830a445c2808d178545f7a29bf264604f6e 100644 (file)
@@ -1052,24 +1052,49 @@ DbgVariable *DwarfDebug::findAbstractVariable(DIVariable &DV,
   return findAbstractVariable(DV, ScopeLoc.getScope(DV->getContext()));
 }
 
-DbgVariable *DwarfDebug::findAbstractVariable(DIVariable &DV,
-                                              const MDNode *ScopeNode) {
+DbgVariable *DwarfDebug::getExistingAbstractVariable(DIVariable &DV,
+                                                     DIVariable &Cleansed) {
   LLVMContext &Ctx = DV->getContext();
   // More then one inlined variable corresponds to one abstract variable.
-  DIVariable Var = cleanseInlinedVariable(DV, Ctx);
-  auto I = AbstractVariables.find(Var);
+  // FIXME: This duplication of variables when inlining should probably be
+  // removed. It's done to allow each DIVariable to describe its location
+  // because the DebugLoc on the dbg.value/declare isn't accurate. We should
+  // make it accurate then remove this duplication/cleansing stuff.
+  Cleansed = cleanseInlinedVariable(DV, Ctx);
+  auto I = AbstractVariables.find(Cleansed);
   if (I != AbstractVariables.end())
     return I->second.get();
+  return nullptr;
+}
 
-  LexicalScope *Scope = LScopes.findAbstractScope(ScopeNode);
-  if (!Scope)
-    return nullptr;
-
+DbgVariable *DwarfDebug::createAbstractVariable(DIVariable &Var,
+                                                LexicalScope *Scope) {
   auto AbsDbgVariable = make_unique<DbgVariable>(Var, nullptr, this);
   addScopeVariable(Scope, AbsDbgVariable.get());
   return (AbstractVariables[Var] = std::move(AbsDbgVariable)).get();
 }
 
+DbgVariable *DwarfDebug::getOrCreateAbstractVariable(DIVariable &DV,
+                                                     const MDNode *ScopeNode) {
+  DIVariable Cleansed = DV;
+  if (DbgVariable *Var = getExistingAbstractVariable(DV, Cleansed))
+    return Var;
+
+  return createAbstractVariable(Cleansed,
+                                LScopes.getOrCreateAbstractScope(ScopeNode));
+}
+
+DbgVariable *DwarfDebug::findAbstractVariable(DIVariable &DV,
+                                              const MDNode *ScopeNode) {
+  DIVariable Cleansed = DV;
+  if (DbgVariable *Var = getExistingAbstractVariable(DV, Cleansed))
+    return Var;
+
+  if (LexicalScope *Scope = LScopes.findAbstractScope(ScopeNode))
+    return createAbstractVariable(Cleansed, Scope);
+  return nullptr;
+}
+
 // If Var is a current function argument then add it to CurrentFnArguments list.
 bool DwarfDebug::addCurrentFnArgument(DbgVariable *Var, LexicalScope *Scope) {
   if (!LScopes.isCurrentFunctionScope(Scope))
@@ -1513,7 +1538,7 @@ void DwarfDebug::endFunction(const MachineFunction *MF) {
       assert(DV && DV.isVariable());
       if (!ProcessedVars.insert(DV))
         continue;
-      findAbstractVariable(DV, DV.getContext());
+      getOrCreateAbstractVariable(DV, DV.getContext());
     }
     constructAbstractSubprogramScopeDIE(TheCU, AScope);
   }
index 2d673791102ee867a7990732142333a2204df5a8..a3b0242fd5f72f148c34a620c803c8605975475e 100644 (file)
@@ -343,6 +343,11 @@ class DwarfDebug : public AsmPrinterHandler {
   }
 
   /// \brief Find abstract variable associated with Var.
+  DbgVariable *getExistingAbstractVariable(DIVariable &DV,
+                                           DIVariable &Cleansed);
+  DbgVariable *createAbstractVariable(DIVariable &DV, LexicalScope *Scope);
+  DbgVariable *getOrCreateAbstractVariable(DIVariable &Var,
+                                           const MDNode *Scope);
   DbgVariable *findAbstractVariable(DIVariable &Var, DebugLoc Loc);
   DbgVariable *findAbstractVariable(DIVariable &Var, const MDNode *Scope);
 
index 2325ab8444ec93ee271ec2128abd138d941e5a08..0bac3bec38917f330aefd772f83e4c402ac052f5 100644 (file)
@@ -1782,12 +1782,10 @@ std::unique_ptr<DIE> DwarfUnit::constructVariableDIEImpl(const DbgVariable &DV,
   // Define variable debug information entry.
   auto VariableDie = make_unique<DIE>(DV.getTag());
   DbgVariable *AbsVar = DV.getAbstractVariable();
-  // FIXME: any missing abstract variable missing a DIE will result in incorrect
-  // DWARF. More details in test/DebugInfo/missing-abstract-variable.ll for an
-  // example of why this is happening.
-  if (AbsVar && AbsVar->getDIE())
+  if (AbsVar) {
+    assert(AbsVar->getDIE());
     addDIEEntry(*VariableDie, dwarf::DW_AT_abstract_origin, *AbsVar->getDIE());
-  else {
+  else {
     if (!Name.empty())
       addString(*VariableDie, dwarf::DW_AT_name, Name);
     addSourceLine(*VariableDie, DV.getVariable());
index d2bfae0bf2bf8f09b7ed9393cd6994094b002bb3..c09227a0a713c3dc8fb330634958512ecc939979 100644 (file)
 ; CHECK: [[ABS_B:.*]]:   DW_TAG_formal_parameter
 ; CHECK-NOT: DW_TAG
 ; CHECK:     DW_AT_name {{.*}} "b"
-; FIXME: Missing 'x's local 's' variable.
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK:     DW_TAG_lexical_block
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK:       DW_TAG_lexical_block
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK: [[ABS_S:.*]]:       DW_TAG_variable
+; CHECK-NOT: DW_TAG
+; CHECK:         DW_AT_name {{.*}} "s"
 
 ; CHECK: DW_TAG_subprogram
 ; CHECK-NOT: DW_TAG
 ; CHECK-NOT: DW_TAG
 ; CHECK:     DW_AT_abstract_origin {{.*}} {[[ABS_X]]}
 ; CHECK-NOT: {{DW_TAG|NULL}}
-; FIXME: This formal parameter goes missing at least at -O2, maybe before that.
+; FIXME: This formal parameter goes missing at least at -O2 (& on
+; mips/powerpc), maybe before that. Perhaps SelectionDAG is to blame (and
+; fastisel succeeds).
 ; CHECK:     DW_TAG_formal_parameter
 ; CHECK-NOT: DW_TAG
 ; CHECK:       DW_AT_abstract_origin {{.*}} {[[ABS_B]]}
 
 ; The two lexical blocks here are caused by the scope of the if that includes
 ; the condition variable, and the scope within the if's composite statement. I'm
-; not sure we really need both of them.
+; not sure we really need both of them since there's no variable declared in the
+; outer of the two
 
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK:     DW_TAG_lexical_block
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK:         DW_TAG_variable
 ; CHECK-NOT: DW_TAG
-
-; FIXME: This shouldn't have a name here, it should use DW_AT_abstract_origin
-; to reference an abstract variable definition instead
-
-; CHECK:           DW_AT_name {{.*}} "s"
-
+; CHECK:           DW_AT_abstract_origin {{.*}} {[[ABS_S]]}
 
 @t = external global i32