Merge pull request #10268 from Pokechu22/code-view-widget-clamp-ub

CodeViewWidget: Fix undefined behavior when centered around address 0
This commit is contained in:
JMC47 2021-12-10 05:59:47 -05:00 committed by GitHub
commit bfddce425d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 52 additions and 18 deletions

View File

@ -20,6 +20,7 @@
#include <QTableWidgetItem> #include <QTableWidgetItem>
#include <QWheelEvent> #include <QWheelEvent>
#include "Common/Assert.h"
#include "Common/GekkoDisassembler.h" #include "Common/GekkoDisassembler.h"
#include "Common/StringUtil.h" #include "Common/StringUtil.h"
#include "Core/Core.h" #include "Core/Core.h"
@ -393,26 +394,27 @@ void CodeViewWidget::CalculateBranchIndentation()
// build a 2D lookup table representing the columns and rows the arrow could be drawn in // build a 2D lookup table representing the columns and rows the arrow could be drawn in
// and try to place all branch arrows in it as far left as possible // and try to place all branch arrows in it as far left as possible
std::vector<bool> arrow_space_used(columns * rows, false); std::vector<bool> arrow_space_used(columns * rows, false);
const auto index = [&](u32 column, u32 row) { return column * rows + row; }; const auto index = [&](u32 column, u32 row) {
const u32 first_visible_addr = AddressForRow(0); ASSERT(row <= rows);
const u32 last_visible_addr = AddressForRow(static_cast<int>(rows - 1)); ASSERT(column <= columns);
for (CodeViewBranch& branch : m_branches) return column * rows + row;
{ };
const auto add_branch_arrow = [&](CodeViewBranch& branch, u32 first_addr, u32 first_row,
u32 last_addr) {
const u32 arrow_src_addr = branch.src_addr; const u32 arrow_src_addr = branch.src_addr;
const u32 arrow_dst_addr = branch.is_link ? branch.src_addr : branch.dst_addr; const u32 arrow_dst_addr = branch.is_link ? branch.src_addr : branch.dst_addr;
const u32 arrow_addr_lower = std::min(arrow_src_addr, arrow_dst_addr); const auto [arrow_addr_lower, arrow_addr_higher] = std::minmax(arrow_src_addr, arrow_dst_addr);
const u32 arrow_addr_higher = std::max(arrow_src_addr, arrow_dst_addr);
const bool is_visible =
last_visible_addr >= arrow_addr_lower || first_visible_addr <= arrow_addr_higher;
if (!is_visible)
continue;
const u32 arrow_first_visible_addr = const bool is_visible =
std::clamp(arrow_addr_lower, first_visible_addr, last_visible_addr); std::max(arrow_addr_lower, first_addr) <= std::min(arrow_addr_higher, last_addr);
const u32 arrow_last_visible_addr = if (!is_visible)
std::clamp(arrow_addr_higher, first_visible_addr, last_visible_addr); return;
const u32 arrow_first_visible_row = (arrow_first_visible_addr - first_visible_addr) / 4;
const u32 arrow_last_visible_row = (arrow_last_visible_addr - first_visible_addr) / 4; const u32 arrow_first_visible_addr = std::clamp(arrow_addr_lower, first_addr, last_addr);
const u32 arrow_last_visible_addr = std::clamp(arrow_addr_higher, first_addr, last_addr);
const u32 arrow_first_visible_row = (arrow_first_visible_addr - first_addr) / 4 + first_row;
const u32 arrow_last_visible_row = (arrow_last_visible_addr - first_addr) / 4 + first_row;
const auto free_column = [&]() -> std::optional<u32> { const auto free_column = [&]() -> std::optional<u32> {
for (u32 column = 0; column < columns; ++column) for (u32 column = 0; column < columns; ++column)
@ -432,11 +434,43 @@ void CodeViewWidget::CalculateBranchIndentation()
}(); }();
if (!free_column) if (!free_column)
continue; return;
branch.indentation = *free_column; branch.indentation = *free_column;
for (u32 row = arrow_first_visible_row; row <= arrow_last_visible_row; ++row) for (u32 row = arrow_first_visible_row; row <= arrow_last_visible_row; ++row)
arrow_space_used[index(*free_column, row)] = true; arrow_space_used[index(*free_column, row)] = true;
};
const u32 first_visible_addr = AddressForRow(0);
const u32 last_visible_addr = AddressForRow(static_cast<int>(rows - 1));
if (first_visible_addr <= last_visible_addr)
{
for (CodeViewBranch& branch : m_branches)
add_branch_arrow(branch, first_visible_addr, 0, last_visible_addr);
}
else
{
// Scrolling defaults to being centered around address 00000000, which means addresses before
// the start are visible (e.g. ffffffa8 - 00000050). We need to do this in two parts, one for
// first_visible_addr to fffffffc, and the second for 00000000 to last_visible_addr.
// That means we need to find the row corresponding to 00000000.
int addr_zero_row = -1;
for (int row = 0; row < rows; row++)
{
if (AddressForRow(row) == 0)
{
addr_zero_row = row;
break;
}
}
ASSERT(addr_zero_row != -1);
for (CodeViewBranch& branch : m_branches)
{
add_branch_arrow(branch, first_visible_addr, 0, 0xfffffffc);
add_branch_arrow(branch, 0x00000000, addr_zero_row, last_visible_addr);
}
} }
} }