linux-kernel - Re: [PATCH 2/9] perf/jit: enable jitdump support without 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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 13 Oct 2016 15:37:42 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Maciej Debski <maciejd@...gle.com>
Cc:     Stephane Eranian <eranian@...gle.com>,
        linux-kernel@...r.kernel.org, jolsa@...hat.com,
        peterz@...radead.org, Ingo Molnar <mingo@...nel.org>,
        anton@...abs.org, namhyung@...nel.org
Subject: Re: [PATCH 2/9] perf/jit: enable jitdump support without dwarf

Em Thu, Oct 13, 2016 at 03:16:43PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Oct 13, 2016 at 03:59:36AM -0700, Stephane Eranian escreveu:
> > From: Maciej Debski <maciejd@...gle.com>
> > 
> > This patch modifies the build dependencies on the jitdump
> > support in perf. As it stands jitdump was wrongfully made dependent
> > 100% on using DWARF. However, the dwarf dependency, only exist if
> > generating the source line table in genelf_debug.c. The rest of the
> > support does not need DWARF.
> > 
> > This patch removes the dependency on DWARF for the entire jitdump
> > support. It keeps it only for the genelf_debug.c support.
> 
> Please add a:
> 
> Fixes: e12b202f8fb9 ("perf jitdump: Build only on supported archs")
> 
> Next time, so that scripts can pick patches for stable kernels, etc.
> 
> Adding it this time.

But, this introduces a bug, breaking the perf's static build:

[acme@...et linux]$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[f5722b101fb37339001e9524262d3eb08b7b1bfe] perf jit: Enable jitdump support without dwarf
[acme@...et linux]$ rm -rf /tmp/build/perf/ ; mkdir -p /tmp/build/perf ; make -C tools/perf LDFLAGS=-static O=/tmp/build/perf install-bin

<big SNIP>

/tmp/build/perf/perf-in.o: In function `perf_event__jit_repipe_mmap2':
/home/acme/git/linux/tools/perf/builtin-inject.c:298: undefined reference to `jit_process'
/tmp/build/perf/perf-in.o: In function `perf_event__jit_repipe_mmap':
/home/acme/git/linux/tools/perf/builtin-inject.c:260: undefined reference to `jit_process'
collect2: error: ld returned 1 exit status
Makefile.perf:470: recipe for target '/tmp/build/perf/perf' failed
make[1]: *** [/tmp/build/perf/perf] Error 1
Makefile:108: recipe for target 'install-bin' failed
make: *** [install-bin] Error 2
make: Leaving directory '/home/acme/git/linux/tools/perf'
[acme@...et linux]$ git bisect bad
f5722b101fb37339001e9524262d3eb08b7b1bfe is the first bad commit
commit f5722b101fb37339001e9524262d3eb08b7b1bfe
Author: Maciej Debski <maciejd@...gle.com>
Date:   Thu Oct 13 03:59:36 2016 -0700

    perf jit: Enable jitdump support without dwarf
    
    This patch modifies the build dependencies on the jitdump support in
    perf. As it stands jitdump was wrongfully made dependent 100% on using
    DWARF. However, the dwarf dependency, only exist if generating the
    source line table in genelf_debug.c. The rest of the support does not
    need DWARF.
    
    This patch removes the dependency on DWARF for the entire jitdump
    support. It keeps it only for the genelf_debug.c support.
    
    Signed-off-by: Maciej Debski <maciejd@...gle.com>
    Reviewed-by: Stephane Eranian <eranian@...gle.com>
    Cc: Anton Blanchard <anton@...abs.org>
    Cc: Jiri Olsa <jolsa@...hat.com>
    Cc: Namhyung Kim <namhyung@...nel.org>
    Cc: Peter Zijlstra <peterz@...radead.org>
    Link: http://lkml.kernel.org/r/1476356383-30100-3-git-send-email-eranian@google.com
    Fixes: e12b202f8fb9 ("perf jitdump: Build only on supported archs")
    Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>

