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

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020-2021, Linus Groh <[email protected]>
+ * Copyright (c) 2021, Andreas Kling <[email protected]>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -24,6 +25,7 @@
 #include <LibGUI/ImageWidget.h>
 #include <LibGUI/Label.h>
 #include <LibGUI/LinkLabel.h>
+#include <LibGUI/Progressbar.h>
 #include <LibGUI/TabWidget.h>
 #include <LibGUI/TextEditor.h>
 #include <LibGUI/Widget.h>
@@ -36,10 +38,42 @@ struct TitleAndText {
  String text;
 };
 
+static NonnullRefPtr<GUI::Window> 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<GUI::Widget>();
+ main_widget.set_fill_with_background_color(true);
+ main_widget.set_layout<GUI::VerticalBoxLayout>();
+ auto& label = main_widget.add<GUI::Label>("Generating crash report...");
+ label.set_fixed_height(30);
+ auto& progressbar = main_widget.add<GUI::Progressbar>();
+ 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<GUI::Progressbar>("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;

Userland/Libraries/LibCoredump/Backtrace.cpp

@@ -42,29 +42,48 @@ 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<void(size_t, size_t)> 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.
  // However, because the first frame represents the faulting
  // 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())

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<void(size_t, size_t)> on_progress = {});
  ~Backtrace();
 
  ELF::Core::ThreadInfo const& thread_info() const { return m_thread_info; }