[debugger] Add options to FindName().
FindName() has two variants, one that returns multiple things and one
that returns just the first (for convenience). The simpler one does not
allow setting the options because for historical usage this was not
necessary.
This patch adds options for the simpler variant of FindName(). Currently
only the default options are used and there should be no behavior
change. But this will allow future uses of the simpler API.
It also plumbs the options through the name lookup callback. As above,
there is no behavior change and the callers currently specify "all
types" (the previous default).
Hook up the "declaration" flag for types. Currently this isn't really
used but will be used in a followup to convert declarations to
definitions when printing.
Slightly improve the debug dumping of the symbol index to indicate the
types of entities referenced. This is never used in real code, only for
manual testing. A commented-out test was added to explicitly dump the
index for a hardcoded file name for certain debugging tasks.
Change-Id: I4d549fb1e5f00d82ae93a2c4ed07756397b54e20
diff --git a/src/developer/debug/zxdb/expr/expr_parser.cc b/src/developer/debug/zxdb/expr/expr_parser.cc
index 8fc2bac..4d4a8db 100644
--- a/src/developer/debug/zxdb/expr/expr_parser.cc
+++ b/src/developer/debug/zxdb/expr/expr_parser.cc
@@ -320,7 +320,8 @@
// The thing we just made is either a type or a name, look it up.
if (name_lookup_callback_) {
- FoundName lookup = name_lookup_callback_(result.ident);
+ FoundName lookup = name_lookup_callback_(
+ result.ident, FindNameOptions(FindNameOptions::kAllKinds));
switch (lookup.kind()) {
case FoundName::kType:
mode = kType;
@@ -367,7 +368,8 @@
// Decode what adding the name just generated.
if (name_lookup_callback_) {
- FoundName lookup = name_lookup_callback_(result.ident);
+ FoundName lookup = name_lookup_callback_(
+ result.ident, FindNameOptions(FindNameOptions::kAllKinds));
switch (lookup.kind()) {
case FoundName::kNamespace:
mode = kNamespace;
diff --git a/src/developer/debug/zxdb/expr/expr_parser_unittest.cc b/src/developer/debug/zxdb/expr/expr_parser_unittest.cc
index 009c2b3..facda80 100644
--- a/src/developer/debug/zxdb/expr/expr_parser_unittest.cc
+++ b/src/developer/debug/zxdb/expr/expr_parser_unittest.cc
@@ -18,13 +18,14 @@
// This name looker-upper declares anything beginning with "Namespace" is a
// namespace, anything beginning with "Template" is a template, and anything
// beginning with "Type" is a type.
-FoundName TestLookupName(const ParsedIdentifier& ident) {
+FoundName TestLookupName(const ParsedIdentifier& ident,
+ const FindNameOptions& opts) {
const ParsedIdentifierComponent& comp = ident.components().back();
const std::string& name = comp.name();
- if (StringBeginsWith(name, "Namespace"))
+ if (opts.find_namespaces && StringBeginsWith(name, "Namespace"))
return FoundName(FoundName::kNamespace, ident.GetFullName());
- if (StringBeginsWith(name, "Type")) {
+ if (opts.find_types && StringBeginsWith(name, "Type")) {
// Make up a random class to go with the type.
auto type = fxl::MakeRefCounted<Collection>(DwarfTag::kClassType);
type->set_assigned_name("Type");
@@ -34,7 +35,7 @@
// to strings properly. This could be added if necessary.
return FoundName(std::move(type));
}
- if (StringBeginsWith(name, "Template")) {
+ if (opts.find_templates && StringBeginsWith(name, "Template")) {
if (comp.has_template()) {
// Assume templates with arguments are types.
return FoundName(fxl::MakeRefCounted<Collection>(DwarfTag::kClassType));
diff --git a/src/developer/debug/zxdb/expr/find_name.cc b/src/developer/debug/zxdb/expr/find_name.cc
index dc34865..2fef705 100644
--- a/src/developer/debug/zxdb/expr/find_name.cc
+++ b/src/developer/debug/zxdb/expr/find_name.cc
@@ -220,10 +220,13 @@
: target_symbols(ts) {}
FoundName FindName(const FindNameContext& context,
+ const FindNameOptions& options,
const ParsedIdentifier& identifier) {
+ FindNameOptions new_opts(options);
+ new_opts.max_results = 1;
+
std::vector<FoundName> results;
- FindName(context, FindNameOptions(FindNameOptions::kAllKinds), identifier,
- &results);
+ FindName(context, new_opts, identifier, &results);
if (!results.empty())
return std::move(results[0]);
return FoundName();
diff --git a/src/developer/debug/zxdb/expr/find_name.h b/src/developer/debug/zxdb/expr/find_name.h
index 800b0b4..31c43f8 100644
--- a/src/developer/debug/zxdb/expr/find_name.h
+++ b/src/developer/debug/zxdb/expr/find_name.h
@@ -103,7 +103,11 @@
// and global scopes for one or more things with a matching name. The first
// version finds the first exact match of any type, the second uses the
// options to customize what and how many results are returned.
+//
+// The variant that returns a single value ignores the max_results of the
+// options and always returns the first thing.
FoundName FindName(const FindNameContext& context,
+ const FindNameOptions& options,
const ParsedIdentifier& identifier);
void FindName(const FindNameContext& context, const FindNameOptions& options,
const ParsedIdentifier& looking_for,
diff --git a/src/developer/debug/zxdb/expr/find_name_unittest.cc b/src/developer/debug/zxdb/expr/find_name_unittest.cc
index 4ef3308..7978e60 100644
--- a/src/developer/debug/zxdb/expr/find_name_unittest.cc
+++ b/src/developer/debug/zxdb/expr/find_name_unittest.cc
@@ -120,14 +120,16 @@
// ACTUAL TEST CODE ----------------------------------------------------------
+ FindNameOptions all_kinds(FindNameOptions::kAllKinds);
+
// Find "value" in the nested block should give the block's one.
ParsedIdentifier value_ident(var_value->GetAssignedName());
- FoundName found = FindName(block_context, value_ident);
+ FoundName found = FindName(block_context, all_kinds, value_ident);
EXPECT_TRUE(found);
EXPECT_EQ(block_value.get(), found.variable());
// Find "value" in the function block should give the function's one.
- found = FindName(function_context, value_ident);
+ found = FindName(function_context, all_kinds, value_ident);
EXPECT_TRUE(found);
EXPECT_EQ(var_value.get(), found.variable());
EXPECT_EQ(var_value->GetAssignedName(), found.GetName());
@@ -136,7 +138,7 @@
ParsedIdentifier value_global_ident(
IdentifierQualification::kGlobal,
ParsedIdentifierComponent(var_value->GetAssignedName()));
- found = FindName(function_context, value_global_ident);
+ found = FindName(function_context, all_kinds, value_global_ident);
EXPECT_FALSE(found);
// Prefix search for "va" should find all three "values".
@@ -157,16 +159,16 @@
// Find "block_local" in the block should be found, but in the function it
// should not be.
ParsedIdentifier block_local_ident(block_other->GetAssignedName());
- found = FindName(block_context, block_local_ident);
+ found = FindName(block_context, all_kinds, block_local_ident);
EXPECT_TRUE(found);
EXPECT_EQ(block_other.get(), found.variable());
EXPECT_EQ(block_other->GetAssignedName(), found.GetName());
- found = FindName(function_context, block_local_ident);
+ found = FindName(function_context, all_kinds, block_local_ident);
EXPECT_FALSE(found);
// Finding the other function parameter in the block should work.
ParsedIdentifier other_param_ident(param_other->GetAssignedName());
- found = FindName(block_context, other_param_ident);
+ found = FindName(block_context, all_kinds, other_param_ident);
EXPECT_TRUE(found);
EXPECT_EQ(param_other.get(), found.variable());
@@ -174,7 +176,7 @@
// namespace) from within the context of the "ns::function()" function.
// The namespace of the function should be implicitly picked up.
ParsedIdentifier ns_value_ident(kNsVarName);
- found = FindName(block_context, ns_value_ident);
+ found = FindName(block_context, all_kinds, ns_value_ident);
EXPECT_TRUE(found);
EXPECT_EQ(ns_value.var.get(), found.variable());
EXPECT_EQ(kNsVarName, found.GetName());
@@ -183,7 +185,7 @@
// should fail and not crash.
FindNameContext block_no_modules_context;
block_no_modules_context.block = block.get();
- found = FindName(block_no_modules_context, ns_value_ident);
+ found = FindName(block_no_modules_context, all_kinds, ns_value_ident);
EXPECT_FALSE(found);
// Break reference cycle for test teardown.
@@ -410,8 +412,10 @@
// ACTUAL TEST CODE ----------------------------------------------------------
+ FindNameOptions all_kinds(FindNameOptions::kAllKinds);
+
// Look up from the global function.
- FoundName found = FindName(function_context, global_type_name);
+ FoundName found = FindName(function_context, all_kinds, global_type_name);
EXPECT_TRUE(found);
EXPECT_EQ(FoundName::kType, found.kind());
EXPECT_EQ(global_type.get(), found.type().get());
@@ -428,7 +432,7 @@
EXPECT_EQ(global_type.get(), found_vect[0].type().get());
// Look up the child function by full name.
- found = FindName(function_context, full_child_type_name);
+ found = FindName(function_context, all_kinds, full_child_type_name);
EXPECT_TRUE(found);
EXPECT_EQ(FoundName::kType, found.kind());
EXPECT_EQ(child_type.get(), found.type().get());
@@ -436,7 +440,7 @@
// Look up the child function by just the child name. Since the function is
// a member of GlobalType, ChildType is a member of "this" so it should be
// found.
- found = FindName(function_context, child_type_name);
+ found = FindName(function_context, all_kinds, child_type_name);
EXPECT_TRUE(found);
EXPECT_EQ(FoundName::kType, found.kind());
EXPECT_EQ(child_type.get(), found.type().get());
@@ -474,15 +478,16 @@
SymbolContext symbol_context(kLoadAddress);
setup.InjectModule("mod", "1234", kLoadAddress, std::move(mod));
+ FindNameOptions all_types(FindNameOptions::kAllKinds);
+
// The string "Template" should be identified as one.
ParsedIdentifier template_name("Template");
- auto found = FindName(context, template_name);
+ auto found = FindName(context, all_types, template_name);
EXPECT_TRUE(found);
EXPECT_EQ(FoundName::kTemplate, found.kind());
EXPECT_EQ("Template", found.GetName());
// The string "TemplateNot" is a type, it should be found as such.
- FindNameOptions all_types(FindNameOptions::kAllKinds);
std::vector<FoundName> found_vect;
FindName(context, all_types, template_not_name, &found_vect);
ASSERT_EQ(1u, found_vect.size());
diff --git a/src/developer/debug/zxdb/expr/mock_expr_eval_context.cc b/src/developer/debug/zxdb/expr/mock_expr_eval_context.cc
index 9f8811c..830df39 100644
--- a/src/developer/debug/zxdb/expr/mock_expr_eval_context.cc
+++ b/src/developer/debug/zxdb/expr/mock_expr_eval_context.cc
@@ -43,9 +43,11 @@
NameLookupCallback MockExprEvalContext::GetSymbolNameLookupCallback() {
// This mock version just integrates with builtin types.
- return [](const ParsedIdentifier& ident) {
- if (auto type = GetBuiltinType(ident.GetFullName()))
- return FoundName(std::move(type));
+ return [](const ParsedIdentifier& ident, const FindNameOptions& opts) {
+ if (opts.find_types) {
+ if (auto type = GetBuiltinType(ident.GetFullName()))
+ return FoundName(std::move(type));
+ }
return FoundName();
};
}
diff --git a/src/developer/debug/zxdb/expr/name_lookup.h b/src/developer/debug/zxdb/expr/name_lookup.h
index e978c55..be21dce 100644
--- a/src/developer/debug/zxdb/expr/name_lookup.h
+++ b/src/developer/debug/zxdb/expr/name_lookup.h
@@ -7,6 +7,7 @@
#include <functional>
+#include "src/developer/debug/zxdb/expr/find_name.h"
#include "src/developer/debug/zxdb/expr/found_name.h"
#include "src/developer/debug/zxdb/expr/parsed_identifier.h"
#include "src/developer/debug/zxdb/symbols/type.h"
@@ -24,7 +25,8 @@
// either a type name or a variable. This happens with "sizeof(X)". The first
// thing (type or variable) matching "X" is used. With this API, we'll see if
// it could possibly be a type and always give the result for the type.
-using NameLookupCallback = std::function<FoundName(const ParsedIdentifier&)>;
+using NameLookupCallback =
+ std::function<FoundName(const ParsedIdentifier&, const FindNameOptions&)>;
} // namespace zxdb
diff --git a/src/developer/debug/zxdb/expr/symbol_eval_context.cc b/src/developer/debug/zxdb/expr/symbol_eval_context.cc
index a0d6224..b7dd325 100644
--- a/src/developer/debug/zxdb/expr/symbol_eval_context.cc
+++ b/src/developer/debug/zxdb/expr/symbol_eval_context.cc
@@ -71,9 +71,9 @@
void SymbolEvalContext::GetNamedValue(const ParsedIdentifier& identifier,
ValueCallback cb) {
- if (FoundName found = FindName(FindNameContext(process_symbols_.get(),
- symbol_context_, block_.get()),
- identifier)) {
+ if (FoundName found =
+ FindName(GetFindNameContext(),
+ FindNameOptions(FindNameOptions::kAllKinds), identifier)) {
switch (found.kind()) {
case FoundName::kVariable:
case FoundName::kMemberVariable:
@@ -123,12 +123,13 @@
NameLookupCallback SymbolEvalContext::GetSymbolNameLookupCallback() {
// The contract for this function is that the callback must not be stored
// so the callback can reference |this|.
- return [this](const ParsedIdentifier& ident) -> FoundName {
+ return [this](const ParsedIdentifier& ident,
+ const FindNameOptions& opts) -> FoundName {
// Look up the symbols in the symbol table if possible.
- FoundName result = DoTargetSymbolsNameLookup(ident);
+ FoundName result = FindName(GetFindNameContext(), opts, ident);
// Fall back on builtin types.
- if (result.kind() == FoundName::kNone) {
+ if (result.kind() == FoundName::kNone && opts.find_types) {
if (auto type = GetBuiltinType(ident.GetFullName()))
return FoundName(std::move(type));
}
@@ -179,9 +180,12 @@
FoundName SymbolEvalContext::DoTargetSymbolsNameLookup(
const ParsedIdentifier& ident) {
- return FindName(
- FindNameContext(process_symbols_.get(), symbol_context_, block_.get()),
- ident);
+ return FindName(GetFindNameContext(),
+ FindNameOptions(FindNameOptions::kAllKinds), ident);
+}
+
+FindNameContext SymbolEvalContext::GetFindNameContext() {
+ return FindNameContext(process_symbols_.get(), symbol_context_, block_.get());
}
} // namespace zxdb
diff --git a/src/developer/debug/zxdb/expr/symbol_eval_context.h b/src/developer/debug/zxdb/expr/symbol_eval_context.h
index 20cc8a6..a488795 100644
--- a/src/developer/debug/zxdb/expr/symbol_eval_context.h
+++ b/src/developer/debug/zxdb/expr/symbol_eval_context.h
@@ -86,6 +86,8 @@
// Implements type name lookup on the target's symbol index.
FoundName DoTargetSymbolsNameLookup(const ParsedIdentifier& ident);
+ FindNameContext GetFindNameContext();
+
fxl::WeakPtr<const ProcessSymbols> process_symbols_; // Possibly null.
SymbolContext symbol_context_;
fxl::RefPtr<SymbolDataProvider> data_provider_; // Possibly null.
diff --git a/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc b/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
index ee50ec1..c5544b3 100644
--- a/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
+++ b/src/developer/debug/zxdb/symbols/dwarf_symbol_factory.cc
@@ -569,6 +569,9 @@
llvm::Optional<uint64_t> byte_size;
main_decoder.AddUnsignedConstant(llvm::dwarf::DW_AT_byte_size, &byte_size);
+ llvm::Optional<bool> is_declaration;
+ main_decoder.AddBool(llvm::dwarf::DW_AT_declaration, &is_declaration);
+
// The type is optional for an enumeration.
llvm::DWARFDie type;
main_decoder.AddReference(llvm::dwarf::DW_AT_type, &type);
@@ -602,7 +605,7 @@
}
});
- if (!main_decoder.Decode(die) || !byte_size)
+ if (!main_decoder.Decode(die))
return fxl::MakeRefCounted<Symbol>();
Enumeration::Map map;
@@ -627,6 +630,8 @@
*byte_size, is_signed, std::move(map));
if (parent)
result->set_parent(MakeLazy(parent));
+ if (is_declaration)
+ result->set_is_declaration(*is_declaration);
return result;
}
diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index.cc b/src/developer/debug/zxdb/symbols/module_symbol_index.cc
index 9147de5..e990144 100644
--- a/src/developer/debug/zxdb/symbols/module_symbol_index.cc
+++ b/src/developer/debug/zxdb/symbols/module_symbol_index.cc
@@ -162,6 +162,7 @@
for (unsigned i = 0; i < die_count; i++) {
// All optional variables need to be reset so we know which ones are set
// by the current DIE.
+ is_declaration.reset();
decl_unit_offset.reset();
decl_global_offset.reset();
diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc b/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc
index aa5eab0..7852e31 100644
--- a/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc
+++ b/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc
@@ -35,8 +35,28 @@
void ModuleSymbolIndexNode::Dump(const std::string& name, std::ostream& out,
int indent_level) const {
out << std::string(indent_level * 2, ' ') << name;
- if (!dies_.empty())
- out << " (" << dies_.size() << ")";
+ if (!dies_.empty()) {
+ out << " (" << dies_.size() << ") ";
+ for (auto& die : dies_) {
+ switch (die.type()) {
+ case RefType::kNamespace:
+ out << "n";
+ break;
+ case RefType::kFunction:
+ out << "f";
+ break;
+ case RefType::kVariable:
+ out << "v";
+ break;
+ case RefType::kTypeDecl:
+ out << "d";
+ break;
+ case RefType::kType:
+ out << "t";
+ break;
+ }
+ }
+ }
out << std::endl;
for (const auto& cur : sub_)
cur.second.Dump(cur.first, out, indent_level + 1);
@@ -86,8 +106,7 @@
}
ModuleSymbolIndexNode* ModuleSymbolIndexNode::AddChild(const char* name) {
- return &sub_.emplace(std::piecewise_construct,
- std::forward_as_tuple(name),
+ return &sub_.emplace(std::piecewise_construct, std::forward_as_tuple(name),
std::forward_as_tuple())
.first->second;
}
@@ -96,8 +115,7 @@
ModuleSymbolIndexNode&& child) {
auto existing = sub_.find(name);
if (existing == sub_.end()) {
- sub_.emplace(std::piecewise_construct,
- std::forward_as_tuple(name),
+ sub_.emplace(std::piecewise_construct, std::forward_as_tuple(name),
std::forward_as_tuple(std::move(child)));
} else {
existing->second.Merge(std::move(child));
diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index_unittest.cc b/src/developer/debug/zxdb/symbols/module_symbol_index_unittest.cc
index db5e906..e553383 100644
--- a/src/developer/debug/zxdb/symbols/module_symbol_index_unittest.cc
+++ b/src/developer/debug/zxdb/symbols/module_symbol_index_unittest.cc
@@ -24,13 +24,6 @@
ModuleSymbolIndex index;
index.CreateIndex(module.object_file());
-#if 0
- // Enable to dump the found index for debugging purposes.
- std::cout << "Index dump:\n";
- index.root().Dump(std::cout, 1);
- index.DumpFileIndex(std::cout);
-#endif
-
// Standalone function search.
auto result = index.FindExact(
TestSymbolModule::SplitName(TestSymbolModule::kMyFunctionName));
@@ -192,6 +185,23 @@
EXPECT_EQ(1u, result.size()) << "int not found.";
}
+// Enable and substitute a path on your system to dump the index for a
+// DWARF file.
+#if 0
+TEST(ModuleSymbolIndex, DumpFileIndex) {
+ TestSymbolModule module;
+ std::string err;
+ ASSERT_TRUE(module.LoadSpecific("chrome", &err)) << err;
+
+ ModuleSymbolIndex index;
+ index.CreateIndex(module.object_file());
+
+ std::cout << "Index dump:\n";
+ index.root().Dump(std::cout, 1);
+ index.DumpFileIndex(std::cout);
+}
+#endif
+
// Enable and substitute a path on your system for kFilename to run the
// indexing benchmark.
#if 0
@@ -208,8 +218,7 @@
}
TEST(ModuleSymbolIndex, BenchmarkIndexing) {
- const char kFilename[] =
- "/usr/local/google/home/brettw/prj/src/out/release/chrome";
+ const char kFilename[] = "chrome";
int64_t begin_us = GetTickMicroseconds();
TestSymbolModule module;
diff --git a/src/developer/debug/zxdb/symbols/type.h b/src/developer/debug/zxdb/symbols/type.h
index 0622dff..96742eef 100644
--- a/src/developer/debug/zxdb/symbols/type.h
+++ b/src/developer/debug/zxdb/symbols/type.h
@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#pragma once
+#ifndef SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_TYPE_H_
+#define SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_TYPE_H_
#include <string>
#include <vector>
@@ -20,6 +21,9 @@
// Returns the type with no "const", "volatile", or similar modifiers that
// don't affect the stored data, and expand typedef and using statements.
//
+ // This does NOT expand forward definitions which will require a symbol
+ // name lookup.
+ //
// It is on the Type class rather than the ModifiedType class so that calling
// code can unconditionally call type->GetConcreteType()->byte_size() or
// other functions to work with the type.
@@ -30,6 +34,13 @@
// Symbol::GetAssignedName).
void set_assigned_name(std::string n) { assigned_name_ = std::move(n); }
+ // Types are declarations when the full definition of the type isn't known.
+ // This corresponds to a C forward declaration. In some cases, the type
+ // definition isn't even encoded in the compilation unit because the full
+ // definition was never seen.
+ bool is_declaration() const { return is_declaration_; }
+ void set_is_declaration(bool id) { is_declaration_ = id; }
+
// For forward-defines where the size of the structure is not known, the
// byte size will be 0.
uint32_t byte_size() const { return byte_size_; }
@@ -44,7 +55,10 @@
private:
std::string assigned_name_;
+ bool is_declaration_ = false;
uint32_t byte_size_ = 0;
};
} // namespace zxdb
+
+#endif // SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_TYPE_H_
