[Crash-utility] Re: [PATCH v2] symbols: skip load .init.* sections if module was successfully initialized

On 2023/11/14 17:32, Tao Liu wrote: > There might be address overlap of one module's .init.text symbols and > another module's .text symbols. As a result, gdb fails to translate the > address to symbol name correctly: > > crash> sym -m virtio_blk | grep MODULE > ffffffffc00a4000 MODULE START: virtio_blk > ffffffffc00a86ec MODULE END: virtio_blk > crash> gdb info address floppy_module_init > Symbol "floppy_module_init" is a function at address 0xffffffffc00a4131. > > Since the .init.* sections of a module had been freed by kernel if the > module was initialized successfully, there is no need to load the .init.* > sections data from "*.ko.debug" in gdb to create such an overlap. > lm->mod_init_module_ptr is used as a flag of whether module is freed. > > Without the patch: > crash> mod -S > crash> struct blk_mq_ops 0xffffffffc00a7160 > struct blk_mq_ops { > queue_rq = 0xffffffffc00a45b0 <floppy_module_init+1151>, <-- symbol translated from module floppy > map_queue = 0xffffffff813015c0 <blk_mq_map_queue>, > ...snip... > complete = 0xffffffffc00a4370 <floppy_module_init+575>, > init_request = 0xffffffffc00a4260 <floppy_module_init+303>, > ...snip... > } > > With the patch: > crash> mod -S > crash> struct blk_mq_ops 0xffffffffc00a7160 > struct blk_mq_ops { > queue_rq = 0xffffffffc00a45b0 <virtio_queue_rq>, <-- symbol translated from module virtio_blk > map_queue = 0xffffffff813015c0 <blk_mq_map_queue>, > ...snip... > complete = 0xffffffffc00a4370 <virtblk_request_done>, > init_request = 0xffffffffc00a4260 <virtblk_init_request>, > ...snip... > } > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > > v1: [PATCH 1/2] symbols: expand kernel modules symtable before symbols translation > [PATCH 2/2] symbols: fix the error belonging of the kernel modules symbols > v2 -> v1: Used different solution, re-drafted patch based on Kazu's comments, > so v1 can be discarded. Thank you for the update, looks good. Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com&gt; We're going to release a new version of crash in a few days, but I would like to evaluate this kind of patch for a while in actual cases. So this will be not included in the release, sorry about that.

Thanks, Kazu > > --- > symbols.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/symbols.c b/symbols.c > index 8e8b4c3..dae5b04 100644 > --- a/symbols.c > +++ b/symbols.c > @@ -13283,7 +13283,7 @@ add_symbol_file_kallsyms(struct load_module *lm, struct gnu_request *req) > shift_string_right(req->buf, strlen(buf)); > BCOPY(buf, req->buf, strlen(buf)); > retval = TRUE; > - } else { > + } else if (lm->mod_init_module_ptr || !STRNEQ(section_name, ".init.")) { > sprintf(buf, " -s %s 0x%lx", section_name, section_vaddr); > while ((len + strlen(buf)) >= buflen) { > RESIZEBUF(req->buf, buflen, buflen * 2);