From 2071c98b552cbccf1fea58bf816cd861f747632a Mon Sep 17 00:00:00 2001 From: Matt Dallmeyer Date: Sun, 14 Jan 2024 04:02:08 -0800 Subject: [PATCH] Fix cases of string formatting with non string literals (#3304) The logger used in `goalc` tries to print an already-formatted string `message` using `fmt::print(message);` Usually this doesn't cause problems, but if you try to print, for example, an exception that has special characters (notably `{`) it will try to do another round of formatting/replacements, despite not having any args to replace with, which ends up throwing another exception. This is why errors when parsing custom level JSON cause the REPL to exit. I've hopefully identified all the various instances of this across the codebase --- common/goos/Interpreter.cpp | 2 +- common/log/log.cpp | 2 +- common/serialization/subtitles/subtitles_v2.cpp | 2 +- common/util/Assert.cpp | 4 ++-- decompiler/ObjectFile/ObjectFileDB_IR2.cpp | 2 +- decompiler/analysis/expression_build.cpp | 2 +- decompiler/analysis/inline_asm_rewrite.cpp | 2 +- game/main.cpp | 2 +- goalc/compiler/Compiler.cpp | 8 ++++---- goalc/compiler/Util.cpp | 4 ++-- goalc/compiler/compilation/Macro.cpp | 4 ++-- goalc/debugger/Debugger.cpp | 2 +- test/offline/framework/file_management.cpp | 2 +- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/common/goos/Interpreter.cpp b/common/goos/Interpreter.cpp index b6c627a6e1..8b8e1df7ac 100644 --- a/common/goos/Interpreter.cpp +++ b/common/goos/Interpreter.cpp @@ -1888,7 +1888,7 @@ Object Interpreter::eval_format(const Object& form, fmt::format_args(args2.data(), static_cast(args2.size()))); if (truthy(dest)) { - lg::print(formatted.c_str()); + lg::print("{}", formatted.c_str()); } return StringObject::make_new(formatted); diff --git a/common/log/log.cpp b/common/log/log.cpp index a985dcc317..06dc7faff4 100644 --- a/common/log/log.cpp +++ b/common/log/log.cpp @@ -106,7 +106,7 @@ void log_print(const char* message) { } if (gLogger.stdout_log_level < lg::level::off_unless_die) { - fmt::print(message); + fmt::print("{}", message); fflush(stdout); fflush(stderr); } diff --git a/common/serialization/subtitles/subtitles_v2.cpp b/common/serialization/subtitles/subtitles_v2.cpp index ffe68e9570..6e8beab329 100644 --- a/common/serialization/subtitles/subtitles_v2.cpp +++ b/common/serialization/subtitles/subtitles_v2.cpp @@ -373,7 +373,7 @@ bool GameSubtitleDB::write_subtitle_db_to_files(const GameVersion game_version) file_util::write_text_file(dump_path, lines_file.dump(2)); } } catch (std::exception& ex) { - lg::error(ex.what()); + lg::error("{}", ex.what()); return false; } return true; diff --git a/common/util/Assert.cpp b/common/util/Assert.cpp index be4e88288c..d31aa70e82 100644 --- a/common/util/Assert.cpp +++ b/common/util/Assert.cpp @@ -16,12 +16,12 @@ void private_assert_failed(const char* expr, if (!msg || msg[0] == '\0') { std::string log = fmt::format("Assertion failed: '{}'\n\tSource: {}:{}\n\tFunction: {}\n", expr, file, line, function); - lg::die(log); + lg::die("{}", log); } else { std::string log = fmt::format("Assertion failed: '{}'\n\tMessage: {}\n\tSource: {}:{}\n\tFunction: {}\n", expr, msg, file, line, function); - lg::die(log); + lg::die("{}", log); } abort(); } diff --git a/decompiler/ObjectFile/ObjectFileDB_IR2.cpp b/decompiler/ObjectFile/ObjectFileDB_IR2.cpp index 0637bc41c6..3a7a7f8b5e 100644 --- a/decompiler/ObjectFile/ObjectFileDB_IR2.cpp +++ b/decompiler/ObjectFile/ObjectFileDB_IR2.cpp @@ -802,7 +802,7 @@ void ObjectFileDB::ir2_insert_lets(int seg, ObjectFileData& data) { "Error while inserting lets: {}. Make sure that the return type is not " "none if something is actually returned.", e.what()); - lg::warn(err); + lg::warn("{}", err); func.warnings.error(err); } } diff --git a/decompiler/analysis/expression_build.cpp b/decompiler/analysis/expression_build.cpp index fa8f53a5ed..41d633f133 100644 --- a/decompiler/analysis/expression_build.cpp +++ b/decompiler/analysis/expression_build.cpp @@ -172,7 +172,7 @@ bool convert_to_expressions( "statement.", f.name()); f.warnings.warning(warn); - lg::warn(warn); + lg::warn("{}", warn); } } diff --git a/decompiler/analysis/inline_asm_rewrite.cpp b/decompiler/analysis/inline_asm_rewrite.cpp index d3f80dea87..5d68c6555f 100644 --- a/decompiler/analysis/inline_asm_rewrite.cpp +++ b/decompiler/analysis/inline_asm_rewrite.cpp @@ -87,7 +87,7 @@ bool rewrite_inline_asm_instructions(Form* top_level_form, } catch (std::exception& e) { std::string warning = fmt::format("ASM instruction re-writing failed in {}: {}", f.name(), e.what()); - lg::warn(warning); + lg::warn("{}", warning); f.warnings.error(";; {}", warning); return false; } diff --git a/game/main.cpp b/game/main.cpp index 449529b59e..4dad7c5390 100644 --- a/game/main.cpp +++ b/game/main.cpp @@ -127,7 +127,7 @@ int main(int argc, char** argv) { CLI11_PARSE(app, argc, argv); if (show_version) { - lg::print(build_revision()); + lg::print("{}", build_revision()); return 0; } diff --git a/goalc/compiler/Compiler.cpp b/goalc/compiler/Compiler.cpp index f1d96bffac..2fe1f02836 100644 --- a/goalc/compiler/Compiler.cpp +++ b/goalc/compiler/Compiler.cpp @@ -197,7 +197,7 @@ Val* Compiler::compile_error_guard(const goos::Object& code, Env* env) { auto loc_info = m_goos.reader.db.get_info_for(code, &term); if (term) { lg::print(fg(fmt::color::yellow) | fmt::emphasis::bold, "Location:\n"); - lg::print(loc_info); + lg::print("{}", loc_info); } lg::print(fg(fmt::color::yellow) | fmt::emphasis::bold, "Code:\n"); @@ -212,7 +212,7 @@ Val* Compiler::compile_error_guard(const goos::Object& code, Env* env) { } std::string line(80, '-'); line.push_back('\n'); - lg::print(line); + lg::print("{}", line); } throw ce; } @@ -224,7 +224,7 @@ Val* Compiler::compile_error_guard(const goos::Object& code, Env* env) { auto loc_info = m_goos.reader.db.get_info_for(code, &term); if (term) { lg::print(fg(fmt::color::yellow) | fmt::emphasis::bold, "Location:\n"); - lg::print(loc_info); + lg::print("{}", loc_info); } lg::print(fg(fmt::color::yellow) | fmt::emphasis::bold, "Code:\n"); @@ -240,7 +240,7 @@ Val* Compiler::compile_error_guard(const goos::Object& code, Env* env) { } std::string line(80, '-'); line.push_back('\n'); - lg::print(line); + lg::print("{}", line); throw ce; } } diff --git a/goalc/compiler/Util.cpp b/goalc/compiler/Util.cpp index 2b80e3aefd..c37006966f 100644 --- a/goalc/compiler/Util.cpp +++ b/goalc/compiler/Util.cpp @@ -162,7 +162,7 @@ goos::Arguments Compiler::get_va(const goos::Object& form, const goos::Object& r std::string err; if (!goos::get_va(rest, &err, &args)) { - throw_compiler_error(form, err); + throw_compiler_error(form, "{}", err); } return args; } @@ -189,7 +189,7 @@ void Compiler::va_check( named) { std::string err; if (!goos::va_check(args, unnamed, named, &err)) { - throw_compiler_error(form, err); + throw_compiler_error(form, "{}", err); } } diff --git a/goalc/compiler/compilation/Macro.cpp b/goalc/compiler/compilation/Macro.cpp index 61bf473e0e..0f207bc5b0 100644 --- a/goalc/compiler/compilation/Macro.cpp +++ b/goalc/compiler/compilation/Macro.cpp @@ -66,7 +66,7 @@ Val* Compiler::compile_goos_macro(const goos::Object& o, } std::string line(80, '-'); line.push_back('\n'); - lg::print(line); + lg::print("{}", line); } throw; @@ -87,7 +87,7 @@ Val* Compiler::compile_goos_macro(const goos::Object& o, } std::string line(80, '-'); line.push_back('\n'); - lg::print(line); + lg::print("{}", line); throw; } diff --git a/goalc/debugger/Debugger.cpp b/goalc/debugger/Debugger.cpp index 364a07fc21..f60415757d 100644 --- a/goalc/debugger/Debugger.cpp +++ b/goalc/debugger/Debugger.cpp @@ -224,7 +224,7 @@ InstructionPointerInfo Debugger::get_rip_info(u64 rip) { void print_and_append_to_string(std::string& str, const std::string& log) { str += log; - lg::print(log); + lg::print("{}", log); } std::vector Debugger::get_backtrace(u64 rip, diff --git a/test/offline/framework/file_management.cpp b/test/offline/framework/file_management.cpp index 9860a69224..d8357f41a5 100644 --- a/test/offline/framework/file_management.cpp +++ b/test/offline/framework/file_management.cpp @@ -75,7 +75,7 @@ std::vector find_source_files(const std::string& game_nam msg += fmt::format("\n- '{}'", path); } } - lg::die(msg); + lg::die("{}", msg); } return result;