From 4cc2b5c38a3f1ee371bdf9d6392aabd4d48ab580 Mon Sep 17 00:00:00 2001 From: Michael Scire Date: Fri, 12 Jul 2019 18:18:31 -0700 Subject: [PATCH] creport: address review commentary --- .../creport/source/creport_crash_report.cpp | 50 +++---------------- stratosphere/creport/source/creport_main.cpp | 4 +- .../creport/source/creport_threads.cpp | 2 +- .../creport/source/creport_threads.hpp | 1 + stratosphere/libstratosphere | 2 +- 5 files changed, 13 insertions(+), 46 deletions(-) diff --git a/stratosphere/creport/source/creport_crash_report.cpp b/stratosphere/creport/source/creport_crash_report.cpp index 931f77c04..fe2cfdd82 100644 --- a/stratosphere/creport/source/creport_crash_report.cpp +++ b/stratosphere/creport/source/creport_crash_report.cpp @@ -29,26 +29,6 @@ namespace sts::creport { constexpr size_t DyingMessageAddressOffset = 0x1C0; /* Helper functions. */ - bool IsAddressReadable(Handle debug_handle, u64 address, size_t size) { - MemoryInfo mi; - u32 pi; - if (R_FAILED(svcQueryDebugProcessMemory(&mi, &pi, debug_handle, address))) { - return false; - } - - /* Must be read or read-write */ - if ((mi.perm | Perm_W) != Perm_Rw) { - return false; - } - - /* Must have space for both userdata address and userdata size. */ - if (address < mi.addr || mi.addr + mi.size < address + size) { - return false; - } - - return true; - } - bool TryGetCurrentTimestamp(u64 *out) { /* Clear output. */ *out = 0; @@ -63,12 +43,12 @@ namespace sts::creport { /* Try to get the current time. */ { - auto time_holder = sm::ScopedServiceHolder(); - return R_SUCCEEDED(time_holder.GetResult()) && R_SUCCEEDED(timeGetCurrentTime(TimeType_LocalSystemClock, out)); + sm::ScopedServiceHolder time_holder; + return time_holder && R_SUCCEEDED(timeGetCurrentTime(TimeType_LocalSystemClock, out)); } } - void EnsureReportDirectories() { + void TryCreateReportDirectories() { mkdir("sdmc:/atmosphere", S_IRWXU); mkdir("sdmc:/atmosphere/crash_reports", S_IRWXU); mkdir("sdmc:/atmosphere/crash_reports/dumps", S_IRWXU); @@ -131,13 +111,14 @@ namespace sts::creport { this->module_list.FindModulesFromThreadInfo(this->debug_handle, this->thread_list.GetThreadInfo(i)); } - /* Nintendo's creport builds the report here, but we'll do it later. */ + /* Nintendo's creport saves the report to erpt here, but we'll save to SD card later. */ } } void CrashReport::GetFatalContext(FatalContext *out) const { std::memset(out, 0, sizeof(*out)); + /* TODO: Support generating 32-bit fatal contexts? */ out->is_aarch32 = false; out->type = static_cast(this->exception_info.type); @@ -162,10 +143,6 @@ namespace sts::creport { } void CrashReport::ProcessExceptions() { - if (!this->IsOpen()) { - return; - } - /* Loop all debug events. */ svc::DebugEventInfo d; while (R_SUCCEEDED(svcGetDebugEvent(reinterpret_cast(&d), this->debug_handle))) { @@ -202,10 +179,6 @@ namespace sts::creport { u64 userdata_address = 0; u64 userdata_size = 0; - if (!IsAddressReadable(this->debug_handle, address, sizeof(userdata_address) + sizeof(userdata_size))) { - return; - } - /* Read userdata address. */ if (R_FAILED(svcReadDebugProcessMemory(&userdata_address, this->debug_handle, address, sizeof(userdata_address)))) { return; @@ -251,9 +224,7 @@ namespace sts::creport { this->result = ResultCreportUserBreak; /* Try to parse out the user break result. */ if (GetRuntimeFirmwareVersion() >= FirmwareVersion_500) { - if (IsAddressReadable(this->debug_handle, d.info.exception.specific.user_break.address, sizeof(this->result))) { - svcReadDebugProcessMemory(&this->result, this->debug_handle, d.info.exception.specific.user_break.address, sizeof(this->result)); - } + svcReadDebugProcessMemory(&this->result, this->debug_handle, d.info.exception.specific.user_break.address, sizeof(this->result)); } break; case svc::DebugExceptionType::UndefinedSystemCall: @@ -292,18 +263,13 @@ namespace sts::creport { return; } - /* Validate that we can read the dying message. */ - if (!IsAddressReadable(this->debug_handle, this->dying_message_address, this->dying_message_size)) { - return; - } - /* Read the dying message. */ svcReadDebugProcessMemory(this->dying_message, this->debug_handle, this->dying_message_address, this->dying_message_size); } void CrashReport::SaveReport() { - /* Ensure path exists. */ - EnsureReportDirectories(); + /* Try to ensure path exists. */ + TryCreateReportDirectories(); /* Get a timestamp. */ u64 timestamp; diff --git a/stratosphere/creport/source/creport_main.cpp b/stratosphere/creport/source/creport_main.cpp index 637ef003b..4b827518f 100644 --- a/stratosphere/creport/source/creport_main.cpp +++ b/stratosphere/creport/source/creport_main.cpp @@ -108,8 +108,8 @@ int main(int argc, char **argv) { /* Try to terminate the process. */ { - auto ns_holder = sts::sm::ScopedServiceHolder(); - if (R_SUCCEEDED(ns_holder.GetResult())) { + sts::sm::ScopedServiceHolder ns_holder; + if (ns_holder) { nsdevTerminateProcess(crashed_pid); } } diff --git a/stratosphere/creport/source/creport_threads.cpp b/stratosphere/creport/source/creport_threads.cpp index f8019f9ee..413c0aed7 100644 --- a/stratosphere/creport/source/creport_threads.cpp +++ b/stratosphere/creport/source/creport_threads.cpp @@ -136,7 +136,7 @@ namespace sts::creport { return false; } - /* In aarch32 mode svcGetDebugThreadParam does not set the LR, FP, and SP registers correctly. */ + /* In aarch32 mode svcGetDebugThreadContext does not set the LR, FP, and SP registers correctly. */ if (!is_64_bit) { this->context.fp = this->context.cpu_gprs[11].x; this->context.sp = this->context.cpu_gprs[13].x; diff --git a/stratosphere/creport/source/creport_threads.hpp b/stratosphere/creport/source/creport_threads.hpp index e350a470b..bc5575352 100644 --- a/stratosphere/creport/source/creport_threads.hpp +++ b/stratosphere/creport/source/creport_threads.hpp @@ -45,6 +45,7 @@ namespace sts::creport { u64 GetGeneralPurposeRegister(size_t i) const { return this->context.cpu_gprs[i].x; } + u64 GetPC() const { return this->context.pc.x; } diff --git a/stratosphere/libstratosphere b/stratosphere/libstratosphere index a8282205b..5ae7b3ea9 160000 --- a/stratosphere/libstratosphere +++ b/stratosphere/libstratosphere @@ -1 +1 @@ -Subproject commit a8282205b5049a098f2a21e976b7261327d64b47 +Subproject commit 5ae7b3ea9e06c7c7b40a8b1d6cbae329d015e2c4