[debugger] Allow querying symbols for one binary at a time
This is closer to our actual usage.
Test: Adjusted unit tests pass.
Change-Id: I4c432cb1efd5d91d51f06d3cb84446e1de9c99c7
diff --git a/bin/debug_agent/debug_agent.cc b/bin/debug_agent/debug_agent.cc
index 88b558e..9e1513d 100644
--- a/bin/debug_agent/debug_agent.cc
+++ b/bin/debug_agent/debug_agent.cc
@@ -375,8 +375,8 @@
proc->UnregisterBreakpoint(bp, address);
}
-zx_status_t DebugAgent::RegisterWatchpoint(Watchpoint* wp,
- zx_koid_t process_koid,
+zx_status_t DebugAgent::RegisterWatchpoint(
+ Watchpoint* wp, zx_koid_t process_koid,
const debug_ipc::AddressRange& range) {
DebuggedProcess* process = GetDebuggedProcess(process_koid);
if (!process) {
@@ -430,7 +430,7 @@
debug_ipc::SymbolTablesReply* reply) {
DebuggedProcess* proc = GetDebuggedProcess(request.process_koid);
if (proc)
- proc->OnSymbolTables(reply);
+ proc->OnSymbolTables(request, reply);
}
DebuggedProcess* DebugAgent::GetDebuggedProcess(zx_koid_t koid) {
diff --git a/bin/debug_agent/debugged_process.cc b/bin/debug_agent/debugged_process.cc
index 4ab9f15..f6018b1 100644
--- a/bin/debug_agent/debugged_process.cc
+++ b/bin/debug_agent/debugged_process.cc
@@ -37,9 +37,7 @@
&kMagicValue, sizeof(kMagicValue));
}
-DebuggedProcess::~DebuggedProcess() {
- DetachFromProcess();
-}
+DebuggedProcess::~DebuggedProcess() { DetachFromProcess(); }
void DebuggedProcess::DetachFromProcess() {
// 1. Remove installed breakpoints.
@@ -346,10 +344,11 @@
GetModulesForProcess(process_, dl_debug_addr_, &reply->modules);
}
-void DebuggedProcess::OnSymbolTables(debug_ipc::SymbolTablesReply* reply) {
- // Modules can only be read after the debug state is set.
- if (dl_debug_addr_)
- GetSymbolTablesForProcess(process_, dl_debug_addr_, &reply->symbol_tables);
+void DebuggedProcess::OnSymbolTables(
+ const debug_ipc::SymbolTablesRequest& request,
+ debug_ipc::SymbolTablesReply* reply) {
+ GetSymbolTableFromProcess(process_, request.base, request.build_id,
+ &reply->symbols);
}
void DebuggedProcess::OnWriteMemory(
diff --git a/bin/debug_agent/debugged_process.h b/bin/debug_agent/debugged_process.h
index 5ba33ad..2538934 100644
--- a/bin/debug_agent/debugged_process.h
+++ b/bin/debug_agent/debugged_process.h
@@ -55,7 +55,8 @@
void OnAddressSpace(const debug_ipc::AddressSpaceRequest& request,
debug_ipc::AddressSpaceReply* reply);
void OnModules(debug_ipc::ModulesReply* reply);
- void OnSymbolTables(debug_ipc::SymbolTablesReply* reply);
+ void OnSymbolTables(const debug_ipc::SymbolTablesRequest& request,
+ debug_ipc::SymbolTablesReply* reply);
void OnWriteMemory(const debug_ipc::WriteMemoryRequest& request,
debug_ipc::WriteMemoryReply* reply);
diff --git a/bin/debug_agent/process_info.cc b/bin/debug_agent/process_info.cc
index efd957e..9ffd6e8 100644
--- a/bin/debug_agent/process_info.cc
+++ b/bin/debug_agent/process_info.cc
@@ -293,34 +293,38 @@
});
}
-zx_status_t GetSymbolTablesForProcess(
- const zx::process& process, uint64_t dl_debug_addr,
- std::vector<debug_ipc::SymbolTable>* symtabs) {
- return WalkModules(
- process, dl_debug_addr,
- [symtabs](const zx::process& process, uint64_t base, uint64_t lmap) {
- debug_ipc::SymbolTable symtab;
- symtab.build_id = debug_ipc::ExtractBuildID(process, base);
+zx_status_t GetSymbolTableFromProcess(
+ const zx::process& process, uint64_t elf_addr, const std::string& build_id,
+ std::vector<debug_ipc::ElfSymbol>* symbols) {
+ auto elf = ElfLib::Create(
+ std::make_unique<ProcessMemoryAccessor>(&process, elf_addr));
- auto elf = ElfLib::Create(
- std::make_unique<ProcessMemoryAccessor>(&process, base));
+ if (!elf) {
+ return ZX_ERR_BAD_STATE;
+ }
- if (elf) {
- if (auto syms = elf->GetAllSymbols()) {
- for (const auto& sym : *syms) {
- debug_ipc::SymbolTable::Symbol ipc_sym;
+ auto got_build_id = debug_ipc::ExtractBuildID(process, elf_addr);
- ipc_sym.name = sym.first;
- ipc_sym.value = sym.second.st_value;
+ if (got_build_id != build_id) {
+ return ZX_ERR_BAD_STATE;
+ }
- symtab.symbols.push_back(std::move(ipc_sym));
- }
- }
- }
+ auto syms = elf->GetAllSymbols();
- symtabs->push_back(std::move(symtab));
- return true;
- });
+ if (!syms) {
+ return ZX_ERR_BAD_STATE;
+ }
+
+ for (const auto& sym : *syms) {
+ debug_ipc::ElfSymbol ipc_sym;
+
+ ipc_sym.name = sym.first;
+ ipc_sym.value = sym.second.st_value;
+
+ symbols->push_back(std::move(ipc_sym));
+ }
+
+ return ZX_OK;
}
std::vector<zx_info_maps_t> GetProcessMaps(const zx::process& process) {
diff --git a/bin/debug_agent/process_info.h b/bin/debug_agent/process_info.h
index 29decad..aeaf855 100644
--- a/bin/debug_agent/process_info.h
+++ b/bin/debug_agent/process_info.h
@@ -46,9 +46,9 @@
// Fills the given vector with the module information for the process.
// "dl_debug_addr" is the address inside "process" of the dynamic loader's
// debug state.
-zx_status_t GetSymbolTablesForProcess(
- const zx::process& process, uint64_t dl_debug_addr,
- std::vector<debug_ipc::SymbolTable>* symtabs);
+zx_status_t GetSymbolTableFromProcess(
+ const zx::process& process, uint64_t elf_addr, const std::string& build_id,
+ std::vector<debug_ipc::ElfSymbol>* symbols);
// Returns the memory mapping for the process. Returns empty on failure.
std::vector<zx_info_maps_t> GetProcessMaps(const zx::process& process);
diff --git a/lib/debug_ipc/agent_protocol.cc b/lib/debug_ipc/agent_protocol.cc
index 2dc727f..3730a93 100644
--- a/lib/debug_ipc/agent_protocol.cc
+++ b/lib/debug_ipc/agent_protocol.cc
@@ -75,12 +75,7 @@
writer->WriteString(module.build_id);
}
-void Serialize(const SymbolTable& symbols, MessageWriter* writer) {
- writer->WriteString(symbols.build_id);
- Serialize(symbols.symbols, writer);
-}
-
-void Serialize(const SymbolTable::Symbol& symbol, MessageWriter* writer) {
+void Serialize(const ElfSymbol& symbol, MessageWriter* writer) {
writer->WriteString(symbol.name);
writer->WriteUint64(symbol.value);
}
@@ -410,7 +405,7 @@
void WriteReply(const SymbolTablesReply& reply, uint32_t transaction_id,
MessageWriter* writer) {
writer->WriteHeader(MsgHeader::Type::kSymbolTables, transaction_id);
- Serialize(reply.symbol_tables, writer);
+ Serialize(reply.symbols, writer);
}
// JobFilter ------------------------------------------------------------------
diff --git a/lib/debug_ipc/client_protocol.cc b/lib/debug_ipc/client_protocol.cc
index b47833d..e0e8e5a 100644
--- a/lib/debug_ipc/client_protocol.cc
+++ b/lib/debug_ipc/client_protocol.cc
@@ -83,13 +83,7 @@
return true;
}
-bool Deserialize(MessageReader* reader, SymbolTable* symbols) {
- if (!reader->ReadString(&symbols->build_id))
- return false;
- return Deserialize(reader, &symbols->symbols);
-}
-
-bool Deserialize(MessageReader* reader, SymbolTable::Symbol* symbol) {
+bool Deserialize(MessageReader* reader, ElfSymbol* symbol) {
if (!reader->ReadString(&symbol->name))
return false;
if (!reader->ReadUint64(&symbol->value))
@@ -491,7 +485,7 @@
return false;
*transaction_id = header.transaction_id;
- Deserialize(reader, &reply->symbol_tables);
+ Deserialize(reader, &reply->symbols);
return true;
}
diff --git a/lib/debug_ipc/protocol.h b/lib/debug_ipc/protocol.h
index c45930d..f7bfc6b 100644
--- a/lib/debug_ipc/protocol.h
+++ b/lib/debug_ipc/protocol.h
@@ -241,9 +241,16 @@
struct SymbolTablesRequest {
uint64_t process_koid = 0;
+ uint64_t base = 0; // Load address of the module.
+
+ // If we give an invalid base we'll either fault, which should be graceful
+ // through the ReadMemory interface, read a bad header, which we can detect,
+ // or we'll find a different module. We can use the build ID to check against
+ // the last case.
+ std::string build_id;
};
struct SymbolTablesReply {
- std::vector<SymbolTable> symbol_tables;
+ std::vector<ElfSymbol> symbols;
};
// Request to set filter.
diff --git a/lib/debug_ipc/protocol_unittests.cc b/lib/debug_ipc/protocol_unittests.cc
index 25c96d8..8c230f3 100644
--- a/lib/debug_ipc/protocol_unittests.cc
+++ b/lib/debug_ipc/protocol_unittests.cc
@@ -452,44 +452,20 @@
TEST(Protocol, SymbolTablesReply) {
SymbolTablesReply initial;
- initial.symbol_tables.resize(2);
- initial.symbol_tables[0].build_id = "abcdefghijkl";
- initial.symbol_tables[0].symbols.resize(2);
- initial.symbol_tables[0].symbols[0].name = "somefunc";
- initial.symbol_tables[0].symbols[0].value = 0xbeefUL;
- initial.symbol_tables[0].symbols[1].name = "wompwomp";
- initial.symbol_tables[0].symbols[1].value = 0xb0efUL;
- initial.symbol_tables[1].build_id = "h4sh3s";
- initial.symbol_tables[1].symbols.resize(2);
- initial.symbol_tables[1].symbols[0].name = "phresh";
- initial.symbol_tables[1].symbols[0].value = 0xf00dUL;
- initial.symbol_tables[1].symbols[1].name = "ackermann";
- initial.symbol_tables[1].symbols[1].value = 0xb0b0b0b0UL;
+ initial.symbols.resize(2);
+ initial.symbols[0].name = "somefunc";
+ initial.symbols[0].value = 0xbeefUL;
+ initial.symbols[1].name = "wompwomp";
+ initial.symbols[1].value = 0xb0efUL;
SymbolTablesReply second;
ASSERT_TRUE(SerializeDeserializeReply(initial, &second));
- EXPECT_EQ(2u, second.symbol_tables.size());
- EXPECT_EQ(initial.symbol_tables[0].build_id,
- second.symbol_tables[0].build_id);
- EXPECT_EQ(initial.symbol_tables[0].symbols[0].name,
- second.symbol_tables[0].symbols[0].name);
- EXPECT_EQ(initial.symbol_tables[0].symbols[0].value,
- second.symbol_tables[0].symbols[0].value);
- EXPECT_EQ(initial.symbol_tables[0].symbols[1].name,
- second.symbol_tables[0].symbols[1].name);
- EXPECT_EQ(initial.symbol_tables[0].symbols[1].value,
- second.symbol_tables[0].symbols[1].value);
- EXPECT_EQ(initial.symbol_tables[1].build_id,
- second.symbol_tables[1].build_id);
- EXPECT_EQ(initial.symbol_tables[1].symbols[0].name,
- second.symbol_tables[1].symbols[0].name);
- EXPECT_EQ(initial.symbol_tables[1].symbols[0].value,
- second.symbol_tables[1].symbols[0].value);
- EXPECT_EQ(initial.symbol_tables[1].symbols[1].name,
- second.symbol_tables[1].symbols[1].name);
- EXPECT_EQ(initial.symbol_tables[1].symbols[1].value,
- second.symbol_tables[1].symbols[1].value);
+ EXPECT_EQ(2u, second.symbols.size());
+ EXPECT_EQ(initial.symbols[0].name, second.symbols[0].name);
+ EXPECT_EQ(initial.symbols[0].value, second.symbols[0].value);
+ EXPECT_EQ(initial.symbols[1].name, second.symbols[1].name);
+ EXPECT_EQ(initial.symbols[1].value, second.symbols[1].value);
}
// ASpace ----------------------------------------------------------------------
@@ -568,7 +544,7 @@
WriteMemoryRequest initial;
initial.process_koid = 91823765;
initial.address = 0x3468234;
- initial.data = { 0, 1, 2, 3, 4, 5};
+ initial.data = {0, 1, 2, 3, 4, 5};
WriteMemoryRequest second;
ASSERT_TRUE(SerializeDeserializeRequest(initial, &second));
diff --git a/lib/debug_ipc/records.h b/lib/debug_ipc/records.h
index 3ab09f7..414d737 100644
--- a/lib/debug_ipc/records.h
+++ b/lib/debug_ipc/records.h
@@ -191,7 +191,7 @@
struct AddressRange {
uint64_t begin = 0;
- uint64_t end = 0; // Non-inclusive.
+ uint64_t end = 0; // Non-inclusive.
};
struct ProcessWatchpointSetings {
@@ -223,15 +223,9 @@
std::string build_id;
};
-// Information on the linkage symbol table for a module.
-struct SymbolTable {
- struct Symbol {
- std::string name;
- uint64_t value = 0;
- };
-
- std::string build_id;
- std::vector<Symbol> symbols;
+struct ElfSymbol {
+ std::string name;
+ uint64_t value = 0;
};
struct AddressRegion {
@@ -241,7 +235,7 @@
uint64_t depth;
};
-// ReadRegisters -------------------------------------------------------------------
+// ReadRegisters ---------------------------------------------------------------
// Value representing a particular register.
struct Register {
