Re: [PATCH] modpost, kallsyms: Treat add '$'-prefixed symbols as mapping symbols

[Masahiro Yamada]

On Sat, Jul 22, 2023 at 12:20 AM Palmer Dabbelt <palmer@xxxxxxxxxxxx> wrote:
>
> Trying to restrict the '$'-prefix change to RISC-V caused some fallout,
> so let's just treat all those symbols as special.
>
> Fixes: c05780ef3c1 ("module: Ignore RISC-V mapping symbols too")
> Link: https://lore.kernel.org/all/20230712015747.77263-1-wangkefeng.wang@xxxxxxxxxx/
> Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx>


Reviewed-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>


> ---
> It wasn't clear if we should just revert the original patch and spin a
> new one, but looks like this is in at least some trees so I sent a
> follow-on patch -- at least this way I can make coffee before forgetting
> to send it ;)
> ---
>  include/linux/module_symbol.h | 16 ++--------------
>  kernel/module/kallsyms.c      |  2 +-
>  scripts/mod/modpost.c         |  2 +-
>  3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
> index 5b799942b243..1269543d0634 100644
> --- a/include/linux/module_symbol.h
> +++ b/include/linux/module_symbol.h
> @@ -3,25 +3,13 @@
>  #define _LINUX_MODULE_SYMBOL_H
>
>  /* This ignores the intensely annoying "mapping symbols" found in ELF files. */
> -static inline int is_mapping_symbol(const char *str, int is_riscv)
> +static inline int is_mapping_symbol(const char *str)
>  {
>         if (str[0] == '.' && str[1] == 'L')
>                 return true;
>         if (str[0] == 'L' && str[1] == '0')
>                 return true;
> -       /*
> -        * RISC-V defines various special symbols that start with "$".  The
> -        * mapping symbols, which exist to differentiate between incompatible
> -        * instruction encodings when disassembling, show up all over the place
> -        * and are generally not meant to be treated like other symbols.  So
> -        * just ignore any of the special symbols.
> -        */
> -       if (is_riscv)
> -               return str[0] == '$';
> -
> -       return str[0] == '$' &&
> -              (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x')
> -              && (str[2] == '\0' || str[2] == '.');
> +       return str[0] == '$';
>  }
>
>  #endif /* _LINUX_MODULE_SYMBOL_H */
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index 78a1ffc399d9..ef73ae7c8909 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -289,7 +289,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
>                  * and inserted at a whim.
>                  */
>                 if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
> -                   is_mapping_symbol(kallsyms_symbol_name(kallsyms, i), IS_ENABLED(CONFIG_RISCV)))
> +                   is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
>                         continue;
>
>                 if (thisval <= addr && thisval > bestval) {
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 7c71429d6502..b29b29707f10 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1052,7 +1052,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
>
>         if (!name || !strlen(name))
>                 return 0;
> -       return !is_mapping_symbol(name, elf->hdr->e_machine == EM_RISCV);
> +       return !is_mapping_symbol(name);
>  }
>
>  /* Look up the nearest symbol based on the section and the address */
> --
> 2.41.0
>


-- 
Best Regards
Masahiro Yamada