Re: [Crash-utility] [PATCH 1/2] Let gdb get kernel module symbols info from crash

Hi, Tao Thank you for the patch. On Wed, Aug 24, 2022 at 12:12 PM <crash-utility-request(a)redhat.com&gt; wrote: > > > Date: Wed, 24 Aug 2022 12:10:33 +0800 > From: Tao Liu <ltao(a)redhat.com&gt; > To: crash-utility(a)redhat.com > Subject: [Crash-utility] [PATCH 1/2] Let gdb get kernel module symbols > info from crash > Message-ID: <20220824041033.40559-1-ltao(a)redhat.com&gt; > Content-Type: text/plain; charset="US-ASCII"; x-default=true > > Gdb will try to resolve an address to its corresponding symbol name such as > when printing a structure. It works fine for kernel symbols, because gdb can > find them through vmlinux. However as for kernel modules symbols, crash > resolves them by dig into "struct module", which gdb don't know. As a > results, gdb fails to translate an kernel modules address to it's symbolic > name. For example we can reproduce the issue as follows. > > crash> timer > .... > 4331308176 336 ffff94ea24240860 ffffffffc03762c0 <estimation_timer> > .... > crash> sym 0xffffffffc03762c0 > ffffffffc03762c0 (t) estimation_timer [ip_vs] > > Before patch: > crash> timer_list ffff94ea24240860 > struct timer_list { > .... > function = 0xffffffffc03762c0, > .... > } > > After patch: > crash> timer_list ffff94ea24240860 > struct timer_list { > .... > function = 0xffffffffc03762c0 <estimation_timer>, > .... > } > > In this patch, we add an interface for gdb, when gdb trying to build kernel > module's address symbolic, the info can be get from crash. > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > defs.h | 2 ++ > gdb-10.2.patch | 35 +++++++++++++++++++++++++++++++++-- > gdb_interface.c | 12 ++++++++++++ > 3 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/defs.h b/defs.h > index 9d6d891..afdcf6c 100644 > --- a/defs.h > +++ b/defs.h > @@ -4874,6 +4874,7 @@ extern "C" int patch_kernel_symbol(struct gnu_request *); > struct syment *symbol_search(char *); > int gdb_line_number_callback(ulong, ulong, ulong); > int gdb_print_callback(ulong); > +char *gdb_lookup_module_symbol(ulong, ulong *); > extern "C" int same_file(char *, char *); > #endif > > @@ -7284,6 +7285,7 @@ int gdb_pass_through(char *, FILE *, ulong); > int gdb_readmem_callback(ulong, void *, int, int); > int gdb_line_number_callback(ulong, ulong, ulong); > int gdb_print_callback(ulong); > +char *gdb_lookup_module_symbol(ulong, ulong *); > void gdb_error_hook(void); > void restore_gdb_sanity(void); > int is_gdb_command(int, ulong); > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > index f0034ed..d08e9ab 100644 > --- a/gdb-10.2.patch > +++ b/gdb-10.2.patch > @@ -399,7 +399,38 @@ exit 0 > if (build_address_symbolic (gdbarch, addr, do_demangle, false, &name, > &offset, &filename, &line, &unmapped)) > return 0; > -@@ -1221,6 +1230,43 @@ print_command_1 (const char *args, int voidprint) > +@@ -567,6 +576,10 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr, > + > + /* See valprint.h. */ > + > ++#ifdef CRASH_MERGE > ++extern "C" char *gdb_lookup_module_symbol(unsigned long, unsigned long *); > ++#endif > ++ > + int > + build_address_symbolic (struct gdbarch *gdbarch, > + CORE_ADDR addr, /* IN */ > +@@ -673,7 +686,19 @@ build_address_symbolic (struct gdbarch *gdbarch, > + } > + } > + if (symbol == NULL && msymbol.minsym == NULL) > ++#ifdef CRASH_MERGE > ++ { > ++ char *name_ptr = gdb_lookup_module_symbol(addr, (unsigned long *)offset); > ++ if (name_ptr) { > ++ *name = name_ptr; > ++ return 0; > ++ } else { > ++ return 1; > ++ } > ++ } > ++#else > + return 1; > ++#endif > + > + /* If the nearest symbol is too far away, don't print anything symbolic. */ > + > +@@ -1221,6 +1246,43 @@ print_command_1 (const char *args, int voidprint) > print_value (val, print_opts); > } > Usually, the gdb patch needs to be put at the end of gdb-10.2.patch, which can help to apply your patch correctly. Could you please refer to the following commit? 4c85e982d25a ("gdb: fix for assigning NULL to std::string")

In some commands, crash will trigger symbols loading via the value_search(), and also decide if the module symbols need to be loaded there, but for this case, there seems to be no better solution.

Thanks. Lianbo > @@ -443,7 +474,7 @@ exit 0 > /* See valprint.h. */ > > void > -@@ -2855,6 +2901,12 @@ but no count or size letter (see \"x\" command)."), > +@@ -2855,6 +2917,12 @@ but no count or size letter (see \"x\" command)."), > c = add_com ("print", class_vars, print_command, print_help.c_str ()); > set_cmd_completer_handle_brkchars (c, print_command_completer); > add_com_alias ("p", "print", class_vars, 1); > diff --git a/gdb_interface.c b/gdb_interface.c > index 3a7fcc9..b14319c 100644 > --- a/gdb_interface.c > +++ b/gdb_interface.c > @@ -935,6 +935,18 @@ gdb_print_callback(ulong addr) > return IS_KVADDR(addr); > } > > +char * > +gdb_lookup_module_symbol(ulong addr, ulong *offset) > +{ > + struct syment *sp; > + > + if ((sp = value_search_module(addr, offset))) { > + return sp->name; > + } else { > + return NULL; > + } > +} > + > /* > * Used by gdb_interface() to catch gdb-related errors, if desired. > */ > -- > 2.33.1 >