[PATCH 06/10] oslib: report each thread's program counter

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

A kernel stack describes a thread asleep in a syscall and says nothing
about one spinning in userspace, which is the other way a thread stops
making progress.

Ask each thread where it is by signalling it and reading the program
counter out of the signal frame. That is a register read and a store,
which a handler may do, unlike unwinding: backtrace() is not async
signal safe and takes the loader lock the first time it runs.

Ask all of them at once, once every passive read is taken. The signal
interrupts whatever a thread was blocked in, so probing earlier would
make the kernel stack and the wait channel describe the interruption
instead of the hang, and probing one at a time would spend the timeout
again for every thread that never answers. Insert the answers from the
back, so the offsets recorded during the reads still point where they
did.

A thread that never answers within the deadline can still run the
handler later, once whatever held it lets go, and by then the slots it
would write to are gone. Count handlers between entry and exit,
withdraw the slots when the deadline expires and wait for the last
handler out before returning to the caller that frees them. A handler
that arrives after the withdrawal finds nothing to write to.

Where the counter sits in the signal frame is a host property, so it is
answered from host/include like the other per-host questions, with a
generic fallback that reports no counter and leaves the host otherwise
untouched. The Linux hosts covered are the ones whose signal frames
linux-user already describes in host-signal.h.

Name the address through dladdr() where there is a symbol for it. There
often is not, because dladdr() sees exported symbols only and a thread
parked inside a library sits in its internals, so fall back to the
object and the offset within it. An offset resolves against the library
afterwards; the raw address does not survive the process.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Cc: Zhao Liu <zhao1.liu@intel.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 host/include/aarch64/host/signal-pc.h     |  20 +++
 host/include/generic/host/signal-pc.h     |  11 ++
 host/include/loongarch64/host/signal-pc.h |  20 +++
 host/include/ppc64/host/signal-pc.h       |  22 +++
 host/include/riscv64/host/signal-pc.h     |  20 +++
 host/include/s390x/host/signal-pc.h       |  20 +++
 host/include/x86_64/host/signal-pc.h      |  20 +++
 include/qemu/osdep.h                      |   7 +-
 meson.build                               |   2 +
 util/oslib-posix.c                        | 181 +++++++++++++++++++++-
 10 files changed, 318 insertions(+), 5 deletions(-)
 create mode 100644 host/include/aarch64/host/signal-pc.h
 create mode 100644 host/include/generic/host/signal-pc.h
 create mode 100644 host/include/loongarch64/host/signal-pc.h
 create mode 100644 host/include/ppc64/host/signal-pc.h
 create mode 100644 host/include/riscv64/host/signal-pc.h
 create mode 100644 host/include/s390x/host/signal-pc.h
 create mode 100644 host/include/x86_64/host/signal-pc.h

