[Crash-utility] [PATCH] Clean up on top of one-thread-v2 (unfinished)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi folks, please take a look on this patch.
It is a clean up on top of one-thread-v2. And it is still in progress.

Things done:
1. Rename machdep->get_cpu_reg to machdep->get_current_task_reg and rename
   x86_64_get_cpu_reg to x86_64_get_current_task_reg.
2. Untied gdb's TPID from any tasks or CPUs ID. Always use CURRENT_CONTEXT
   for regs and properties fetching.
3. When CURRENT switched, gdb_change_thread_context() must be called to
   invalidate caches.
4. Update x86_64_get_current_task_reg to support inactive tasks for offline
   debugging.

TODO:
1. Arch specific (Arm and PPC) changes for (1) above are required.
2. x86_64_get_current_task_reg is questionable about fetching pt_regs for
   inactive tasks. I added my approach. More testing required.

Known issues
1. After switching from all tasks to one thread:
   a. gdb frame selection stopped working.
   b. unwinding started to work differently around top (garbaged) frames

Signed-off-by: Alexey Makhalov <alexey.makhalov@xxxxxxxxxxxx>
---
 crash_target.c  | 38 +++++++++++------------------
 defs.h          | 10 +++-----
 gdb_interface.c |  8 +++----
 kernel.c        | 23 ------------------
 task.c          | 16 ++++---------
 x86_64.c        | 64 ++++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 83 insertions(+), 76 deletions(-)

diff --git a/crash_target.c b/crash_target.c
index fb3b634..58f74b4 100644
--- a/crash_target.c
+++ b/crash_target.c
@@ -26,11 +26,8 @@
 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,
+extern "C" int crash_get_current_task_reg (int regno, const char *regname,
                                   int regsize, void *val);
-extern "C" void gdb_refresh_regcache(unsigned int cpu);
-extern "C" int set_cpu(int cpu, int print_context);
-extern "C" int crash_set_thread(ulong);
 extern "C" int gdb_change_thread_context (ulong);
 extern "C" void crash_get_current_task_info(unsigned long *pid, char **comm);
 
@@ -72,22 +69,27 @@ 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))
         error (_("fatal error: buffer size is not enough to fit register value"));
 
-      if (crash_get_cpu_reg (cpu, r, regname, regsize, (void *)&regval))
+      if (crash_get_current_task_reg (r, regname, regsize, (void *)&regval))
         regcache->raw_supply (r, regval);
       else
         regcache->raw_supply (r, NULL);
@@ -142,19 +144,7 @@ crash_target_init (void)
 extern "C" int
 gdb_change_thread_context (ulong task)
 {
-  inferior* inf = current_inferior();
-  int cpu = crash_set_thread(task);
-  if (cpu < 0)
-    return FALSE;
-
-  ptid_t ptid = ptid_t(CRASH_INFERIOR_PID, 0, cpu);
-   thread_info *tp = find_thread_ptid (inf, ptid);
-
-   if (tp == nullptr)
-     return FALSE;
-
-   target_fetch_registers(get_thread_regcache(tp), -1);
-   switch_to_thread(tp);
-   reinit_frame_cache ();
-   return TRUE;
-}
\ No newline at end of file
+  registers_changed();
+  reinit_frame_cache();
+  return TRUE;
+}
diff --git a/defs.h b/defs.h
index d7e3b18..7596179 100644
--- a/defs.h
+++ b/defs.h
@@ -1081,7 +1081,7 @@ struct machdep_table {
         void (*get_irq_affinity)(int);
         void (*show_interrupts)(int, ulong *);
 	int (*is_page_ptr)(ulong, physaddr_t *);
-	int (*get_cpu_reg)(int, int, const char *, int, void *);
+	int (*get_current_task_reg)(int, const char *, int, void *);
 	int (*is_cpu_prstatus_valid)(int cpu);
 };
 
@@ -6124,7 +6124,6 @@ void dump_kernel_table(int);
 void dump_bt_info(struct bt_info *, char *where);
 void dump_log(int);
 void parse_kernel_version(char *);
-int crash_set_thread(ulong);
 
 #define LOG_LEVEL(v) ((v) & 0x07)
 #define SHOW_LOG_LEVEL    (0x1)
