[Crash-utility] Re: [PATCH v4 04/16] Rename get_cpu_reg to get_current_task_reg

On 6/20/24 3:07 PM, Lianbo Jiang wrote: > Date: Fri, 31 May 2024 17:19:27 +0800 > From: Tao Liu&lt;ltao(a)redhat.com&gt; > Subject: [Crash-utility] [PATCH v4 04/16] Rename get_cpu_reg to >     get_current_task_reg > To:devel@lists.crash-utility.osci.io > Cc: Alexey Makhalov&lt;alexey.makhalov(a)broadcom.com&gt;, Mahesh J >     Salgaonkar&lt;mahesh(a)linux.ibm.com&gt;, "Naveen N . Rao" >     &lt;naveen.n.rao(a)linux.vnet.ibm.com&gt;, Lianbo Jiang&lt;lijiang(a)redhat.com&gt; > Message-ID:<20240531091939.97828-5-ltao@redhat.com> > Content-Type: text/plain; charset=UTF-8 > > Since we only leave one gdb thread, or 1 cpu thread, and can use cmd > "set" > to switch task context by reusing the thread. So the word "get_cpu_reg", > which stands for "fetch registers' value for cpu thread x", is no longer > appropriate, better using "get_current_task_reg", which stands for > "fetch > registers' value for the current task", and makes more sense. > > Co-developed-by: Aditya Gupta&lt;adityag(a)linux.ibm.com&gt; > Co-developed-by: Alexey Makhalov&lt;alexey.makhalov(a)broadcom.com&gt; > Co-developed-by: Tao Liu&lt;ltao(a)redhat.com&gt; > Cc: Sourabh Jain&lt;sourabhjain(a)linux.ibm.com&gt; > Cc: Hari Bathini&lt;hbathini(a)linux.ibm.com&gt; > Cc: Mahesh J Salgaonkar&lt;mahesh(a)linux.ibm.com&gt; > Cc: Naveen N. Rao&lt;naveen.n.rao(a)linux.vnet.ibm.com&gt; > Cc: Lianbo Jiang&lt;lijiang(a)redhat.com&gt; > Cc: HAGIO KAZUHITO(萩尾 一仁)&lt;k-hagio-ab(a)nec.com&gt; > Cc: Tao Liu&lt;ltao(a)redhat.com&gt; > Cc: Alexey Makhalov&lt;alexey.makhalov(a)broadcom.com&gt; > Signed-off-by: Tao Liu&lt;ltao(a)redhat.com&gt; > --- >   crash_target.c  | 5 ++--- >   defs.h          | 2 +- >   gdb_interface.c | 6 +++--- >   x86_64.c        | 2 +- >   4 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/crash_target.c b/crash_target.c > index 03718b5..52966f7 100644 > --- a/crash_target.c > +++ b/crash_target.c > @@ -26,7 +26,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, > +extern "C" int crash_get_current_task_reg (int regno, const char > *regname, >                                     int regsize, void *val); >   extern "C" int gdb_change_thread_context (); >   @@ -68,7 +68,6 @@ 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 (); >       if (regno >= 0) { > @@ -84,7 +83,7 @@ onetime: >         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); > diff --git a/defs.h b/defs.h > index 0d872b2..012ffdc 100644 > --- a/defs.h > +++ b/defs.h > @@ -1080,7 +1080,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); >   }; >   diff --git a/gdb_interface.c b/gdb_interface.c > index ab1bd52..b13d5fd 100644 > --- a/gdb_interface.c > +++ b/gdb_interface.c > @@ -1067,11 +1067,11 @@ 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 *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/x86_64.c b/x86_64.c > index 0c21eb8..5ab2852 100644 > --- a/x86_64.c > +++ b/x86_64.c > @@ -195,7 +195,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_cpu_reg; Still getting a compilation error: cc -c -g -DX86_64 -DLZO -DGDB_10_2  x86_64.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security In file included from x86_64.c:16: defs.h:8058:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]  8058 | extern int gdb_change_thread_context ();       | ^~~~~~ x86_64.c: In function ‘x86_64_init’: x86_64.c:198:47: error: assignment to ‘int (*)(int,  const char *, int,  void *)’ from incompatible pointer type ‘int (*)(int,  int, const char *, int,  void *)’ [-Wincompatible-pointer-types]   198 |                 machdep->get_current_task_reg = x86_64_get_cpu_reg;       |                                               ^ make[4]: *** [Makefile:432: x86_64.o] Error 1 make[3]: *** [Makefile:1877: gdb] Error 2 make[2]: *** [Makefile:278: rebuild] Error 2 make[1]: *** [Makefile:266: gdb_merge] Error 2 make: *** [Makefile:258: all] Error 2 Looks like the current changes are not complete because of removing a parameter in the get_cpu_reg().

Thanks Lianbo >                   if (machdep->cmdline_args[0]) >                           parse_cmdline_args(); >           if ((string = pc->read_vmcoreinfo("relocate"))) { > -- 2.40.1