labath updated this revision to Diff 150149.
labath added a comment.
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.
https://reviews.llvm.org/D47781
Files:
lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
lit/SymbolFile/DWARF/find-basic-variable.cpp
lit/SymbolFile/DWARF/find-qualified-variable.cpp
source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
source/Plugins/SymbolFile/DWARF/DWARFIndex.h
source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -27,6 +27,7 @@
#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"
@@ -2024,14 +2025,25 @@
// 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];
@@ -2062,6 +2074,13 @@
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;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -21,7 +21,7 @@
void Preload() override { Index(); }
- void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
+ void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
void GetGlobalVariables(const RegularExpression ®ex,
DIEArray &offsets) override;
void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -345,12 +345,8 @@
// 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;
@@ -361,9 +357,9 @@
}
}
-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 ®ex,
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -11,6 +11,7 @@
#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"
@@ -23,9 +24,9 @@
void Preload() override {}
- void GetGlobalVariables(ConstString name, DIEArray &offsets) override {}
+ void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
void GetGlobalVariables(const RegularExpression ®ex,
- 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,
@@ -42,7 +43,7 @@
void ReportInvalidDIEOffset(dw_offset_t offset,
llvm::StringRef name) override {}
- void Dump(Stream &s) override {}
+ void Dump(Stream &s) override;
private:
DebugNamesDWARFIndex(Module &module,
@@ -57,7 +58,12 @@
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
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -8,6 +8,8 @@
//===----------------------------------------------------------------------===//
#include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
+#include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/Stream.h"
using namespace lldb_private;
using namespace lldb;
@@ -23,11 +25,68 @@
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 ®ex,
+ 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());
+}
Index: source/Plugins/SymbolFile/DWARF/DWARFIndex.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -25,7 +25,11 @@
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 ®ex,
DIEArray &offsets) = 0;
virtual void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) = 0;
Index: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -33,7 +33,7 @@
void Preload() override {}
- void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
+ void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
void GetGlobalVariables(const RegularExpression ®ex,
DIEArray &offsets) override;
void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
Index: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -13,7 +13,6 @@
#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"
@@ -56,19 +55,9 @@
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 ®ex,
Index: lit/SymbolFile/DWARF/find-qualified-variable.cpp
===================================================================
--- /dev/null
+++ lit/SymbolFile/DWARF/find-qualified-variable.cpp
@@ -0,0 +1,15 @@
+// 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;
Index: lit/SymbolFile/DWARF/find-basic-variable.cpp
===================================================================
--- lit/SymbolFile/DWARF/find-basic-variable.cpp
+++ lit/SymbolFile/DWARF/find-basic-variable.cpp
@@ -20,6 +20,18 @@
// 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:
Index: lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
===================================================================
--- /dev/null
+++ lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
@@ -0,0 +1,14 @@
+// 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-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
