linux-kernel - Re: [PATCH v2] perf callchain: Return directly when use '--call-graph dwarf' under !HAVE_DWARF

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] [day] [month] [year] [list]

Message-ID: <20201215153448.GA258566@kernel.org>
Date:   Tue, 15 Dec 2020 12:34:48 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Xuefeng Li <lixuefeng@...ngson.cn>
Subject: Re: [PATCH v2] perf callchain: Return directly when use
 '--call-graph dwarf' under !HAVE_DWARF_SUPPORT

Em Tue, Dec 15, 2020 at 10:49:59PM +0900, Namhyung Kim escreveu:
> Hello,
> 
> On Tue, Dec 15, 2020 at 10:35 AM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
> >
> > DWARF register mappings have not been defined for some architectures,
> > at least for mips, so we can print an error message and then return
> > directly when use '--call-graph dwarf'.
> >
> > E.g. without this patch:
> >
> > [root@...ux perf]# ./perf record --call-graph dwarf cd
> > Error:
> > The sys_perf_event_open() syscall returned with 89 (Function not implemented) for event (cycles).
> > /bin/dmesg | grep -i perf may provide additional information.
> >
> > With this patch:
> >
> > [root@...ux perf]# ./perf record --call-graph dwarf cd
> > DWARF is not supported for architecture mips64
> >
> >  Usage: perf record [<options>] [<command>]
> >     or: perf record [<options>] -- <command> [<options>]
> >
> >         --call-graph <record_mode[,record_size]>
> >                           setup and enables call-graph (stack chain/backtrace):
> >
> >                                 record_mode:    call graph recording mode (fp|dwarf|lbr)
> >                                 record_size:    if record_mode is 'dwarf', max size of stack recording (<bytes>)
> >                                                 default: 8192 (bytes)
> >
> >                                 Default: fp
> >
> > Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> > ---
> >
> > v2: Use HAVE_DWARF_SUPPORT to check
> 
> I'm not sure whether this is because of lack of dwarf library or kernel support.
> Based on the error message, I guess it's from the kernel.  Then I think this
> patch won't be sufficient..

tools/perf/Makefile.config

  ifndef NO_DWARF
    ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
      msg := $(warning DWARF register mappings have not been defined for architecture $(SRCARCH), DWARF support disabled);
      NO_DWARF := 1
    else
      CFLAGS += -DHAVE_DWARF_SUPPORT $(LIBDW_CFLAGS)
      LDFLAGS += $(LIBDW_LDFLAGS)
      EXTLIBS += ${DWARFLIBS}
      $(call detected,CONFIG_DWARF)
    endif # PERF_HAVE_DWARF_REGS
  endif # NO_DWARF

[acme@...e perf]$ find tools/perf -type f | xargs grep PERF_HAVE_DWARF_REGS
tools/perf/arch/xtensa/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/x86/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/arm64/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/arm/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/s390/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/powerpc/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/sh/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/riscv/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/sparc/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/csky/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/Makefile.config:    ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
tools/perf/Makefile.config:    endif # PERF_HAVE_DWARF_REGS
[acme@...e perf]$

Ouch:

[acme@...e perf]$ cat tools/perf/arch/xtensa/Makefile
# SPDX-License-Identifier: GPL-2.0-only
ifndef NO_DWARF
PERF_HAVE_DWARF_REGS := 1
endif
[acme@...e perf]$

So you have a point, only if NO_DWARF is not defined, then
PERF_HAVE_DWARF_REGS is can be used to define HAVE_DWARF_SUPPORT, too
clumsy.

So probably my hunch that this should be done at
evsel__open_strerror() and use perf_missing_features.dwarf_regs is
what we should go...

I'm removing this patch from my current tree, please take a look at:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/core&id=f55c66234c1967f6e56e56c3e084f80b417c124b

For how I did this for another feature that the kernel may or not
support, perf_event_open() fails, it notices that in
perf_missing_features and then later evsel__open_strerror() returns a
sensible warning, reacting to something the running kernel returned.

- Arnaldo