8:03 p.m.
On 2023/05/26 21:40, lijiang wrote:
On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com>
wrote:
> Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)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.
ok, thanks.
+ 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.
This path is used by "mod -d|-D" and reinit_modules() for unloaded modules.
The old procedure always does the cleanup and reinstall of the hash
entries, so kept it as it is.
Thanks,
Kazu
>
>
>> + 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
>>
>>
>>