diff --git a/host/include/aarch64/host/signal-pc.h b/host/include/aarch64/host/signal-pc.h
new file mode 100644
index 0000000000..fe60ed1c96
--- /dev/null
+++ b/host/include/aarch64/host/signal-pc.h
@@ -0,0 +1,20 @@
+/*
+ * Program counter of an interrupted aarch64 thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef AARCH64_HOST_SIGNAL_PC_H
+#define AARCH64_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.pc;
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/host/include/generic/host/signal-pc.h b/host/include/generic/host/signal-pc.h
new file mode 100644
index 0000000000..8ce3822069
--- /dev/null
+++ b/host/include/generic/host/signal-pc.h
@@ -0,0 +1,11 @@
+/*
+ * No host specific way to read the program counter from a signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HOST_SIGNAL_PC_H
+#define HOST_SIGNAL_PC_H
+
+#define HAVE_HOST_SIGNAL_PC 0
+
+#endif
diff --git a/host/include/loongarch64/host/signal-pc.h b/host/include/loongarch64/host/signal-pc.h
new file mode 100644
index 0000000000..ccb2ac1c56
--- /dev/null
+++ b/host/include/loongarch64/host/signal-pc.h
@@ -0,0 +1,20 @@
+/*
+ * Program counter of an interrupted loongarch64 thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef LOONGARCH64_HOST_SIGNAL_PC_H
+#define LOONGARCH64_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.__pc;
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/host/include/ppc64/host/signal-pc.h b/host/include/ppc64/host/signal-pc.h
new file mode 100644
index 0000000000..1473570a39
--- /dev/null
+++ b/host/include/ppc64/host/signal-pc.h
@@ -0,0 +1,22 @@
+/*
+ * Program counter of an interrupted ppc64 thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef PPC64_HOST_SIGNAL_PC_H
+#define PPC64_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#include <asm/ptrace.h>
+
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.gp_regs[PT_NIP];
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/host/include/riscv64/host/signal-pc.h b/host/include/riscv64/host/signal-pc.h
new file mode 100644
index 0000000000..362b446040
--- /dev/null
+++ b/host/include/riscv64/host/signal-pc.h
@@ -0,0 +1,20 @@
+/*
+ * Program counter of an interrupted riscv64 thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef RISCV64_HOST_SIGNAL_PC_H
+#define RISCV64_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.__gregs[REG_PC];
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/host/include/s390x/host/signal-pc.h b/host/include/s390x/host/signal-pc.h
new file mode 100644
index 0000000000..6ad809c911
--- /dev/null
+++ b/host/include/s390x/host/signal-pc.h
@@ -0,0 +1,20 @@
+/*
+ * Program counter of an interrupted s390x thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef S390X_HOST_SIGNAL_PC_H
+#define S390X_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.psw.addr;
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/host/include/x86_64/host/signal-pc.h b/host/include/x86_64/host/signal-pc.h
new file mode 100644
index 0000000000..c94ad2e976
--- /dev/null
+++ b/host/include/x86_64/host/signal-pc.h
@@ -0,0 +1,20 @@
+/*
+ * Program counter of an interrupted x86_64 thread, from its signal frame.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef X86_64_HOST_SIGNAL_PC_H
+#define X86_64_HOST_SIGNAL_PC_H
+
+#ifdef CONFIG_LINUX
+#define HAVE_HOST_SIGNAL_PC 1
+
+static inline uintptr_t host_signal_pc(const ucontext_t *uc)
+{
+    return uc->uc_mcontext.gregs[REG_RIP];
+}
+#else
+#define HAVE_HOST_SIGNAL_PC 0
+#endif
+
+#endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0bd08e0f7f..00eeb5dcc9 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -680,9 +680,10 @@ bool qemu_debugger_attached(void);
  * qemu_thread_states:
  *
  * Returns a newly allocated, newline terminated description of every thread
- * in this process apart from the caller: its id, name, scheduler state and
- * kernel stack, or NULL on a host that cannot report any of that. Where the
- * kernel stack is privileged the description falls back to a wait channel.
+ * in this process apart from the caller: its id, name, scheduler state, the
+ * program counter it stopped at and its kernel stack, or NULL on a host that
+ * cannot report any of that. A privileged kernel stack falls back to a wait
+ * channel, and a host with no way to read a program counter omits it.
  */
 char *qemu_thread_states(void);
 
diff --git a/meson.build b/meson.build
index cfac634cf1..25e8842918 100644
--- a/meson.build
+++ b/meson.build
@@ -2646,6 +2646,8 @@ config_host_data.set('HAVE_GETIFADDRS', cc.has_function('getifaddrs'))
 config_host_data.set('HAVE_GLIB_WITH_SLICE_ALLOCATOR', glib_has_gslice)
 config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
 config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul', prefix: osdep_prefix))
