Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1541552 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-14 00:10 +0100
    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 10:20 +0100
      Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-14 10:40 +0100
        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:00 +0100
        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:00 +0100
          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:10 +0100
            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:20 +0100
              Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:40 +0100
          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:20 +0100

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

Dne 9.12.2016 v 23:46 Dodji Seketeli napsal(a):
> 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)

For completeness, with a foo.symref file in the tree, genksyms would print

foo.c: warning: foo: modversion changed because of changes in struct blah

So there is some sort of diagnostics already. Does the abidiff tool
handle the case when an exported symbol is moved between .c files? This
is always a mess with genksyms, because the two .c files have different
includes and thus the type expansion stops at different points. So
typically the move needs to be reverted as a workaround.

Michal

[toc] | [next] | [standalone]

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

Michal Marek <mmarek@suse.com> a écrit:

[...]

> Does the abidiff tool handle the case when an exported symbol is moved
> between .c files? This is always a mess with genksyms, because the two
> .c files have different includes and thus the type expansion stops at
> different points. So typically the move needs to be reverted as a
> workaround.

Let's consider the function:

  'void foo(struct S*);'

If two ELF binaries contain a definition of that function foo which ELF
symbol is exported, if the type struct S hasn't changed, and if the only
difference between the ELF binaries is that foo was defined in the
translation unit a.c in the first binary and in b.c in the second
binary, then the comparison engine of libabigail (which is the library
that abidiff uses) will consider the declarations of the two foo
functions as being equal -- no matter what include file comes before the
definition point of foo in a.c and b.c.  If it does not, then it's a bug
that ought to be fixed.

If you feel that I haven't understood your question, then I guess a
minimal standalone example (in the form of C source code) that
illustrates your use case could be helpful to me.

Thanks.

-- 
		Dodji

[toc] | [prev] | [next] | [standalone]

#1541792

On 2016-12-14 09:58, Dodji Seketeli wrote:
> Michal Marek <mmarek@suse.com> a écrit:
> 
> [...]
> 
>> Does the abidiff tool handle the case when an exported symbol is moved
>> between .c files? This is always a mess with genksyms, because the two
>> .c files have different includes and thus the type expansion stops at
>> different points. So typically the move needs to be reverted as a
>> workaround.
> 
> Let's consider the function:
> 
>   'void foo(struct S*);'
> 
> If two ELF binaries contain a definition of that function foo which ELF
> symbol is exported, if the type struct S hasn't changed, and if the only
> difference between the ELF binaries is that foo was defined in the
> translation unit a.c in the first binary and in b.c in the second
> binary, then the comparison engine of libabigail (which is the library
> that abidiff uses) will consider the declarations of the two foo
> functions as being equal -- no matter what include file comes before the
> definition point of foo in a.c and b.c.  If it does not, then it's a bug
> that ought to be fixed.
> 
> If you feel that I haven't understood your question, then I guess a
> minimal standalone example (in the form of C source code) that
> illustrates your use case could be helpful to me.

A minimal example would be

t1.c:
struct s1;
struct s2 {
	int i;
}
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}
void foo(struct s3*);
EXPORT_SYMBOL(foo);

t2.c:
struct s1 {
	int j;
}
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}
void foo(struct s3*);
EXPORT_SYMBOL(foo);

genksyms expands this to
void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * )

or

void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * )

respectively. The types are the same, but their visibility in the
different compilation units differs.

Michal

[toc] | [prev] | [next] | [standalone]

#1541793

On 2016-12-14 10:15, Michal Marek wrote:
> A minimal example would be
> 
> t1.c:
> struct s1;
> struct s2 {
> 	int i;
> }
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> }
> void foo(struct s3*);
> EXPORT_SYMBOL(foo);
> 
> t2.c:
> struct s1 {
> 	int j;
> }
> struct s2;
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> }
> void foo(struct s3*);
> EXPORT_SYMBOL(foo);

Note that the above, if passed to genksyms verbatim, would result in
genksyms treating all the types as internal. Here is a complete
example including linemarkers:

