[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20200708183351.GB3581918@krava>
Date: Wed, 8 Jul 2020 20:33:51 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: Rémi Bernon <rbernon@...eweavers.com>
Cc: linux-kernel@...r.kernel.org,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Jacek Caban <jacek@...eweavers.com>
Subject: Re: [PATCH 2/3] perf symbols: Try reading the symbol table with
libbfd
On Wed, Jul 08, 2020 at 11:44:24AM +0200, Rémi Bernon wrote:
> On 2020-06-24 15:40, Rémi Bernon wrote:
> > On 2020-06-24 15:25, Jiri Olsa wrote:
> > > On Wed, Jun 24, 2020 at 12:30:40PM +0200, Remi Bernon wrote:
> > >
> > > SNIP
> > >
> > > > +
> > > > + symbols__insert(&dso->symbols, symbol);
> > > > + }
> > > > +#ifdef bfd_get_section
> > > > +#undef bfd_asymbol_section
> > > > +#endif
> > > > +
> > > > + symbols__fixup_end(&dso->symbols);
> > > > + symbols__fixup_duplicate(&dso->symbols);
> > > > + dso->adjust_symbols = 1;
> > > > +
> > > > + err = 0;
> > > > +out_free:
> > > > + free(symbols);
> > > > +out_close:
> > > > + bfd_close(abfd);
> > > > + return err;
> > > > +}
> > > > +#endif
> > > > +
> > > > static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
> > > > enum dso_binary_type type)
> > > > {
> > > > @@ -1695,6 +1816,7 @@ int dso__load(struct dso *dso, struct map *map)
> > > > bool next_slot = false;
> > > > bool is_reg;
> > > > bool nsexit;
> > > > + int bfdrc = -1;
> > > > int sirc = -1;
> > > > enum dso_binary_type symtab_type = binary_type_symtab[i];
> > > > @@ -1713,12 +1835,19 @@ int dso__load(struct dso *dso, struct map *map)
> > > > nsinfo__mountns_exit(&nsc);
> > > > is_reg = is_regular_file(name);
> > > > +#ifdef HAVE_LIBBFD_SUPPORT
> > > > if (is_reg)
> > > > + bfdrc = dso__load_bfd_symbols(dso, name);
> > > > +#endif
> > >
> > > so the comment says that we try bfd when the libelf fails,
> > > but the code seems to do the oposite.. starts with bfd
> > > and skip libelf if it's ok
> > >
> > > jirka
> > >
> >
> > Ah, sorry I forgot to reword the commit comment to match the changes,
> > thanks for noticing. It should probably be something like that:
> >
> > Trying to read symbols on non-ELF binaries with libbfd, when
> > supported, makes it possible for perf to report symbols and
> > annotations for Windows applications running under Wine.
> >
> > As you suggested, the libbfd code now tries to detect the binary format,
> > and early rejects if ELF is detected, so it then falls back to the
> > libelf code path.
>
> Hi, it doesn't seem there's more comments, should I just resend the series
> with the updated commit message?
yes, please fix the changelog.. patches look good to me, but
I'd like somebody else with better bfd knowledge to review them ;-)
jirka
Powered by blists - more mailing lists