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

Nicholas Piggin
2016-12-09T03:33:24Z

On Thu, 1 Dec 2016 17:12:41 +0100
Michal Marek <mmarek@suse.com> wrote:

> On 2016-12-01 04:39, Nicholas Piggin wrote:
> > On Thu, 01 Dec 2016 02:35:54 +0000
> > Ben Hutchings <ben@decadent.org.uk> wrote:
> >> As I understand it, genksyms incorporates the definitions of a
> >> function's parameter and return types - not just their names - and all
> >> the types they refer to, recursively. So a structure size change
> >> should change the version of all functions where the function and its
> >> caller pass that structure between them, however indirectly. It finds
> >> such indirect ABI breakage for me fairly regularly, though of course I
> >> don't know that it finds everything.
> >
> > It is only the type name.
> >
> > Not only that but even if you did extend it further to structure type
> > arrangement then you still have to deal with other structures followed
> > via pointers. Or (rarer but not unheard of):
> >
> > - changes to structures without changes of the types of their members
> > - changes to arguments without changes of their type
>
> This is already covered by genksyms. Try make V=1 with
> CONFIG_MODVERSIONS=y and add the -D option to one of the genksyms
> command. I wanted to paste the expanded signature for
> register_filesystem() as an example, but vger would probably drop the
> mail for being too big :).

Well I simply tested the outcome. If you have:

struct blah {
int x;
};
int foo(struct blah *blah)
{
return blah->x;
}
EXPORT(foo);

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

Now change to

struct blah {
int y;
int x;
};

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

It just doesn't catch these things. Honestly, stable ABI distros *have*
to review all patches to ensure the ABI is unchanged. Some tools could
help significantly, but for that, the debug info ABI checking tools that
have been mentioned in this thread are far better tool for this job than
modversions.

Thanks,
Nick