$ cat t1.i
# 1 "t1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "t1.c"
# 1 "t1.h" 1
# 1 "t.h" 1
struct s1;
struct s2;
struct s3 {
 struct s1 *ptr1;
 struct s2 *ptr2;
};
# 2 "t1.h" 2
struct s2 {
 int i;
};
# 2 "t1.c" 2
void foo(struct s3 *s) { }
EXPORT_SYMBOL(foo);

$ cat t2.i
# 1 "t2.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "t2.c"
# 1 "t2.h" 1
# 1 "t.h" 1
struct s1;
struct s2;
struct s3 {
 struct s1 *ptr1;
 struct s2 *ptr2;
};
# 2 "t2.h" 2
struct s1 {
 int j;
};
# 2 "t2.c" 2
void foo(struct s3 *s) { }
EXPORT_SYMBOL(foo);

$ ./scripts/genksyms/genksyms -D <t1.i 
Export foo == <void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * ) >
__crc_foo = 0xf731cef8 ;

$ ./scripts/genksyms/genksyms -D <t2.i 
Export foo == <void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * ) >
__crc_foo = 0xc925dae5 ;

Michal

[toc] | [prev] | [next] | [standalone]

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

Michal Marek <mmarek@suse.com> a écrit:

[...]

> A minimal example would be
>
> t1.c:
> struct s1;
> struct s2 {
> 	int i;
> }
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> }
> void foo(struct s3*);
> EXPORT_SYMBOL(foo);
>
> t2.c:
> struct s1 {
> 	int j;
> }
> struct s2;
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> }
> void foo(struct s3*);
> EXPORT_SYMBOL(foo);
>
> genksyms expands this to
> void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * )
>
> or
>
> void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * )
> respectively.

Thanks, I have built an independant test case from this:

$ cat t1.c
struct s1;
struct s2 {
	int i;
};
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};
void foo(struct s3*);
$ cat t2.c
struct s1 {
	int j;
};
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};
void foo(struct s3*);
$ gcc -g -c t1.c
$ gcc -g -c t2.c
$ abidiff t1.o t2.o
$ 

So, as you see here, abidiff considers t1.o and t2.o has having the same
ABI, so it considers the two foo functions to be equivalent.

> The types are the same, but their visibility in the different
> compilation units differs.

I see, for genksyms, the order of declarations matters, especially when
forward declarations are involved.

Libabigail does a "whole binary" analysis of types.

So, consider the point of use of the type 'struct s1*'.  Even if 'struct
s' is just forward-declared at that point, the declaration of struct s1
is "resolved" to its definition.  Even if the definition comes later in
the binary.

In other words, if struct s1 is defined in the binary, you'll never have
that "struct s1 {UNKNOWN} *ptr1;" that you see in genksyms's
representation.

Cheers,

-- 
		Dodji

[toc] | [prev] | [next] | [standalone]

#1541807

On 2016-12-14 10:36, Dodji Seketeli wrote:
> Michal Marek <mmarek@suse.com> a écrit:
> 
> [...]
> 
>> A minimal example would be
>>
>> t1.c:
>> struct s1;
>> struct s2 {
>> 	int i;
>> }
>> struct s3 {
>> 	struct s1 *ptr1;
>> 	struct s2 *ptr2;
>> }
>> void foo(struct s3*);
>> EXPORT_SYMBOL(foo);
>>
>> t2.c:
>> struct s1 {
>> 	int j;
>> }
>> struct s2;
>> struct s3 {
>> 	struct s1 *ptr1;
>> 	struct s2 *ptr2;
>> }
>> void foo(struct s3*);
>> EXPORT_SYMBOL(foo);
>>
>> genksyms expands this to
>> void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * )
>>
>> or
>>
>> void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * )
>> respectively.
> 
> Thanks, I have built an independant test case from this:
> 
> $ cat t1.c
> struct s1;
> struct s2 {
> 	int i;
> };
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> };
> void foo(struct s3*);
> $ cat t2.c
> struct s1 {
> 	int j;
> };
> struct s2;
> struct s3 {
> 	struct s1 *ptr1;
> 	struct s2 *ptr2;
> };
> void foo(struct s3*);
> $ gcc -g -c t1.c
> $ gcc -g -c t2.c
> $ abidiff t1.o t2.o
> $ 
> 
> So, as you see here, abidiff considers t1.o and t2.o has having the same
> ABI, so it considers the two foo functions to be equivalent.

Wow. That sounds too good to be true.


>> The types are the same, but their visibility in the different
>> compilation units differs.
> 
> I see, for genksyms, the order of declarations matters, especially when
> forward declarations are involved.
> 
> Libabigail does a "whole binary" analysis of types.
> 
> So, consider the point of use of the type 'struct s1*'.  Even if 'struct
> s' is just forward-declared at that point, the declaration of struct s1
> is "resolved" to its definition.  Even if the definition comes later in
> the binary.

But there isn't any definition of struct s1 in t1.o. Does abidiff
"steal" the definition from the other object file? That would be
legitimate, I'm just curious.

Thanks,
Michal

[toc] | [prev] | [next] | [standalone]

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

Michal Marek <mmarek@suse.com> a écrit:

>> Libabigail does a "whole binary" analysis of types.
>> 
>> So, consider the point of use of the type 'struct s1*'.  Even if 'struct
>> s' is just forward-declared at that point, the declaration of struct s1
>> is "resolved" to its definition.  Even if the definition comes later in
>> the binary.
>
> But there isn't any definition of struct s1 in t1.o. Does abidiff
> "steal" the definition from the other object file? That would be
> legitimate, I'm just curious.

If there is another translation unit in the *same* binary that defines
struct s1, then yes, it's "stolen", as you say.

But if in the entire binary, struct s1 is just declared (not defined),
then it'll compare equal to any struct s1 that is defined in the
*second* binary.

Cheers,

-- 
		Dodji

[toc] | [prev] | [next] | [standalone]

#1541839

On 2016-12-14 11:02, Dodji Seketeli wrote:
> Michal Marek <mmarek@suse.com> a écrit:
> 
>>> Libabigail does a "whole binary" analysis of types.
>>>
>>> So, consider the point of use of the type 'struct s1*'.  Even if 'struct
>>> s' is just forward-declared at that point, the declaration of struct s1
>>> is "resolved" to its definition.  Even if the definition comes later in
>>> the binary.
>>
>> But there isn't any definition of struct s1 in t1.o. Does abidiff
>> "steal" the definition from the other object file? That would be
>> legitimate, I'm just curious.
> 
> If there is another translation unit in the *same* binary that defines
> struct s1, then yes, it's "stolen", as you say.
> 
> But if in the entire binary, struct s1 is just declared (not defined),
> then it'll compare equal to any struct s1 that is defined in the
> *second* binary.

That makes sense, thanks.

Michal

[toc] | [prev] | [next] | [standalone]

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

Dodji Seketeli <dodji@seketeli.org> a écrit:

Grr, I did paste the wrong content of t1.c and t2.c in my last message sorry.

Here are the correct ones:

$ cat t1.c
struct s1;
struct s2 {
	int i;
};
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};

void foo(struct s3* s __attribute__((unused)))
{
}

$ cat t2.c
struct s1 {
	int j;
};
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};

void foo(struct s3* s __attribute__((unused)))
{
}

$ gcc -g -c t1.c
$ gcc -g -c t2.c
$ abidiff t1.o t2.o
$ 

The rest of my previous message still applies :-)

> So, as you see here, abidiff considers t1.o and t2.o has having the same
> ABI, so it considers the two foo functions to be equivalent.
>
>> The types are the same, but their visibility in the different
>> compilation units differs.
>
> I see, for genksyms, the order of declarations matters, especially when
> forward declarations are involved.
>
> Libabigail does a "whole binary" analysis of types.
>
> So, consider the point of use of the type 'struct s1*'.  Even if 'struct
> s' is just forward-declared at that point, the declaration of struct s1
> is "resolved" to its definition.  Even if the definition comes later in
> the binary.
>
> In other words, if struct s1 is defined in the binary, you'll never have
> that "struct s1 {UNKNOWN} *ptr1;" that you see in genksyms's
> representation.

Thanks.

-- 
		Dodji

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web