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

[Nicholas Piggin]

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

> On Fri, Dec 09, 2016 at 11:46:54PM +0100, Dodji Seketeli wrote:
> > Hello,
> > 
> > Nicholas Piggin <npiggin@xxxxxxxxx> 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