| 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
| ||
|
Message-ID: <Zy0SmlDrmksra_OD@google.com>
Date: Thu, 7 Nov 2024 11:18:50 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Masami Hiramatsu <mhiramat@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...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
Hello,
Sorry for the late reply.
On Mon, Oct 07, 2024 at 08:46:59AM -0700, Ian Rogers wrote:
> 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.
Yep, it's used only in the data type profiling. I don't expect it to
work in the cross environment yet. This change moves it closer to the
working state though. :)
> 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.
Yep, as I said it's not supported yet and I don't think that's the goal
of this patch series. So probably it's ok to merge it without the
extensive testing.
Thanks,
Namhyung
>
> > > 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