+config_host_data.set('CONFIG_DLADDR', cc.has_function('dladdr',
+                     prefix: osdep_prefix + '#include <dlfcn.h>'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
 if rbd.found()
   config_host_data.set('HAVE_RBD_NAMESPACE_EXISTS',
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 6a683b849e..ddc2b48df2 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -58,8 +58,13 @@
 #include <lwp.h>
 #endif
 
+#ifdef CONFIG_DLADDR
+#include <dlfcn.h>
+#endif
+
 #include "qemu/memalign.h"
 #include "qemu/mmap-alloc.h"
+#include "host/signal-pc.h"
 
 #define MAX_MEM_PREALLOC_THREAD_COUNT 32
 
@@ -168,8 +173,164 @@ static bool thread_stacks_readable(void)
     return readable;
 }
 
-static void append_thread_state(GString *out, const char *tid)
+/* Where a thread was, and where its line belongs in the report */
+typedef struct {
+    int tid;
+    size_t at;
+    void *pc;
+    int done;
+} ThreadPc;
+
+#if HAVE_HOST_SIGNAL_PC
+
+/* How long to wait for the answers, however many threads were asked */
+#define THREAD_PC_WAIT_MS 100
+
+/*
+ * Published to the handler, which runs in whichever thread was interrupted
+ * and so may only touch this with atomics: every qemu synchronisation
+ * primitive is built on a mutex, and none of them may be taken here.
+ * pc_handlers counts handlers between entry and exit, so the slots can be
+ * withdrawn and the last handler waited out before the caller frees them.
+ */
+static ThreadPc *pc_slots;
+static int pc_nslots;
+static int pc_handlers;
+
+static void thread_pc_handler(int sig, siginfo_t *si, void *ctx)
+{
+    ThreadPc *slots;
+    int me, i, n;
+
+    qatomic_inc(&pc_handlers);
+    smp_mb__after_rmw();
+    slots = qatomic_load_acquire(&pc_slots);
+    if (slots) {
+        n = qatomic_read(&pc_nslots);
+        me = qemu_get_thread_id();
+        for (i = 0; i < n; i++) {
+            if (qatomic_read(&slots[i].tid) == me) {
+                slots[i].pc = (void *)host_signal_pc(ctx);
+                qatomic_store_release(&slots[i].done, 1);
+                break;
+            }
+        }
+    }
+    qatomic_dec(&pc_handlers);
+}
+
+static bool thread_pc_armed(void)
+{
+    static int armed = -1;
+    struct sigaction sa;
+
+    if (armed >= 0) {
+        return armed;
+    }
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_sigaction = thread_pc_handler;
+    sa.sa_flags = SA_SIGINFO | SA_RESTART;
+    sigemptyset(&sa.sa_mask);
+    armed = sigaction(QEMU_SIG_INTROSPECT, &sa, NULL) == 0;
+    return armed;
+}
+
+/*
+ * Ask every thread at once. Asking one at a time would spend the timeout
+ * again for each thread that never answers, and the threads that never
+ * answer are the ones a hung qemu has most of.
+ */
+static void thread_pc_collect(GArray *slots)
 {
+    int64_t deadline;
+    guint i;
+    int left;
+
+    if (!slots->len || !thread_pc_armed()) {
+        return;
+    }
+    qatomic_set(&pc_nslots, slots->len);
+    qatomic_store_release(&pc_slots, &g_array_index(slots, ThreadPc, 0));
+
+    for (i = 0; i < slots->len; i++) {
+        ThreadPc *slot = &g_array_index(slots, ThreadPc, i);
+
+        if (qemu_kill_thread(slot->tid, QEMU_SIG_INTROSPECT) < 0) {
+            qatomic_store_release(&slot->done, -1);
+        }
+    }
+
+    deadline = g_get_monotonic_time() + THREAD_PC_WAIT_MS * 1000;
+    do {
+        left = 0;
+        for (i = 0; i < slots->len; i++) {
+            if (!qatomic_load_acquire(&g_array_index(slots, ThreadPc,
+                                                     i).done)) {
+                left++;
+            }
+        }
+        if (!left) {
+            break;
+        }
+        g_usleep(1000);
+    } while (g_get_monotonic_time() < deadline);
+
+    qatomic_set(&pc_slots, NULL);
+    /* Pairs with the barrier in thread_pc_handler() */
+    smp_mb();
+    while (qatomic_read(&pc_handlers)) {
+        g_usleep(1000);
+    }
+}
+
+static void insert_thread_pc(GString *out, const ThreadPc *slot)
+{
+    g_autofree char *line = NULL;
+    void *pc = slot->pc;
+#ifdef CONFIG_DLADDR
+    Dl_info info = { };
+#endif
+
+    if (qatomic_read(&slot->done) != 1 || !pc) {
+        return;
+    }
+#ifdef CONFIG_DLADDR
+    if (dladdr(pc, &info) && info.dli_sname) {
+        line = g_strdup_printf("      pc %p %s+0x%tx\n", pc, info.dli_sname,
+                               (char *)pc - (char *)info.dli_saddr);
+    } else if (info.dli_fname && info.dli_fbase) {
+        /*
+         * dladdr() only knows exported symbols, so an address inside a
+         * library's own internals has none. The object and the offset
+         * within it still resolve later, the raw address does not.
+         */
+        g_autofree char *obj = g_path_get_basename(info.dli_fname);
+
+        line = g_strdup_printf("      pc %p %s+0x%tx\n", pc, obj,
+                               (char *)pc - (char *)info.dli_fbase);
+    }
+#endif
+    if (!line) {
+        line = g_strdup_printf("      pc %p\n", pc);
+    }
+    g_string_insert(out, slot->at, line);
+}
+
+#else
+
+static void thread_pc_collect(GArray *slots)
+{
+}
+
+static void insert_thread_pc(GString *out, const ThreadPc *slot)
+{
+}
+
+#endif /* HAVE_HOST_SIGNAL_PC */
+
+static void append_thread_state(GString *out, GArray *slots, const char *tid)
+{
+    ThreadPc slot = { };
     g_autofree char *comm = read_task_file(tid, "comm");
     g_autofree char *stat = NULL;
     g_autofree char *stack = NULL;
@@ -194,6 +355,10 @@ static void append_thread_state(GString *out, const char *tid)
     }
     g_string_append_printf(out, "  %-7s %-16s %c\n", tid, comm, state);
 
+    slot.tid = atoi(tid);
+    slot.at = out->len;
+    g_array_append_val(slots, slot);
+
     /* A running thread has no settled kernel stack to unwind */
     if (state != 'R' && thread_stacks_readable()) {
         stack = read_task_file(tid, "stack");
@@ -220,8 +385,10 @@ static void append_thread_state(GString *out, const char *tid)
 char *qemu_thread_states(void)
 {
     g_autoptr(GDir) dir = g_dir_open("/proc/self/task", 0, NULL);
+    g_autoptr(GArray) slots = g_array_new(FALSE, TRUE, sizeof(ThreadPc));
     const char *tid;
     GString *out;
+    guint i;
 
     if (!dir) {
         return NULL;
@@ -232,13 +399,23 @@ char *qemu_thread_states(void)
     out = g_string_new(NULL);
     while ((tid = g_dir_read_name(dir))) {
         if (atoi(tid) != qemu_get_thread_id()) {
-            append_thread_state(out, tid);
+            append_thread_state(out, slots, tid);
         }
     }
     if (!out->len) {
         g_string_free(out, TRUE);
         return NULL;
     }
+
+    /*
+     * Only now, with every passive read taken, disturb the threads. Insert
+     * from the back so the offsets recorded above stay where they were.
+     */
+    thread_pc_collect(slots);
+    for (i = slots->len; i > 0; i--) {
+        insert_thread_pc(out, &g_array_index(slots, ThreadPc, i - 1));
+    }
+
     return g_string_free(out, FALSE);
 }
 
-- 
2.53.0