From 03115c9f2f52da6a96fb9d0f33ea3e555ff4544c Mon Sep 17 00:00:00 2001 From: Dan McCarthy Date: Sat, 23 Dec 2023 01:02:48 -0800 Subject: [PATCH] Debugger: Add memory search types: GreaterThan(OrEqual), LesserThan(OrEqual), and Not Equal (#10441) * Make memory search search type handling more clear with enum Adds an enum class to represent the Search type used in a memory search. Prior, this was just handled with an integer to represent each type, but it was very unclear what corresponded to which type at first glance. Made this easier to follow by using an enum to represent the type. * Debugger : Add support for greater than/less than/not equal search types Adds support for basic greater than/greater than or equal/less than/less than or equal/not equal search types for the debugger's Memory Scan. This adds a new input to allow selecting the search comparison type, which defaults to Equals, and allows switching to the above mentioned comparisons. It's set up to allow for adding more easily. Restructures some of the functions to make having multiple comparisons quite manageable. Adds an enum for search comparison types for easy logic handling. * Debugger: Update Array/String search type error to mention not handling Not Equals Currently array/string searches don't support Not Equals searches, so this needs to be removed. * Debugger: Code cleanup + feedback changes Sets up if expressions to use constexpr for compile time evaluation and makes the is greater/less than logic simpler to read for int. Also removes an unneeded QPushButton cast and simply compares the pointers directly. --- pcsx2-qt/Debugger/CpuWidget.cpp | 184 +++++++++++++++++++++----------- pcsx2-qt/Debugger/CpuWidget.h | 23 ++++ pcsx2-qt/Debugger/CpuWidget.ui | 34 ++++++ 3 files changed, 178 insertions(+), 63 deletions(-) diff --git a/pcsx2-qt/Debugger/CpuWidget.cpp b/pcsx2-qt/Debugger/CpuWidget.cpp index 264a5b908093b2..3b54a799da23ba 100644 --- a/pcsx2-qt/Debugger/CpuWidget.cpp +++ b/pcsx2-qt/Debugger/CpuWidget.cpp @@ -39,6 +39,9 @@ using namespace QtUtils; using namespace MipsStackWalk; +using SearchComparison = CpuWidget::SearchComparison; +using SearchType = CpuWidget::SearchType; + CpuWidget::CpuWidget(QWidget* parent, DebugInterface& cpu) : m_cpu(cpu) , m_bpModel(cpu) @@ -869,7 +872,7 @@ void CpuWidget::onStackListDoubleClick(const QModelIndex& index) } template -static bool checkAddressValueMatches(DebugInterface* cpu, u32 addr, T value) +static T readValueAtAddress(DebugInterface* cpu, u32 addr) { T val = 0; switch (sizeof(T)) @@ -882,42 +885,88 @@ static bool checkAddressValueMatches(DebugInterface* cpu, u32 addr, T value) break; case sizeof(u32): { - if (std::is_same_v) - { - const float fTop = value + 0.00001f; - const float fBottom = value - 0.00001f; - const float memValue = std::bit_cast(cpu->read32(addr)); - return (fBottom < memValue && memValue < fTop); - } - val = cpu->read32(addr); break; } case sizeof(u64): { - if (std::is_same_v) + val = cpu->read64(addr); + break; + } + } + return val; +} + +template +static bool memoryValueComparator(SearchComparison searchComparison, T searchValue, T readValue) +{ + const bool isNotOperator = searchComparison == SearchComparison::NotEquals; + switch (searchComparison) + { + case SearchComparison::Equals: + case SearchComparison::NotEquals: + { + bool areValuesEqual = false; + if constexpr (std::is_same_v) { - const double dTop = value + 0.00001f; - const double dBottom = value - 0.00001f; - const double memValue = std::bit_cast(cpu->read64(addr)); - return (dBottom < memValue && memValue < dTop); + const T fTop = searchValue + 0.00001f; + const T fBottom = searchValue - 0.00001f; + const T memValue = std::bit_cast(readValue); + areValuesEqual = (fBottom < memValue && memValue < fTop); } - - val = cpu->read64(addr); + else if constexpr (std::is_same_v) + { + const double dTop = searchValue + 0.00001f; + const double dBottom = searchValue - 0.00001f; + const double memValue = std::bit_cast(readValue); + areValuesEqual = (dBottom < memValue && memValue < dTop); + } + else + { + areValuesEqual = searchValue == readValue; + } + return isNotOperator ? !areValuesEqual : areValuesEqual; break; } + case SearchComparison::GreaterThan: + case SearchComparison::GreaterThanOrEqual: + case SearchComparison::LessThan: + case SearchComparison::LessThanOrEqual: + { + const bool hasEqualsCheck = searchComparison == SearchComparison::GreaterThanOrEqual || searchComparison == SearchComparison::LessThanOrEqual; + if (hasEqualsCheck && memoryValueComparator(SearchComparison::Equals, searchValue, readValue)) + return true; + + const bool isGreaterOperator = searchComparison == SearchComparison::GreaterThan || searchComparison == SearchComparison::GreaterThanOrEqual; + if (std::is_same_v) + { + const T fTop = searchValue + 0.00001f; + const T fBottom = searchValue - 0.00001f; + const T memValue = std::bit_cast(readValue); + const bool isGreater = memValue > fTop; + const bool isLesser = memValue < fBottom; + return isGreaterOperator ? isGreater : isLesser; + } + else if (std::is_same_v) + { + const double dTop = searchValue + 0.00001f; + const double dBottom = searchValue - 0.00001f; + const double memValue = std::bit_cast(readValue); + const bool isGreater = memValue > dTop; + const bool isLesser = memValue < dBottom; + return isGreaterOperator ? isGreater : isLesser; + } + return isGreaterOperator ? (readValue > searchValue) : (readValue < searchValue); + } default: Console.Error("Debugger: Unknown type when doing memory search!"); return false; - break; } - - return val == value; } template -static std::vector searchWorker(DebugInterface* cpu, std::vector searchAddresses, u32 start, u32 end, T value) +std::vector searchWorker(DebugInterface* cpu, std::vector searchAddresses, SearchComparison searchComparison, u32 start, u32 end, T searchValue) { std::vector hitAddresses; const bool isSearchingRange = searchAddresses.size() <= 0; @@ -925,7 +974,10 @@ static std::vector searchWorker(DebugInterface* cpu, std::vector searc { for (u32 addr = start; addr < end; addr += sizeof(T)) { - if (checkAddressValueMatches(cpu, addr, value)) + if (!cpu->isValidAddress(addr)) + continue; + T readValue = readValueAtAddress(cpu, addr); + if (memoryValueComparator(searchComparison, searchValue, readValue)) { hitAddresses.push_back(addr); } @@ -935,11 +987,13 @@ static std::vector searchWorker(DebugInterface* cpu, std::vector searc { for (const u32 addr : searchAddresses) { - if (checkAddressValueMatches(cpu, addr, value)) + if (!cpu->isValidAddress(addr)) + continue; + T readValue = readValueAtAddress(cpu, addr); + if (memoryValueComparator(searchComparison, searchValue, readValue)) { hitAddresses.push_back(addr); } - } } return hitAddresses; @@ -959,7 +1013,6 @@ static bool compareByteArrayAtAddress(DebugInterface* cpu, u32 addr, QByteArray static std::vector searchWorkerByteArray(DebugInterface* cpu, std::vector searchAddresses, u32 start, u32 end, QByteArray value) { - std::vector hitAddresses; const bool isSearchingRange = searchAddresses.size() <= 0; if (isSearchingRange) @@ -986,27 +1039,26 @@ static std::vector searchWorkerByteArray(DebugInterface* cpu, std::vector startWorker(DebugInterface* cpu, int type, std::vector searchAddresses, u32 start, u32 end, QString value, int base) +std::vector startWorker(DebugInterface* cpu, const SearchType type, const SearchComparison searchComparison, std::vector searchAddresses, u32 start, u32 end, QString value, int base) { - const bool isSigned = value.startsWith("-"); switch (type) { - case 0: - return isSigned ? searchWorker(cpu, searchAddresses, start, end, value.toShort(nullptr, base)) : searchWorker(cpu, searchAddresses, start, end, value.toUShort(nullptr, base)); - case 1: - return isSigned ? searchWorker(cpu, searchAddresses, start, end, value.toShort(nullptr, base)) : searchWorker(cpu, searchAddresses, start, end, value.toUShort(nullptr, base)); - case 2: - return isSigned ? searchWorker(cpu, searchAddresses, start, end, value.toInt(nullptr, base)) : searchWorker(cpu, searchAddresses, start, end, value.toUInt(nullptr, base)); - case 3: - return isSigned ? searchWorker(cpu, searchAddresses, start, end, value.toLong(nullptr, base)) : searchWorker(cpu, searchAddresses, start, end, value.toULongLong(nullptr, base)); - case 4: - return searchWorker(cpu, searchAddresses, start, end, value.toFloat()); - case 5: - return searchWorker(cpu, searchAddresses, start, end, value.toDouble()); - case 6: + case SearchType::ByteType: + return isSigned ? searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toShort(nullptr, base)) : searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toUShort(nullptr, base)); + case SearchType::Int16Type: + return isSigned ? searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toShort(nullptr, base)) : searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toUShort(nullptr, base)); + case SearchType::Int32Type: + return isSigned ? searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toInt(nullptr, base)) : searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toUInt(nullptr, base)); + case SearchType::Int64Type: + return isSigned ? searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toLong(nullptr, base)) : searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toULongLong(nullptr, base)); + case SearchType::FloatType: + return searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toFloat()); + case SearchType::DoubleType: + return searchWorker(cpu, searchAddresses, searchComparison, start, end, value.toDouble()); + case SearchType::StringType: return searchWorkerByteArray(cpu, searchAddresses, start, end, value.toUtf8()); - case 7: + case SearchType::ArrayType: return searchWorkerByteArray(cpu, searchAddresses, start, end, QByteArray::fromHex(value.toUtf8())); default: Console.Error("Debugger: Unknown type when doing memory search!"); @@ -1020,7 +1072,7 @@ void CpuWidget::onSearchButtonClicked() if (!m_cpu.isAlive()) return; - const int searchType = m_ui.cmbSearchType->currentIndex(); + const SearchType searchType = static_cast(m_ui.cmbSearchType->currentIndex()); const bool searchHex = m_ui.chkSearchHex->isChecked(); bool ok; @@ -1047,25 +1099,33 @@ void CpuWidget::onSearchButtonClicked() } const QString searchValue = m_ui.txtSearchValue->text(); - + const SearchComparison searchComparison = static_cast(m_ui.cmbSearchComparison->currentIndex()); + const bool isFilterSearch = sender() == m_ui.btnFilterSearch; unsigned long long value; + const bool isVariableSize = searchType == SearchType::ArrayType || searchType == SearchType::StringType; + if (isVariableSize && searchComparison != SearchComparison::Equals) + { + QMessageBox::critical(this, tr("Debugger"), tr("Search types Array and String can only be used with Equals search comparisons.")); + return; + } + switch (searchType) { - case 0: - case 1: - case 2: - case 3: + case SearchType::ByteType: + case SearchType::Int16Type: + case SearchType::Int32Type: + case SearchType::Int64Type: value = searchValue.toULongLong(&ok, searchHex ? 16 : 10); break; - case 4: - case 5: + case SearchType::FloatType: + case SearchType::DoubleType: searchValue.toDouble(&ok); break; - case 6: + case SearchType::StringType: ok = !searchValue.isEmpty(); break; - case 7: + case SearchType::ArrayType: ok = !searchValue.trimmed().isEmpty(); break; } @@ -1078,21 +1138,21 @@ void CpuWidget::onSearchButtonClicked() switch (searchType) { - case 7: - case 6: - case 5: - case 4: + case SearchType::ArrayType: + case SearchType::StringType: + case SearchType::DoubleType: + case SearchType::FloatType: break; - case 3: + case SearchType::Int64Type: if (value <= std::numeric_limits::max()) break; - case 2: + case SearchType::Int32Type: if (value <= std::numeric_limits::max()) break; - case 1: + case SearchType::Int16Type: if (value <= std::numeric_limits::max()) break; - case 0: + case SearchType::ByteType: if (value <= std::numeric_limits::max()) break; default: @@ -1111,19 +1171,16 @@ void CpuWidget::onSearchButtonClicked() m_searchResults = results; loadSearchResults(); m_ui.btnFilterSearch->setDisabled(m_ui.listSearchResults->count() == 0); - }); m_ui.btnSearch->setDisabled(true); - QPushButton* senderButton = qobject_cast(sender()); - bool isFilterSearch = senderButton == m_ui.btnFilterSearch; std::vector addresses; if (isFilterSearch) { addresses = m_searchResults; } QFuture> workerFuture = - QtConcurrent::run(startWorker, &m_cpu, searchType, addresses, searchStart, searchEnd, searchValue, searchHex ? 16 : 10); + QtConcurrent::run(startWorker, &m_cpu, searchType, searchComparison, addresses, searchStart, searchEnd, searchValue, searchHex ? 16 : 10); workerWatcher->setFuture(workerFuture); } @@ -1139,7 +1196,8 @@ void CpuWidget::onSearchResultsListScroll(u32 value) } } -void CpuWidget::loadSearchResults() { +void CpuWidget::loadSearchResults() +{ const u32 numLoaded = m_ui.listSearchResults->count(); const u32 amountLeftToLoad = m_searchResults.size() - numLoaded; if (amountLeftToLoad < 1) diff --git a/pcsx2-qt/Debugger/CpuWidget.h b/pcsx2-qt/Debugger/CpuWidget.h index 5adc3f24f46a4a..2ec4faa82491f7 100644 --- a/pcsx2-qt/Debugger/CpuWidget.h +++ b/pcsx2-qt/Debugger/CpuWidget.h @@ -44,6 +44,29 @@ class CpuWidget final : public QWidget CpuWidget(QWidget* parent, DebugInterface& cpu); ~CpuWidget(); + enum class SearchType + { + ByteType, + Int16Type, + Int32Type, + Int64Type, + FloatType, + DoubleType, + StringType, + ArrayType + }; + + // Note: The order of these enum values must reflect the order in thee Search Comparison combobox. + enum class SearchComparison + { + Equals, + NotEquals, + GreaterThan, + GreaterThanOrEqual, + LessThan, + LessThanOrEqual + }; + public slots: void paintEvent(QPaintEvent* event); diff --git a/pcsx2-qt/Debugger/CpuWidget.ui b/pcsx2-qt/Debugger/CpuWidget.ui index d4c94bb84cb603..bfddb52742bc9b 100644 --- a/pcsx2-qt/Debugger/CpuWidget.ui +++ b/pcsx2-qt/Debugger/CpuWidget.ui @@ -306,6 +306,40 @@ + + + + + Equals + + + + + Not Equals + + + + + Greater Than + + + + + Greater Than Or Equal + + + + + Less Than + + + + + Less Than Or Equal + + + +