Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm

Nicholas Piggin
2016-12-12T03:50:50Z

On Sat, 10 Dec 2016 13:41:03 +0100
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Fri, Dec 09, 2016 at 11:46:54PM +0100, Dodji Seketeli wrote:
> > Hello,
> >
> > Nicholas Piggin <npiggin@gmail.com> a écrit:
> >
> > [...]
> >
> > > That said, a dwarf based checker tool should be able to do as good a job
> > > (maybe a bit better because report is very informative and it may pick up
> > > compiler alignments or padding options).
> >
> > So, Nicholas was kind enough to send me the two Linux Kernel binaries
> > that he built with the tiny little interface change that we were
> > discussing earlier. Here is what the abidiff[1] tools says about that
> > interface change:
> >
> > $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
> > Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> >
> > 1 function with some indirect sub-type change:
> >
> > [C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
> > parameter 1 of type 'blah*' has sub-type changes:
> > in pointed to type 'struct blah' at memory.c:78:1:
> > type size changed from 32 to 64 bits
> > 1 data member insertion:
> > 'int blah::y', at offset 0 (in bits) at memory.c:79:1
> > 1 data member change:
> > 'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)
> >
> >
> >
> > real 0m2.595s
> > user 0m2.489s
> > sys 0m0.108s
> > $
> >
> > I kept the timing information to give you an idea of the time it takes
> > on a non-optimized build of abidiff.
> >
> > One could for instance want that types that are not defined in header
> > files be kept out of the change report. In that case it's possible to
> > write a little suppression specification file like this one:
> >
> > $ cat vmlinux.abignore
> > [suppress_type]
> > source_location_not_regexp = .*\\.h
> > $
> >
> > You can then pass that suppression file to the tool:
> >
> > $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi
> > Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
> > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> >
> >
> > real 0m2.574s
> > user 0m2.473s
> > sys 0m0.102s
> > $
> >
> > So this is the kind of interface change analysis tool we are working on
> > at the moment.
> >
> > One could also imagine a tool that would compute a CRC that takes the
> > very same suppression specification files into account, letting people
> > to decide that some interface changes are OK. That CRC would thus be
> > added to the special ELF sections we already have today. We could keep
> > the modversion machinery, but with a greater dose of flexibility.
> > Whenever modversion detects a change, abidiff would tell people what the
> > change is exactly.
> >
> > What do you guys think?
>
> YES YES YES!!!
>
> Now I don't work on a distro anymore, but I would think that something
> like this would be really useful, pointing out exactly what changed is
> very important for distro maintainers to determine what they want to do
> (either fix up the abi change with strange hacks, or ignore it due to
> the change being in an area they don't care at all about, i.e. a random
> driver subsystem.)
>
> So yes, I think this is really good stuff. But if the distro
> maintainers correct me and think it's useless, then I need to revisit my
> view of exactly what they do for their customers :)

Agree completely. BTW (for those who might be looking into these tools),
we also have https://github.com/skozina/kabi-dw that Stanislav (cc'ed)
mentioned earlier.

It's true that the current modversions __crc_ matching infrastructure is
"just" a symbol versioning system, and we could keep it and just populate
it with something other than genksyms (e.g., a symbol version list provided
by distros). But the starting point should be *no* versioning and simply
using names to break linkage. Unless there's a compelling reason not to,
symbols are simpler, easier, everyone knows how they work.

The other question would be whether to pull a minimal tool into the kernel
source or keep them out of tree (but possibly add some helper scripts etc).
I guess we'll need to see what distros want.

Thanks,
Nick