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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fU4hTL1hfB7-FpMnFopJJriZAOXY_8iakW5yHC_gfhTWg@mail.gmail.com>
Date: Mon, 7 Oct 2024 08:46:59 -0700
From: Ian Rogers <irogers@...gle.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>, 
	James Clark <james.clark@...aro.org>, Mike Leach <mike.leach@...aro.org>, 
	Leo Yan <leo.yan@...ux.dev>, Guo Ren <guoren@...nel.org>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Nick Terrell <terrelln@...com>, 
	"Steven Rostedt (Google)" <rostedt@...dmis.org>, Guilherme Amadio <amadio@...too.org>, 
	Changbin Du <changbin.du@...wei.com>, Daniel Bristot de Oliveira <bristot@...nel.org>, 
	Daniel Wagner <dwagner@...e.de>, Aditya Gupta <adityag@...ux.ibm.com>, 
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, Masahiro Yamada <masahiroy@...nel.org>, 
	Kajol Jain <kjain@...ux.ibm.com>, Huacai Chen <chenhuacai@...nel.org>, 
	Bibo Mao <maobibo@...ngson.cn>, Anup Patel <anup@...infault.org>, 
	Atish Patra <atishp@...osinc.com>, Shenlin Liang <liangshenlin@...incomputing.com>, 
	Oliver Upton <oliver.upton@...ux.dev>, "Steinar H. Gunderson" <sesse@...gle.com>, 
	"Dr. David Alan Gilbert" <linux@...blig.org>, Chen Pei <cp0613@...ux.alibaba.com>, 
	Dima Kogan <dima@...retsauce.net>, Yury Norov <yury.norov@...il.com>, 
	Alexander Lobakin <aleksander.lobakin@...el.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-csky@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 16/31] perf dwarf-regs: Pass accurate disassembly
 machine to get_dwarf_regnum

On Mon, Oct 7, 2024 at 1:08 AM Masami Hiramatsu <mhiramat@...nel.org> wrote:
>
> On Sat,  5 Oct 2024 12:55:26 -0700
> Ian Rogers <irogers@...gle.com> wrote:
>
> > Rather than pass 0/EM_NONE, use the value computed in the disasm
> > struct arch. Switch the EM_NONE case to EM_HOST, rewriting EM_NONE if
> > it were passed to get_dwarf_regnum. Pass a flags value as
> > architectures like csky need the flags to determine the ABI variant.
> >
>
> Does this change the command output when we use it for cross-build
> environment? E.g. remote arch is different from host arch? If so,
> please add output examples with/without this change.

The cases where this would apply are small as get_arch_regnum is only
implemented for x86. get_dwarf_regnum likewise only works for x86 and
it is only called by annotate.
In this code without this patch the behavior is to return -ENOTSUP, ie
the code is set up to fail and this code just makes it not fail for
the x86 case (when not on x86) with code that is well tested on x86.
The code exists as x86 registers may be the same dwarf number but have
different names: e.g. rax, eax, ax, al. I'm not sure this reaches a
high complexity level for extensive testing. I'll see if I can grab an
x86 perf.data file to analyze on ARM, but I don't think doing this
should gate the series.

Thanks,
Ian

> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/perf/util/annotate.c           | 6 +++---
> >  tools/perf/util/dwarf-regs.c         | 8 ++++++--
> >  tools/perf/util/include/dwarf-regs.h | 5 +++--
> >  3 files changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 37ce43c4eb8f..b1d98da79be8 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -2292,7 +2292,7 @@ static int extract_reg_offset(struct arch *arch, const char *str,
> >       if (regname == NULL)
> >               return -1;
> >
> > -     op_loc->reg1 = get_dwarf_regnum(regname, 0);
> > +     op_loc->reg1 = get_dwarf_regnum(regname, arch->e_machine, arch->e_flags);
> >       free(regname);
> >
> >       /* Get the second register */
> > @@ -2305,7 +2305,7 @@ static int extract_reg_offset(struct arch *arch, const char *str,
> >               if (regname == NULL)
> >                       return -1;
> >
> > -             op_loc->reg2 = get_dwarf_regnum(regname, 0);
> > +             op_loc->reg2 = get_dwarf_regnum(regname, arch->e_machine, arch->e_flags);
> >               free(regname);
> >       }
> >       return 0;
> > @@ -2405,7 +2405,7 @@ int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl,
> >                               return -1;
> >
> >                       if (*s == arch->objdump.register_char)
> > -                             op_loc->reg1 = get_dwarf_regnum(s, 0);
> > +                             op_loc->reg1 = get_dwarf_regnum(s, arch->e_machine, arch->e_flags);
> >                       else if (*s == arch->objdump.imm_char) {
> >                               op_loc->offset = strtol(s + 1, &p, 0);
> >                               if (p && p != s + 1)
> > diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
> > index 7c01bc4d7e5b..1321387f6948 100644
> > --- a/tools/perf/util/dwarf-regs.c
> > +++ b/tools/perf/util/dwarf-regs.c
> > @@ -70,7 +70,7 @@ __weak int get_arch_regnum(const char *name __maybe_unused)
> >  }
> >
> >  /* Return DWARF register number from architecture register name */
> > -int get_dwarf_regnum(const char *name, unsigned int machine)
> > +int get_dwarf_regnum(const char *name, unsigned int machine, unsigned int flags __maybe_unused)
> >  {
> >       char *regname = strdup(name);
> >       int reg = -1;
> > @@ -84,8 +84,12 @@ int get_dwarf_regnum(const char *name, unsigned int machine)
> >       if (p)
> >               *p = '\0';
> >
> > +     if (machine == EM_NONE) {
> > +             /* Generic arch - use host arch */
> > +             machine = EM_HOST;
> > +     }
> >       switch (machine) {
> > -     case EM_NONE:   /* Generic arch - use host arch */
> > +     case EM_HOST:
> >               reg = get_arch_regnum(regname);
> >               break;
> >       default:
> > diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
> > index f4f87ded5e3d..ee0a734564c7 100644
> > --- a/tools/perf/util/include/dwarf-regs.h
> > +++ b/tools/perf/util/include/dwarf-regs.h
> > @@ -93,12 +93,13 @@ int get_arch_regnum(const char *name);
> >   * name: architecture register name
> >   * machine: ELF machine signature (EM_*)
> >   */
> > -int get_dwarf_regnum(const char *name, unsigned int machine);
> > +int get_dwarf_regnum(const char *name, unsigned int machine, unsigned int flags);
> >
> >  #else /* HAVE_LIBDW_SUPPORT */
> >
> >  static inline int get_dwarf_regnum(const char *name __maybe_unused,
> > -                                unsigned int machine __maybe_unused)
> > +                                unsigned int machine __maybe_unused,
> > +                                unsigned int flags __maybe_unused)
> >  {
> >       return -1;
> >  }
> > --
> > 2.47.0.rc0.187.ge670bccf7e-goog
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@...nel.org>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