Re: [PATCH 1/3] perf tools: Allow vmlinux to fallback to kallsyms on NO_LIBELF=1

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim escreveu:
> Hi Peter,
> 
> On Mon, Nov 10, 2014 at 9:11 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote:
> >> Hmm.. I don't think it's specific to the minimal elf parser.  The return
> >> value of dso__load_sym() is a number of symbols found so when it sees a
> >> dso with 0 symbols it'll fall back to the next option IMHO (not
> >> tested).  Did you see a problem with the current code?
> >
> > So your patch:
> >
> > +++ b/tools/perf/util/symbol-minimal.c
> > @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> >         unsigned char *build_id[BUILD_ID_SIZE];
> >         int ret;
> >
> > +       if (dso->kernel)
> > +               return 0;  /* always use kallsyms */
> > +
> >
> > changes the symbol-minimal.c file to add this exception. That is very
> > much specific to the minimal elf parser, or am I just seeing things?
> 
> What minimal parser does here is just skip kernel dsos (vmlinux) since
> it didn't deal with all the details of parsing vmlinux currently.
> 
> 
> >
> > What I was saying, why not have a util/symbol.c change that disregards
> > all DSOs with 0 symbols in.
> 
> The util/symbol.c doesn't need this because it can handle vmlinux
> reliably.  So after reading symbol table, it'll use the dso if it

symbol.c should not be able to handle anything, since the actual ELF
loader is in symbol-elf.c or simbol-minimal.c, no?

I.e.:

	symbol.c::dso__load()
		symbol.c::dso__load_kernel_sym()
			symbol.c::dso__load_vmlinux()

And then it heads into one of the ELF loaders, right now, without your
patch, I see, when the minimal loader is used:

		symbol-minimal.c::dso__load_sym()

reads the build-id and if it works, returns 1, which is a bug, it should
return 0 in this case, possibly -1 if it doesn't read the build-id :-\

I.e. it should return 0, because that way it signals: "I can't read it,
thus no symbols were loaded, symbol.c: please go on looking for them
somewhere else, kallsyms perhaps?".

And this is the bug, probably. Now to look at your patches to see if
this is touched somehow...

> actually contains symbols or fallback to next dso if it has 0 symbols.
> IOW it already disregards all dsos with 0 symbols in.

See above.
 
> -- 
> Thanks,
> Namhyung