Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Qt/Patches: Improve patch validation and patch creator #14172

Merged
merged 7 commits into from Jul 13, 2023
Merged

Qt/Patches: Improve patch validation and patch creator #14172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d2569cf
Qt: add validator to patch creator offset lineedit
Megamouse Jul 9, 2023
09ef7c6
Patches: validate yml patch offset values
Megamouse Jul 11, 2023
e4f710a
Patches: fix logging for locations of node iterators
Megamouse Jul 11, 2023
85fb2a9
Patches: do not allow empty patch names
Megamouse Jul 11, 2023
51d2583
Patches: improve location logging of some invalid nodes
Megamouse Jul 11, 2023
d62040f
Qt/patches: highlight faulty lines in patch creator
Megamouse Jul 11, 2023
27f8b01
Merge branch 'master' into fixes
elad335 Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 60 additions & 18 deletions Utilities/bin_patch.cpp
View file
Open in desktop
Expand Up @@ -219,18 +219,18 @@ bool patch_engine::load(patch_map& patches_map, const std::string& path, std::st
// Go through each main key in the file
for (auto pair : root)
{
const auto& main_key = pair.first.Scalar();
const std::string& main_key = pair.first.Scalar();

if (const auto yml_type = pair.second.Type(); yml_type != YAML::NodeType::Map)
if (main_key.empty())
{
append_log_message(log_messages, fmt::format("Error: Skipping key %s: expected Map, found %s (location: %s, file: %s)", main_key, yml_type, get_yaml_node_location(pair.second), path), &patch_log.error);
append_log_message(log_messages, fmt::format("Error: Skipping empty key (location: %s, file: %s)", get_yaml_node_location(pair.first), path), &patch_log.error);
is_valid = false;
continue;
}

if (main_key.empty())
if (const auto yml_type = pair.second.Type(); yml_type != YAML::NodeType::Map)
{
append_log_message(log_messages, fmt::format("Error: Skipping empty key (location: %s, file: %s)", get_yaml_node_location(pair.second), path), &patch_log.error);
append_log_message(log_messages, fmt::format("Error: Skipping key %s: expected Map, found %s (location: %s, file: %s)", main_key, yml_type, get_yaml_node_location(pair.second), path), &patch_log.error);
is_valid = false;
continue;
}
Expand All @@ -242,7 +242,7 @@ bool patch_engine::load(patch_map& patches_map, const std::string& path, std::st
}

// Find or create an entry matching the key/hash in our map
auto& container = patches_map[main_key];
patch_container& container = patches_map[main_key];
container.hash = main_key;
container.version = version;

Expand All @@ -252,6 +252,13 @@ bool patch_engine::load(patch_map& patches_map, const std::string& path, std::st
// Each key in "Patches" is also the patch description
const std::string& description = patches_entry.first.Scalar();

if (description.empty())
{
append_log_message(log_messages, fmt::format("Error: Empty patch name (key: %s, location: %s, file: %s)", main_key, get_yaml_node_location(patches_entry.first), path), &patch_log.error);
is_valid = false;
continue;
}

// Compile patch information

if (const auto yml_type = patches_entry.second.Type(); yml_type != YAML::NodeType::Map)
Expand Down Expand Up @@ -330,7 +337,7 @@ bool patch_engine::load(patch_map& patches_map, const std::string& path, std::st
continue;
}

patch_engine::patch_app_versions app_versions;
patch_app_versions app_versions;

for (const auto version : serial_node.second)
{
Expand Down Expand Up @@ -579,6 +586,14 @@ bool patch_engine::load(patch_map& patches_map, const std::string& path, std::st
{
if (!read_patch_node(info, patch_node, root, log_messages))
{
for (const auto& it : patches_entry.second)
{
if (it.first.Scalar() == patch_key::patch)
{
append_log_message(log_messages, fmt::format("Skipping invalid patch node %s: (key: %s, location: %s)", info.description, main_key, get_yaml_node_location(it.first)), &patch_log.error);
break;
}
}
is_valid = false;
}
}
Expand Down Expand Up @@ -703,6 +718,33 @@ bool patch_engine::add_patch_data(YAML::Node node, patch_info& info, u32 modifie

std::string error_message;

// Validate offset
switch (p_data.type)
{
case patch_type::move_file:
case patch_type::hide_file:
break;
default:
{
const u32 offset = get_yaml_node_value<u32>(addr_node, error_message);
if (!error_message.empty())
{
error_message = fmt::format("Skipping patch data entry: [ %s, 0x%.8x, %s ] (key: %s, location: %s) Invalid patch offset '%s' (not a valid u32 or overflow)",
p_data.type, p_data.offset, p_data.original_value.empty() ? "?" : p_data.original_value, info.hash, get_yaml_node_location(node), p_data.original_offset);
append_log_message(log_messages, error_message, &patch_log.error);
return false;
}
if ((0xFFFFFFFF - modifier) < p_data.offset)
{
error_message = fmt::format("Skipping patch data entry: [ %s, 0x%.8x, %s ] (key: %s, location: %s) Invalid combination of patch offset 0x%.8x and modifier 0x%.8x (overflow)",
p_data.type, p_data.offset, p_data.original_value.empty() ? "?" : p_data.original_value, info.hash, get_yaml_node_location(node), p_data.offset, modifier);
append_log_message(log_messages, error_message, &patch_log.error);
return false;
}
break;
}
}

switch (p_data.type)
{
case patch_type::utf8:
Expand Down Expand Up @@ -1338,9 +1380,9 @@ std::basic_string<u32> patch_engine::apply(const std::string& name, std::functio
}

std::basic_string<u32> applied_total;
const auto& container = ::at32(m_map, name);
const auto& serial = Emu.GetTitleID();
const auto& app_version = Emu.GetAppVersion();
const patch_container& container = ::at32(m_map, name);
const std::string& serial = Emu.GetTitleID();
const std::string& app_version = Emu.GetAppVersion();

// Different containers in order to separate the patches
std::vector<std::shared_ptr<patch_info>> patches_for_this_serial_and_this_version;
Expand Down Expand Up @@ -1374,7 +1416,7 @@ std::basic_string<u32> patch_engine::apply(const std::string& name, std::functio
continue;
}

const auto& app_versions = ::at32(serials, found_serial);
const patch_app_versions& app_versions = ::at32(serials, found_serial);
std::string found_app_version;

if (app_versions.contains(app_version))
Expand Down Expand Up @@ -1492,11 +1534,11 @@ void patch_engine::unload(const std::string& name)
return;
}

const auto& container = ::at32(m_map, name);
const patch_container& container = ::at32(m_map, name);

for (const auto& [description, patch] : container.patch_info_map)
{
for (auto& entry : patch.data_list)
for (const patch_data& entry : patch.data_list)
{
// Deallocate used memory
if (u32 addr = std::exchange(entry.alloc_addr, 0))
Expand Down Expand Up @@ -1624,7 +1666,7 @@ static void append_patches(patch_engine::patch_map& existing_patches, const patc
continue;
}

auto& container = existing_patches[hash];
patch_engine::patch_container& container = existing_patches[hash];

for (const auto& [description, new_info] : new_container.patch_info_map)
{
Expand All @@ -1635,7 +1677,7 @@ static void append_patches(patch_engine::patch_map& existing_patches, const patc
continue;
}

auto& info = container.patch_info_map[description];
patch_engine::patch_info& info = container.patch_info_map[description];

bool ok;
const bool version_is_bigger = utils::compare_versions(new_info.patch_version, info.patch_version, ok) > 0;
Expand Down Expand Up @@ -1798,7 +1840,7 @@ bool patch_engine::save_patches(const patch_map& patches, const std::string& pat

bool patch_engine::import_patches(const patch_engine::patch_map& patches, const std::string& path, usz& count, usz& total, std::stringstream* log_messages)
{
patch_engine::patch_map existing_patches;
patch_map existing_patches;

if (load(existing_patches, path, "", true, log_messages))
{
Expand All @@ -1811,13 +1853,13 @@ bool patch_engine::import_patches(const patch_engine::patch_map& patches, const

bool patch_engine::remove_patch(const patch_info& info)
{
patch_engine::patch_map patches;
patch_map patches;

if (load(patches, info.source_path))
{
if (patches.contains(info.hash))
{
auto& container = patches[info.hash];
patch_container& container = patches[info.hash];

if (container.patch_info_map.contains(info.description))
{
Expand Down
6 changes: 3 additions & 3 deletions rpcs3/rpcs3qt/debugger_frame.cpp
View file
Open in desktop
Expand Up @@ -435,7 +435,7 @@ void debugger_frame::keyPressEvent(QKeyEvent* event)
dlg.set_input_font(mono, false);
dlg.set_clear_button_enabled(false);
dlg.set_button_enabled(QDialogButtonBox::StandardButton::Ok, false);
dlg.set_validator(new QRegularExpressionValidator(QRegularExpression("^[1-9][0-9]*$")));
dlg.set_validator(new QRegularExpressionValidator(QRegularExpression("^[1-9][0-9]*$"), &dlg));

u32 max = 0;

Expand Down Expand Up @@ -1188,11 +1188,11 @@ void debugger_frame::ShowGotoAddressDialog()

if (const auto thread = get_cpu(); !thread || thread->id_type() != 2)
{
expression_input->setValidator(new QRegularExpressionValidator(QRegularExpression("^(0[xX])?0*[a-fA-F0-9]{0,8}$")));
expression_input->setValidator(new QRegularExpressionValidator(QRegularExpression("^(0[xX])?0*[a-fA-F0-9]{0,8}$"), this));
}
else
{
expression_input->setValidator(new QRegularExpressionValidator(QRegularExpression("^(0[xX])?0*[a-fA-F0-9]{0,5}$")));
expression_input->setValidator(new QRegularExpressionValidator(QRegularExpression("^(0[xX])?0*[a-fA-F0-9]{0,5}$"), this));
}

// Ok/Cancel
Expand Down
4 changes: 2 additions & 2 deletions rpcs3/rpcs3qt/elf_memory_dumping_dialog.cpp
View file
Open in desktop
Expand Up @@ -34,13 +34,13 @@ elf_memory_dumping_dialog::elf_memory_dumping_dialog(u32 ppu_debugger_addr, std:
m_seg_list->setMinimumWidth(gui::utils::get_label_width(tr("PPU Address: 0x00000000, LS Address: 0x00000, Segment Size: 0x00000, Flags: 0x0")));

// Address expression input
auto make_hex_edit = [mono](u32 max_digits)
auto make_hex_edit = [this, mono](u32 max_digits)
{
QLineEdit* le = new QLineEdit();
le->setFont(mono);
le->setMaxLength(max_digits + 2);
le->setPlaceholderText("0x" + QStringLiteral("0").repeated(max_digits));
le->setValidator(new QRegularExpressionValidator(QRegularExpression(QStringLiteral("^(0[xX])?0*[a-fA-F0-9]{0,%1}$").arg(max_digits))));
le->setValidator(new QRegularExpressionValidator(QRegularExpression(QStringLiteral("^(0[xX])?0*[a-fA-F0-9]{0,%1}$").arg(max_digits)), this));
return le;
};

Expand Down
2 changes: 1 addition & 1 deletion rpcs3/rpcs3qt/log_frame.cpp
View file
Open in desktop
Expand Up @@ -679,7 +679,7 @@ void log_frame::UpdateUI()

const auto font_start_tag = [](const QColor& color) -> const QString { return QStringLiteral("<font color = \"") % color.name() % QStringLiteral("\">"); };
const QString font_start_tag_stack = "<font color = \"" % m_color_stack.name() % "\">";
const QString font_end_tag = QStringLiteral("</font>");
static const QString font_end_tag = QStringLiteral("</font>");

static constexpr auto escaped = [](const QString& text)
{
Expand Down
2 changes: 1 addition & 1 deletion rpcs3/rpcs3qt/main_window.cpp
View file
Open in desktop
Expand Up @@ -1633,7 +1633,7 @@ void main_window::DecryptSPRXLibraries()
dlg->set_input_font(mono, true, '0');
dlg->set_clear_button_enabled(false);
dlg->set_button_enabled(QDialogButtonBox::StandardButton::Ok, false);
dlg->set_validator(new QRegularExpressionValidator(QRegularExpression("^((((((K?L)?I)?C)?=)?0)?x)?[a-fA-F0-9]{0,32}$"))); // HEX only (with additional KLIC=0x prefix for convenience)
dlg->set_validator(new QRegularExpressionValidator(QRegularExpression("^((((((K?L)?I)?C)?=)?0)?x)?[a-fA-F0-9]{0,32}$"), this)); // HEX only (with additional KLIC=0x prefix for convenience)
dlg->setAttribute(Qt::WA_DeleteOnClose);

connect(dlg, &input_dialog::text_changed, dlg, [dlg](const QString& text)
Expand Down
2 changes: 1 addition & 1 deletion rpcs3/rpcs3qt/memory_viewer_panel.cpp
View file
Open in desktop
Expand Up @@ -92,7 +92,7 @@ memory_viewer_panel::memory_viewer_panel(QWidget* parent, std::shared_ptr<CPUDis
m_addr_line->setMaxLength(18);
m_addr_line->setFixedWidth(75);
m_addr_line->setFocus();
m_addr_line->setValidator(new QRegularExpressionValidator(QRegularExpression(m_type == thread_type::spu ? "^(0[xX])?0*[a-fA-F0-9]{0,5}$" : "^(0[xX])?0*[a-fA-F0-9]{0,8}$")));
m_addr_line->setValidator(new QRegularExpressionValidator(QRegularExpression(m_type == thread_type::spu ? "^(0[xX])?0*[a-fA-F0-9]{0,5}$" : "^(0[xX])?0*[a-fA-F0-9]{0,8}$"), this));
hbox_tools_mem_addr->addWidget(m_addr_line);
tools_mem_addr->setLayout(hbox_tools_mem_addr);

Expand Down