@@ -8023,7 +8022,7 @@ extern int have_full_symbols(void);
 
 /*
  * Register numbers must be in sync with gdb/features/i386/64bit-core.c
- * to make crash_target->fetch_registers() ---> machdep->get_cpu_reg()
+ * to make crash_target->fetch_registers() ---> machdep->get_current_task_reg()
  * working properly.
  */
 enum x86_64_regnum {
@@ -8081,7 +8080,7 @@ enum arm64_regnum {
 
 /*
  * Register numbers to make crash_target->fetch_registers()
- * ---> machdep->get_cpu_reg() work properly.
+ * ---> machdep->get_current_task_reg() work properly.
  *
  *  These register numbers and names are given according to output of
  *  `rs6000_register_name`, because that is what was being used by
@@ -8199,7 +8198,4 @@ enum ppc64_regnum {
 	PPC64_VRSAVE_REGNU = 139
 };
 
-/* crash_target.c */
-extern void gdb_refresh_regcache (unsigned int cpu);
-
 #endif /* !GDB_COMMON */
diff --git a/gdb_interface.c b/gdb_interface.c
index 28264c2..2e9117a 100644
--- a/gdb_interface.c
+++ b/gdb_interface.c
@@ -1073,14 +1073,14 @@ unsigned long crash_get_kaslr_offset(void)
 }
 
 /* Callbacks for crash_target */
-int crash_get_cpu_reg (int cpu, int regno, const char *regname,
+int crash_get_current_task_reg (int regno, const char *regname,
                        int regsize, void *val);
 
-int crash_get_cpu_reg (int cpu, int regno, const char *regname,
+int crash_get_current_task_reg (int regno, const char *regname,
                        int regsize, void *value)
 {
-        if (!machdep->get_cpu_reg)
+        if (!machdep->get_current_task_reg)
                 return FALSE;
-        return machdep->get_cpu_reg(cpu, regno, regname, regsize, value);
+        return machdep->get_current_task_reg(regno, regname, regsize, value);
 }
 
diff --git a/kernel.c b/kernel.c
index 446fd78..15b00dc 100644
--- a/kernel.c
+++ b/kernel.c
@@ -6543,29 +6543,6 @@ set_cpu(int cpu, int print_context)
 		show_context(CURRENT_CONTEXT());
 }
 
-int
-crash_set_thread(ulong task)
-{
-	bool found = FALSE;
-	struct task_context *tc = FIRST_CONTEXT();
-
-	for (int i = 0; i < RUNNING_TASKS(); i++, tc++) {
-		if (tc->task == task) {
-			found = TRUE;
-			break;
-		}
-	}
-
-	if (!found)
-		return -1;
-
-	if (CURRENT_TASK() == tc->task)
-		return 0;
-
-	set_context(tc->task, NO_PID, TRUE);
-	return 0;
-}
-
 /*
  *  Collect the irq_desc[] entry along with its associated handler and
  *  action structures.
diff --git a/task.c b/task.c
index f7e5b05..5f1ddad 100644
--- a/task.c
+++ b/task.c
@@ -11286,19 +11286,11 @@ check_stack_end_magic:
 		fprintf(fp, "No stack overflows detected\n");
 }
 
+void crash_get_current_task_info(unsigned long *pid, char **comm);
 void crash_get_current_task_info(unsigned long *pid, char **comm)
 {
-	int i;
-	struct task_context *tc;
+	struct task_context *tc = CURRENT_CONTEXT();
 
-	tc = FIRST_CONTEXT();
-	for (i = 0; i < RUNNING_TASKS(); i++, tc++)
-		if (tc->task == CURRENT_TASK()) {
-			*pid = tc->pid;
-			*comm = tc->comm;
-			return;
-		}
-	*pid = 0;
-	*comm = NULL;
-	return;
+	*pid = tc->pid;
+	*comm = tc->comm;
 }
diff --git a/x86_64.c b/x86_64.c
index 41e67c6..73bb353 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -126,7 +126,7 @@ static int x86_64_get_framesize(struct bt_info *, ulong, ulong, char *);
 static void x86_64_framesize_debug(struct bt_info *);
 static void x86_64_get_active_set(void);
 static int x86_64_get_kvaddr_ranges(struct vaddr_range *);
-static int x86_64_get_cpu_reg(int, int, const char *, int, void *);
+static int x86_64_get_current_task_reg(int, const char *, int, void *);
 static int x86_64_verify_paddr(uint64_t);
 static void GART_init(void);
 static void x86_64_exception_stacks_init(void);
@@ -207,7 +207,7 @@ x86_64_init(int when)
 		machdep->machspec->irq_eframe_link = UNINITIALIZED;
 		machdep->machspec->irq_stack_gap = UNINITIALIZED;
 		machdep->get_kvaddr_ranges = x86_64_get_kvaddr_ranges;
-		machdep->get_cpu_reg = x86_64_get_cpu_reg;
+		machdep->get_current_task_reg = x86_64_get_current_task_reg;
                 if (machdep->cmdline_args[0])
                         parse_cmdline_args();
 		if ((string = pc->read_vmcoreinfo("relocate"))) {
@@ -898,7 +898,7 @@ x86_64_dump_machdep_table(ulong arg)
         fprintf(fp, "        is_page_ptr: x86_64_is_page_ptr()\n");
         fprintf(fp, "       verify_paddr: x86_64_verify_paddr()\n");
         fprintf(fp, "  get_kvaddr_ranges: x86_64_get_kvaddr_ranges()\n");
-	fprintf(fp, "        get_cpu_reg: x86_64_get_cpu_reg()\n");
+	fprintf(fp, "get_current_task_reg: x86_64_get_current_task_reg()\n");
         fprintf(fp, "    init_kernel_pgd: x86_64_init_kernel_pgd()\n");
         fprintf(fp, "clear_machdep_cache: x86_64_clear_machdep_cache()\n");
 	fprintf(fp, " xendump_p2m_create: %s\n", PVOPS_XEN() ?
@@ -9186,7 +9186,7 @@ x86_64_get_kvaddr_ranges(struct vaddr_range *vrp)
 		}
 
 static int
-x86_64_get_cpu_reg(int cpu, int regno, const char *name,
+x86_64_get_current_task_reg(int regno, const char *name,
                    int size, void *value)
 {
 	struct bt_info bt_info, bt_setup;
@@ -9195,8 +9195,6 @@ x86_64_get_cpu_reg(int cpu, int regno, const char *name,
 	ulong ip, sp;
 	bool ret = FALSE;
 
-	if (VMSS_DUMPFILE())
-		return vmware_vmss_get_cpu_reg(cpu, regno, name, size, value);
 	switch (regno) {
 	case RAX_REGNUM ... GS_REGNUM:
 	case FS_BASE_REGNUM ... ORIG_RAX_REGNUM:
@@ -9208,6 +9206,60 @@ x86_64_get_cpu_reg(int cpu, int regno, const char *name,
 	tc = CURRENT_CONTEXT();
 	if (!tc)
 		return FALSE;
+
+	/*
+	 * For inactive task, grab rip, rbp, rbx, r12, r13, r14 and r15 from
+	 * inactive_task_frame (see __switch_to_asm). Other regs saved on
+	 * regular frame.
+	 */
+	if (!is_task_active(tc->task)) {
+		int frame_size = STRUCT_SIZE("inactive_task_frame");
+
+		/* Only modern kernels supported. */
+		if (tt->flags & THREAD_INFO && frame_size == 7 * 8) {
+			ulong rsp;
+			int offset = 0;
+			switch (regno) {
+				case RSP_REGNUM:
+					readmem(tc->task + OFFSET(task_struct_thread) +
+						OFFSET(thread_struct_rsp), KVADDR,
+						&rsp, sizeof(void *),
+						"thread_struct rsp", FAULT_ON_ERROR);
+					rsp += frame_size;
+					memcpy(value, &rsp, size);
+					return TRUE;
+				case RIP_REGNUM:
+					offset += 8;
+				case RBP_REGNUM:
+					offset += 8;
+				case RBX_REGNUM:
+					offset += 8;
+				case R12_REGNUM:
+					offset += 8;
+				case R13_REGNUM:
+					offset += 8;
+				case R14_REGNUM:
+					offset += 8;
+				case R15_REGNUM:
+					readmem(tc->task + OFFSET(task_struct_thread) +
+						OFFSET(thread_struct_rsp), KVADDR,
+						&rsp, sizeof(void *),
+						"thread_struct rsp", FAULT_ON_ERROR);
+					readmem(rsp + offset, KVADDR, value, sizeof(void *),
+							"inactive_thread_frame saved regs", FAULT_ON_ERROR);
+					return TRUE;
+			}
+		}
+		/* TBD: older kernels support. */
+		return FALSE;
+	}
+
+	/*
+	 * Task is active, grab CPU's registers
+	 */
+	if (VMSS_DUMPFILE())
+		return vmware_vmss_get_cpu_reg(tc->processor, regno, name, size, value);
+
 	BZERO(&bt_setup, sizeof(struct bt_info));
 	clone_bt_info(&bt_setup, &bt_info, tc);
 	fill_stackbuf(&bt_info);
-- 
2.39.0
--
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




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux