2:47 a.m.
Hi Lianbo,
thank you for the review.
On 2023/06/20 10:46, lijiang wrote:
>> +#define for_each_mod_mem_type(type) \
>> + for (int (type) = MOD_TEXT; (type) < MOD_MEM_NUM_TYPES; (type)++)
I found that this cannot build with an old gcc, e.g. on RHEL7.
Please note that -std=gnu99 or later is required for such a gcc.
$ gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)
...
$ make -j 16 warn CFLAGS='-std=gnu99'
...
ar: creating crashlib.a
CXXLD gdb
$ make clean
...
$ make -j 16 warn
...
In file included from symbols.c:18:0:
symbols.c: In function 'module_symbol_dump':
defs.h:3007:2: error: 'for' loop initial declarations are only allowed
in C99 mode
for (int (type) = MOD_TEXT; (type) < MOD_MEM_NUM_TYPES; (type)++)
^
symbols.c:1352:3: note: in expansion of macro 'for_each_mod_mem_type'
for_each_mod_mem_type(t) {
^
defs.h:3007:2: note: use option -std=c99 or -std=gnu99 to compile your code
for (int (type) = MOD_TEXT; (type) < MOD_MEM_NUM_TYPES; (type)++)
^
symbols.c:1352:3: note: in expansion of macro 'for_each_mod_mem_type'
for_each_mod_mem_type(t) {
^
...
>> +--- 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);
>> + }
>> + }
>>
>
BTW: I can still get the following warnings:
...
CXX symtab.o
symtab.c: In function ‘void gdb_command_funnel_1(gnu_request*)’:
symtab.c:7519:64: warning: ‘%lx’ directive writing between 1 and 16 bytes
into a region of size between 10 and 73 [-Wformat-overflow=]
7519 | sprintf(buf, " -s %s
0x%lx", secname, lm->mod_section_data[i].addr);
| ^~~
symtab.c:7519:54: note: directive argument in the range [1,
18446744073709551615]
7519 | sprintf(buf, " -s %s
0x%lx", secname, lm->mod_section_data[i].addr);
| ^~~~~~~~~~~~~~
Oops, thanks, I missed this. will fix.
>> +static int module_mem_type(ulong, struct load_module *);
>> +static ulong module_mem_end(ulong, struct load_module *);
>> +static int in_module_range(ulong, struct load_module *, int, int);
>> +struct syment *value_search_module_6_4(ulong, ulong *);
>> +struct syment *next_symbol_by_symname(char *);
>> +struct syment *prev_symbol_by_symname(char *);
>> +struct syment *next_module_symbol_by_value(ulong);
>> +struct syment *prev_module_symbol_by_value(ulong);
>> +struct syment *next_module_symbol_by_syment(struct syment *);
>> +struct syment *prev_module_symbol_by_syment(struct syment *);
>> +
>>
>
> The above functions are only used in the symbols.c, It should be good to
> add a 'static' keyword to them.
Agreed.
>> +/* val_in should be a pseudo module end symbol. */
>> +struct syment *
>> +next_module_symbol_by_value(ulong val_in)
>> +{
>> + struct load_module *lm;
>> + struct syment *sp, *sp_end;
>> + ulong start, min;
>> + int i;
>> +
>> +retry:
>> + sp = sp_end = NULL;
>> + min = (ulong)-1;
>> + for (i = 0; i < st->mods_installed; i++) {
>> + lm = &st->load_modules[i];
>> +
>> + /* Search for the next lowest symtable. */
>> + for_each_mod_mem_type(t) {
>> + if (!lm->symtable[t])
>> + continue;
>> +
>> + start = lm->symtable[t]->value;
>> + if (start > val_in && start < min) {
>> + min = start;
>> + sp = lm->symtable[t];
>> + sp_end = lm->symend[t];
>> + }
>> + }
>> + }
>> +
>> + if (!sp)
>> + return NULL;
>> +
>> + for ( ; sp < sp_end; sp++) {
>> + if (MODULE_PSEUDO_SYMBOL(sp))
>> + continue;
>> + if (sp->value > val_in)
>> + return sp;
>> + }
>> +
>> + /* Found a table that has only pseudo symbols. */
>> + val_in = sp_end->value;
>> + goto retry;
>>
>
> Is it possible for 'retry' to become an infinite loop? And there is also a
> similar 'retry' in the prev_module_symbol_by_value().
No, it should return NULL if (!sp) as above, i.e. there is no
higher/lower module symbol.
For example, it tested ok with the highest module symbol:
crash-6.4> sym -M | sort | tail
ffffffffc2d242c0 (r) rd_ptr.16
ffffffffc2d24300 (r) suffixes.15
ffffffffc2d24340 (r) tjmax_model_table
ffffffffc2d24380 (r) tjmax_pci_table
ffffffffc2d243a0 (r) __param_str_tjmax
ffffffffc2d243a8 (r) __param
ffffffffc2d243a8 (r) __param_tjmax
ffffffffc2d25000 MODULE RODATA END: coretemp
ffffffffc2d26000 MODULE RO_AFTER_INIT START: coretemp
ffffffffc2d27000 MODULE RO_AFTER_INIT END: coretemp
crash-6.4> sym -n __param_tjmax
ffffffffc2d243a8 (r) __param_tjmax [coretemp]
crash-6.4>
Thanks,
Kazu