DebugNamesDWARFIndex: Add ability to lookup variables

[labath]

Summary

This adds the ability to lookup variables to the DWARF v5 index class.
find-basic-variable.cpp test is extended to cover this scenario as well,
and I have added a new test which verifies that the dwarf v5 index class
is indeed used.

[clayborg]

Check the context as noted in inline comments.

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

70 ↗

(On Diff #149967)

Need to check for the context matches here if context is not empty.

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

14 ↗

(On Diff #149967)

move to .cpp file?

[labath]

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

70 ↗

(On Diff #149967)

Hmm... thanks for pointing this out. There is some index-independent filtering based on CompilerDeclContexts happening in SymbolFileDWARF (which is why my context test actually passes here).

I was basing this off of how apple and manual indexes work, and neither of them check the context. In fact, the manual index does not even extract the identifier portion of the name (it just looks up the whole string), so maybe nobody passes qualified names names into this function anyway? I am starting to think I should just remove the ExtractContextAndIdentifier call. If we see failures down the line, at least we will learn why was this call there in the first place.

[clayborg]

Feel free to check into it and modify the contract for GetGlobalVariables and possibly rename "name" to "basename" to be clear what the argument is. Then modify the headerdoc to clearly state what we are looking for if the filtering is going on higher up

[labath]

Ok, so it turns out we were passing qualified names into the
GetGlobalVariables function. In the apple-tables case we were then searching
based on the basename and completely ignoring the context. This resulted in
incorrect matches being returned (I believe this is the bug Jonas is
investigating right now). In the manual-index case, we weren't doing the search
based on the basename, but things worked because our index also contained
demangled names.

This version of the patch does the following:

• remove context handling from the index classes. The GetGlobalVariables function now takes a basename like you suggested.
• in particular, this means that the manual index can now stop storing demangled names of global variables. This makes it consistent with the other indexes and with the function case, where we removed demangled names a couple of weeks ago.
• puts the context extraction into SymbolFileDWARF, so that it is available to all indexes.
• implements context filtering ("fixing" Jonas's bug), also in SymbolFileDWARF. I am not particularly proud of how this is implemented (substring search), but this is the same way that function context pruning is done (cf. Module::LookupInfo::Prune). A more sensible framework for context handling would be in order, but I consider that out of scope for this patch.

[clayborg]

Much better. Anything we can do to clearly define and simplify the indexers is great stuff.

lldb/trunk/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp

+// Test that we use the DWARF v5 name indexes.
+
+// REQUIRES: lld
+
+// RUN: clang %s -g -c -emit-llvm -o - --target=x86_64-pc-linux | \
+// RUN:   llc -accel-tables=Dwarf -filetype=obj -o %t.o
+// RUN: ld.lld %t.o -o %t
+// RUN: lldb-test symbols %t | FileCheck %s
+
+// CHECK: Name Index
+// CHECK: String: 0x{{.*}} "_start"
+// CHECK: Tag: DW_TAG_subprogram
+
+extern "C" void _start() {}

lldb/trunk/lit/SymbolFile/DWARF/find-basic-variable.cpp

 // RUN: lldb-test symbols --name=foo --find=variable --context=context %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=foo --find=variable %t | \
 // RUN:   FileCheck --check-prefix=NAME %s
 // RUN: lldb-test symbols --regex --name=foo --find=variable %t | \
 // RUN:   FileCheck --check-prefix=REGEX %s
 // RUN: lldb-test symbols --name=not_there --find=variable %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+//
+// RUN: clang %s -g -c -emit-llvm -o - --target=x86_64-pc-linux | \
+// RUN:   llc -accel-tables=Dwarf -filetype=obj -o %t.o
+// RUN: ld.lld %t.o -o %t
+// RUN: lldb-test symbols --name=foo --find=variable --context=context %t | \
+// RUN:   FileCheck --check-prefix=CONTEXT %s
+// RUN: lldb-test symbols --name=foo --find=variable %t | \
+// RUN:   FileCheck --check-prefix=NAME %s
+// RUN: lldb-test symbols --regex --name=foo --find=variable %t | \
+// RUN:   FileCheck --check-prefix=REGEX %s
+// RUN: lldb-test symbols --name=not_there --find=variable %t | \
+// RUN:   FileCheck --check-prefix=EMPTY %s
 
 // EMPTY: Found 0 variables:
 // NAME: Found 4 variables:
 // CONTEXT: Found 1 variables:
 // REGEX: Found 5 variables:
 int foo;
 // NAME-DAG: name = "foo", type = {{.*}} (int), {{.*}} decl = find-basic-variable.cpp:[[@LINE-1]]
 // REGEX-DAG: name = "foo", type = {{.*}} (int), {{.*}} decl = find-basic-variable.cpp:[[@LINE-2]]

lldb/trunk/lit/SymbolFile/DWARF/find-qualified-variable.cpp

+// RUN: clang %s -g -c -o %t --target=x86_64-apple-macosx
+// RUN: lldb-test symbols --name=A::foo --find=variable %t | FileCheck %s
+
+// CHECK: Found 1 variables:
+
+struct A {
+  static int foo;
+};
+int A::foo;
+// NAME-DAG: name = "foo", {{.*}} decl = find-qualified-variable.cpp:[[@LINE-1]]
+
+struct B {
+  static int foo;
+};
+int B::foo;

lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h

AppleDWARFIndex(
       std::unique_ptr<DWARFMappedHash::MemoryTable> apple_objc)
       : DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
         m_apple_namespaces_up(std::move(apple_namespaces)),
         m_apple_types_up(std::move(apple_types)),
         m_apple_objc_up(std::move(apple_objc)) {}
 
   void Preload() override {}
 
-  void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
+  void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
                           DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override;
   void GetCompleteObjCClass(ConstString class_name, bool must_be_implementation,
                             DIEArray &offsets) override;
   void GetTypes(ConstString name, DIEArray &offsets) override;
   void GetTypes(const DWARFDeclContext &context, DIEArray &offsets) override;

lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp

 //===-- AppleDWARFIndex.cpp ------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/SymbolFile/DWARF/AppleDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
 #include "Plugins/SymbolFile/DWARF/DWARFUnit.h"
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/Function.h"
 
 using namespace lldb_private;
 using namespace lldb;
 
 std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
if (apple_names_table_up || apple_names_table_up || apple_types_table_up ||
     return llvm::make_unique<AppleDWARFIndex>(
         module, std::move(apple_names_table_up),
         std::move(apple_namespaces_table_up), std::move(apple_types_table_up),
         std::move(apple_objc_table_up));
 
   return nullptr;
 }
 
-void AppleDWARFIndex::GetGlobalVariables(ConstString name, DIEArray &offsets) {
-  if (!m_apple_names_up)
-    return;
-
-  const char *name_cstr = name.GetCString();
-  llvm::StringRef basename;
-  llvm::StringRef context;
-
-  if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name_cstr, context,
-                                                      basename))
-    basename = name_cstr;
-
-  m_apple_names_up->FindByName(basename, offsets);
+void AppleDWARFIndex::GetGlobalVariables(ConstString basename, DIEArray &offsets) {
+  if (m_apple_names_up)
+    m_apple_names_up->FindByName(basename.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetGlobalVariables(const RegularExpression &regex,
                                          DIEArray &offsets) {
   if (!m_apple_names_up)
     return;
 
   DWARFMappedHash::DIEInfoArray hash_data;

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h

 namespace lldb_private {
 class DWARFIndex {
 public:
   DWARFIndex(Module &module) : m_module(module) {}
   virtual ~DWARFIndex();
 
   virtual void Preload() = 0;
 
-  virtual void GetGlobalVariables(ConstString name, DIEArray &offsets) = 0;
+  /// Finds global variables with the given base name. Any additional filtering
+  /// (e.g., to only retrieve variables from a given context) should be done by
+  /// the consumer.
+  virtual void GetGlobalVariables(ConstString basename, DIEArray &offsets) = 0;
+
   virtual void GetGlobalVariables(const RegularExpression &regex,
                                   DIEArray &offsets) = 0;
   virtual void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) = 0;
   virtual void GetObjCMethods(ConstString class_name, DIEArray &offsets) = 0;
   virtual void GetCompleteObjCClass(ConstString class_name,
                                     bool must_be_implementation,
                                     DIEArray &offsets) = 0;
   virtual void GetTypes(ConstString name, DIEArray &offsets) = 0;

lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

 //===-- DebugNamesDWARFIndex.h ---------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_DEBUGNAMESDWARFINDEX_H
 #define LLDB_DEBUGNAMESDWARFINDEX_H
 
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Utility/ConstString.h"
 #include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 
 namespace lldb_private {
 class DebugNamesDWARFIndex : public DWARFIndex {
 public:
   static llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>>
   Create(Module &module, DWARFDataExtractor debug_names,
          DWARFDataExtractor debug_str, DWARFDebugInfo *debug_info);
 
   void Preload() override {}
 
-  void GetGlobalVariables(ConstString name, DIEArray &offsets) override {}
+  void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
-                          DIEArray &offsets) override {}
+                          DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override {}
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override {}
   void GetCompleteObjCClass(ConstString class_name, bool must_be_implementation,
                             DIEArray &offsets) override {}
   void GetTypes(ConstString name, DIEArray &offsets) override {}
   void GetTypes(const DWARFDeclContext &context, DIEArray &offsets) override {}
   void GetNamespaces(ConstString name, DIEArray &offsets) override {}
   void GetFunctions(ConstString name, DWARFDebugInfo &info,
                     const CompilerDeclContext &parent_decl_ctx,
                     uint32_t name_type_mask,
                     std::vector<DWARFDIE> &dies) override {}
   void GetFunctions(const RegularExpression &regex,
                     DIEArray &offsets) override {}
 
   void ReportInvalidDIEOffset(dw_offset_t offset,
                               llvm::StringRef name) override {}
-  void Dump(Stream &s) override {}
+  void Dump(Stream &s) override;
 
 private:
   DebugNamesDWARFIndex(Module &module,
                        std::unique_ptr<llvm::DWARFDebugNames> debug_names_up,
                        DWARFDataExtractor debug_names_data,
                        DWARFDataExtractor debug_str_data,
                        DWARFDebugInfo *debug_info)
       : DWARFIndex(module), m_debug_names_up(std::move(debug_names_up)) {}
 
   // LLVM DWARFDebugNames will hold a non-owning reference to this data, so keep
   // track of the ownership here.
   DWARFDataExtractor m_debug_names_data;
   DWARFDataExtractor m_debug_str_data;
 
-  std::unique_ptr<llvm::DWARFDebugNames> m_debug_names_up;
+  using DebugNames = llvm::DWARFDebugNames;
+  std::unique_ptr<DebugNames> m_debug_names_up;
+
+  void Append(const DebugNames::Entry &entry, DIEArray &offsets);
+  void MaybeLogLookupError(llvm::Error error, const DebugNames::NameIndex &ni,
+                           llvm::StringRef name);
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_DEBUGNAMESDWARFINDEX_H

lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

 //===-- DebugNamesDWARFIndex.cpp -------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
+#include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/Stream.h"
 
 using namespace lldb_private;
 using namespace lldb;
 
 static llvm::DWARFDataExtractor ToLLVM(const DWARFDataExtractor &data) {
   return llvm::DWARFDataExtractor(
       llvm::StringRef(reinterpret_cast<const char *>(data.GetDataStart()),
                       data.GetByteSize()),
       data.GetByteOrder() == eByteOrderLittle, data.GetAddressByteSize());
 }
 
 llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>>
 DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
                              DWARFDataExtractor debug_str,
                              DWARFDebugInfo *debug_info) {
-  auto index_up = llvm::make_unique<llvm::DWARFDebugNames>(ToLLVM(debug_names),
-                                                           ToLLVM(debug_str));
+  auto index_up =
+      llvm::make_unique<DebugNames>(ToLLVM(debug_names), ToLLVM(debug_str));
   if (llvm::Error E = index_up->extract())
     return std::move(E);
 
   return std::unique_ptr<DebugNamesDWARFIndex>(new DebugNamesDWARFIndex(
       module, std::move(index_up), debug_names, debug_str, debug_info));
 }
+
+void DebugNamesDWARFIndex::Append(const DebugNames::Entry &entry,
+                                  DIEArray &offsets) {
+  llvm::Optional<uint64_t> cu_offset = entry.getCUOffset();
+  llvm::Optional<uint64_t> die_offset = entry.getDIESectionOffset();
+  if (cu_offset && die_offset)
+    offsets.emplace_back(*cu_offset, *die_offset);
+}
+
+void DebugNamesDWARFIndex::MaybeLogLookupError(llvm::Error error,
+                                               const DebugNames::NameIndex &ni,
+                                               llvm::StringRef name) {
+  // Ignore SentinelErrors, log everything else.
+  LLDB_LOG_ERROR(
+      LogChannelDWARF::GetLogIfAll(DWARF_LOG_LOOKUPS),
+      handleErrors(std::move(error), [](const DebugNames::SentinelError &) {}),
+      "Failed to parse index entries for index at {1:x}, name {2}: {0}",
+      ni.getUnitOffset(), name);
+}
+
+void DebugNamesDWARFIndex::GetGlobalVariables(ConstString basename,
+                                              DIEArray &offsets) {
+  for (const DebugNames::Entry &entry :
+       m_debug_names_up->equal_range(basename.GetStringRef())) {
+    if (entry.tag() != DW_TAG_variable)
+      continue;
+
+    Append(entry, offsets);
+  }
+}
+
+void DebugNamesDWARFIndex::GetGlobalVariables(const RegularExpression &regex,
+                                              DIEArray &offsets) {
+  for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
+    for (DebugNames::NameTableEntry nte: ni) {
+      if (!regex.Execute(nte.getString()))
+        continue;
+
+      uint32_t entry_offset = nte.getEntryOffset();
+      llvm::Expected<DebugNames::Entry> entry_or = ni.getEntry(&entry_offset);
+      for (; entry_or; entry_or = ni.getEntry(&entry_offset)) {
+        if (entry_or->tag() != DW_TAG_variable)
+          continue;
+
+        Append(*entry_or, offsets);
+      }
+      MaybeLogLookupError(entry_or.takeError(), ni, nte.getString());
+    }
+  }
+}
+
+void DebugNamesDWARFIndex::Dump(Stream &s) {
+  std::string data;
+  llvm::raw_string_ostream os(data);
+  m_debug_names_up->dump(os);
+  s.PutCString(os.str());
+}

lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h

 namespace lldb_private {
 class ManualDWARFIndex : public DWARFIndex {
 public:
   ManualDWARFIndex(Module &module, DWARFDebugInfo *debug_info)
       : DWARFIndex(module), m_debug_info(debug_info) {}
 
   void Preload() override { Index(); }
 
-  void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
+  void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
                           DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override;
   void GetCompleteObjCClass(ConstString class_name, bool must_be_implementation,
                             DIEArray &offsets) override;
   void GetTypes(ConstString name, DIEArray &offsets) override;
   void GetTypes(const DWARFDeclContext &context, DIEArray &offsets) override;

lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

case DW_TAG_variable:
         // "(anonymous namespace)::i"...
 
         // Make sure our mangled name isn't the same string table entry as our
         // name. If it starts with '_', then it is ok, else compare the string
         // to make sure it isn't the same and we don't end up with duplicate
         // entries
         if (mangled_cstr && name != mangled_cstr &&
             ((mangled_cstr[0] == '_') || (::strcmp(name, mangled_cstr) != 0))) {
-          Mangled mangled(ConstString(mangled_cstr), true);
-          set.globals.Insert(mangled.GetMangledName(),
+          set.globals.Insert(ConstString(mangled_cstr),
                              DIERef(cu_offset, die.GetOffset()));
-          ConstString demangled = mangled.GetDemangledName(cu_language);
-          if (demangled)
-            set.globals.Insert(demangled, DIERef(cu_offset, die.GetOffset()));
         }
       }
       break;
 
     default:
       continue;
     }
   }
 }
 
-void ManualDWARFIndex::GetGlobalVariables(ConstString name, DIEArray &offsets) {
+void ManualDWARFIndex::GetGlobalVariables(ConstString basename, DIEArray &offsets) {
   Index();
-  m_set.globals.Find(name, offsets);
+  m_set.globals.Find(basename, offsets);
 }
 
 void ManualDWARFIndex::GetGlobalVariables(const RegularExpression &regex,
                                           DIEArray &offsets) {
   Index();
   m_set.globals.Find(regex, offsets);
 }
 

lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Symbols.h"
 
 #include "lldb/Interpreter/OptionValueFileSpecList.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 
uint32_t SymbolFileDWARF::FindGlobalVariables(
 
   DWARFDebugInfo *info = DebugInfo();
   if (info == NULL)
     return 0;
 
   // Remember how many variables are in the list before we search.
   const uint32_t original_size = variables.GetSize();
 
+  llvm::StringRef basename;
+  llvm::StringRef context;
+
+  if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
+                                                      context, basename))
+    basename = name.GetStringRef();
+
   DIEArray die_offsets;
-  m_index->GetGlobalVariables(name, die_offsets);
+  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
   if (num_die_matches) {
     SymbolContext sc;
     sc.module_sp = m_obj_file->GetModule();
     assert(sc.module_sp);
 
+    // Loop invariant: Variables up to this index have been checked for context
+    // matches.
+    uint32_t pruned_idx = original_size;
+
     bool done = false;
     for (size_t i = 0; i < num_die_matches && !done; ++i) {
       const DIERef &die_ref = die_offsets[i];
       DWARFDIE die = GetDIE(die_ref);
 
       if (die) {
         switch (die.Tag()) {
         default:
for (size_t i = 0; i < num_die_matches && !done; ++i) {
               if (!actual_parent_decl_ctx ||
                   actual_parent_decl_ctx != *parent_decl_ctx)
                 continue;
             }
           }
 
           ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
                          &variables);
+          while (pruned_idx < variables.GetSize()) {
+            VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+            if (var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+              ++pruned_idx;
+            else
+              variables.RemoveVariableAtIndex(pruned_idx);
+          }
 
           if (variables.GetSize() - original_size >= max_matches)
             done = true;
         } break;
         }
       } else {
         m_index->ReportInvalidDIEOffset(die_ref.die_offset,
                                         name.GetStringRef());