5:46 a.m.
On 5/31/24 5:22 PM, devel-request(a)lists.crash-utility.osci.io wrote:
Date: Fri, 31 May 2024 17:19:26 +0800
From: Tao Liu<ltao(a)redhat.com>
Subject: [Crash-utility] [PATCH v4 03/16] Let crash change gdb context
To:devel@lists.crash-utility.osci.io
Cc: Alexey Makhalov<alexey.makhalov(a)broadcom.com>, Mahesh J
Salgaonkar<mahesh(a)linux.ibm.com>, "Naveen N . Rao"
<naveen.n.rao(a)linux.vnet.ibm.com>, Lianbo Jiang<lijiang(a)redhat.com>
Message-ID:<20240531091939.97828-4-ltao@redhat.com>
Content-Type: text/plain; charset=UTF-8
This patch is a preparation of gdb stack unwinding support. "set" command
is 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(a)linux.ibm.com>
Co-developed-by: Alexey Makhalov<alexey.makhalov(a)broadcom.com>
Co-developed-by: Tao Liu<ltao(a)redhat.com>
Cc: Sourabh Jain<sourabhjain(a)linux.ibm.com>
Cc: Hari Bathini<hbathini(a)linux.ibm.com>
Cc: Mahesh J Salgaonkar<mahesh(a)linux.ibm.com>
Cc: Naveen N. Rao<naveen.n.rao(a)linux.vnet.ibm.com>
Cc: Lianbo Jiang<lijiang(a)redhat.com>
Cc: HAGIO KAZUHITO(萩尾 一仁)<k-hagio-ab(a)nec.com>
Cc: Tao Liu<ltao(a)redhat.com>
Cc: Alexey Makhalov<alexey.makhalov(a)broadcom.com>
Signed-off-by: Tao Liu<ltao(a)redhat.com>
---
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;
+}
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);
else
error(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