On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com> wrote:
Signed-off-by: Kazuhito Hagio <k-hagio-ab@nec.com>
---
 symbols.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/symbols.c b/symbols.c
index 40e992e9ee12..5399c7494cb1 100644
--- a/symbols.c
+++ b/symbols.c
@@ -13565,7 +13565,7 @@ append_section_symbols:
 void
 delete_load_module(ulong base_addr)
 {
-       int i;
+       int i, j;
         struct load_module *lm;
        struct gnu_request request, *req;

@@ -13580,7 +13580,23 @@ delete_load_module(ulong base_addr)
                                req->name = lm->mod_namelist;
                                gdb_interface(req);
                        }
-                       mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
+                       if (MODULE_MEMORY()) {
+                               if (lm->mod_load_symtable) {
+                                       mod_symtable_hash_remove_range(lm->mod_load_symtable, lm->mod_load_symend);

The following code can be replaced with the macro for_each_mod_mem_type() or memset(), for example:
memset(lm->load_symtable, 0, MOD_MEM_NUM_TYPES-1);
memset(lm->load_symend, 0, MOD_MEM_NUM_TYPES-1);

The disadvantage is to invoke the memset() twice, but the code looks very clean.

+                                       for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
+                                               lm->load_symtable[j] = NULL;
+                                               lm->load_symend[j] = NULL;
+                                       }
+                               } else { /* somehow this function runs for unloaded modules */

Could you please explain why it may get into the "else" code path? And why do we need to handle this situation for now? But not needed before the module memory changes.
 
+                                       for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
+                                               if (!lm->symtable[j])
+                                                       continue;
+                                               mod_symtable_hash_remove_range(lm->symtable[j], lm->symend[j]);
+                                       }
+                               }
+                       } else
+                               mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
+
                        if (lm->mod_load_symtable) {
                                free(lm->mod_load_symtable);
                                 namespace_ctl(NAMESPACE_FREE,
@@ -13588,9 +13604,19 @@ delete_load_module(ulong base_addr)
                        }
                        if (lm->mod_flags & MOD_REMOTE)
                                unlink_module(lm);
-                       lm->mod_symtable = lm->mod_ext_symtable;
-                       lm->mod_symend = lm->mod_ext_symend;
-                       mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
+                       if (MODULE_MEMORY()) {
+                               lm->symtable = lm->ext_symtable;
+                               lm->symend = lm->ext_symend;
+                               for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
+                                       if (!lm->symtable[j])
+                                               continue;
+                                       mod_symtable_hash_install_range(lm->symtable[j], lm->symend[j]);
+                               }
+                       } else {
+                               lm->mod_symtable = lm->mod_ext_symtable;
+                               lm->mod_symend = lm->mod_ext_symend;
+                               mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
+                       }
                        lm->mod_flags &= ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH);
                        lm->mod_flags |= MOD_EXT_SYMS;
                        lm->mod_load_symtable = NULL;
@@ -13619,7 +13645,23 @@ delete_load_module(ulong base_addr)
                                req->name = lm->mod_namelist;
                                gdb_interface(req);
                        }
-                       mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
+                       if (MODULE_MEMORY())
+                               if (lm->mod_load_symtable) {
+                                       mod_symtable_hash_remove_range(lm->mod_load_symtable, lm->mod_load_symend);
+                                       for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {

Ditto.
 
+                                               lm->load_symtable[j] = NULL;
+                                               lm->load_symend[j] = NULL;
+                                       }
+                               } else { /* somehow this function runs for unloaded modules */

Ditto.

Thanks.
Lianbo
 
+                                       for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
+                                               if (!lm->symtable[j])
+                                                       continue;
+                                               mod_symtable_hash_remove_range(lm->symtable[j], lm->symend[j]);
+                                       }
+                               }
+                       else
+                               mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
+
                        if (lm->mod_load_symtable) {
                                free(lm->mod_load_symtable);
                                namespace_ctl(NAMESPACE_FREE,
@@ -13627,9 +13669,19 @@ delete_load_module(ulong base_addr)
                        }
                        if (lm->mod_flags & MOD_REMOTE)
                                unlink_module(lm);
-                       lm->mod_symtable = lm->mod_ext_symtable;
-                       lm->mod_symend = lm->mod_ext_symend;
-                       mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
+                       if (MODULE_MEMORY()) {
+                               lm->symtable = lm->ext_symtable;
+                               lm->symend = lm->ext_symend;
+                               for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
+                                       if (!lm->symtable[j])
+                                               continue;
+                                       mod_symtable_hash_install_range(lm->symtable[j], lm->symend[j]);
+                               }
+                       } else {
+                               lm->mod_symtable = lm->mod_ext_symtable;
+                               lm->mod_symend = lm->mod_ext_symend;
+                               mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
+                       }
                         lm->mod_flags &= ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH);
                         lm->mod_flags |= MOD_EXT_SYMS;
                         lm->mod_load_symtable = NULL;
--
2.31.1