Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230721150147.11720-2-palmer@rivosinc.com>
In-Reply-To: <20230721150147.11720-2-palmer@rivosinc.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sat, 22 Jul 2023 02:02:56 +0900
Message-ID: <CAK7LNATF9pxJc0nU3NPnX_PFJr7gF6Baras548ULkO8XE98_VQ@mail.gmail.com>
Subject: Re: [PATCH] modpost, kallsyms: Treat add '$'-prefixed symbols as
 mapping symbols
To:     Palmer Dabbelt <palmer@rivosinc.com>
Cc:     wangkefeng.wang@huawei.com, mcgrof@kernel.org, nathan@kernel.org,
        ndesaulniers@google.com, nicolas@fjasle.eu,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-kbuild@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sat, Jul 22, 2023 at 12:20=E2=80=AFAM Palmer Dabbelt <palmer@rivosinc.co=
m> 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@=
huawei.com/
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>


Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>


> ---
> 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 fi=
les. */
> -static inline int is_mapping_symbol(const char *str, int is_riscv)
> +static inline int is_mapping_symbol(const char *str)
>  {
>         if (str[0] =3D=3D '.' && str[1] =3D=3D 'L')
>                 return true;
>         if (str[0] =3D=3D 'L' && str[1] =3D=3D '0')
>                 return true;
> -       /*
> -        * RISC-V defines various special symbols that start with "$".  T=
he
> -        * mapping symbols, which exist to differentiate between incompat=
ible
> -        * 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] =3D=3D '$';
> -
> -       return str[0] =3D=3D '$' &&
> -              (str[1] =3D=3D 'a' || str[1] =3D=3D 'd' || str[1] =3D=3D '=
t' || str[1] =3D=3D 'x')
> -              && (str[2] =3D=3D '\0' || str[2] =3D=3D '.');
> +       return str[0] =3D=3D '$';
>  }
>
>  #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) =3D=3D '\0' ||
> -                   is_mapping_symbol(kallsyms_symbol_name(kallsyms, i), =
IS_ENABLED(CONFIG_RISCV)))
> +                   is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
>                         continue;
>
>                 if (thisval <=3D 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 *el=
f, Elf_Sym *sym)
>
>         if (!name || !strlen(name))
>                 return 0;
> -       return !is_mapping_symbol(name, elf->hdr->e_machine =3D=3D EM_RIS=
CV);
> +       return !is_mapping_symbol(name);
>  }
>
>  /* Look up the nearest symbol based on the section and the address */
> --
> 2.41.0
>


--=20
Best Regards
Masahiro Yamada