linux-kernel - Re: [PATCH] perf tools riscv: Add support for riscv lookup_binutils

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fWkAqX+Uy_XhXHjEx6sj-wpOJ+WQf=vMtRzRBS-0Kj32Q@mail.gmail.com>
Date:   Sat, 11 Mar 2023 22:27:41 -0800
From:   Ian Rogers <irogers@...gle.com>
To:     paranlee <p4ranlee@...il.com>
Cc:     Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Ananth N Mavinakayanahalli <ananth@...ibm.com>,
        Anton Blanchard <anton@...abs.org>,
        Daniel Axtens <dja@...ens.net>,
        Michael Ellerman <mpe@...erman.id.au>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-riscv@...ts.infradead.org
Subject: Re: [PATCH] perf tools riscv: Add support for riscv lookup_binutils_path

On Sat, Mar 11, 2023 at 3:22 AM paranlee <p4ranlee@...il.com> wrote:
>
> Add to know RISC-V binutils path.
> Secondarily, edit the code block with alphabetical order.
>
> Signed-off-by: Paran Lee <p4ranlee@...il.com>
> ---
>  tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 59dd875fd5e4..058527ededdd 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -29,11 +29,23 @@ const char *const arm_triplets[] = {
>  };
>
>  const char *const arm64_triplets[] = {
> +       "aarch64-unknown-linux-",

Modifying ARM64 behavior should be a separate change.

>         "aarch64-linux-android-",
>         "aarch64-linux-gnu-",
>         NULL
>  };
>
> +const char *const mips_triplets[] = {
> +       "mips-unknown-linux-gnu-",
> +       "mipsel-linux-android-",
> +       "mips-linux-gnu-",
> +       "mips64-linux-gnu-",
> +       "mips64el-linux-gnuabi64-",
> +       "mips64-linux-gnuabi64-",
> +       "mipsel-linux-gnu-",
> +       NULL
> +};
> +

This will affect the blame history. It should probably be its own change too.

>  const char *const powerpc_triplets[] = {
>         "powerpc-unknown-linux-gnu-",
>         "powerpc-linux-gnu-",
> @@ -43,6 +55,20 @@ const char *const powerpc_triplets[] = {
>         NULL
>  };
>
> +const char *const riscv32_triplets[] = {
> +       "riscv32-unknown-linux-gnu-",
> +       "riscv32-linux-android-",
> +       "riscv32-linux-gnu-",
> +       NULL
> +};
> +
> +const char *const riscv64_triplets[] = {
> +       "riscv64-unknown-linux-gnu-",
> +       "riscv64-linux-android-",
> +       "riscv64-linux-gnu-",
> +       NULL
> +};
> +
>  const char *const s390_triplets[] = {
>         "s390-ibm-linux-",
>         "s390x-linux-gnu-",
> @@ -78,17 +104,6 @@ const char *const x86_triplets[] = {
>         NULL
>  };
>
> -const char *const mips_triplets[] = {
> -       "mips-unknown-linux-gnu-",
> -       "mipsel-linux-android-",
> -       "mips-linux-gnu-",
> -       "mips64-linux-gnu-",
> -       "mips64el-linux-gnuabi64-",
> -       "mips64-linux-gnuabi64-",
> -       "mipsel-linux-gnu-",
> -       NULL
> -};
> -
>  static bool lookup_path(char *name)
>  {
>         bool found = false;
> @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env,
>                 path_list = arm_triplets;
>         else if (!strcmp(arch, "arm64"))
>                 path_list = arm64_triplets;
> +       else if (!strcmp(arch, "mips"))
> +               path_list = mips_triplets;
>         else if (!strcmp(arch, "powerpc"))
>                 path_list = powerpc_triplets;
> -       else if (!strcmp(arch, "sh"))
> -               path_list = sh_triplets;
> +       else if (!strcmp(arch, "riscv32"))
> +               path_list = riscv32_triplets;
> +       else if (!strcmp(arch, "riscv64"))
> +               path_list = riscv64_triplets;
>         else if (!strcmp(arch, "s390"))
> -               path_list = s390_triplets;
> +               path_list = s390_triplets;

whitespace issue?

> +       else if (!strcmp(arch, "sh"))
> +               path_list = sh_triplets;
>         else if (!strcmp(arch, "sparc"))
>                 path_list = sparc_triplets;
>         else if (!strcmp(arch, "x86"))
>                 path_list = x86_triplets;
> -       else if (!strcmp(arch, "mips"))
> -               path_list = mips_triplets;
>         else {
>                 ui__error("binutils for %s not supported.\n", arch);
>                 goto out_error;

I think in general we need to revamp this code. Two things that I see
that are missing are (1) support for perf config and (2) addr2line
should be configurable, you may want llvm-addr2line. Adding RISC-V is
of course important too :-)

Thanks,
Ian

> --
> 2.34.1
>