From e21c471c3ede7d38e8d7b2e7f1a81a8b24018cdd Mon Sep 17 00:00:00 2001 From: kotcrab <4594081+kotcrab@users.noreply.github.com> Date: Tue, 12 Nov 2024 21:05:11 +0100 Subject: [PATCH] Support copy address and value in Struct viewer Reorganize add breakpoint menu Style fixes --- Common/GhidraClient.cpp | 8 +-- Common/GhidraClient.h | 2 +- UI/ImDebugger/ImStructViewer.cpp | 88 ++++++++++++++++++++------------ UI/ImDebugger/ImStructViewer.h | 3 +- 4 files changed, 62 insertions(+), 39 deletions(-) diff --git a/Common/GhidraClient.cpp b/Common/GhidraClient.cpp index b26e5a374229..e0c5bbe3cacf 100644 --- a/Common/GhidraClient.cpp +++ b/Common/GhidraClient.cpp @@ -74,7 +74,7 @@ bool GhidraClient::FetchSymbols() { return false; } - for (const auto pEntry: entries) { + for (const auto pEntry : entries) { JsonGet entry = pEntry->value; GhidraSymbol symbol; @@ -104,7 +104,7 @@ bool GhidraClient::FetchTypes() { return false; } - for (const auto pEntry: entries) { + for (const auto pEntry : entries) { const JsonGet entry = pEntry->value; GhidraType type; @@ -124,7 +124,7 @@ bool GhidraClient::FetchTypes() { pendingResult_.error = "missing enum members"; return false; } - for (const JsonNode* pEnumEntry: enumEntries->value) { + for (const JsonNode* pEnumEntry : enumEntries->value) { JsonGet enumEntry = pEnumEntry->value; GhidraEnumMember member; member.name = enumEntry.getStringOr("name", ""); @@ -153,7 +153,7 @@ bool GhidraClient::FetchTypes() { pendingResult_.error = "missing composite members"; return false; } - for (const JsonNode* pCompositeEntry: compositeEntries->value) { + for (const JsonNode* pCompositeEntry : compositeEntries->value) { JsonGet compositeEntry = pCompositeEntry->value; GhidraCompositeMember member; member.fieldName = compositeEntry.getStringOr("fieldName", ""); diff --git a/Common/GhidraClient.h b/Common/GhidraClient.h index efaeb00c7c41..1884e4a19438 100644 --- a/Common/GhidraClient.h +++ b/Common/GhidraClient.h @@ -63,13 +63,13 @@ struct GhidraType { }; class GhidraClient { -public: enum class Status { Idle, Pending, Ready, }; +public: struct Result { std::vector symbols; std::unordered_map types; diff --git a/UI/ImDebugger/ImStructViewer.cpp b/UI/ImDebugger/ImStructViewer.cpp index 8604a0817f70..84d1701a10be 100644 --- a/UI/ImDebugger/ImStructViewer.cpp +++ b/UI/ImDebugger/ImStructViewer.cpp @@ -3,6 +3,7 @@ #include "ext/imgui/imgui.h" +#include "Common/System/Request.h" #include "Core/MemMap.h" #include "Core/Debugger/Breakpoints.h" #include "Core/MIPS/MIPSDebugInterface.h" @@ -190,7 +191,7 @@ static constexpr int COLUMN_CONTENT = 2; void ImStructViewer::Draw(MIPSDebugInterface* mipsDebug, bool* open) { mipsDebug_ = mipsDebug; - ImGui::SetNextWindowSize(ImVec2(430, 450), ImGuiCond_FirstUseEver); + ImGui::SetNextWindowSize(ImVec2(750, 550), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Struct viewer", open) || !mipsDebug->isAlive() || !Memory::IsActive()) { ImGui::End(); return; @@ -275,7 +276,7 @@ void ImStructViewer::DrawGlobals() { ImGui::TableSetupColumn("Content"); ImGui::TableHeadersRow(); - for (const auto& symbol: ghidraClient_.result.symbols) { + for (const auto& symbol : ghidraClient_.result.symbols) { if (!symbol.label || !symbol.userDefined || symbol.dataTypePathName.empty()) { continue; } @@ -304,7 +305,7 @@ void ImStructViewer::DrawWatch() { ImGui::TableHeadersRow(); int watchIndex = -1; - for (const auto& watch: watches_) { + for (const auto& watch : watches_) { watchIndex++; if (!watchFilter_.PassFilter(watch.name.c_str())) { continue; @@ -313,10 +314,9 @@ void ImStructViewer::DrawWatch() { if (!watch.expression.empty()) { u32 val; PostfixExpression postfix; - if (mipsDebug_->initExpression(watch.expression.c_str(), postfix)) { - if (mipsDebug_->parseExpression(postfix, val)) { - address = val; - } + if (mipsDebug_->initExpression(watch.expression.c_str(), postfix) + && mipsDebug_->parseExpression(postfix, val)) { + address = val; } } else { address = watch.address; @@ -346,7 +346,7 @@ void ImStructViewer::DrawNewWatchEntry() { ImGui::SetKeyboardFocusHere(0); } newWatch_.typeFilter.Draw(); - for (const auto& entry: ghidraClient_.result.types) { + for (const auto& entry : ghidraClient_.result.types) { const auto& type = entry.second; if (newWatch_.typeFilter.PassFilter(type.displayName.c_str())) { ImGui::PushID(type.pathName.c_str()); @@ -498,7 +498,7 @@ static void DrawPointerContent( static std::string FormatEnumValue(const std::vector& enumMembers, const u64 value) { std::stringstream ss; bool hasPrevious = false; - for (const auto& member: enumMembers) { + for (const auto& member : enumMembers) { if (value & member.value) { if (hasPrevious) { ss << " | "; @@ -559,12 +559,12 @@ void ImStructViewer::DrawType( ImGui::AlignTextToFramePadding(); // Flags used for nodes that can't be further opened const ImGuiTreeNodeFlags leafFlags = ImGuiTreeNodeFlags_Leaf | ImGuiTreeNodeFlags_NoTreePushOnOpen | - ImGuiTreeNodeFlags_Bullet | extraTreeNodeFlags; + extraTreeNodeFlags; // Type is missing in fetched types, this can happen e.g. if type used for watch is removed from Ghidra if (!hasType) { ImGui::TreeNodeEx("Field", leafFlags, "%s", name); - DrawContextMenu(base, offset, 0, typePathName, name, watchIndex); + DrawContextMenu(base, offset, 0, typePathName, name, watchIndex, nullptr); ImGui::PushStyleColor(ImGuiCol_Text, COLOR_RED); DrawTypeColumn("", typePathName, base, offset); ImGui::PopStyleColor(); @@ -580,7 +580,7 @@ void ImStructViewer::DrawType( // Handle cases where pointers or expressions point to invalid memory if (!Memory::IsValidAddress(address)) { ImGui::TreeNodeEx("Field", leafFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, nullptr); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); ImGui::PushStyleColor(ImGuiCol_Text, COLOR_GRAY); @@ -596,23 +596,24 @@ void ImStructViewer::DrawType( switch (type.kind) { case ENUM: { ImGui::TreeNodeEx("Enum", leafFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + const u64 enumValue = ReadMemoryInt(address, type.length); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, &enumValue); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); - const u64 value = ReadMemoryInt(address, type.length); - const std::string stringValue = FormatEnumValue(type.enumMembers, value); - ImGui::Text("= %llx (%s)", value, stringValue.c_str()); + const std::string enumString = FormatEnumValue(type.enumMembers, enumValue); + ImGui::Text("= %llx (%s)", enumValue, enumString.c_str()); DrawIntBuiltInEditPopup(address, type.length); break; } case POINTER: { const bool nodeOpen = ImGui::TreeNodeEx("Pointer", extraTreeNodeFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + const u32 pointer = Memory::Read_U32(address); + const u64 pointer64 = pointer; + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, &pointer64); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); - const u32 pointer = Memory::Read_U32(address); DrawPointerContent(types, type, pointer); if (nodeOpen) { @@ -649,7 +650,7 @@ void ImStructViewer::DrawType( } case ARRAY: { const bool nodeOpen = ImGui::TreeNodeEx("Array", extraTreeNodeFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, nullptr); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); @@ -669,11 +670,11 @@ void ImStructViewer::DrawType( case STRUCTURE: case UNION: { const bool nodeOpen = ImGui::TreeNodeEx("Composite", extraTreeNodeFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, nullptr); DrawTypeColumn("%s", typeDisplayName, base, offset); if (nodeOpen) { - for (const auto& member: type.compositeMembers) { + for (const auto& member : type.compositeMembers) { DrawType(base, offset + member.offset, member.typePathName, nullptr, member.fieldName.c_str(), -1); } @@ -683,7 +684,7 @@ void ImStructViewer::DrawType( } case FUNCTION_DEFINITION: ImGui::TreeNodeEx("Field", leafFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, nullptr); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); @@ -691,9 +692,11 @@ void ImStructViewer::DrawType( break; case BUILT_IN: { ImGui::TreeNodeEx("Field", leafFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); if (knownBuiltIns.count(typePathName)) { + // This will copy float as int, but we can live with that for now + const u64 value = ReadMemoryInt(address, type.alignedLength); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, &value); DrawTypeColumn("%s", typeDisplayName, base, offset); ImGui::TableSetColumnIndex(COLUMN_CONTENT); DrawBuiltInContent(knownBuiltIns.at(typePathName), address); @@ -709,7 +712,7 @@ void ImStructViewer::DrawType( // At this point there is most likely some issue in the Ghidra plugin and the type wasn't // classified to any category ImGui::TreeNodeEx("Field", leafFlags, "%s", name); - DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex); + DrawContextMenu(base, offset, type.alignedLength, typePathName, name, watchIndex, nullptr); ImGui::PushStyleColor(ImGuiCol_Text, COLOR_RED); DrawTypeColumn("", typeDisplayName, base, offset); ImGui::PopStyleColor(); @@ -721,18 +724,33 @@ void ImStructViewer::DrawType( ImGui::PopID(); } +static void CopyHexNumberToClipboard(u64 value) { + std::stringstream ss; + ss << std::hex << value; + const std::string valueString = ss.str(); + System_CopyStringToClipboard(valueString); +} + void ImStructViewer::DrawContextMenu( const u32 base, const u32 offset, const int length, const std::string& typePathName, const char* name, - const int watchIndex + const int watchIndex, + const u64* value ) { ImGui::OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); if (ImGui::BeginPopup("context")) { const u32 address = base + offset; + if (ImGui::MenuItem("Copy address")) { + CopyHexNumberToClipboard(address); + } + if (value && ImGui::MenuItem("Copy value")) { + CopyHexNumberToClipboard(*value); + } + // This might be called when iterating over existing watches so can't modify the watch vector directly here if (watchIndex < 0) { if (ImGui::MenuItem("Add watch")) { @@ -760,21 +778,25 @@ void ImStructViewer::DrawContextMenu( CBreakPoints::RemoveMemCheck(address, end); } } - if (!hasMemCheck || !(memCheck.cond & MEMCHECK_READ)) { - if (ImGui::MenuItem("Add memory read breakpoint")) { + const bool canAddRead = !hasMemCheck || !(memCheck.cond & MEMCHECK_READ); + const bool canAddWrite = !hasMemCheck || !(memCheck.cond & MEMCHECK_WRITE); + const bool canAddWriteOnChange = !hasMemCheck || !(memCheck.cond & MEMCHECK_WRITE_ONCHANGE); + if ((canAddRead || canAddWrite || canAddWriteOnChange) && ImGui::BeginMenu("Add memory breakpoint")) { + if (canAddRead && canAddWrite && ImGui::MenuItem("Read/Write")) { + constexpr auto cond = static_cast(MEMCHECK_READ | MEMCHECK_WRITE); + CBreakPoints::AddMemCheck(address, end, cond, BREAK_ACTION_PAUSE); + } + if (canAddRead && ImGui::MenuItem("Read")) { CBreakPoints::AddMemCheck(address, end, MEMCHECK_READ, BREAK_ACTION_PAUSE); } - } - if (!hasMemCheck || !(memCheck.cond & MEMCHECK_WRITE)) { - if (ImGui::MenuItem("Add memory write breakpoint")) { + if (canAddWrite && ImGui::MenuItem("Write")) { CBreakPoints::AddMemCheck(address, end, MEMCHECK_WRITE, BREAK_ACTION_PAUSE); } - } - if (!hasMemCheck || !(memCheck.cond & MEMCHECK_WRITE_ONCHANGE)) { - if (ImGui::MenuItem("Add memory write on change breakpoint")) { + if (canAddWriteOnChange && ImGui::MenuItem("Write Change")) { constexpr auto cond = static_cast(MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE); CBreakPoints::AddMemCheck(address, end, cond, BREAK_ACTION_PAUSE); } + ImGui::EndMenu(); } } diff --git a/UI/ImDebugger/ImStructViewer.h b/UI/ImDebugger/ImStructViewer.h index 473f4b4f5d28..808e92cbd2b0 100644 --- a/UI/ImDebugger/ImStructViewer.h +++ b/UI/ImDebugger/ImStructViewer.h @@ -67,5 +67,6 @@ class ImStructViewer { int length, const std::string& typePathName, const char* name, - int watchIndex); + int watchIndex, + const u64* value); };