On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com> wrote:
and fix off-by-one bug in "help -s" output with CRASHDEBUG(1)

Signed-off-by: Kazuhito Hagio <k-hagio-ab@nec.com>
---
 symbols.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 2 deletions(-)

diff --git a/symbols.c b/symbols.c
index 5399c7494cb1..a432909ff28e 100644
--- a/symbols.c
+++ b/symbols.c
@@ -110,6 +110,7 @@ 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_v2(ulong, ulong *);
+struct syment *next_module_symbol(char *, struct syment *, ulong);

 static const char *module_start_tags[];
 static const char *module_start_strs[];
@@ -4116,9 +4117,9 @@ dump_symbol_table(void)

                fprintf(fp, "        loaded_objfile: %lx\n", (ulong)lm->loaded_objfile);

-               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?

                                fprintf(fp, "  %lx  %s\n",
                                        sp->value, sp->name);   
                        }
@@ -5857,6 +5858,106 @@ closest_symbol_value(ulong value)
                 return(0);
 }

+/* 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.

+                       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;
+       }
+       */

I tried to find the old code, but still did not get the changed history, and did not see the similar symbols  " module)" in the latest kernel.

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.

+        if ((sp = symbol_search(symbol))) {
+               sp++;
+               if (MODULE_PSEUDO_SYMBOL(sp) && strstr(sp->name, "_END"))
+                       return next_module_symbol(NULL, NULL, sp->value);
+
+               return sp;
+       }
+
+       return NULL;
+}
+
 /*
  *  For a given symbol, return a pointer to the next (higher) symbol's syment.
  *  Either a symbol name or syment pointer may be passed as an argument.
@@ -5886,6 +5987,9 @@ next_symbol(char *symbol, struct syment *sp_in)

                search_init = FALSE;

+               if (MODULE_MEMORY())
+                       return next_module_symbol(NULL, sp_in, 0);
+

The above if-code block should be moved before the "search_init = FALSE".

Thanks.
Lianbo

                for (i = 0; i < st->mods_installed; i++) {
                        lm = &st->load_modules[i];
                        if (lm->mod_flags & MOD_INIT)
@@ -5928,6 +6032,9 @@ next_symbol(char *symbol, struct syment *sp_in)
        }


+       if (MODULE_MEMORY())
+               return next_module_symbol(symbol, NULL, 0);
+
        /*
         *  Deal with a few special cases...
         */
--
2.31.1