[Crash-utility] Re: [PATCH v4 3/5] synchronise cpu context changes between crash/gdb

Currently, both crash context and gdb's current thread context were pretty independent, and could be different, for example, crash commands might be working on thread 6 (CPU 5), but GDB passthroughs will be working on thread thread 2 (CPU 1). This was not a problem earlier since interaction of crash and gdb was not depending on current context for most part. But for gdb passthroughs to work correctly, gdb needs register values from crash, which depend on current context in crash. Synchronise 'thread' command in gdb with 'set -c' command in crash. 1. crash -> gdb synchronisation: Everytime crash's context changes, a helper is called to switch to the thread on that CPU in gdb. The function has been implemented in crash_target.c, since gdb functions are accessible inside 'crash_target.c', and the thread ID to CPU ID mapping is also done by the crash_target, during initially registering the threads with gdb. With this implementation, GDB's default thread initially also changes to the crashing thread, so a switch to crashing thread manually isn't required anymore 2. gdb -> crash synchronisation: gdb has been patched to call 'set_cpu' whenever user switches to any thread. Signed-off-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; --- crash_target.c | 22 ++++++++++++++++++++++ defs.h | 3 +++ gdb-10.2.patch | 30 ++++++++++++++++++++++++++++++ kernel.c | 8 +++++++- ppc64.c | 3 --- task.c | 5 +++++ 6 files changed, 67 insertions(+), 4 deletions(-) diff --git a/crash_target.c b/crash_target.c index 455480679741..223a51acb1e1 100644 --- a/crash_target.c +++ b/crash_target.c @@ -29,6 +29,8 @@ extern "C" int gdb_readmem_callback(unsigned long, void *, int, int); 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); /* The crash target. */ @@ -133,3 +135,23 @@ crash_target_init (void) /* Now, set up the frame cache. */ reinit_frame_cache (); } + +/* + * Change gdb's thread context to the thread on given CPU + * */ +extern "C" int gdb_change_cpu_context (unsigned int cpu) { + ptid_t ptid = ptid_t(CRASH_INFERIOR_PID, 0, cpu); + inferior* inf = current_inferior (); + thread_info *tp = find_thread_ptid (inf, ptid); + + if (tp == nullptr) + return FALSE; + + /* Making sure that crash's context is same */ + set_cpu(cpu); + + /* Switch to the thread */ + switch_to_thread(tp); + return TRUE; +} + diff --git a/defs.h b/defs.h index 0c80de72e89e..5e6df14e947d 100644 --- a/defs.h +++ b/defs.h @@ -7978,4 +7978,7 @@ enum ppc64_renum { PPC64_VRSAVE_REGNU = 139 }; +/* crash_target.c */ +extern int gdb_change_cpu_context (unsigned int cpu); + #endif /* !GDB_COMMON */ diff --git a/gdb-10.2.patch b/gdb-10.2.patch index 2f7d585105aa..8c1b43eb07b7 100644 --- a/gdb-10.2.patch +++ b/gdb-10.2.patch @@ -10,6 +10,7 @@ tar xvzmf gdb-10.2.tar.gz \ gdb-10.2/gdb/symtab.c \ + gdb-10.2/gdb/thread.c \ gdb-10.2/gdb/printcmd.c \ gdb-10.2/gdb/symfile.c \ gdb-10.2/gdb/Makefile.in \ @@ -485,6 +486,35 @@ exit 0 return best_pst; } +--- gdb-10.2/gdb/thread.c.orig ++++ gdb-10.2/gdb/thread.c +@@ -58,6 +58,11 @@ static int highest_thread_num; + /* The current/selected thread. */ + static thread_info *current_thread_; + ++#ifdef CRASH_MERGE ++/* Function to set cpu, defined by crash-utility */ ++extern "C" void set_cpu (int); ++#endif ++ + /* RAII type used to increase / decrease the refcount of each thread + in a given list of threads. */ + +@@ -1896,7 +1901,13 @@ thread_command (const char *tidstr, int from_tty) + { + ptid_t previous_ptid = inferior_ptid; + +- thread_select (tidstr, parse_thread_id (tidstr, NULL)); ++ struct thread_info* thread_id = parse_thread_id (tidstr, NULL); ++ ++#ifdef CRASH_MERGE ++ set_cpu(thread_id->ptid.tid()); ++#endif ++ ++ thread_select (tidstr, thread_id); + + /* Print if the thread has not changed, otherwise an event will + be sent. */ --- gdb-10.2/gdb/symfile.c.orig +++ gdb-10.2/gdb/symfile.c @@ -652,7 +652,26 @@ default_symfile_offsets (struct objfile *objfile, diff --git a/kernel.c b/kernel.c index 52b7ba09f390..a8d60507dd95 100644 --- a/kernel.c +++ b/kernel.c @@ -6504,7 +6504,13 @@ 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))) + 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);

else error(FATAL, "cannot determine active task on cpu %ld\n", cpu); diff --git a/ppc64.c b/ppc64.c index f5b0b7241ea4..1816972fcf2c 100644 --- a/ppc64.c +++ b/ppc64.c @@ -2542,9 +2542,6 @@ ppc64_get_cpu_reg(int cpu, int regno, const char *name, int size, return FALSE; } - /* FIXME: Always setting the context to CURRENT_CONTEXT irrespective of whicher - * thread we switched to, in gdb - */ tc = CURRENT_CONTEXT(); BZERO(&bt_setup, sizeof(struct bt_info)); clone_bt_info(&bt_setup, &bt_info, tc); diff --git a/task.c b/task.c index ebdb5be3786f..64088eca29a1 100644 --- a/task.c +++ b/task.c @@ -5301,6 +5301,11 @@ set_context(ulong task, ulong pid) if (found) { CURRENT_CONTEXT() = tc; + + /* change the selected thread in gdb, according to current context */ + if ( gdb_change_cpu_context(tc->processor) == FALSE ) + return FALSE; + return TRUE;

} else { if (task)