LibCoreDump+Crash{Daemon,Reporter}: Make backtraces thread-specific

We want to show an individual backtrace for each thread, not one
containing backtrace entries from all threads.

Fixes #4778.

6 files changed, 86 insertions, 46 deletions

diff --git a/Userland/Applications/CrashReporter/main.cpp b/Userland/Applications/CrashReporter/main.cpp
index 1676e1df56..80b8ea162a 100644
--- a/Userland/Applications/CrashReporter/main.cpp
+++ b/Userland/Applications/CrashReporter/main.cpp
@@ -36,40 +36,56 @@
 #include <LibDesktop/Launcher.h>
 #include <LibELF/CoreDump.h>
 #include <LibGUI/Application.h>
+#include <LibGUI/BoxLayout.h>
 #include <LibGUI/Button.h>
 #include <LibGUI/FileIconProvider.h>
 #include <LibGUI/Icon.h>
 #include <LibGUI/ImageWidget.h>
 #include <LibGUI/Label.h>
-#include <LibGUI/Layout.h>
 #include <LibGUI/LinkLabel.h>
 #include <LibGUI/TabWidget.h>
 #include <LibGUI/TextEditor.h>
+#include <LibGUI/Widget.h>
 #include <LibGUI/Window.h>
 #include <string.h>
 
-static String build_backtrace(const CoreDump::Reader& coredump)
+struct TitleAndText {
+    String title;
+    String text;
+};
+
+static TitleAndText build_backtrace(const CoreDump::Reader& coredump, const ELF::Core::ThreadInfo& thread_info, size_t thread_index)
 {
+    CoreDump::Backtrace backtrace(coredump, thread_info);
+
     StringBuilder builder;
 
-    auto assertion = coredump.metadata().get("assertion");
-    if (assertion.has_value() && !assertion.value().is_empty()) {
+    auto prepend_assertion = [&] {
+        auto assertion = coredump.metadata().get("assertion");
+        if (!assertion.has_value() || assertion.value().is_empty())
+            return;
         builder.append("ASSERTION FAILED: ");
         builder.append(assertion.value().characters());
         builder.append('\n');
         builder.append('\n');
-    }
+    };
 
-    auto first = true;
-    for (auto& entry : coredump.backtrace().entries()) {
-        if (first)
-            first = false;
-        else
+    auto first_entry = true;
+    for (auto& entry : backtrace.entries()) {
+        if (first_entry) {
+            if (entry.function_name.starts_with("__assertion_failed"))
+                prepend_assertion();
+            first_entry = false;
+        } else {
             builder.append('\n');
+        }
         builder.append(entry.to_string());
     }
 
-    return builder.build();
+    return {
+        String::formatted("Thread #{} (TID {})", thread_index, thread_info.tid),
+        builder.build()
+    };
 }
 
 int main(int argc, char** argv)
@@ -86,7 +102,8 @@ int main(int argc, char** argv)
     args_parser.add_positional_argument(coredump_path, "Coredump path", "coredump-path");
     args_parser.parse(argc, argv);
 
-    String backtrace;
+    Vector<TitleAndText> thread_backtraces;
+
     String executable_path;
     int pid { 0 };
     u8 termination_signal { 0 };
@@ -97,7 +114,14 @@ int main(int argc, char** argv)
             warnln("Could not open coredump '{}'", coredump_path);
             return 1;
         }
-        backtrace = build_backtrace(*coredump);
+
+        size_t thread_index = 0;
+        coredump->for_each_thread_info([&](auto& thread_info) {
+            thread_backtraces.append(build_backtrace(*coredump, thread_info, thread_index));
+            ++thread_index;
+            return IterationDecision::Continue;
+        });
+
         executable_path = coredump->process_executable_path();
         pid = coredump->process_pid();
         termination_signal = coredump->process_termination_signal();
@@ -166,10 +190,24 @@ int main(int argc, char** argv)
 
     auto& tab_widget = *widget.find_descendant_of_type_named<GUI::TabWidget>("tab_widget");
 
-    auto& backtrace_text_editor = tab_widget.add_tab<GUI::TextEditor>("Backtrace");
-    backtrace_text_editor.set_mode(GUI::TextEditor::Mode::ReadOnly);
-    backtrace_text_editor.set_text(backtrace);
-    backtrace_text_editor.set_should_hide_unnecessary_scrollbars(true);
+    auto& backtrace_tab = tab_widget.add_tab<GUI::Widget>("Backtrace");
+    backtrace_tab.set_layout<GUI::VerticalBoxLayout>();
+    backtrace_tab.layout()->set_margins({ 4, 4, 4, 4 });
+
+    auto& backtrace_label = backtrace_tab.add<GUI::Label>("A backtrace for each thread alive during the crash is listed below:");
+    backtrace_label.set_text_alignment(Gfx::TextAlignment::CenterLeft);
+    backtrace_label.set_fixed_height(16);
+
+    auto& backtrace_tab_widget = backtrace_tab.add<GUI::TabWidget>();
+    backtrace_tab_widget.set_tab_position(GUI::TabWidget::TabPosition::Bottom);
+    for (auto& backtrace : thread_backtraces) {
+        auto& backtrace_text_editor = backtrace_tab_widget.add_tab<GUI::TextEditor>(backtrace.title);
+        backtrace_text_editor.set_layout<GUI::VerticalBoxLayout>();
+        backtrace_text_editor.layout()->set_margins({ 4, 4, 4, 4 });
+        backtrace_text_editor.set_text(backtrace.text);
+        backtrace_text_editor.set_mode(GUI::TextEditor::Mode::ReadOnly);
+        backtrace_text_editor.set_should_hide_unnecessary_scrollbars(true);
+    }
 
     auto& close_button = *widget.find_descendant_of_type_named<GUI::Button>("close_button");
     close_button.on_click = [&](auto) {
diff --git a/Userland/Libraries/LibCoreDump/Backtrace.cpp b/Userland/Libraries/LibCoreDump/Backtrace.cpp
index afa6220447..1352fa9e72 100644
--- a/Userland/Libraries/LibCoreDump/Backtrace.cpp
+++ b/Userland/Libraries/LibCoreDump/Backtrace.cpp
@@ -68,29 +68,27 @@ static const ELFObjectInfo* object_info_for_region(const ELF::Core::MemoryRegion
     return info_ptr;
 }
 
-Backtrace::Backtrace(const Reader& coredump)
+Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread_info)
+    : m_thread_info(move(thread_info))
 {
-    coredump.for_each_thread_info([this, &coredump](const ELF::Core::ThreadInfo& thread_info) {
-        uint32_t* ebp = (uint32_t*)thread_info.regs.ebp;
-        uint32_t* eip = (uint32_t*)thread_info.regs.eip;
-        while (ebp && eip) {
-            add_backtrace_entry(coredump, (FlatPtr)eip);
-            auto next_eip = coredump.peek_memory((FlatPtr)(ebp + 1));
-            auto next_ebp = coredump.peek_memory((FlatPtr)(ebp));
-            if (!next_eip.has_value() || !next_ebp.has_value())
-                break;
-            eip = (uint32_t*)next_eip.value();
-            ebp = (uint32_t*)next_ebp.value();
-        }
-        return IterationDecision::Continue;
-    });
+    uint32_t* ebp = (uint32_t*)m_thread_info.regs.ebp;
+    uint32_t* eip = (uint32_t*)m_thread_info.regs.eip;
+    while (ebp && eip) {
+        add_entry(coredump, (FlatPtr)eip);
+        auto next_eip = coredump.peek_memory((FlatPtr)(ebp + 1));
+        auto next_ebp = coredump.peek_memory((FlatPtr)(ebp));
+        if (!next_eip.has_value() || !next_ebp.has_value())
+            break;
+        eip = (uint32_t*)next_eip.value();
+        ebp = (uint32_t*)next_ebp.value();
+    }
 }
 
 Backtrace::~Backtrace()
 {
 }
 
-void Backtrace::add_backtrace_entry(const Reader& coredump, FlatPtr eip)
+void Backtrace::add_entry(const Reader& coredump, FlatPtr eip)
 {
     auto* region = coredump.region_containing((FlatPtr)eip);
     if (!region) {
diff --git a/Userland/Libraries/LibCoreDump/Backtrace.h b/Userland/Libraries/LibCoreDump/Backtrace.h
index 6915b170c2..a0ca69d480 100644
--- a/Userland/Libraries/LibCoreDump/Backtrace.h
+++ b/Userland/Libraries/LibCoreDump/Backtrace.h
@@ -29,6 +29,7 @@
 #include <AK/Types.h>
 #include <LibCoreDump/Reader.h>
 #include <LibDebug/DebugInfo.h>
+#include <LibELF/CoreDump.h>
 
 namespace CoreDump {
 
@@ -54,14 +55,16 @@ public:
         String to_string(bool color = false) const;
     };
 
-    Backtrace(const Reader&);
+    Backtrace(const Reader&, const ELF::Core::ThreadInfo&);
     ~Backtrace();
 
+    const ELF::Core::ThreadInfo thread_info() const { return m_thread_info; }
     const Vector<Entry> entries() const { return m_entries; }
 
 private:
-    void add_backtrace_entry(const Reader&, FlatPtr eip);
+    void add_entry(const Reader&, FlatPtr eip);
 
+    ELF::Core::ThreadInfo m_thread_info;
     Vector<Entry> m_entries;
 };
 
diff --git a/Userland/Libraries/LibCoreDump/Reader.cpp b/Userland/Libraries/LibCoreDump/Reader.cpp
index d489520af5..843513e04c 100644
--- a/Userland/Libraries/LibCoreDump/Reader.cpp
+++ b/Userland/Libraries/LibCoreDump/Reader.cpp
@@ -26,7 +26,6 @@
 
 #include <AK/JsonObject.h>
 #include <AK/JsonValue.h>
-#include <LibCoreDump/Backtrace.h>
 #include <LibCoreDump/Reader.h>
 #include <signal_numbers.h>
 #include <string.h>
@@ -161,11 +160,6 @@ const ELF::Core::MemoryRegionInfo* Reader::region_containing(FlatPtr address) co
     return ret;
 }
 
-const Backtrace Reader::backtrace() const
-{
-    return Backtrace(*this);
-}
-
 int Reader::process_pid() const
 {
     auto process_info = this->process_info();
diff --git a/Userland/Libraries/LibCoreDump/Reader.h b/Userland/Libraries/LibCoreDump/Reader.h
index dbba53fa4d..15a7f38198 100644
--- a/Userland/Libraries/LibCoreDump/Reader.h
+++ b/Userland/Libraries/LibCoreDump/Reader.h
@@ -30,7 +30,6 @@
 #include <AK/MappedFile.h>
 #include <AK/Noncopyable.h>
 #include <AK/OwnPtr.h>
-#include <LibCoreDump/Forward.h>
 #include <LibELF/CoreDump.h>
 #include <LibELF/Image.h>
 
@@ -63,8 +62,6 @@ public:
     };
     const LibraryData* library_containing(FlatPtr address) const;
 
-    const Backtrace backtrace() const;
-
     int process_pid() const;
     u8 process_termination_signal() const;
     String process_executable_path() const;
diff --git a/Userland/Services/CrashDaemon/main.cpp b/Userland/Services/CrashDaemon/main.cpp
index 2453f73699..8dcd318566 100644
--- a/Userland/Services/CrashDaemon/main.cpp
+++ b/Userland/Services/CrashDaemon/main.cpp
@@ -56,8 +56,18 @@ static void print_backtrace(const String& coredump_path)
         dbgln("Could not open coredump '{}'", coredump_path);
         return;
     }
-    for (auto& entry : coredump->backtrace().entries())
-        dbgln("{}", entry.to_string(true));
+
+    size_t thread_index = 0;
+    coredump->for_each_thread_info([&](auto& thread_info) {
+        CoreDump::Backtrace backtrace(*coredump, thread_info);
+        if (thread_index > 0)
+            dbgln();
+        dbgln("--- Backtrace for thread #{} (TID {}) ---", thread_index, thread_info.tid);
+        for (auto& entry : backtrace.entries())
+            dbgln("{}", entry.to_string(true));
+        ++thread_index;
+        return IterationDecision::Continue;
+    });
 }
 
 static void launch_crash_reporter(const String& coredump_path)