[debugger] Add tests for FindGlobalVariable.
Adds tests for FindGlobalVariable which were missing before. Does some
cleanup around how this function and the main FindVariable function
interact to make it more easily testable.
Adds a new helper to set up a symbol test environment. I plan to use
this to improve some other tests in a followup.
TEST=this is
Change-Id: I231f92864072467fbb6984a5962705016a7e8eb1
diff --git a/bin/zxdb/expr/find_variable.cc b/bin/zxdb/expr/find_variable.cc
index 0ca358b..18ccf17 100644
--- a/bin/zxdb/expr/find_variable.cc
+++ b/bin/zxdb/expr/find_variable.cc
@@ -70,8 +70,19 @@
// Fall back to searching global vars.
if (process_symbols) {
- return FindGlobalVariable(process_symbols, block, block_symbol_context,
- identifier);
+ // Get the scope for the current function. This may fail in which case
+ // we'll be left with an empty current scope. This is non-fatal: it just
+ // means we won't implicitly search the current namespace and will search
+ // only the global one.
+ Identifier current_scope;
+ if (const Function* function = block->GetContainingFunction()) {
+ auto [err, func_name] = Identifier::FromString(function->GetFullName());
+ if (!err.has_error())
+ current_scope = func_name.GetScope();
+ }
+
+ return FindGlobalVariable(process_symbols, current_scope,
+ block_symbol_context, identifier);
}
return std::nullopt;
}
@@ -155,37 +166,23 @@
}
std::optional<FoundVariable> FindGlobalVariable(
- const ProcessSymbols* process_symbols, const CodeBlock* block,
- const SymbolContext* block_symbol_context, const Identifier& identifier) {
+ const ProcessSymbols* process_symbols, const Identifier& current_scope,
+ const SymbolContext* symbol_context, const Identifier& identifier) {
std::vector<const LoadedModuleSymbols*> modules =
process_symbols->GetLoadedModuleSymbols();
if (modules.empty())
return std::nullopt;
- Identifier current_scope;
-
// When we're given a block to start searching from, always search
// that module for symbol matches first. If there are duplicates in other
// modules, one normally wants the current one.
const LoadedModuleSymbols* current_module = nullptr;
- if (block) {
- // If block is non-null, so must be the symbol context.
- FXL_DCHECK(block_symbol_context);
-
- // Get the scope for the current block. This may fail in which case we'll
- // be left with an empty current scope. This is non-fatal so we can ignore
- // the err: it must means we won't implicitly search the current namespace.
- Err err;
- std::tie(err, current_scope) = Identifier::FromString(block->GetFullName());
- current_scope = current_scope.GetScope();
-
- // There's not currently a great way to map a symbol (the code block) back
- // to the module it came from. So use the symbol_context to find the module
- // that corresponds to its base address.
- uint64_t block_module_load_address =
- block_symbol_context->RelativeToAbsolute(0);
+ if (symbol_context) {
+ // Find the module that corresponds to the symbol context.
+ uint64_t module_load_address =
+ symbol_context->RelativeToAbsolute(0);
for (const LoadedModuleSymbols* mod : modules) {
- if (mod->load_address() == block_module_load_address) {
+ if (mod->load_address() == module_load_address) {
current_module = mod;
break;
}
@@ -203,7 +200,7 @@
for (const LoadedModuleSymbols* loaded_mod : modules) {
if (loaded_mod != current_module) {
if (auto found = FindGlobalVariableInModule(
- current_module->module_symbols(), current_scope, identifier))
+ loaded_mod->module_symbols(), current_scope, identifier))
return found;
}
}
diff --git a/bin/zxdb/expr/find_variable.h b/bin/zxdb/expr/find_variable.h
index f765115..fea90d1 100644
--- a/bin/zxdb/expr/find_variable.h
+++ b/bin/zxdb/expr/find_variable.h
@@ -56,11 +56,12 @@
const Identifier& identifier);
// Attempts to resolve the named variable in the global namespace and any
-// other namespaces that the given block is in. The block[_symbol_context]
-// can be null to only search from the global namespace.
+// other namespaces that the given block is in. The symbol_context is used
+// to prioritize the current module. It can be null to search in a
+// non-guaranteed order.
std::optional<FoundVariable> FindGlobalVariable(
- const ProcessSymbols* process_symbols, const CodeBlock* block,
- const SymbolContext* block_symbol_context, const Identifier& identifier);
+ const ProcessSymbols* process_symbols, const Identifier& current_scope,
+ const SymbolContext* symbol_context, const Identifier& identifier);
// Searches a specific index and current namespace for a global variable of
// the given name. The current_scope would be the current namespace + class
diff --git a/bin/zxdb/expr/find_variable_unittest.cc b/bin/zxdb/expr/find_variable_unittest.cc
index bb306e6..24fb36b 100644
--- a/bin/zxdb/expr/find_variable_unittest.cc
+++ b/bin/zxdb/expr/find_variable_unittest.cc
@@ -8,6 +8,8 @@
#include "garnet/bin/zxdb/symbols/base_type.h"
#include "garnet/bin/zxdb/symbols/function.h"
#include "garnet/bin/zxdb/symbols/mock_module_symbols.h"
+#include "garnet/bin/zxdb/symbols/namespace.h"
+#include "garnet/bin/zxdb/symbols/process_symbols_impl_test_setup.h"
#include "garnet/bin/zxdb/symbols/symbol_context.h"
#include "garnet/bin/zxdb/symbols/type_test_support.h"
#include "garnet/bin/zxdb/symbols/variable_test_support.h"
@@ -20,10 +22,48 @@
namespace zxdb {
+namespace {
+
+// Creates a global variable that's inserted into the index and the mock
+// ModuleSymbols.
+struct TestGlobalVariable {
+ // Index of the next DieRef to generated. This ensures the generated IDs are
+ // unique.
+ static int next_die_ref;
+
+ TestGlobalVariable(MockModuleSymbols* mod_sym,
+ ModuleSymbolIndexNode* index_parent,
+ const std::string& var_name)
+ : die_ref(next_die_ref++),
+ index_node(index_parent->AddChild(std::string(var_name))) {
+ index_node->AddDie(die_ref);
+ var = MakeVariableForTest(var_name, MakeInt32Type(), 0x100, 0x200,
+ std::vector<uint8_t>());
+ mod_sym->AddDieRef(die_ref, var);
+ }
+
+ // The DieRef links the index and the entry injected into the ModuleSymbols.
+ ModuleSymbolIndexNode::DieRef die_ref;
+
+ // Place where this variable is indexed.
+ ModuleSymbolIndexNode* index_node;
+
+ // The variable itself.
+ fxl::RefPtr<Variable> var;
+};
+
+int TestGlobalVariable::next_die_ref = 1;
+
+} // namespace
+
// This test declares the following structure. There are three levels of
// variables, each one has one unique variable, and one labeled "value" for
// testing ambiguity.
//
+// namespace ns {
+//
+// int32_t ns_value;
+//
// void Foo(int32_t value, int32_t other_param) {
// int32_t value; // 2nd declaration.
// int32_t function_local;
@@ -32,22 +72,44 @@
// int32_t block_local;
// }
// }
+//
+// } // namespace ns
TEST(FindVariable, FindLocalVariable) {
+ ProcessSymbolsImplTestSetup setup;
+
auto int32_type = MakeInt32Type();
// Empyt DWARF location expression. Since we don't evaluate any variables
// they can all be empty.
std::vector<uint8_t> var_loc;
- SymbolContext symbol_context = SymbolContext::ForRelativeAddresses();
+ // Set up the module symbols. This creates "ns" and "ns_value" in the
+ // symbol index.
+ auto mod = std::make_unique<MockModuleSymbols>("mod.so");
+ auto& root = mod->index().root(); // Root of the index for module 1.
- // Function.
+ const char kNsName[] = "ns";
+ auto ns_node = root.AddChild(kNsName);
+
+ const char kNsVarName[] = "ns_value";
+ TestGlobalVariable ns_value(mod.get(), ns_node, kNsVarName);
+
+ constexpr uint64_t kLoadAddress = 0x1000;
+ SymbolContext symbol_context(kLoadAddress);
+ setup.InjectModule("mod", "1234", kLoadAddress, std::move(mod));
+
+ // Namespace.
+ auto ns = fxl::MakeRefCounted<Namespace>();
+ ns->set_assigned_name(kNsName);
+
+ // Function inside the namespace.
auto function = fxl::MakeRefCounted<Function>();
function->set_assigned_name("function");
uint64_t kFunctionBeginAddr = 0x1000;
uint64_t kFunctionEndAddr = 0x2000;
function->set_code_ranges(
{AddressRange(kFunctionBeginAddr, kFunctionEndAddr)});
+ function->set_parent(LazySymbol(ns));
// Function parameters.
auto param_value = MakeVariableForTest(
@@ -119,59 +181,118 @@
EXPECT_TRUE(found);
EXPECT_EQ(param_other.get(), found->variable());
+ // Look up the variable "ns::ns_value" using the name "ns_value" (no
+ // namespace) from within the context of the "ns::function()" function.
+ // The namespace of the function should be implicitly picked up.
+ Identifier ns_value_ident(ExprToken(ExprToken::kName, kNsVarName, 0));
+ found = FindVariable(&setup.process(), block.get(), &symbol_context,
+ ns_value_ident);
+ EXPECT_TRUE(found);
+ EXPECT_EQ(ns_value.var.get(), found->variable());
+
+ // Loop up the global "ns_value" var with no global symbol context. This
+ // should fail and not crash.
+ found = FindVariable(nullptr, block.get(), &symbol_context, ns_value_ident);
+ EXPECT_FALSE(found);
+
// Break reference cycle for test teardown.
+ function->set_parent(LazySymbol());
block->set_parent(LazySymbol());
}
+// This only tests the ModulSymbols and function naming integration, the
+// details of the index searching are tested by FindGlobalVariableInModule()
+TEST(FindVariable, FindGlobalVariable) {
+ ProcessSymbolsImplTestSetup setup;
+
+ const char kGlobalName[] = "global"; // Different variable in each.
+ const char kVar1Name[] = "var1"; // Only in module 1
+ const char kVar2Name[] = "var2"; // Only in module 2
+ const char kNotFoundName[] = "notfound";
+
+ Identifier global_ident(ExprToken(ExprToken::kName, kGlobalName, 0));
+ Identifier var1_ident(ExprToken(ExprToken::kName, kVar1Name, 0));
+ Identifier var2_ident(ExprToken(ExprToken::kName, kVar2Name, 0));
+ Identifier notfound_ident(ExprToken(ExprToken::kName, kNotFoundName, 0));
+
+ // Module 1.
+ auto mod1 = std::make_unique<MockModuleSymbols>("mod1.so");
+ auto& root1 = mod1->index().root(); // Root of the index for module 1.
+ TestGlobalVariable global1(mod1.get(), &root1, kGlobalName);
+ TestGlobalVariable var1(mod1.get(), &root1, kVar1Name);
+ constexpr uint64_t kLoadAddress1 = 0x1000;
+ SymbolContext symbol_context1(kLoadAddress1);
+ setup.InjectModule("mod1", "1234", kLoadAddress1, std::move(mod1));
+
+ // Module 2.
+ auto mod2 = std::make_unique<MockModuleSymbols>("mod2.so");
+ auto& root2 = mod2->index().root(); // Root of the index for module 1.
+ TestGlobalVariable global2(mod2.get(), &root2, kGlobalName);
+ TestGlobalVariable var2(mod2.get(), &root2, kVar2Name);
+ constexpr uint64_t kLoadAddress2 = 0x2000;
+ SymbolContext symbol_context2(kLoadAddress2);
+ setup.InjectModule("mod2", "5678", kLoadAddress2, std::move(mod2));
+
+ // Searching for "global" in module1's context should give the global in that
+ // module.
+ auto found = FindGlobalVariable(&setup.process(), Identifier(),
+ &symbol_context1, global_ident);
+ ASSERT_TRUE(found);
+ EXPECT_EQ(global1.var.get(), found->variable());
+
+ // Searching for "global" in module2's context should give the global in that
+ // module.
+ found = FindGlobalVariable(&setup.process(), Identifier(),
+ &symbol_context2, global_ident);
+ ASSERT_TRUE(found);
+ EXPECT_EQ(global2.var.get(), found->variable());
+
+ // Searching for "var1" in module2's context should still find it even though
+ // its in the other module.
+ found = FindGlobalVariable(&setup.process(), Identifier(),
+ &symbol_context2, var1_ident);
+ ASSERT_TRUE(found);
+ EXPECT_EQ(var1.var.get(), found->variable());
+
+ // Searching for "var2" with no context should still find it.
+ found = FindGlobalVariable(&setup.process(), Identifier(),
+ nullptr, var2_ident);
+ ASSERT_TRUE(found);
+ EXPECT_EQ(var2.var.get(), found->variable());
+}
+
TEST(FindVariable, FindGlobalVariableInModule) {
MockModuleSymbols mod_sym("test.so");
- auto int32_type = MakeInt32Type();
auto& root = mod_sym.index().root(); // Root of the index.
const char kVarName[] = "var";
const char kNsName[] = "ns";
- // Empyt DWARF location expression. Since we don't evaluate any variables
- // they can all be empty.
- std::vector<uint8_t> var_loc;
-
// Make a global variable in the toplevel namespace.
- ModuleSymbolIndexNode::DieRef global_ref(1);
- auto global_node = root.AddChild(kVarName);
- global_node->AddDie(global_ref);
- auto global_var =
- MakeVariableForTest(kVarName, int32_type, 0x100, 0x200, var_loc);
- mod_sym.AddDieRef(global_ref, global_var);
+ TestGlobalVariable global(&mod_sym, &root, kVarName);
- Identifier var_ident(Identifier::Component(
- ExprToken(), ExprToken(ExprToken::kName, kVarName, 0)));
+ Identifier var_ident(ExprToken(ExprToken::kName, kVarName, 0));
auto found = FindGlobalVariableInModule(&mod_sym, Identifier(), var_ident);
ASSERT_TRUE(found);
- EXPECT_EQ(global_var.get(), found->variable());
+ EXPECT_EQ(global.var.get(), found->variable());
// Say we're in some nested namespace and search for the same name. It should
// find the variable in the upper namespace.
- Identifier nested_ns(Identifier::Component(
- ExprToken(), ExprToken(ExprToken::kName, kNsName, 0)));
+ Identifier nested_ns(ExprToken(ExprToken::kName, kNsName, 0));
found = FindGlobalVariableInModule(&mod_sym, nested_ns, var_ident);
ASSERT_TRUE(found);
- EXPECT_EQ(global_var.get(), found->variable());
+ EXPECT_EQ(global.var.get(), found->variable());
// Add a variable in the nested namespace with the same name.
- ModuleSymbolIndexNode::DieRef ns_var_ref(2);
auto ns_node = root.AddChild(kNsName);
- auto ns_var_node = ns_node->AddChild(kVarName);
- ns_var_node->AddDie(ns_var_ref);
- auto ns_var =
- MakeVariableForTest(kVarName, int32_type, 0x300, 0x400, var_loc);
- mod_sym.AddDieRef(ns_var_ref, ns_var);
+ TestGlobalVariable ns(&mod_sym, ns_node, kVarName);
// Re-search for the same name in the nested namespace, it should get the
// nested one first.
found = FindGlobalVariableInModule(&mod_sym, nested_ns, var_ident);
ASSERT_TRUE(found);
- EXPECT_EQ(ns_var.get(), found->variable());
+ EXPECT_EQ(ns.var.get(), found->variable());
// Now do the same search but globally qualify the input "::var" which should
// match only the toplevel one.
@@ -180,7 +301,7 @@
ExprToken(ExprToken::kName, kVarName, 0)));
found = FindGlobalVariableInModule(&mod_sym, nested_ns, var_global_ident);
ASSERT_TRUE(found);
- EXPECT_EQ(global_var.get(), found->variable());
+ EXPECT_EQ(global.var.get(), found->variable());
}
} // namespace zxdb
diff --git a/bin/zxdb/symbols/BUILD.gn b/bin/zxdb/symbols/BUILD.gn
index 5f325e4..0a2d906 100644
--- a/bin/zxdb/symbols/BUILD.gn
+++ b/bin/zxdb/symbols/BUILD.gn
@@ -145,6 +145,8 @@
"mock_process_symbols.h",
"mock_symbol_data_provider.cc",
"mock_symbol_data_provider.h",
+ "process_symbols_impl_test_setup.cc",
+ "process_symbols_impl_test_setup.h",
"type_test_support.cc",
"type_test_support.h",
"variable_test_support.cc",
diff --git a/bin/zxdb/symbols/mock_process_symbols.h b/bin/zxdb/symbols/mock_process_symbols.h
index 2efd47a..63b2135 100644
--- a/bin/zxdb/symbols/mock_process_symbols.h
+++ b/bin/zxdb/symbols/mock_process_symbols.h
@@ -10,9 +10,10 @@
namespace zxdb {
-// Provides a ProcessSymbols implementation that just returns empty values for
-// everything. Tests can override this to implement the subset of
-// functionality they need.
+// This is useful for testing things that only use a ProcessSymbols and have
+// minimal requirements. More elaborate tests can use the real ProcessSymbols
+// implementation with MockModuleSymbols backing it. See
+// ProcessSymbolsImplTestSetup.
class MockProcessSymbols : public ProcessSymbols {
public:
MockProcessSymbols();
diff --git a/bin/zxdb/symbols/process_symbols_impl.cc b/bin/zxdb/symbols/process_symbols_impl.cc
index f58276d..ce7bd01 100644
--- a/bin/zxdb/symbols/process_symbols_impl.cc
+++ b/bin/zxdb/symbols/process_symbols_impl.cc
@@ -97,6 +97,25 @@
notifications_->OnSymbolLoadFailure(err);
}
+void ProcessSymbolsImpl::InjectModuleForTesting(
+ const std::string& name, const std::string& build_id,
+ std::unique_ptr<LoadedModuleSymbols> mod_sym) {
+ LoadedModuleSymbols* loaded_ptr = mod_sym.get();
+ FXL_DCHECK(loaded_ptr);
+
+ ModuleInfo info;
+ info.name = name;
+ info.build_id = build_id;
+ info.base = loaded_ptr->load_address();
+ info.symbols = std::move(mod_sym);
+ modules_[loaded_ptr->load_address()] = std::move(info);
+
+ // Issue notifications.
+ target_symbols_->AddModule(
+ fxl::RefPtr<SystemSymbols::ModuleRef>(loaded_ptr->module_ref()));
+ notifications_->DidLoadModuleSymbols(loaded_ptr);
+}
+
TargetSymbols* ProcessSymbolsImpl::GetTargetSymbols() {
return target_symbols_;
}
diff --git a/bin/zxdb/symbols/process_symbols_impl.h b/bin/zxdb/symbols/process_symbols_impl.h
index 5bc6113..094dc3b 100644
--- a/bin/zxdb/symbols/process_symbols_impl.h
+++ b/bin/zxdb/symbols/process_symbols_impl.h
@@ -33,9 +33,9 @@
// See the corresponding functions in ProcessObserver for docs.
class Notifications {
public:
- virtual void DidLoadModuleSymbols(LoadedModuleSymbols* module) = 0;
- virtual void WillUnloadModuleSymbols(LoadedModuleSymbols* module) = 0;
- virtual void OnSymbolLoadFailure(const Err& err) = 0;
+ virtual void DidLoadModuleSymbols(LoadedModuleSymbols* module) {}
+ virtual void WillUnloadModuleSymbols(LoadedModuleSymbols* module) {}
+ virtual void OnSymbolLoadFailure(const Err& err) {}
};
// The passed-in pointers must outlive this class.
@@ -48,6 +48,13 @@
// Replaces all modules with the given list.
void SetModules(const std::vector<debug_ipc::Module>& modules);
+ // Appends the ModuleSymbols implementation to the current list (unlike
+ // SetModules which does a replacement). This is typically used to populate
+ // a ProcessSymbols with one or more MockModuleSymbols for testing purposes.
+ void InjectModuleForTesting(const std::string& name,
+ const std::string& build_id,
+ std::unique_ptr<LoadedModuleSymbols> mod_sym);
+
// ProcessSymbols implementation.
fxl::WeakPtr<const ProcessSymbols> GetWeakPtr() const override;
TargetSymbols* GetTargetSymbols() override;
diff --git a/bin/zxdb/symbols/process_symbols_impl_test_setup.cc b/bin/zxdb/symbols/process_symbols_impl_test_setup.cc
new file mode 100644
index 0000000..eb2b11a
--- /dev/null
+++ b/bin/zxdb/symbols/process_symbols_impl_test_setup.cc
@@ -0,0 +1,28 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "garnet/bin/zxdb/symbols/process_symbols_impl_test_setup.h"
+
+#include "garnet/bin/zxdb/symbols/loaded_module_symbols.h"
+#include "garnet/bin/zxdb/symbols/module_symbols.h"
+
+namespace zxdb {
+
+ProcessSymbolsImplTestSetup::ProcessSymbolsImplTestSetup()
+ : system_(),
+ target_(&system_),
+ process_notifications_(),
+ process_(&process_notifications_, &target_) {}
+
+ProcessSymbolsImplTestSetup::~ProcessSymbolsImplTestSetup() = default;
+
+void ProcessSymbolsImplTestSetup::InjectModule(
+ const std::string& name, const std::string& build_id, uint64_t base,
+ std::unique_ptr<ModuleSymbols> mod_sym) {
+ auto loaded = std::make_unique<LoadedModuleSymbols>(
+ system_.InjectModuleForTesting(build_id, std::move(mod_sym)), base);
+ process_.InjectModuleForTesting(name, build_id, std::move(loaded));
+}
+
+} // namespace zxdb
diff --git a/bin/zxdb/symbols/process_symbols_impl_test_setup.h b/bin/zxdb/symbols/process_symbols_impl_test_setup.h
new file mode 100644
index 0000000..7918a66
--- /dev/null
+++ b/bin/zxdb/symbols/process_symbols_impl_test_setup.h
@@ -0,0 +1,45 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#pragma once
+
+#include <memory>
+
+#include "garnet/bin/zxdb/symbols/process_symbols_impl.h"
+#include "garnet/bin/zxdb/symbols/system_symbols.h"
+#include "garnet/bin/zxdb/symbols/target_symbols_impl.h"
+
+namespace zxdb {
+
+class ModuleSymbols;
+
+// This class sets up a ProcessSymbolsImpl for testing purposes. It allows
+// MockModuleSymbols to be easily injected into the process.
+//
+// This class is only useful for tests that use the symbol system but not the
+// client objects (Process/Target, etc.).
+class ProcessSymbolsImplTestSetup {
+ public:
+ ProcessSymbolsImplTestSetup();
+ ~ProcessSymbolsImplTestSetup();
+
+ SystemSymbols& system() { return system_; }
+ TargetSymbolsImpl& target() { return target_; }
+ ProcessSymbolsImpl& process() { return process_; }
+
+ // Appends the given module symbols implementation to the process. This will
+ // typically be a MockModuleSymbols.
+ void InjectModule(const std::string& name, const std::string& build_id,
+ uint64_t base, std::unique_ptr<ModuleSymbols> mod_sym);
+
+ private:
+ SystemSymbols system_;
+
+ TargetSymbolsImpl target_;
+
+ ProcessSymbolsImpl::Notifications process_notifications_;
+ ProcessSymbolsImpl process_;
+};
+
+} // namespace zxdb
