From bb8ea65a808a9cb19f15ead79a85d03ce387329f Mon Sep 17 00:00:00 2001 From: Aditya Gupta Date: Mon, 4 Dec 2023 16:16:19 +0530 Subject: [PATCH] fix 'info threads' command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the output is like this ppc64le: crash> info threads Id Target Id Frame 1 CPU 0 in ?? () 2 CPU 1 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 3 CPU 2 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 4 CPU 3 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 * 5 CPU 4 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 6 CPU 5 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 7 CPU 6 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 8 CPU 7 0xc000000000025d00 in ppc_panic_event (this=, event=, ptr=0x0) at arch/powerpc/kernel/setup-common.c:712 There are two issues here: 1. CPU 0 always shows '' frame: This is due to 'thread 0' being initialised in 'crash_target_init', but it's very early in crash init, so it could not get any registers from the vmcore. And, since GDB caches registers (whether it is there or now), it keeps thinking that registers for this thread are unavailable, so all future usage of thread 0 also shows unavailable registers & frame Fix this by refreshing the register cache of all threads in 'gdb_refresh_regcache', at task_init 2. All other CPUs show the same frame: For each thread, GDB depends on crash to give it registers, ie. crash_target::fetch_registers, but this in turn depends on crash's current context GDB internally switches it's thread context internally to each thread one by one, while printing the thread's frame, similarly switch crash's context when the thread context is changed, so that it matches the thread context in gdb (such as, CPU 3 in crash for thread 2(CPU 3) in gdb). Also, since we are switching context multiple times, add an argument to 'set_cpu' to not print the context everytime With these fixed, it will show all threads, as well as correct registers and frame. It might still show similar frame for many threads, due to all of them being on same NMI exception crash path, but if checked with 'info registers', they will have different registers. Cc: Sourabh Jain Cc: Hari Bathini Cc: Mahesh J Salgaonkar Cc: Naveen N. Rao Cc: Lianbo Jiang Cc: HAGIO KAZUHITO(萩尾 一仁) Cc: Tao Liu Signed-off-by: Aditya Gupta --- crash_target.c | 37 ++++++++++++++++++++++++++++++++-- defs.h | 5 +++-- gdb-10.2.patch | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ kernel.c | 8 +++++--- task.c | 30 ++++++++++++++++++++-------- tools.c | 10 +++++----- 6 files changed, 124 insertions(+), 20 deletions(-) diff --git a/crash_target.c b/crash_target.c index a9d7eea8..d06383f5 100644 --- a/crash_target.c +++ b/crash_target.c @@ -30,7 +30,8 @@ extern "C" int crash_get_nr_cpus(void); extern "C" int crash_get_cpu_reg (int cpu, int regno, const char *regname, int regsize, void *val); extern "C" int gdb_change_cpu_context (unsigned int cpu); -extern "C" int set_cpu (int cpu); +extern "C" void gdb_refresh_regcache(unsigned int cpu); +extern "C" int set_cpu(int cpu, int print_context); /* The crash target. */ @@ -150,10 +151,42 @@ gdb_change_cpu_context(unsigned int cpu) return FALSE; /* Making sure that crash's context is same */ - set_cpu(cpu); + set_cpu(cpu, FALSE); /* Switch to the thread */ switch_to_thread(tp); + + /* Fetch/Refresh thread's registers */ + gdb_refresh_regcache(cpu); + return TRUE; } +/* Refresh regcache of gdb thread on given CPU + * + * When gdb threads were initially added by 'crash_target_init', crash was not + * yet initialised, and hence crash_target::fetch_registers didn't give any + * registers to gdb. + * + * This is meant to be called after tasks in crash have been initialised, and + * possible machdep->get_cpu_reg is also set so architecture can give registers + */ +extern "C" void +gdb_refresh_regcache(unsigned int cpu) +{ + int saved_cpu = inferior_thread()->ptid.tid(); + ptid_t ptid = ptid_t(CRASH_INFERIOR_PID, 0, cpu); + inferior *inf = current_inferior(); + thread_info *tp = find_thread_ptid (inf, ptid); + + if (tp == NULL) { + warning("gdb thread for cpu %d not found\n", cpu); + return; + } + + /* temporarily switch to the cpu so we get correct registers */ + set_cpu(cpu, FALSE); + target_fetch_registers(get_thread_regcache(tp), -1); + + set_cpu(saved_cpu, FALSE); +} diff --git a/defs.h b/defs.h index 50fe25c0..6b980468 100644 --- a/defs.h +++ b/defs.h @@ -6013,7 +6013,7 @@ extern char *help_map[]; * task.c */ void task_init(void); -int set_context(ulong, ulong); +int set_context(ulong, ulong, uint); void show_context(struct task_context *); ulong pid_to_task(ulong); ulong task_to_pid(ulong); @@ -6118,7 +6118,7 @@ void parse_kernel_version(char *); #define SHOW_LOG_CTIME (0x10) #define SHOW_LOG_SAFE (0x20) #define SHOW_LOG_CALLER (0x40) -void set_cpu(int); +void set_cpu(int cpu, int print_context); void clear_machdep_cache(void); struct stack_hook *gather_text_list(struct bt_info *); int get_cpus_online(void); @@ -8175,5 +8175,6 @@ enum ppc64_regnum { /* crash_target.c */ extern int gdb_change_cpu_context (unsigned int cpu); +extern void gdb_refresh_regcache (unsigned int cpu); #endif /* !GDB_COMMON */ diff --git a/gdb-10.2.patch b/gdb-10.2.patch index 1fb42128..138413fa 100644 --- a/gdb-10.2.patch +++ b/gdb-10.2.patch @@ -16140,3 +16140,57 @@ exit 0 } /* +--- gdb-10.2/gdb/thread.c.orig ++++ gdb-10.2/gdb/thread.c +@@ -60,7 +60,7 @@ static thread_info *current_thread_; + + #ifdef CRASH_MERGE + /* Function to set cpu, defined by crash-utility */ +-extern "C" void set_cpu (int); ++extern "C" void set_cpu(int cpu, int print_context); + #endif + + /* RAII type used to increase / decrease the refcount of each thread +@@ -1098,6 +1098,9 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, + uiout->table_body (); + } + ++#ifdef CRASH_MERGE ++ int current_cpu = current_thread ? current_thread->ptid.tid() : 0; ++#endif + for (inferior *inf : all_inferiors ()) + for (thread_info *tp : inf->threads ()) + { +@@ -1113,6 +1116,11 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, + + ui_out_emit_tuple tuple_emitter (uiout, NULL); + ++ /* Switch to the thread's CPU in crash */ ++#ifdef CRASH_MERGE ++ set_cpu(tp->ptid.tid(), FALSE); ++#endif ++ + if (!uiout->is_mi_like_p ()) + { + if (tp == current_thread) +@@ -1185,6 +1193,11 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, + /* This end scope restores the current thread and the frame + selected before the "info threads" command, and it finishes the + ui-out list or table. */ ++ ++#ifdef CRASH_MERGE ++ /* Restore crash's context */ ++ set_cpu(current_cpu, FALSE); ++#endif + } + + if (pid == -1 && requested_threads == NULL) +@@ -1904,7 +1917,7 @@ thread_command (const char *tidstr, int from_tty) + struct thread_info* thread_id = parse_thread_id (tidstr, NULL); + + #ifdef CRASH_MERGE +- set_cpu(thread_id->ptid.tid()); ++ set_cpu(thread_id->ptid.tid(), FALSE); + #endif + + thread_select (tidstr, thread_id); diff --git a/kernel.c b/kernel.c index 93af36fc..c7de73a3 100644 --- a/kernel.c +++ b/kernel.c @@ -6514,9 +6514,10 @@ dump_kernel_table(int verbose) /* * Set the context to the active task on a given cpu -- dumpfiles only. + * Optionally show the context if print_context is TRUE */ void -set_cpu(int cpu) +set_cpu(int cpu, int print_context) { ulong task; @@ -6534,11 +6535,12 @@ set_cpu(int cpu) return; if (task) - set_context(task, NO_PID); + set_context(task, NO_PID, TRUE); else error(FATAL, "cannot determine active task on cpu %ld\n", cpu); - show_context(CURRENT_CONTEXT()); + if (print_context) + show_context(CURRENT_CONTEXT()); } diff --git a/task.c b/task.c index 3a190caf..a405b05a 100644 --- a/task.c +++ b/task.c @@ -672,7 +672,7 @@ task_init(void) if (ACTIVE()) { active_pid = REMOTE() ? pc->server_pid : LOCAL_ACTIVE() ? pc->program_pid : 1; - set_context(NO_TASK, active_pid); + set_context(NO_TASK, active_pid, FALSE); tt->this_task = pid_to_task(active_pid); } else { @@ -684,7 +684,7 @@ task_init(void) else if (ELF_NOTES_VALID() && DISKDUMP_DUMPFILE()) map_cpus_to_prstatus_kdump_cmprs(); please_wait("determining panic task"); - set_context(get_panic_context(), NO_PID); + set_context(get_panic_context(), NO_PID, TRUE); please_wait_done(); } @@ -706,6 +706,17 @@ task_init(void) kt->boot_date.tv_nsec = 0; } + /* + * Refresh CPU 0's thread's regcache + * + * This is required since, it's registers were initialised in + * crash_target_init when crash was not initialised yet and hence could + * not pass registers to gdb when gdb requests via + * crash_target::fetch_registers, so CPU 0's registers are shown as + * in gdb mode + * */ + gdb_refresh_regcache(0); + tt->flags |= TASK_INIT_DONE; } @@ -2985,9 +2996,9 @@ refresh_context(ulong curtask, ulong curpid) struct task_context *tc; if (task_exists(curtask) && pid_exists(curpid)) { - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, FALSE); } else { - set_context(tt->this_task, NO_PID); + set_context(tt->this_task, NO_PID, FALSE); complain = TRUE; if (STREQ(args[0], "set") && (argcnt == 2) && @@ -3053,7 +3064,7 @@ sort_context_array(void) curtask = CURRENT_TASK(); qsort((void *)tt->context_array, (size_t)tt->running_tasks, sizeof(struct task_context), sort_by_pid); - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, FALSE); sort_context_by_task(); } @@ -3100,7 +3111,7 @@ sort_context_array_by_last_run(void) curtask = CURRENT_TASK(); qsort((void *)tt->context_array, (size_t)tt->running_tasks, sizeof(struct task_context), sort_by_last_run); - set_context(curtask, NO_PID); + set_context(curtask, NO_PID, FALSE); sort_context_by_task(); } @@ -5281,7 +5292,7 @@ comm_exists(char *s) * that pid is selected. */ int -set_context(ulong task, ulong pid) +set_context(ulong task, ulong pid, uint update_gdb_thread) { int i; struct task_context *tc; @@ -5303,7 +5314,10 @@ set_context(ulong task, ulong pid) CURRENT_CONTEXT() = tc; /* change the selected thread in gdb, according to current context */ - return gdb_change_cpu_context(tc->processor); + if (update_gdb_thread) + return gdb_change_cpu_context(tc->processor); + else + return TRUE; } else { if (task) error(INFO, "cannot set context for task: %lx\n", task); diff --git a/tools.c b/tools.c index 0f2db108..18d088dd 100644 --- a/tools.c +++ b/tools.c @@ -1860,7 +1860,7 @@ cmd_set(void) break; } cpu = dtoi(optarg, FAULT_ON_ERROR, NULL); - set_cpu(cpu); + set_cpu(cpu, TRUE); return; case 'p': @@ -1871,7 +1871,7 @@ cmd_set(void) return; if (ACTIVE()) { - set_context(tt->this_task, NO_PID); + set_context(tt->this_task, NO_PID, TRUE); show_context(CURRENT_CONTEXT()); return; } @@ -1880,7 +1880,7 @@ cmd_set(void) error(INFO, "no panic task found!\n"); return; } - set_context(tt->panic_task, NO_PID); + set_context(tt->panic_task, NO_PID, TRUE); show_context(CURRENT_CONTEXT()); return; @@ -2559,14 +2559,14 @@ cmd_set(void) case STR_PID: pid = value; task = NO_TASK; - if (set_context(task, pid)) + if (set_context(task, pid, TRUE)) show_context(CURRENT_CONTEXT()); break; case STR_TASK: task = value; pid = NO_PID; - if (set_context(task, pid)) + if (set_context(task, pid, TRUE)) show_context(CURRENT_CONTEXT()); break;