On 6/19/24 6:46 PM, Lianbo Jiang wrote:
On 5/31/24 5:22 PM, devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx wrote:Date: Fri, 31 May 2024 17:19:26 +0800 From: Tao Liu<ltao@xxxxxxxxxx> Subject: [PATCH v4 03/16] Let crash change gdb context To:devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx Cc: Alexey Makhalov<alexey.makhalov@xxxxxxxxxxxx>, Mahesh J Salgaonkar<mahesh@xxxxxxxxxxxxx>, "Naveen N . Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx>, Lianbo Jiang<lijiang@xxxxxxxxxx> Message-ID:<20240531091939.97828-4-ltao@xxxxxxxxxx> Content-Type: text/plain; charset=UTF-8This patch is a preparation of gdb stack unwinding support. "set" commandis extended with gdb context change: crash> set <pid> or crash> set <task> Then the task context of crash and gdb will both be switched to pid/task, and the following command: "bt" "gdb bt" will output the same task context. Co-developed-by: Aditya Gupta<adityag@xxxxxxxxxxxxx> Co-developed-by: Alexey Makhalov<alexey.makhalov@xxxxxxxxxxxx> Co-developed-by: Tao Liu<ltao@xxxxxxxxxx> Cc: Sourabh Jain<sourabhjain@xxxxxxxxxxxxx> Cc: Hari Bathini<hbathini@xxxxxxxxxxxxx> Cc: Mahesh J Salgaonkar<mahesh@xxxxxxxxxxxxx> Cc: Naveen N. Rao<naveen.n.rao@xxxxxxxxxxxxxxxxxx> Cc: Lianbo Jiang<lijiang@xxxxxxxxxx> Cc: HAGIO KAZUHITO(萩尾 一仁)<k-hagio-ab@xxxxxxx> Cc: Tao Liu<ltao@xxxxxxxxxx> Cc: Alexey Makhalov<alexey.makhalov@xxxxxxxxxxxx> Signed-off-by: Tao Liu<ltao@xxxxxxxxxx> --- crash_target.c | 20 +++++++++++++++++--- defs.h | 5 ++++- kernel.c | 11 ++++++++--- task.c | 21 +++++++++++++-------- tools.c | 8 ++++---- 5 files changed, 46 insertions(+), 19 deletions(-) diff --git a/crash_target.c b/crash_target.c index 1f62bf6..03718b5 100644 --- a/crash_target.c +++ b/crash_target.c @@ -28,7 +28,7 @@ void crash_target_init (void); extern "C" int gdb_readmem_callback(unsigned long, void *, int, int);extern "C" int crash_get_cpu_reg (int cpu, int regno, const char *regname,int regsize, void *val); - +extern "C" int gdb_change_thread_context (); /* The crash target. */ @@ -63,16 +63,22 @@ public: }; -/* We just get all the registers, so we don't use regno. */ void crash_target::fetch_registers (struct regcache *regcache, int regno) { + int r; gdb_byte regval[16]; int cpu = inferior_ptid.tid(); struct gdbarch *arch = regcache->arch (); - for (int r = 0; r < gdbarch_num_regs (arch); r++) + if (regno >= 0) { + r = regno; + goto onetime; + } + + for (r = 0; regno == -1 && r < gdbarch_num_regs (arch); r++) { +onetime: const char *regname = gdbarch_register_name(arch, r); int regsize = register_size (arch, r); if (regsize > sizeof (regval)) @@ -129,3 +135,11 @@ crash_target_init (void) /* Now, set up the frame cache. */ reinit_frame_cache (); }This function looks weird to me. Can you try to refactor this one? For example: +static void supply_registers(struct regcache *regcache, int regno) +{ + gdb_byte regval[16]; + struct gdbarch *arch = regcache->arch (); + const char *regname = gdbarch_register_name(arch, regno); + int regsize = register_size(arch, regno); + + if (regsize > sizeof (regval))+ error (_("fatal error: buffer size is not enough to fit register value"));++ if (crash_get_current_task_reg (regno, regname, regsize, (void *)®val))+ regcache->raw_supply (regno, regval); + else + regcache->raw_supply (regno, NULL); +} void crash_target::fetch_registers (struct regcache *regcache, int regno) { int r; if (regno >= 0) { supply_registers(regcache, regno); return; }for (r = 0; regno == -1 && r < gdbarch_num_regs (regcache->arch ()); r++)supply_registers(regcache, r); } That can avoid jumping into a for-loop code block with goto. And It looks more friendly and readable. Just an idea, What do you think? Thanks Lianbo+ +extern "C" int +gdb_change_thread_context () +{ + target_fetch_registers(get_current_regcache(), -1); + reinit_frame_cache(); + return TRUE; +}
Also, the above function is missing a declaration and I see the following warning:
cc -c -g -DX86_64 -DLZO -DGDB_10_2 main.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
In file included from main.c:18:defs.h:8058:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
8058 | extern int gdb_change_thread_context (); | ^~~~~~cc -c -g -DX86_64 -DLZO -DGDB_10_2 tools.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
In file included from tools.c:18:defs.h:8058:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
8058 | extern int gdb_change_thread_context (); | ^~~~~~ Thanks Lianbo
diff --git a/defs.h b/defs.h index 3cb8e63..0d872b2 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); @@ -8051,4 +8051,7 @@ enum x86_64_regnum { LAST_REGNUM }; +/* crash_target.c */ +extern int gdb_change_thread_context (); + #endif /* !GDB_COMMON */ diff --git a/kernel.c b/kernel.c index 1728b70..78b7b1e 100644 --- a/kernel.c +++ b/kernel.c @@ -6494,15 +6494,20 @@ set_cpu(int cpu) if (hide_offline_cpu(cpu)) error(FATAL, "invalid cpu number: cpu %d is OFFLINE\n", cpu); - if ((task = get_active_task(cpu))) - set_context(task, NO_PID); + task = get_active_task(cpu); + + /* Check if context is already set to given cpu */ + if (task == CURRENT_TASK()) + return; + + if (task) + set_context(task, NO_PID, TRUE); elseerror(FATAL, "cannot determine active task on cpu %ld\n", cpu);show_context(CURRENT_CONTEXT()); } - /* * Collect the irq_desc[] entry along with its associated handler and * action structures. diff --git a/task.c b/task.c index ebdb5be..d47d268 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(); } @@ -2985,9 +2985,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 +3053,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, TRUE); sort_context_by_task(); } @@ -3100,7 +3100,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, TRUE); sort_context_by_task(); } @@ -5281,7 +5281,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; @@ -5301,7 +5301,12 @@ set_context(ulong task, ulong pid) if (found) { CURRENT_CONTEXT() = tc; - return TRUE; ++ /* change the selected thread in gdb, according to current context */+ if (update_gdb_thread) + return gdb_change_thread_context(); + 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 0f2db10..80d4244 100644 --- a/tools.c +++ b/tools.c @@ -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; -- 2.40.1
-- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
