On 2023/05/26 15:20, lijiang wrote:
> On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>
> 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-ab@xxxxxxx>
>> ---
>> 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.
Modifying the gdb patch will be unavoidable, because the calculation in
gdb/symtab.c is different from the existing one.
And if we reuse the offset member, anyway we need an additional flag or
something to switch the calculation above. So I decided to add the addr
member to clarify its meaning.
>
> };
>>
>> /* 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.
These are the information of each module, I think they should not be
moved to struct symbol_table_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.
Yes, agree, also I was thinking about something like this.
will try it in the next version.
(I'd like to use int as I wrote in the 02/15 thread though :)
Thanks,
Kazu
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
