From 44e4d88f587f71accddf979c2f8771638d0e2947 Mon Sep 17 00:00:00 2001 From: sysadmin Date: Fri, 6 Feb 2026 06:52:44 -0700 Subject: [PATCH] Provider refactor: 2-method base class, ProcessProvider, ProcessPicker Collapse Provider interface from 9 virtual methods to 2 (read + size), move providers to src/providers/, add name()/kind()/getSymbol() virtuals. Replace FileProvider with BufferProvider, add ProcessProvider (Win32) with module-based symbol resolution, wire ProcessPicker dialog, and integrate getSymbol into pointer display and command row. - Fix isReadable overflow for large addresses - Guard deferred showSourcePicker/showTypeAutocomplete against stale edits - 7/7 tests pass including 3 new provider test suites Co-Authored-By: Claude Opus 4.6 --- CMakeLists.txt | 27 +++ src/compose.cpp | 14 +- src/controller.cpp | 118 +++++++++--- src/controller.h | 2 + src/core.h | 110 ++--------- src/editor.cpp | 65 +++++-- src/editor.h | 3 + src/format.cpp | 20 +- src/main.cpp | 21 ++- src/processpicker.cpp | 194 ++++++++++++++++++++ src/processpicker.h | 46 +++++ src/processpicker.ui | 163 ++++++++++++++++ src/providers/buffer_provider.h | 47 +++++ src/providers/null_provider.h | 14 ++ src/providers/process_provider.h | 102 ++++++++++ src/providers/provider.h | 78 ++++++++ tests/test_command_row.cpp | 139 ++++++++++++++ tests/test_compose.cpp | 20 +- tests/test_core.cpp | 10 +- tests/test_editor.cpp | 76 ++------ tests/test_format.cpp | 8 +- tests/test_provider.cpp | 296 ++++++++++++++++++++++++++++++ tests/test_provider_getSymbol.cpp | 105 +++++++++++ 23 files changed, 1457 insertions(+), 221 deletions(-) create mode 100644 src/processpicker.cpp create mode 100644 src/processpicker.h create mode 100644 src/processpicker.ui create mode 100644 src/providers/buffer_provider.h create mode 100644 src/providers/null_provider.h create mode 100644 src/providers/process_provider.h create mode 100644 src/providers/provider.h create mode 100644 tests/test_command_row.cpp create mode 100644 tests/test_provider.cpp create mode 100644 tests/test_provider_getSymbol.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6adaf8e..3443e35 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,7 @@ set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_AUTOMOC ON) set(CMAKE_AUTORCC ON) +set(CMAKE_AUTOUIC ON) find_package(Qt6 REQUIRED COMPONENTS Widgets PrintSupport) @@ -19,6 +20,9 @@ add_executable(ReclassX src/controller.cpp src/compose.cpp src/format.cpp + src/processpicker.h + src/processpicker.cpp + src/processpicker.ui src/resources.qrc ) @@ -29,6 +33,7 @@ target_link_libraries(ReclassX PRIVATE Qt6::PrintSupport QScintilla::QScintilla dbghelp + psapi ) add_custom_target(screenshot ALL @@ -43,6 +48,10 @@ file(WRITE ${_combine_script} " set(_out \"${CMAKE_BINARY_DIR}/h_cpp_combined.txt\") file(WRITE \${_out} \"\") foreach(_f + \"${CMAKE_SOURCE_DIR}/src/providers/provider.h\" + \"${CMAKE_SOURCE_DIR}/src/providers/buffer_provider.h\" + \"${CMAKE_SOURCE_DIR}/src/providers/null_provider.h\" + \"${CMAKE_SOURCE_DIR}/src/providers/process_provider.h\" \"${CMAKE_SOURCE_DIR}/src/core.h\" \"${CMAKE_SOURCE_DIR}/src/editor.h\" \"${CMAKE_SOURCE_DIR}/src/editor.cpp\" @@ -90,4 +99,22 @@ if(BUILD_TESTING) Qt6::Widgets Qt6::PrintSupport Qt6::Test QScintilla::QScintilla) add_test(NAME test_editor COMMAND test_editor) + + add_executable(test_provider tests/test_provider.cpp) + target_include_directories(test_provider PRIVATE src) + target_link_libraries(test_provider PRIVATE Qt6::Core Qt6::Test) + add_test(NAME test_provider COMMAND test_provider) + + add_executable(test_command_row tests/test_command_row.cpp) + target_include_directories(test_command_row PRIVATE src) + target_link_libraries(test_command_row PRIVATE Qt6::Core Qt6::Test) + add_test(NAME test_command_row COMMAND test_command_row) + + add_executable(test_provider_getSymbol tests/test_provider_getSymbol.cpp) + target_include_directories(test_provider_getSymbol PRIVATE src) + target_link_libraries(test_provider_getSymbol PRIVATE Qt6::Core Qt6::Test) + if(WIN32) + target_link_libraries(test_provider_getSymbol PRIVATE psapi) + endif() + add_test(NAME test_provider_getSymbol COMMAND test_provider_getSymbol) endif() diff --git a/src/compose.cpp b/src/compose.cpp index 85f0e63..f53cfbf 100644 --- a/src/compose.cpp +++ b/src/compose.cpp @@ -39,7 +39,7 @@ struct ComposeState { if (currentLine > 0) text += '\n'; // 3-char fold indicator column: " - " expanded, " + " collapsed, " " other if (lm.lineKind == LineKind::CommandRow) - text += QStringLiteral(" * "); + text += QStringLiteral(" "); else if (lm.foldHead) text += lm.foldCollapsed ? QStringLiteral(" + ") : QStringLiteral(" - "); else @@ -135,7 +135,7 @@ void composeLeaf(ComposeState& state, const NodeTree& tree, lm.isContinuation = isCont; lm.lineKind = isCont ? LineKind::Continuation : LineKind::Field; lm.nodeKind = node.kind; - lm.offsetText = fmt::fmtOffsetMargin(tree.baseAddress + absAddr, isCont); + lm.offsetText = fmt::fmtOffsetMargin(absAddr, isCont); lm.markerMask = computeMarkers(node, prov, absAddr, isCont, depth); lm.foldLevel = computeFoldLevel(depth, false); lm.effectiveTypeW = typeW; @@ -171,7 +171,7 @@ void composeParent(ComposeState& state, const NodeTree& tree, lm.nodeId = node.id; lm.depth = depth; lm.lineKind = LineKind::Field; - lm.offsetText = fmt::fmtOffsetMargin(tree.baseAddress + absAddr, false); + lm.offsetText = fmt::fmtOffsetMargin(absAddr, false); lm.nodeKind = node.kind; lm.markerMask = (1u << M_CYCLE) | (1u << M_ERR); lm.foldLevel = computeFoldLevel(depth, false); @@ -188,7 +188,7 @@ void composeParent(ComposeState& state, const NodeTree& tree, lm.nodeId = node.id; lm.depth = depth; lm.lineKind = LineKind::ArrayElementSeparator; - lm.offsetText = fmt::fmtOffsetMargin(tree.baseAddress + absAddr, false); + lm.offsetText = fmt::fmtOffsetMargin(absAddr, false); lm.nodeKind = node.kind; lm.foldLevel = computeFoldLevel(depth, false); lm.markerMask = 0; @@ -207,7 +207,7 @@ void composeParent(ComposeState& state, const NodeTree& tree, lm.nodeId = node.id; lm.depth = depth; lm.lineKind = LineKind::Header; - lm.offsetText = fmt::fmtOffsetMargin(tree.baseAddress + absAddr, false); + lm.offsetText = fmt::fmtOffsetMargin(absAddr, false); lm.nodeKind = node.kind; lm.foldHead = true; lm.foldCollapsed = node.collapsed; @@ -289,7 +289,7 @@ void composeNode(ComposeState& state, const NodeTree& tree, lm.nodeId = node.id; lm.depth = depth; lm.lineKind = LineKind::Field; - lm.offsetText = fmt::fmtOffsetMargin(tree.baseAddress + absAddr, false); + lm.offsetText = fmt::fmtOffsetMargin(absAddr, false); lm.nodeKind = node.kind; lm.foldHead = true; lm.foldCollapsed = node.collapsed; @@ -445,7 +445,7 @@ ComposeResult compose(const NodeTree& tree, const Provider& prov) { lm.markerMask = 0; lm.effectiveTypeW = state.typeW; lm.effectiveNameW = state.nameW; - state.emitLine(QStringLiteral("SRC: File : 0x0"), lm); + state.emitLine(QStringLiteral("File Address: 0x0"), lm); } QVector roots = state.childMap.value(0); diff --git a/src/controller.cpp b/src/controller.cpp index 79dedaf..a3de008 100644 --- a/src/controller.cpp +++ b/src/controller.cpp @@ -1,4 +1,6 @@ #include "controller.h" +#include "providers/process_provider.h" +#include "processpicker.h" #include #include #include @@ -11,6 +13,10 @@ #include #include #include +#include +#ifdef _WIN32 +#include +#endif namespace rcx { @@ -94,7 +100,8 @@ void RcxDocument::loadData(const QString& binaryPath) { if (!file.open(QIODevice::ReadOnly)) return; undoStack.clear(); - provider = std::make_unique(file.readAll()); + provider = std::make_unique( + file.readAll(), QFileInfo(binaryPath).fileName()); dataPath = binaryPath; tree.baseAddress = 0; emit documentChanged(); @@ -102,7 +109,7 @@ void RcxDocument::loadData(const QString& binaryPath) { void RcxDocument::loadData(const QByteArray& data) { undoStack.clear(); - provider = std::make_unique(data); + provider = std::make_unique(data); tree.baseAddress = 0; emit documentChanged(); } @@ -266,7 +273,16 @@ void RcxController::connectEditor(RcxEditor* editor) { QString path = QFileDialog::getOpenFileName(w, "Load Binary Data", {}, "All Files (*)"); if (!path.isEmpty()) m_doc->loadData(path); } - // "Process" is a placeholder — no action yet + else if (text == QStringLiteral("Process")) { +#ifdef _WIN32 + auto* w = qobject_cast(parent()); + ProcessPicker picker(w); + if (picker.exec() == QDialog::Accepted) { + attachToProcess(picker.selectedProcessId(), + picker.selectedProcessName()); + } +#endif + } break; } case EditTarget::ArrayIndex: @@ -619,22 +635,22 @@ void RcxController::showContextMenu(RcxEditor* editor, int line, int nodeIdx, && node.kind != NodeKind::Padding && node.kind != NodeKind::Mat4x4 && m_doc->provider->isWritable(); if (isEditable) { - menu.addAction("Edit &Value", [editor, line]() { + menu.addAction("Edit &Value\tEnter", [editor, line]() { editor->beginInlineEdit(EditTarget::Value, line); }); } - menu.addAction("Re&name", [editor, line]() { + menu.addAction("Re&name\tF2", [editor, line]() { editor->beginInlineEdit(EditTarget::Name, line); }); - menu.addAction("Change &Type", [editor, line]() { + menu.addAction("Change &Type\tT", [editor, line]() { editor->beginInlineEdit(EditTarget::Type, line); }); menu.addSeparator(); - menu.addAction("&Add Field Below", [this, parentId]() { + menu.addAction("&Add Field Below\tInsert", [this, parentId]() { insertNode(parentId, -1, NodeKind::Hex64, "newField"); }); @@ -649,11 +665,11 @@ void RcxController::showContextMenu(RcxEditor* editor, int line, int nodeIdx, }); } - menu.addAction("D&uplicate", [this, nodeId]() { + menu.addAction("D&uplicate\tCtrl+D", [this, nodeId]() { int ni = m_doc->tree.indexOfId(nodeId); if (ni >= 0) duplicateNode(ni); }); - menu.addAction("&Delete", [this, nodeId]() { + menu.addAction("&Delete\tDelete", [this, nodeId]() { int ni = m_doc->tree.indexOfId(nodeId); if (ni >= 0) removeNode(ni); }); @@ -803,29 +819,87 @@ void RcxController::applySelectionOverlays() { } void RcxController::updateCommandRow() { + // -- Source label: driven by provider metadata -- QString src; - if (!m_doc->filePath.isEmpty()) - src = QFileInfo(m_doc->filePath).fileName(); - else - src = QStringLiteral("File"); - if (!m_doc->dataPath.isEmpty()) - src += QStringLiteral(" @ ") + QFileInfo(m_doc->dataPath).fileName(); + QString provName = m_doc->provider->name(); + if (provName.isEmpty()) { + src = QStringLiteral(" in command row + // kind() returns "File" via base default +}; + +} // namespace rcx diff --git a/src/providers/process_provider.h b/src/providers/process_provider.h new file mode 100644 index 0000000..43a723d --- /dev/null +++ b/src/providers/process_provider.h @@ -0,0 +1,102 @@ +#pragma once +#include "provider.h" + +#ifdef _WIN32 +#include +#include + +namespace rcx { + +class ProcessProvider : public Provider { + HANDLE m_handle = nullptr; + uint64_t m_base = 0; + int m_size = 0; + QString m_name; + + struct ModuleInfo { + QString name; + uint64_t base; + uint64_t size; + }; + QVector m_modules; + +public: + ProcessProvider(HANDLE proc, uint64_t base, int regionSize, const QString& name) + : m_handle(proc), m_base(base), m_size(regionSize), m_name(name) + { + cacheModules(); + } + + ~ProcessProvider() override { + if (m_handle) CloseHandle(m_handle); + } + + ProcessProvider(const ProcessProvider&) = delete; + ProcessProvider& operator=(const ProcessProvider&) = delete; + + int size() const override { return m_size; } + + bool read(uint64_t addr, void* buf, int len) const override { + SIZE_T got = 0; + BOOL ok = ReadProcessMemory(m_handle, + (LPCVOID)(m_base + addr), buf, len, &got); + return ok && (int)got == len; + } + + bool isWritable() const override { return true; } + + bool write(uint64_t addr, const void* buf, int len) override { + SIZE_T got = 0; + BOOL ok = WriteProcessMemory(m_handle, + (LPVOID)(m_base + addr), buf, len, &got); + return ok && (int)got == len; + } + + QString name() const override { return m_name; } + QString kind() const override { return QStringLiteral("Process"); } + + // getSymbol takes an absolute virtual address and resolves it to + // "module.dll+0xOFFSET" using the cached module list. + QString getSymbol(uint64_t absAddr) const override { + for (const auto& mod : m_modules) { + if (absAddr >= mod.base && absAddr < mod.base + mod.size) { + uint64_t offset = absAddr - mod.base; + return QStringLiteral("%1+0x%2") + .arg(mod.name) + .arg(offset, 0, 16, QChar('0')); + } + } + return {}; + } + + HANDLE handle() const { return m_handle; } + uint64_t baseAddress() const { return m_base; } + void refreshModules() { m_modules.clear(); cacheModules(); } + +private: + void cacheModules() { + HMODULE mods[1024]; + DWORD needed = 0; + if (!EnumProcessModulesEx(m_handle, mods, sizeof(mods), + &needed, LIST_MODULES_ALL)) + return; + int count = qMin((int)(needed / sizeof(HMODULE)), 1024); + m_modules.reserve(count); + for (int i = 0; i < count; ++i) { + MODULEINFO mi{}; + WCHAR modName[MAX_PATH]; + if (GetModuleInformation(m_handle, mods[i], &mi, sizeof(mi)) + && GetModuleBaseNameW(m_handle, mods[i], modName, MAX_PATH)) + { + m_modules.append({ + QString::fromWCharArray(modName), + (uint64_t)mi.lpBaseOfDll, + (uint64_t)mi.SizeOfImage + }); + } + } + } +}; + +} // namespace rcx +#endif // _WIN32 diff --git a/src/providers/provider.h b/src/providers/provider.h new file mode 100644 index 0000000..5f376c5 --- /dev/null +++ b/src/providers/provider.h @@ -0,0 +1,78 @@ +#pragma once +#include +#include +#include +#include + +namespace rcx { + +class Provider { +public: + virtual ~Provider() = default; + + // --- Subclasses MUST implement these two --- + virtual bool read(uint64_t addr, void* buf, int len) const = 0; + virtual int size() const = 0; + + // --- Optional overrides --- + virtual bool write(uint64_t addr, const void* buf, int len) { + Q_UNUSED(addr); Q_UNUSED(buf); Q_UNUSED(len); + return false; + } + virtual bool isWritable() const { return false; } + + // Human-readable label for this source. + // Examples: "notepad.exe", "dump.bin", "tcp://10.0.0.1:1337" + virtual QString name() const { return {}; } + + // Category tag for the command row Source span. + // Examples: "File", "Process", "Socket" + virtual QString kind() const { return QStringLiteral("File"); } + + // Resolve an absolute address to a symbol name. + // Returns empty string if no symbol is known. + // ProcessProvider: "ntdll.dll+0x1A30" + // BufferProvider: "" (no symbols in flat files) + virtual QString getSymbol(uint64_t addr) const { + Q_UNUSED(addr); + return {}; + } + + // --- Derived convenience (non-virtual, never override) --- + + bool isValid() const { return size() > 0; } + + bool isReadable(uint64_t addr, int len) const { + if (len <= 0) return (len == 0); + uint64_t ulen = (uint64_t)len; + return addr <= (uint64_t)size() && ulen <= (uint64_t)size() - addr; + } + + template + T readAs(uint64_t addr) const { + T v{}; + read(addr, &v, sizeof(T)); + return v; + } + + uint8_t readU8 (uint64_t a) const { return readAs(a); } + uint16_t readU16(uint64_t a) const { return readAs(a); } + uint32_t readU32(uint64_t a) const { return readAs(a); } + uint64_t readU64(uint64_t a) const { return readAs(a); } + float readF32(uint64_t a) const { return readAs(a); } + double readF64(uint64_t a) const { return readAs(a); } + + QByteArray readBytes(uint64_t addr, int len) const { + if (len <= 0) return {}; + QByteArray buf(len, Qt::Uninitialized); + if (!read(addr, buf.data(), len)) + buf.fill('\0'); + return buf; + } + + bool writeBytes(uint64_t addr, const QByteArray& d) { + return write(addr, d.constData(), d.size()); + } +}; + +} // namespace rcx diff --git a/tests/test_command_row.cpp b/tests/test_command_row.cpp new file mode 100644 index 0000000..d60de33 --- /dev/null +++ b/tests/test_command_row.cpp @@ -0,0 +1,139 @@ +#include +#include +#include +#include "providers/provider.h" +#include "providers/buffer_provider.h" +#include "providers/null_provider.h" + +using namespace rcx; + +// -- Replicate the label-building logic from updateCommandRow so we can test it +// without needing a full RcxController/RcxDocument/RcxEditor stack. + +static QString buildSourceLabel(const Provider& prov) { + QString provName = prov.name(); + if (provName.isEmpty()) + return QStringLiteral("")); + } + + void label_bufferNoName_showsSelectSource() { + // BufferProvider with empty name also triggers ")); + } + + void label_bufferWithName_showsFileAndName() { + BufferProvider p(QByteArray(4, '\0'), "dump.bin"); + QCOMPARE(buildSourceLabel(p), QStringLiteral("File 'dump.bin'")); + } + + // --------------------------------------------------------------- + // Full command row text + // --------------------------------------------------------------- + + void row_nullProvider() { + NullProvider p; + QString row = buildCommandRow(p, 0); + QCOMPARE(row, QStringLiteral(" ")); + } + + void span_fileProvider() { + BufferProvider p(QByteArray(4, '\0'), "dump.bin"); + QString row = buildCommandRow(p, 0x140000000ULL); + auto span = commandRowSrcSpan(row); + QVERIFY(span.valid); + QString extracted = row.mid(span.start, span.end - span.start); + QCOMPARE(extracted, QStringLiteral("File 'dump.bin'")); + } + + void span_processProvider_simulated() { + // Simulate a process provider without needing Windows APIs + // by building the string directly + QString row = QStringLiteral(" Process 'notepad.exe' Address: 0x7FF600000000"); + auto span = commandRowSrcSpan(row); + QVERIFY(span.valid); + QString extracted = row.mid(span.start, span.end - span.start); + QCOMPARE(extracted, QStringLiteral("Process 'notepad.exe'")); + } + + // --------------------------------------------------------------- + // Provider switching simulation + // --------------------------------------------------------------- + + void switching_nullToFileToProcess() { + // Start with NullProvider + std::unique_ptr prov = std::make_unique(); + QCOMPARE(buildSourceLabel(*prov), QStringLiteral("