From f6f9599899359aa05d3b8b60e1de69b6dd1d56fd Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 21 Sep 2021 18:46:10 +0200 Subject: [PATCH] CrashReporter+LibCoredump: Show progress window while loading coredump Some coredumps take a long time to symbolicate, so let's show a simple window with a progress bar while they are loading. I'm not super happy with the factoring of this feature, but it's an absolutely kickass feature that makes crashing feel 100% more responsive than before, since you now get GUI feedback almost immediately after a crash occurs. :^) --- Userland/Applications/CrashReporter/main.cpp | 40 ++++++++++++++++++-- Userland/Libraries/LibCoredump/Backtrace.cpp | 39 ++++++++++++++----- Userland/Libraries/LibCoredump/Backtrace.h | 2 +- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/Userland/Applications/CrashReporter/main.cpp b/Userland/Applications/CrashReporter/main.cpp index 01c4c04903ed5e..f9a192ce01a148 100644 --- a/Userland/Applications/CrashReporter/main.cpp +++ b/Userland/Applications/CrashReporter/main.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020-2021, Linus Groh + * Copyright (c) 2021, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -24,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -36,10 +38,42 @@ struct TitleAndText { String text; }; +static NonnullRefPtr create_progress_window() +{ + auto window = GUI::Window::construct(); + window->set_title("CrashReporter"); + window->set_resizable(false); + window->resize(240, 64); + window->center_on_screen(); + auto& main_widget = window->set_main_widget(); + main_widget.set_fill_with_background_color(true); + main_widget.set_layout(); + auto& label = main_widget.add("Generating crash report..."); + label.set_fixed_height(30); + auto& progressbar = main_widget.add(); + progressbar.set_name("progressbar"); + progressbar.set_fixed_width(150); + progressbar.set_fixed_height(22); + return window; +} + static TitleAndText build_backtrace(Coredump::Reader const& coredump, ELF::Core::ThreadInfo const& thread_info, size_t thread_index) { + // Show a very simple progress window ASAP to make crashing feel more responsive. + // FIXME: This is not the most beautifully factored thing. + auto progress_window = create_progress_window(); + progress_window->show(); + + auto& progressbar = *progress_window->main_widget()->find_descendant_of_type_named("progressbar"); + auto timer = Core::ElapsedTimer::start_new(); - Coredump::Backtrace backtrace(coredump, thread_info); + Coredump::Backtrace backtrace(coredump, thread_info, [&](size_t frame_index, size_t frame_count) { + progressbar.set_value(frame_index + 1); + progressbar.set_max(frame_count); + Core::EventLoop::current().pump(Core::EventLoop::WaitMode::PollForEvents); + }); + progress_window->close(); + auto metadata = coredump.metadata(); dbgln("Generating backtrace took {} ms", timer.elapsed()); @@ -118,6 +152,8 @@ int main(int argc, char** argv) return 1; } + auto app = GUI::Application::construct(argc, argv); + const char* coredump_path = nullptr; bool unlink_after_use = false; @@ -163,8 +199,6 @@ int main(int argc, char** argv) dbgln("Failed deleting coredump file"); } - auto app = GUI::Application::construct(argc, argv); - if (pledge("stdio recvfd sendfd rpath unix", nullptr) < 0) { perror("pledge"); return 1; diff --git a/Userland/Libraries/LibCoredump/Backtrace.cpp b/Userland/Libraries/LibCoredump/Backtrace.cpp index 1edbccc256949b..814d6f1d38b12b 100644 --- a/Userland/Libraries/LibCoredump/Backtrace.cpp +++ b/Userland/Libraries/LibCoredump/Backtrace.cpp @@ -42,20 +42,37 @@ ELFObjectInfo const* Backtrace::object_info_for_region(ELF::Core::MemoryRegionIn return info_ptr; } -Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread_info) +Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread_info, Function on_progress) : m_thread_info(move(thread_info)) { - FlatPtr* bp; - FlatPtr* ip; #if ARCH(I386) - bp = (FlatPtr*)m_thread_info.regs.ebp; - ip = (FlatPtr*)m_thread_info.regs.eip; + auto* start_bp = (FlatPtr*)m_thread_info.regs.ebp; + auto* start_ip = (FlatPtr*)m_thread_info.regs.eip; #else - bp = (FlatPtr*)m_thread_info.regs.rbp; - ip = (FlatPtr*)m_thread_info.regs.rip; + auto* start_bp = (FlatPtr*)m_thread_info.regs.rbp; + auto* start_ip = (FlatPtr*)m_thread_info.regs.rip; #endif - bool first_frame = true; + // In order to provide progress updates, we first have to walk the + // call stack to determine how many frames it has. + size_t frame_count = 0; + { + auto* bp = start_bp; + auto* ip = start_ip; + while (bp && ip) { + ++frame_count; + auto next_ip = coredump.peek_memory((FlatPtr)(bp + 1)); + auto next_bp = coredump.peek_memory((FlatPtr)(bp)); + if (!next_ip.has_value() || !next_bp.has_value()) + break; + ip = (FlatPtr*)next_ip.value(); + bp = (FlatPtr*)next_bp.value(); + } + } + + auto* bp = start_bp; + auto* ip = start_ip; + size_t frame_index = 0; while (bp && ip) { // We use eip - 1 because the return address from a function frame // is the instruction that comes after the 'call' instruction. @@ -63,8 +80,10 @@ Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread // instruction rather than the return address we don't subtract // 1 there. VERIFY((FlatPtr)ip > 0); - add_entry(coredump, (FlatPtr)ip - (first_frame ? 0 : 1)); - first_frame = false; + add_entry(coredump, (FlatPtr)ip - ((frame_index == 0) ? 0 : 1)); + if (on_progress) + on_progress(frame_index, frame_count); + ++frame_index; auto next_ip = coredump.peek_memory((FlatPtr)(bp + 1)); auto next_bp = coredump.peek_memory((FlatPtr)(bp)); if (!next_ip.has_value() || !next_bp.has_value()) diff --git a/Userland/Libraries/LibCoredump/Backtrace.h b/Userland/Libraries/LibCoredump/Backtrace.h index e450e31753f94a..e22ecb42695d89 100644 --- a/Userland/Libraries/LibCoredump/Backtrace.h +++ b/Userland/Libraries/LibCoredump/Backtrace.h @@ -37,7 +37,7 @@ class Backtrace { String to_string(bool color = false) const; }; - Backtrace(const Reader&, const ELF::Core::ThreadInfo&); + Backtrace(const Reader&, const ELF::Core::ThreadInfo&, Function on_progress = {}); ~Backtrace(); ELF::Core::ThreadInfo const& thread_info() const { return m_thread_info; }