Hi, Tao
Thank you for the patch.
On Wed, Aug 24, 2022 at 12:12 PM <[email protected]> wrote:

>
> Date: Wed, 24 Aug 2022 12:10:33 +0800
> From: Tao Liu <[email protected]>
> To: [email protected]
> Subject: [Crash-utility] [PATCH 1/2] Let gdb get kernel module symbols
>         info from crash
> Message-ID: <[email protected]>
> 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 <[email protected]>
> ---
>  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
>
>
--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to