From e7f9f58fa408ac89ed1ce709494d84090b63bff0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 5 Dec 2022 21:26:00 -0500 Subject: reporter: Eliminate undefined behavior in SaveErrorReport The optionals are unconditionally dereferenced when setting the custom error text, and in a few cases this function is called using the default value of the optionals. This means we'd be dereferencing uninitialized storage. Since they're used unconditionally, we can use value_or to set a default when storage is uninitialized. --- src/core/reporter.cpp | 8 ++++---- src/core/reporter.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/reporter.cpp b/src/core/reporter.cpp index 6e21296f6..543b91d9a 100644 --- a/src/core/reporter.cpp +++ b/src/core/reporter.cpp @@ -339,8 +339,8 @@ void Reporter::SavePlayReport(PlayReportType type, u64 title_id, std::vector custom_text_main, - std::optional custom_text_detail) const { + const std::optional& custom_text_main, + const std::optional& custom_text_detail) const { if (!IsReportingEnabled()) { return; } @@ -354,8 +354,8 @@ void Reporter::SaveErrorReport(u64 title_id, Result result, out["backtrace"] = GetBacktraceData(system); out["error_custom_text"] = { - {"main", *custom_text_main}, - {"detail", *custom_text_detail}, + {"main", custom_text_main.value_or("")}, + {"detail", custom_text_detail.value_or("")}, }; SaveToFile(std::move(out), GetPath("error_report", title_id, timestamp)); diff --git a/src/core/reporter.h b/src/core/reporter.h index 68755cbde..983a9545a 100644 --- a/src/core/reporter.h +++ b/src/core/reporter.h @@ -61,8 +61,8 @@ public: // Used by error applet void SaveErrorReport(u64 title_id, Result result, - std::optional custom_text_main = {}, - std::optional custom_text_detail = {}) const; + const std::optional& custom_text_main = {}, + const std::optional& custom_text_detail = {}) const; void SaveFSAccessLog(std::string_view log_message) const; -- cgit v1.2.3 From eadc1ae1e7a4ce873467b9ea244af752f57ce5e7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 5 Dec 2022 21:42:22 -0500 Subject: reporter: Pass by const reference where applicable Same behavior, but without memory churn. --- src/core/reporter.cpp | 27 ++++++++++++++------------- src/core/reporter.h | 12 ++++++------ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/core/reporter.cpp b/src/core/reporter.cpp index 543b91d9a..77821e047 100644 --- a/src/core/reporter.cpp +++ b/src/core/reporter.cpp @@ -38,7 +38,7 @@ std::string GetTimestamp() { using namespace nlohmann; -void SaveToFile(json json, const std::filesystem::path& filename) { +void SaveToFile(const json& json, const std::filesystem::path& filename) { if (!Common::FS::CreateParentDirs(filename)) { LOG_ERROR(Core, "Failed to create path for '{}' to save report!", Common::FS::PathToUTF8String(filename)); @@ -81,8 +81,8 @@ json GetReportCommonData(u64 title_id, Result result, const std::string& timesta } json GetProcessorStateData(const std::string& architecture, u64 entry_point, u64 sp, u64 pc, - u64 pstate, std::array registers, - std::optional> backtrace = {}) { + u64 pstate, const std::array& registers, + const std::optional>& backtrace = {}) { auto out = json{ {"entry_point", fmt::format("{:016X}", entry_point)}, {"sp", fmt::format("{:016X}", sp)}, @@ -224,11 +224,11 @@ void Reporter::SaveCrashReport(u64 title_id, Result result, u64 set_flags, u64 e out["processor_state"] = std::move(proc_out); - SaveToFile(std::move(out), GetPath("crash_report", title_id, timestamp)); + SaveToFile(out, GetPath("crash_report", title_id, timestamp)); } void Reporter::SaveSvcBreakReport(u32 type, bool signal_debugger, u64 info1, u64 info2, - std::optional> resolved_buffer) const { + const std::optional>& resolved_buffer) const { if (!IsReportingEnabled()) { return; } @@ -250,7 +250,7 @@ void Reporter::SaveSvcBreakReport(u32 type, bool signal_debugger, u64 info1, u64 out["svc_break"] = std::move(break_out); - SaveToFile(std::move(out), GetPath("svc_break_report", title_id, timestamp)); + SaveToFile(out, GetPath("svc_break_report", title_id, timestamp)); } void Reporter::SaveUnimplementedFunctionReport(Kernel::HLERequestContext& ctx, u32 command_id, @@ -271,13 +271,13 @@ void Reporter::SaveUnimplementedFunctionReport(Kernel::HLERequestContext& ctx, u out["function"] = std::move(function_out); - SaveToFile(std::move(out), GetPath("unimpl_func_report", title_id, timestamp)); + SaveToFile(out, GetPath("unimpl_func_report", title_id, timestamp)); } void Reporter::SaveUnimplementedAppletReport( u32 applet_id, u32 common_args_version, u32 library_version, u32 theme_color, - bool startup_sound, u64 system_tick, std::vector> normal_channel, - std::vector> interactive_channel) const { + bool startup_sound, u64 system_tick, const std::vector>& normal_channel, + const std::vector>& interactive_channel) const { if (!IsReportingEnabled()) { return; } @@ -308,10 +308,11 @@ void Reporter::SaveUnimplementedAppletReport( out["applet_normal_data"] = std::move(normal_out); out["applet_interactive_data"] = std::move(interactive_out); - SaveToFile(std::move(out), GetPath("unimpl_applet_report", title_id, timestamp)); + SaveToFile(out, GetPath("unimpl_applet_report", title_id, timestamp)); } -void Reporter::SavePlayReport(PlayReportType type, u64 title_id, std::vector> data, +void Reporter::SavePlayReport(PlayReportType type, u64 title_id, + const std::vector>& data, std::optional process_id, std::optional user_id) const { if (!IsReportingEnabled()) { return; @@ -335,7 +336,7 @@ void Reporter::SavePlayReport(PlayReportType type, u64 title_id, std::vector(type)); out["play_report_data"] = std::move(data_out); - SaveToFile(std::move(out), GetPath("play_report", title_id, timestamp)); + SaveToFile(out, GetPath("play_report", title_id, timestamp)); } void Reporter::SaveErrorReport(u64 title_id, Result result, @@ -358,7 +359,7 @@ void Reporter::SaveErrorReport(u64 title_id, Result result, {"detail", custom_text_detail.value_or("")}, }; - SaveToFile(std::move(out), GetPath("error_report", title_id, timestamp)); + SaveToFile(out, GetPath("error_report", title_id, timestamp)); } void Reporter::SaveFSAccessLog(std::string_view log_message) const { diff --git a/src/core/reporter.h b/src/core/reporter.h index 983a9545a..9fdb9d6c1 100644 --- a/src/core/reporter.h +++ b/src/core/reporter.h @@ -36,7 +36,7 @@ public: // Used by syscall svcBreak void SaveSvcBreakReport(u32 type, bool signal_debugger, u64 info1, u64 info2, - std::optional> resolved_buffer = {}) const; + const std::optional>& resolved_buffer = {}) const; // Used by HLE service handler void SaveUnimplementedFunctionReport(Kernel::HLERequestContext& ctx, u32 command_id, @@ -44,10 +44,10 @@ public: const std::string& service_name) const; // Used by stub applet implementation - void SaveUnimplementedAppletReport(u32 applet_id, u32 common_args_version, u32 library_version, - u32 theme_color, bool startup_sound, u64 system_tick, - std::vector> normal_channel, - std::vector> interactive_channel) const; + void SaveUnimplementedAppletReport( + u32 applet_id, u32 common_args_version, u32 library_version, u32 theme_color, + bool startup_sound, u64 system_tick, const std::vector>& normal_channel, + const std::vector>& interactive_channel) const; enum class PlayReportType { Old, @@ -56,7 +56,7 @@ public: System, }; - void SavePlayReport(PlayReportType type, u64 title_id, std::vector> data, + void SavePlayReport(PlayReportType type, u64 title_id, const std::vector>& data, std::optional process_id = {}, std::optional user_id = {}) const; // Used by error applet -- cgit v1.2.3