On 2/25/19 9:45 PM, Andrii Nakryiko wrote:
> On Mon, Feb 25, 2019 at 9:38 PM Yonghong Song <yhs@xxxxxx> wrote:
>>
>>
>>
>> On 2/25/19 1:36 PM, Andrii Nakryiko wrote:
>>> 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.
>>>
>>> $ 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?
>>
>> BPF does not support floating point instructions. As the result,
>> BTF design does not contain floating point types.
>>
>> Looks like kernel also uses float types (in certain drivers and arch's),
>> but types referred by bpf program mostly will not contain float types.
>>
>> Currently, both llvm and pahole will ignore floating types, so
>> from type comparison point of view, we will be fine.
>> The issue may come up with people using vmlinux BTF inside the
>> kernel for different purpose where a float type needs to be
>> specified, e.g., for a floating point function argument in ftrace.
>> But this is rare too.
>>
>> I am not 100% sure whether pahole dwarf/btf parity is considered
>> as a strong reason to add floating point type support in BTF or not.
>
> Isn't float and double just another base type with corresponding name?
> Fundamentally, how is it different from int or long? It's still just a
> collection of bits in 32-bit, 64-bit, etc number, except interpreted
> differently. Do you mean that for BTF to properly support floats,
> we'll need to extends set of possible encodings in INT type (in
> addition to today's CHAR, SIGNED and UNSIGNED)?
Probably another kind like BTF_KIND_FLOAT with additional u32 (in
addition to btf_type) indicating the size (4 bytes, 8 bytes and 16 bytes).
>
>>
>>>
>>> Though for the latter, it probably would be better to resolve base
>>> type size using BTF data itself.
>>>
>>>>> [acme@quaco pahole]$
