On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com> wrote:
> Support "mod -s|-S" with introducing lm->load_sym{table,end} > > but many functions like "mod -d|-D" are still not supported. > > Signed-off-by: Kazuhito Hagio <k-hagio...@nec.com> > --- > defs.h | 9 +- > gdb-10.2.patch | 16 +++ > symbols.c | 350 ++++++++++++++++++++++++++++++++++++++++++++----- > 3 files changed, 340 insertions(+), 35 deletions(-) > > diff --git a/defs.h b/defs.h > index 95e44e8cb87c..b2478b6741ec 100644 > --- a/defs.h > +++ b/defs.h > @@ -2925,6 +2925,7 @@ struct mod_section_data { > ulong size; > int priority; > int flags; > + ulong addr; > Is it possible to reuse the member offset in module memory patches? I noticed that the offset is not used in the calculate_load_order_v3(). If it is doable to reuse the offset, that may avoid modifying the gdb patch? I haven't investigated the details. }; > > /* This is unlikely to change, so imported from kernel for now. */ > @@ -2982,8 +2983,12 @@ struct load_module { > > /* For 6.4 module_memory */ > struct module_memory mem[MOD_MEM_NUM_TYPES]; - struct syment *symtable[MOD_MEM_NUM_TYPES]; > - struct syment *symend[MOD_MEM_NUM_TYPES]; > + struct syment **symtable; > + struct syment **symend; > Some similar member definitions are in the struct symbol_table_data and struct load_module, it looks confusing to me. But I'm not sure if it is better to move some of them to the struct symbol_talbe_data. + struct syment *ext_symtable[MOD_MEM_NUM_TYPES]; > + struct syment *ext_symend[MOD_MEM_NUM_TYPES]; > + struct syment *load_symtable[MOD_MEM_NUM_TYPES]; > + struct syment *load_symend[MOD_MEM_NUM_TYPES]; > int address_order[MOD_MEM_NUM_TYPES]; > int nr_mems; > }; > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > index 835aae9859be..b3f6d8b086eb 100644 > --- a/gdb-10.2.patch > +++ b/gdb-10.2.patch > @@ -3120,3 +3120,19 @@ exit 0 > return result; > } > > +--- gdb-10.2/gdb/symtab.c.orig > ++++ gdb-10.2/gdb/symtab.c > +@@ -7515,8 +7515,11 @@ gdb_add_symbol_file(struct gnu_request * > + secname = lm->mod_section_data[i].name; > + if ((lm->mod_section_data[i].flags & > SEC_FOUND) && > + !STREQ(secname, ".text")) { > +- sprintf(buf, " -s %s 0x%lx", secname, > +- lm->mod_section_data[i].offset + > lm->mod_base); > ++ if (lm->mod_section_data[i].addr) > ++ sprintf(buf, " -s %s 0x%lx", > secname, lm->mod_section_data[i].addr); > ++ else > ++ sprintf(buf, " -s %s 0x%lx", > secname, > ++ > lm->mod_section_data[i].offset + lm->mod_base); > + strcat(req->buf, buf); > + } > + } > diff --git a/symbols.c b/symbols.c > index ef00ce0b79ca..8343081f51f7 100644 > --- a/symbols.c > +++ b/symbols.c > @@ -50,6 +50,8 @@ static void store_section_data(struct load_module *, bfd > *, asection *); > static void calculate_load_order_v1(struct load_module *, bfd *); > static void calculate_load_order_v2(struct load_module *, bfd *, int, > void *, long, unsigned int); > +static void calculate_load_order_v3(struct load_module *, bfd *, int, > + void *, long, unsigned int); static void check_insmod_builtin(struct load_module *, int, ulong *); > static int is_insmod_builtin(struct load_module *, struct syment *); > struct load_module; > @@ -2288,20 +2290,22 @@ store_module_symbols_v3(ulong total, int > mods_installed) > > for (sp = st->ext_module_symtable; sp < > st->ext_module_symend; sp++) { > if (STREQ(sp->name, buf1)) { > - lm->symtable[i] = sp; > + lm->ext_symtable[i] = sp; > break; > } > } > for ( ; sp < st->ext_module_symend; sp++) { > if (STREQ(sp->name, buf2)) { > - lm->symend[i] = sp; > + lm->ext_symend[i] = sp; > break; > } > } > > - if (lm->symtable[i] && lm->symend[i]) > - > mod_symtable_hash_install_range(lm->symtable[i], lm->symend[i]); > + if (lm->ext_symtable[i] && lm->ext_symend[i]) > + > mod_symtable_hash_install_range(lm->ext_symtable[i], lm->ext_symend[i]); > } > + lm->symtable = lm->ext_symtable; > + lm->symend = lm->ext_symend; > } > > st->flags |= MODULE_SYMS; > @@ -4090,15 +4094,27 @@ dump_symbol_table(void) > for (j = 0; j < lm->nr_mems; j++) > fprintf(fp, " %d", lm->address_order[j]); > fprintf(fp, "\n"); > + fprintf(fp, " symtable: %lx\n", > (ulong)lm->symtable); > + fprintf(fp, " ext_symtable: %lx\n", > (ulong)lm->ext_symtable); > + for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) { > + fprintf(fp, " ext_symtable[%d]: %lx > - %lx\n", > + j, (ulong)lm->ext_symtable[j], > (ulong)lm->ext_symend[j]); > + } > + fprintf(fp, " load_symtable: %lx\n", > (ulong)lm->load_symtable); > + for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) { > + fprintf(fp, " load_symtable[%d]: %lx > - %lx\n", > + j, (ulong)lm->load_symtable[j], > (ulong)lm->load_symend[j]); > + } > } > > for (s = 0; s < lm->mod_sections; s++) { > fprintf(fp, > - " %12s prio: %x flags: %05x offset: %-8lx size: > %lx\n", > + " %20s prio: %x flags: %08x offset: %-8lx addr: > %-16lx size: %lx\n", > lm->mod_section_data[s].name, > lm->mod_section_data[s].priority, > lm->mod_section_data[s].flags, > lm->mod_section_data[s].offset, > + lm->mod_section_data[s].addr, > lm->mod_section_data[s].size); > } > > @@ -12122,6 +12138,7 @@ store_section_data(struct load_module *lm, bfd > *bfd, asection *section) > } > lm->mod_section_data[i].size = bfd_section_size(section); > lm->mod_section_data[i].offset = 0; > + lm->mod_section_data[i].addr = 0; > if (strlen(name) < MAX_MOD_SEC_NAME) > strcpy(lm->mod_section_data[i].name, name); > else > @@ -12367,6 +12384,133 @@ calculate_load_order_v2(struct load_module *lm, > bfd *bfd, int dynamic, > } > } > > +/* Linux 6.4 and later */ > +static void > +calculate_load_order_v3(struct load_module *lm, bfd *bfd, int dynamic, > + void *minisyms, long symcount, unsigned int size) > +{ > + struct syment *s1, *s2; > + ulong sec_start; > + bfd_byte *from, *fromend; > + asymbol *store; > + asymbol *sym; > + symbol_info syminfo; > + char *secname; > + int i, j; > + > + if ((store = bfd_make_empty_symbol(bfd)) == NULL) > + error(FATAL, "bfd_make_empty_symbol() failed\n"); > + > + for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) { > + > The above for-loop is frequently used in these patches, can we introduce the following macro definition from the kernel? #define for_each_mod_mem_type(type) \ for (enum mod_mem_type (type) = 0; \ (type) < MOD_MEM_NUM_TYPES; (type)++) Looks more convenient to me. + s1 = lm->symtable[j]; > + s2 = lm->symend[j]; > + while (s1 < s2) { > + > + if (MODULE_PSEUDO_SYMBOL(s1)) { > + s1++; > + continue; > + } > + > + /* Skip over symbols whose sections have been > identified. */ > + for (i = 0; i < lm->mod_sections; i++) { > + if ((lm->mod_section_data[i].flags & > SEC_FOUND) == 0) > + continue; > + > + if (s1->value >= lm->mod_section_data[i].addr > + && s1->value < lm->mod_section_data[i].addr > + + lm->mod_section_data[i].size) { > + break; > + } > + } > + > + /* Matched one of the sections. Skip symbol. */ > + if (i < lm->mod_sections) { > + if (CRASHDEBUG(2)) { > + fprintf(fp, "skip %lx %s %s\n", s1->value, > s1->name, > + lm->mod_section_data[i].name); > + } > + s1++; > + continue; > + } > + > + /* Find the symbol in the object file. */ > + from = (bfd_byte *) minisyms; > + fromend = from + symcount * size; > + secname = NULL; > + for (; from < fromend; from += size) { > + if ((sym = bfd_minisymbol_to_symbol(bfd, > dynamic, from, > + store)) == NULL) > + error(FATAL, > + "bfd_minisymbol_to_symbol() > failed\n"); > + > + bfd_get_symbol_info(bfd, sym, &syminfo); > + if (CRASHDEBUG(3)) { > + fprintf(fp,"matching sym %s %lx > against bfd %s %lx\n", > + s1->name, (long) s1->value, > syminfo.name, > + (long) syminfo.value); > + } > + if (strcmp(syminfo.name, s1->name) == 0) { > + secname = (char > *)bfd_section_name(sym->section); > + break; > + } > + > + } > + if (secname == NULL) { > + if (CRASHDEBUG(1)) { > + fprintf(fp, "symbol %s not found in > module\n", s1->name); > + } > + s1++; > + continue; > + } > + > + /* Match the section it came in. */ > + for (i = 0; i < lm->mod_sections; i++) { > + if (STREQ(lm->mod_section_data[i].name, > secname)) { > + break; > + } > + } > + > + if (i == lm->mod_sections) { > + fprintf(fp, "?? Section %s not found for > symbol %s\n", > + secname, s1->name); > + s1++; > + continue; > + } > + > + if (lm->mod_section_data[i].flags & SEC_FOUND) { > + s1++; > + continue; > + } > + > + /* Update the offset information for the section */ > + sec_start = s1->value - syminfo.value; > + //lm->mod_section_data[i].offset = sec_start - > lm->mem[j].base; > + /* keep the address instead of offset */ > + lm->mod_section_data[i].addr = sec_start; > + lm->mod_section_data[i].flags |= SEC_FOUND; > + > + if (CRASHDEBUG(2)) { > + fprintf(fp, "update sec offset sym %s @ %lx > val %lx section %s\n", > + s1->name, s1->value, > (ulong)syminfo.value, secname); > + } > + > + if (strcmp(secname, ".text") == 0) > + lm->mod_text_start = sec_start; > + > + if (strcmp(secname, ".bss") == 0) > + lm->mod_bss_start = sec_start; > + > + if (strcmp(secname, ".data") == 0) > + lm->mod_data_start = sec_start; > + > + if (strcmp(secname, ".rodata") == 0) > + lm->mod_rodata_start = sec_start; > + s1++; > + } > + } > +} > + > /* > * Later versons of insmod store basic address information of each > * module in a format that looks like the following example of the > @@ -12561,6 +12705,7 @@ add_symbol_file(struct load_module *lm) > req = &request; > BZERO(req, sizeof(struct gnu_request)); > > + /* TODO: ugh, module section data structures changed? */ > if ((lm->mod_flags & MOD_KALLSYMS) && > add_symbol_file_kallsyms(lm, req)) > return TRUE; > @@ -12572,8 +12717,11 @@ add_symbol_file(struct load_module *lm) > (!STREQ(secname, ".text") && > !STREQ(secname, ".data.percpu") && > !STREQ(secname, ".data..percpu"))) { > - sprintf(buf, " -s %s 0x%lx", secname, > - lm->mod_section_data[i].offset + > lm->mod_base); > + if (MODULE_MEMORY()) > + sprintf(buf, " -s %s 0x%lx", secname, > lm->mod_section_data[i].addr); > + else > + sprintf(buf, " -s %s 0x%lx", secname, > + lm->mod_section_data[i].offset + > lm->mod_base); > len += strlen(buf); > } > } > @@ -12914,24 +13062,43 @@ static struct syment * > kallsyms_module_symbol(struct load_module *lm, symbol_info *syminfo) > { > struct syment *sp, *spx; > - int cnt; > + int i, cnt; > > if (!(lm->mod_flags & MOD_KALLSYMS)) > return NULL; > > sp = NULL; > cnt = 0; > - for (spx = lm->mod_ext_symtable; spx <= lm->mod_ext_symend; spx++) > { > - if (!STREQ(spx->name, syminfo->name)) > - continue; > - if (spx->cnt) { > - cnt++; > - continue; > - } > + if (MODULE_MEMORY()) { > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. > + if (!lm->ext_symtable[i]) > + continue; > + for (spx = lm->ext_symtable[i]; spx <= > lm->ext_symend[i]; spx++) { > + if (!STREQ(spx->name, syminfo->name)) > + continue; > + if (spx->cnt) { > + cnt++; > + continue; > + } > > - spx->cnt++; > - sp = spx; > - break; > + spx->cnt++; > + sp = spx; > + break; > + } > + } > + } else { > + for (spx = lm->mod_ext_symtable; spx <= > lm->mod_ext_symend; spx++) { > + if (!STREQ(spx->name, syminfo->name)) > + continue; > + if (spx->cnt) { > + cnt++; > + continue; > + } > + > + spx->cnt++; > + sp = spx; > + break; > + } > } > > if (CRASHDEBUG(2)) { > @@ -12968,7 +13135,7 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > char *nameptr, *secname; > long index; > long symalloc; > - int found; > + int found = FALSE; > > if ((store = bfd_make_empty_symbol(bfd)) == NULL) > error(FATAL, "bfd_make_empty_symbol() failed\n"); > @@ -13025,8 +13192,17 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > lm->mod_rodata_start = lm->mod_bss_start = 0; > lm->mod_load_symcnt = 0; > lm->mod_sections = 0; > - for (spx = lm->mod_ext_symtable; spx <= > lm->mod_ext_symend; spx++) > - spx->cnt = 0; > + if (MODULE_MEMORY()) { > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. > + if (!lm->ext_symtable[i]) > + continue; > + for (spx = lm->ext_symtable[i]; spx <= > lm->ext_symend[i]; spx++) > + spx->cnt = 0; > + } > + } else { > + for (spx = lm->mod_ext_symtable; spx <= > lm->mod_ext_symend; spx++) > + spx->cnt = 0; > + } > sp = lm->mod_load_symtable; > > if (!(lm->mod_section_data = (struct mod_section_data *) > @@ -13037,7 +13213,17 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > > bfd_map_over_sections(bfd, section_header_info, MODULE_SECTIONS); > > - if (kt->flags & KMOD_V1) > + /* for debug > + for (i = 0; i < lm->mod_sections; i++) { > + struct mod_section_data *m = &lm->mod_section_data[i]; > + fprintf(fp, "%d: name %s offset %ld size %ld flags %x\n", > + i, m->name, m->offset, m->size, m->flags); > + } > + */ > + > + if (MODULE_MEMORY()) > + calculate_load_order_v3(lm, bfd, dynamic, minisyms, > symcount, size); > + else if (kt->flags & KMOD_V1) > calculate_load_order_v1(lm, bfd); > else > calculate_load_order_v2(lm, bfd, dynamic, minisyms, > @@ -13146,7 +13332,10 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > syminfo.value += lm->mod_percpu; > found = TRUE; > } else { > - syminfo.value += > lm->mod_section_data[i].offset + lm->mod_base; > + if (MODULE_MEMORY()) > + syminfo.value += > lm->mod_section_data[i].addr; > + else > + syminfo.value += > lm->mod_section_data[i].offset + lm->mod_base; > found = TRUE; > } > } > @@ -13181,6 +13370,51 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > * syminfo data types accepted above, plus the two pseudo symbols. > * Note that the new syment name pointers haven't been resolved > yet. > */ > + if (MODULE_MEMORY()) { > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. > + if (!lm->ext_symtable[i]) > + continue; > + for (spx = lm->ext_symtable[i]; spx <= > lm->ext_symend[i]; spx++) { > + found = FALSE; > + for (sp = lm->mod_load_symtable; sp < > lm->mod_load_symend; sp++) { > + index = (long)sp->name; > + nameptr = > &lm->mod_load_namespace.address[index]; > + if (STREQ(spx->name, nameptr)) { > + found = TRUE; > + if (spx->value == > sp->value) { > + if (CRASHDEBUG(2)) > + > fprintf(fp, "%s: %s matches!\n", > + > lm->mod_name, nameptr); > + } else { > + if (CRASHDEBUG(2)) > + fprintf(fp, > + "[%s] %s: %lx != > extern'd value: %lx\n", > + > lm->mod_name, > + > nameptr, sp->value, > + > spx->value); > + } > + break; > + } > + } > + if (!found) { > + if (CRASHDEBUG(2)) > + fprintf(fp, "append ext %s > (%lx)\n", spx->name, spx->value); > + /* append it here... */ > + namespace_ctl(NAMESPACE_INSTALL, > + &lm->mod_load_namespace, > + lm->mod_load_symend, > spx->name); > + > + lm->mod_load_symend->value = > spx->value; > + lm->mod_load_symend->type = > spx->type; > + lm->mod_load_symend->flags |= > MODULE_SYMBOL; > + lm->mod_load_symend++; > + lm->mod_load_symcnt++; > + } > + } > + } > + goto append_section_symbols; > + } > + > for (spx = lm->mod_ext_symtable; spx <= lm->mod_ext_symend; spx++) > { > found = FALSE; > for (sp = lm->mod_load_symtable; > @@ -13223,6 +13457,7 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > } > } > > +append_section_symbols: > /* > * Append helpful pseudo symbols about found out sections. > * Use 'S' as its type which is never seen in existing symbols. > @@ -13232,8 +13467,11 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > if (!(lm->mod_section_data[i].flags & SEC_FOUND)) > continue; > /* Section start */ > - lm->mod_load_symend->value = lm->mod_base + > - > lm->mod_section_data[i].offset; > + if (MODULE_MEMORY()) > + lm->mod_load_symend->value = > lm->mod_section_data[i].addr; > + else > + lm->mod_load_symend->value = lm->mod_base + > + > lm->mod_section_data[i].offset; > lm->mod_load_symend->type = 'S'; > lm->mod_load_symend->flags |= MODULE_SYMBOL; > sprintf(name, "_MODULE_SECTION_START [%s]", > @@ -13244,9 +13482,12 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > lm->mod_load_symcnt++; > > /* Section end */ > - lm->mod_load_symend->value = lm->mod_base + > - > lm->mod_section_data[i].offset + > - lm->mod_section_data[i].size; > + if (MODULE_MEMORY()) > + lm->mod_load_symend->value = > lm->mod_section_data[i].addr; > + else > + lm->mod_load_symend->value = lm->mod_base + > + > lm->mod_section_data[i].offset; > + lm->mod_load_symend->value += lm->mod_section_data[i].size; > lm->mod_load_symend->type = 'S'; > lm->mod_load_symend->flags |= MODULE_SYMBOL; > sprintf(name, "_MODULE_SECTION_END [%s]", > @@ -13263,16 +13504,59 @@ store_load_module_symbols(bfd *bfd, int dynamic, > void *minisyms, > qsort(lm->mod_load_symtable, lm->mod_load_symcnt, sizeof(struct > syment), > compare_syms); > > + /* keep load symtable addresses to lm->load_symtable[] */ > + /* TODO: make more efficient */ > + for (sp = lm->mod_load_symtable; sp < lm->mod_load_symend; sp++) { > + char buf1[BUFSIZE], buf2[BUFSIZE]; > + > + if (CRASHDEBUG(1)) > + fprintf(fp, "DEBUG: value %16lx name %s\n", > sp->value, sp->name); > + if (!MODULE_PSEUDO_SYMBOL(sp)) > + continue; > + > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. > + if (!lm->mem[i].base) > + continue; > + > + sprintf(buf1, "%s%s", module_start_tags[i], > lm->mod_name); > + sprintf(buf2, "%s%s", module_end_tags[i], > lm->mod_name); > + > + if (STREQ(sp->name, buf1)) { > + lm->load_symtable[i] = sp; > + break; > + } else if (STREQ(sp->name, buf2)) { > + lm->load_symend[i] = sp; > + break; > + } > + } > + } > + > + /* TODO: add check for 6.4 and later */ > lm->mod_load_symend--; > - if (!MODULE_END(lm->mod_load_symend) && > + if (!MODULE_MEMORY() && !MODULE_END(lm->mod_load_symend) && > !IN_MODULE_PERCPU(lm->mod_load_symend->value, lm)) > error(INFO, "%s: last symbol: %s is not _MODULE_END_%s?\n", > lm->mod_name, lm->mod_load_symend->name, > lm->mod_name); > > - mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend); > - lm->mod_symtable = lm->mod_load_symtable; > - lm->mod_symend = lm->mod_load_symend; > - mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend); > + if (MODULE_MEMORY()) { > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. > + if (!lm->symtable[i]) > + continue; > + mod_symtable_hash_remove_range(lm->symtable[i], > lm->symend[i]); > + } > + lm->symtable = lm->load_symtable; > + lm->symend = lm->load_symend; > + for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) { > Ditto. Thanks. Lianbo + if (!lm->symtable[i]) > + continue; > + mod_symtable_hash_install_range(lm->symtable[i], > lm->symend[i]); > + } > + } else { > + mod_symtable_hash_remove_range(lm->mod_symtable, > lm->mod_symend); > + lm->mod_symtable = lm->mod_load_symtable; > + lm->mod_symend = lm->mod_load_symend; > + mod_symtable_hash_install_range(lm->mod_symtable, > lm->mod_symend); > + } > > lm->mod_flags &= ~MOD_EXT_SYMS; > lm->mod_flags |= MOD_LOAD_SYMS; > -- > 2.31.1 > >
-- Crash-utility mailing list Crash-utility@redhat.com https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki