On Wed, Jun 17, 2020 at 10:36:20AM -0700, Matt Helsley wrote:
> On Fri, Jun 12, 2020 at 06:05:34PM +0200, Peter Zijlstra wrote:
> > On top of that, I suppose we can do something like the below.
> >
> > Then you can simply write:
> >
> > if (reloc->sym->class == SYM_MCOUNT && ..)
>
> This looks like a good way to move towards a "single pass" through the ELF
> data
> for mcount.
>
> What order do you want to see this patch go in? Before this series (i.e.
> perhaps
> just a kcov SYM_ class to start)? Early or late in this series? After?
>
> Right now I'm thinking of putting this on the end of my series because
> I'm focusing on converting recordmcount in the series and this isn't
> strictly necessary but is definitely nicer.
No particular thoughts about where, so at the end would be fine.
> > ---
> >
> > diff --git a/kernel/locking/Makefile b/kernel/locking/Makefile
> > index 45452facff3b..94e4b8fcf9c1 100644
> > --- a/kernel/locking/Makefile
> > +++ b/kernel/locking/Makefile
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> > # Any varying coverage in these files is non-deterministic
> > # and is generally not a function of system call inputs.
> > -KCOV_INSTRUMENT := n
> > +# KCOV_INSTRUMENT := n
> >
> > obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o
> >
> > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > index 432417a83902..133c0c285be6 100644
> > --- a/tools/objtool/elf.c
> > +++ b/tools/objtool/elf.c
> > @@ -341,6 +341,24 @@ static int read_sections(struct elf *elf)
> > return 0;
> > }
> >
> > +static bool is_mcount_symbol(const char *name)
> > +{
> > + if (name[0] == '.')
> > + name++;
> > +
> > + if (name[0] == '_')
> > + name++;
> > +
> > + return !strcmp(name, "mcount", 6) ||
>
> Looks like you intended this to be a strncmp() but I don't see a reason to
> use strncmp(). Am I missing something?
typing hard :-)
> > + !strcmp(name, "_fentry__") ||
> > + !strcmp(name, "_gnu_mcount_nc");
> > +}
>
> This mashes all of the arch-specific mcount name checks together. I
> don't see a problem with that because I doubt there will be a collision
> with other functions.
This, I assumed it would just work.
> Just to be careful I looked through the Clang and
> GCC sources, though I only dug through the history of Clang's output --
> GCC's history with respect to mcount symbol names across architectures is
> much harder to trace so I only looked at the current sources.
Fair enough; thanks for the due-dilligence.
