Em Mon, Feb 25, 2019 at 01:36:57PM -0800, Andrii Nakryiko escreveu:
> On Mon, Feb 25, 2019 at 12:51 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Mon, Feb 25, 2019 at 12:08 PM Arnaldo Carvalho de Melo
> > <arnaldo.melo@xxxxxxxxx> wrote:
> > >
> > > Em Mon, Feb 25, 2019 at 11:42:29AM -0800, Andrii Nakryiko escreveu:
> > > > On Mon, Feb 25, 2019 at 11:00 AM Arnaldo Carvalho de Melo> <arnaldo.melo@xxxxxxxxx> wrote:
> > >
> > > > > Em Mon, Feb 25, 2019 at 01:02:39PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > > So yeah, the BTF encoder/decoder is working just fine, the problem is in
> > > > > > pahole's DWARF code, lemme see...
> > > > >
> > > > > Please try the patch below, for me btfdiff continues to show no diff for
> > > > > all types in my vmlinux and now it also produces the same output for
> > > > > when the first element of a bitfield has its bit_size equal to its
> > > > > byte_size * 8:
> > > >
> > > > Yes, this fixes all the issues I've seen. btfdiff output is now empty
> > > > for my kernel image. Thanks for quick fix!
> > > >
> > > > Reviewed-by: Andrii Nakryiko <andriin@xxxxxx>
> > >
> > > Thanks, just for some extra testing I tried encoding BTF for glibc 2.28:
> > >
> > > [acme@quaco pahole]$ pahole -JV libc-2.28.so.debug
> > > <BIG snip>
> >
> > <snip>
> >
> > > [386075] INT long unsigned int size=8 bit_offset=0 nr_bits=64 encoding=(none)
> > > Cannot open libc-2.28.so.debug
> > > Failed to encode BTF
> >
> > I've tried the same on libc-2.26.so.debug and it worked.
> >
> > It failes after BTF encoding and deduping is done, exactly when it
> > tries to re-open libc-2.28.so.debug to write out new contents. Not
> > sure why it would fail, as it seems to be just plain open() call, but
> > the fact that we both can repro this means there is something there.
> >
> > It also seems that libc-2.28.so.debug is corrupted:
> >
> > $ readelf -n /home/andriin/local/btf/libc-2.26.so.debug | grep
> > 'Unknown note type' | wc -l
> > 0
> > $ readelf -n /home/andriin/local/btf/libc-2.28.so.debug | grep
> > 'Unknown note type' | wc -l
> > 17953
> >
>
> Ignore everything I've said (though 'Unknown note type' issue is
> strange), for me it was just permissions issues.
yeah, we need to fix up these messages, I did the chmod here and got
further.
[acme@quaco pahole]$ ls -la libc-2.28.so.debug
-r--r--r--. 1 acme acme 17189368 Feb 25 17:04 libc-2.28.so.debug
[acme@quaco pahole]$ pahole -J libc-2.28.so.debug
Cannot open libc-2.28.so.debug
Failed to encode BTF
[acme@quaco pahole]$ chmod +w libc-2.28.so.debug
[acme@quaco pahole]$ ls -la libc-2.28.so.debug
-rw-rw-r--. 1 acme acme 17189368 Feb 25 17:04 libc-2.28.so.debug
[acme@quaco pahole]$ pahole -J libc-2.28.so.debug
[acme@quaco pahole]$
And I get the same warnings.
base_type__name_to_size: base_type _Float128
If we just add _Float128 to the base_type__name_to_size arrays I think
its good enough for pretty printing, i.e. we won't know from encoding
that this is a float or double, as CTF and DWARF can, but for printing
purposes, even regenerating the type for consumption by other tools, it
should just work, no?
So, two bugs, the bit_size for "long double" is 128, not 64, and if we
add an entry for _Float128, then btfdiff is clean for libc, without any
further work on properly supporting float types as in CTF or DWARF:
[acme@quaco pahole]$ btfdiff libc-2.28.so.debug
[acme@quaco pahole]$ git diff
diff --git a/dwarves.c b/dwarves.c
index adc411a8279d..48a018a9c354 100644
--- a/dwarves.c
+++ b/dwarves.c
@@ -168,11 +168,12 @@ static struct base_type_name_to_size {
{ .name = "double double", .size = 64, },
{ .name = "single float", .size = 32, },
{ .name = "float", .size = 32, },
- { .name = "long double", .size = 64, },
- { .name = "long double long double", .size = 64, },
+ { .name = "long double", .size = sizeof(long double) * 8, },
+ { .name = "long double long double", .size = sizeof(long double) * 8, },
{ .name = "__int128", .size = 128, },
{ .name = "unsigned __int128", .size = 128, },
{ .name = "__int128 unsigned", .size = 128, },
+ { .name = "_Float128", .size = 128, },
{ .name = NULL },
};
[acme@quaco pahole]$
I'll add a series of patches fixing up the types that don't have encoded
in its name the size in bits, so that it works on all arches, one more
for _Float128, should be enough for now, right?
- Arnaldo
> $ chmod +rw /home/andriin/local/btf/libc-2.28.so.debug
>
> $ ~/local/pahole/build/pahole -J ~/local/btf/libc-2.28.so.debug
>
> $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> base_type__name_to_size: base_type _Float128
> class__fixup_btf_bitfields: unknown base type name "_Float128"!
> --- /tmp/btfdiff.dwarf.aV4wSL 2019-02-25 13:31:54.787923673 -0800
> +++ /tmp/btfdiff.btf.22NQmJ 2019-02-25 13:31:54.802923668 -0800
> @@ -461,9 +461,15 @@ struct La_x86_64_retval {
> uint64_t lrv_rdx; /* 8 8 */
> La_x86_64_xmm lrv_xmm0; /* 16 16 */
> La_x86_64_xmm lrv_xmm1; /* 32 16 */
> - long double lrv_st0; /* 48 16 */
> + long double lrv_st0; /* 48 8 */
> +
> + /* XXX 8 bytes hole, try to pack */
> +
> /* --- cacheline 1 boundary (64 bytes) --- */
> - long double lrv_st1; /* 64 16 */
> + long double lrv_st1; /* 64 8 */
> +
> + /* XXX 8 bytes hole, try to pack */
> +
> La_x86_64_vector lrv_vector0; /* 80 64 */
> /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
> La_x86_64_vector lrv_vector1; /* 144 64 */
> @@ -472,6 +478,7 @@ struct La_x86_64_retval {
> __int128 lrv_bnd1; /* 224 16 */
>
> /* size: 240, cachelines: 4, members: 10 */
> + /* sum members: 224, holes: 2, sum holes: 16 */
> /* last cacheline: 48 bytes */
> };
> struct r_debug {
> @@ -2044,7 +2051,7 @@ union ieee754_float {
> } ieee_nan; /* 0 4 */
> };
> union ieee854_long_double {
> - long double d; /* 0 16 */
> + long double d; /* 0 8 */
> struct {
> unsigned int mantissa1:32; /* 0: 0 4 */
> unsigned int mantissa0:32; /* 4: 0 4 */
> @@ -2141,7 +2148,7 @@ struct ucontext_t {
> /* last cacheline: 8 bytes */
> };
> union ieee854_float128 {
> - _Float128 d; /* 0 16 */
> + _Float128 d; /* 0 0 */
> struct {
> unsigned int mantissa3:32; /* 0: 0 4 */
> unsigned int mantissa2:32; /* 4: 0 4 */
> @@ -2219,7 +2226,7 @@ union printf_arg {
> long unsigned int pa_u_long_int; /* 0 8 */
> long long unsigned int pa_u_long_long_int; /* 0 8 */
> double pa_double; /* 0 8 */
> - long double pa_long_double; /* 0 16 */
> + long double pa_long_double; /* 0 8 */
> const char * pa_string; /* 0 8 */
> const wchar_t * pa_wstring; /* 0 8 */
> void * pa_pointer; /* 0 8 */
>
> Seems like "long double" size is invalid for BTF. And we should
> probably add "_Float128" as another "well known" base type name?
>
> Though for the latter, it probably would be better to resolve base
> type size using BTF data itself.
>
> > > [acme@quaco pahole]$
--
- Arnaldo
