9:33 p.m.
On 2023/06/01 18:46, lijiang wrote:
On Thu, Jun 1, 2023 at 4:01 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com>
wrote:
>>> - if (CRASHDEBUG(1)) {
>>> + if (CRASHDEBUG(1) && lm->mod_load_symtable) {
>>> for (sp = lm->mod_load_symtable;
>>> - sp < lm->mod_load_symend; sp++) {
>>> + sp <= lm->mod_load_symend; sp++) {
>>>
>>
>> The real problem might not be here? Some member variables are not
> properly
>> initialized?
>
> sorry, I don't understand this.
>
There is the read problem here. lm->mod_load_symend is the last
> valid syment address.
>
Ok, Thank you for the explanation, Kazu. But I saw the following code in
other patches(06/15,etc):
+ for (sp = lm->mod_load_symtable; sp <
lm->mod_load_symend; sp++) {
Are there any differences?
Yes, because those are before this subtraction in store_load_module_symbols():
lm->mod_load_symend--;
if (!MODULE_MEMORY() && !MODULE_END(lm->mod_load_symend) &&
>
>>> +/* Only for 6.4 and later */
>>> +struct syment *
>>> +next_module_symbol(char *symbol, struct syment *sp_in, ulong val_in)
>>> +{
>>> + int i, j, k;
>>> + struct load_module *lm;
>>> + struct syment *sp, *sp_end;
>>> +
>>> + if (symbol)
>>> + goto symbol_search;
>>> + if (val_in)
>>> + goto value_search;
>>> +
>>> + /* for sp_in */
>>> + for (i = 0; i < st->mods_installed; i++) {
>>> + lm = &st->load_modules[i];
>>> +
>>> + /* quick check: sp_in is not in the module range. */
>>> + if (sp_in < lm->symtable[lm->address_order[0]] ||
>>> + sp_in >
> lm->symend[lm->address_order[lm->nr_mems-1]])
>>> + continue;
>>> +
>>> + for (j = 0; j < lm->nr_mems; j++) {
>>> + k = lm->address_order[j];
>>> + if (sp_in < lm->symtable[k] || sp_in >
>>> lm->symend[k])
>>> + continue;
>>> +
>>> + if (sp_in == lm->symend[k])
>>> + return next_module_symbol(NULL, NULL,
>>> sp_in->value);
>>> +
>>>
>>
>> This means it has to be invoked recursively.
>
> It looks to be a recursive call, but actually it's just a composite
> of three functions, i.e. symbol, syment and value search, and the
> value search does not do a recursive call. So I think it's not a
> real recursive call.
>
>
Ok, but it looks unusual and hard to understand.
ok, I will try to split the function into e.g.
next_module_symbol_by_syment()
next_module_symbol_by_symname()
next_module_symbol_by_value()
Thanks,
Kazu
>>
>> + sp = sp_in + 1;
>>> + if (MODULE_PSEUDO_SYMBOL(sp))
>>> + return next_module_symbol(NULL, NULL,
>>> sp->value);
>>> +
>>> + return sp;
>>> + }
>>> + }
>>> + return NULL;
>>> +
>>> +value_search:
>>> + sp = sp_end = NULL;
>>> + for (i = 0; i < st->mods_installed; i++) {
>>> + lm = &st->load_modules[i];
>>> +
>>> + /* quick check: val_in is higher than the highest in the
>>> module. */
>>> + if (val_in >
>>> lm->symend[lm->address_order[lm->nr_mems-1]]->value)
>>> + continue;
>>> +
>>> + for (j = 0; j < lm->nr_mems; j++) {
>>> + k = lm->address_order[j];
>>> + if (val_in < lm->symtable[k]->value
&&
>>> + (sp == NULL || lm->symtable[k]->value
<
>>> sp->value)) {
>>> + sp = lm->symtable[k];
>>> + sp_end = lm->symend[k];
>>> + break;
>>> + }
>>> + }
>>> + }
>>> + for ( ; sp < sp_end; sp++) {
>>> + if (MODULE_PSEUDO_SYMBOL(sp))
>>> + continue;
>>> + if (sp->value > val_in)
>>> + return sp;
>>> + }
>>> + return NULL;
>>> +
>>> +symbol_search:
>>> + /*
>>> + * Deal with a few special cases...
>>> + *
>>> + * hmm, looks like crash now does not use these special cases.
>>> + *
>>> + if (strstr(symbol, " module)")) {
>>> + sprintf(buf, "_MODULE_START_");
>>> + strcat(buf, &symbol[1]);
>>> + p1 = strstr(buf, " module)");
>>> + *p1 = NULLCHAR;
>>> + symbol = buf;
>>> + }
>>> +
>>> + if (STREQ(symbol, "_end")) {
>>> + if (!st->mods_installed)
>>> + return NULL;
>>> +
>>> + lm = &st->load_modules[0];
>>> +
>>> + return lm->mod_symtable;
>>> + }
>>> + */
>>>
>>
>> The symbol_search code block can be moved to the beginning of this
>> function, the current code has become very simple, that can avoid the
> goto
>> statement.
>
> Isn't it pointless?
> If we do so, we will need something like "goto syment_search" instead.
>
>
It's true, I did not see that the next_module_symbol() is called again the
label symbol_search.
Thanks
Lianbo