:040000 040000 281764609ccfd745f984caae6a331b5b9ebd15b0 4f1ebed718481cacdd901f28a05b347a135e46cf M	tools
[acme@...et linux]$

Please use:

  make -C tools/perf build-test

That will do may build tests, including this one, the static build.

I'm trying to figure this out...

- Arnaldo
 
> Thanks,
> 
> - Arnaldo
> 
>  
> > Signed-off-by: Maciej Debski <maciejd@...gle.com>
> > Reviewed-by: Stephane Eranian <eranian@...gle.com>
> > ---
> >  tools/perf/Makefile.config | 2 --
> >  tools/perf/util/Build      | 2 +-
> >  tools/perf/util/genelf.c   | 9 +++++++--
> >  tools/perf/util/genelf.h   | 2 ++
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 72edf83d76b7..8cfc310d4358 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -366,10 +366,8 @@ ifndef NO_SDT
> >  endif
> >  
> >  ifdef PERF_HAVE_JITDUMP
> > -  ifndef NO_DWARF
> >      $(call detected,CONFIG_JITDUMP)
> >      CFLAGS += -DHAVE_JITDUMP
> > -  endif
> >  endif
> >  
> >  ifeq ($(ARCH),powerpc)
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index eb60e613d795..1dc67efad634 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -120,7 +120,7 @@ libperf-y += demangle-rust.o
> >  ifdef CONFIG_JITDUMP
> >  libperf-$(CONFIG_LIBELF) += jitdump.o
> >  libperf-$(CONFIG_LIBELF) += genelf.o
> > -libperf-$(CONFIG_LIBELF) += genelf_debug.o
> > +libperf-$(CONFIG_DWARF) += genelf_debug.o
> >  endif
> >  
> >  CFLAGS_config.o   += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
> > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> > index c1ef805c6a8f..14a73acc549c 100644
> > --- a/tools/perf/util/genelf.c
> > +++ b/tools/perf/util/genelf.c
> > @@ -19,7 +19,9 @@
> >  #include <limits.h>
> >  #include <fcntl.h>
> >  #include <err.h>
> > +#ifdef HAVE_DWARF_SUPPORT
> >  #include <dwarf.h>
> > +#endif
> >  
> >  #include "perf.h"
> >  #include "genelf.h"
> > @@ -157,7 +159,7 @@ gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *cod
> >  int
> >  jit_write_elf(int fd, uint64_t load_addr, const char *sym,
> >  	      const void *code, int csize,
> > -	      void *debug, int nr_debug_entries)
> > +	      void *debug __maybe_unused, int nr_debug_entries __maybe_unused)
> >  {
> >  	Elf *e;
> >  	Elf_Data *d;
> > @@ -386,11 +388,14 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
> >  	shdr->sh_size = sizeof(bnote);
> >  	shdr->sh_entsize = 0;
> >  
> > +#ifdef HAVE_DWARF_SUPPORT
> >  	if (debug && nr_debug_entries) {
> >  		retval = jit_add_debug_info(e, load_addr, debug, nr_debug_entries);
> >  		if (retval)
> >  			goto error;
> > -	} else {
> > +	} else
> > +#endif
> > +	{
> >  		if (elf_update(e, ELF_C_WRITE) < 0) {
> >  			warnx("elf_update 4 failed");
> >  			goto error;
> > diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h
> > index 2fbeb59c4bdd..5c933ac71451 100644
> > --- a/tools/perf/util/genelf.h
> > +++ b/tools/perf/util/genelf.h
> > @@ -4,8 +4,10 @@
> >  /* genelf.c */
> >  int jit_write_elf(int fd, uint64_t code_addr, const char *sym,
> >  		  const void *code, int csize, void *debug, int nr_debug_entries);
> > +#ifdef HAVE_DWARF_SUPPORT
> >  /* genelf_debug.c */
> >  int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries);
> > +#endif
> >  
> >  #if   defined(__arm__)
> >  #define GEN_ELF_ARCH	EM_ARM
> > -- 
> > 1.9.1