Re: [PATCH dwarves 2/2] btf_encoder: Normalize array index type for parallel dwarf loading case

[Yonghong Song <yhs@fb.com>]

On 5/11/22 5:32 PM, Andrii Nakryiko wrote:
> On Wed, May 11, 2022 at 3:02 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> With latest llvm15 built kernel (make -j LLVM=1), I hit the following
>> error when build selftests (make -C tools/testing/selftests/bpf -j LLVM=1):
>>    In file included from skeleton/pid_iter.bpf.c:3:
>>    .../selftests/bpf/tools/build/bpftool/vmlinux.h:84050:9: error: unknown type name
>>         '__builtin_va_list___2'; did you mean '__builtin_va_list'?
>>    typedef __builtin_va_list___2 va_list___2;
>>            ^~~~~~~~~~~~~~~~~~~~~
>>            __builtin_va_list
>>    note: '__builtin_va_list' declared here
>>    In file included from skeleton/profiler.bpf.c:3:
>>    .../selftests/bpf/tools/build/bpftool/vmlinux.h:84050:9: error: unknown type name
>>         '__builtin_va_list__ _2'; did you mean '__builtin_va_list'?
>>    typedef __builtin_va_list___2 va_list___2;
>>            ^~~~~~~~~~~~~~~~~~~~~
>>            __builtin_va_list
>>    note: '__builtin_va_list' declared here
>>
>> The error can be easily explained with after-dedup vmlinux btf:
>>    [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
>>    [2300] STRUCT '__va_list_tag' size=24 vlen=4
>>          'gp_offset' type_id=2 bits_offset=0
>>          'fp_offset' type_id=2 bits_offset=32
>>          'overflow_arg_area' type_id=32 bits_offset=64
>>          'reg_save_area' type_id=32 bits_offset=128
>>    [2308] TYPEDEF 'va_list' type_id=2309
>>    [2309] TYPEDEF '__builtin_va_list' type_id=2310
>>    [2310] ARRAY '(anon)' type_id=2300 index_type_id=21 nr_elems=1
>>
>>    [5289] PTR '(anon)' type_id=2308
>>    [158520] STRUCT 'warn_args' size=32 vlen=2
>>          'fmt' type_id=14 bits_offset=0
>>          'args' type_id=2308 bits_offset=64
>>    [27299] INT '__ARRAY_SIZE_TYPE__' size=4 bits_offset=0 nr_bits=32 encoding=(none)
>>    [34590] TYPEDEF '__builtin_va_list' type_id=34591
>>    [34591] ARRAY '(anon)' type_id=2300 index_type_id=27299 nr_elems=1
>>
>> The typedef __builtin_va_list is a builtin type for the compiler.
>> In the above case, two typedef __builtin_va_list are generated.
>> The reason is due to different array index_type_id. This happened
>> when pahole is running with more than one jobs when parsing dwarf
>> and generating btfs.
>>
>> Function btf_encoder__encode_cu() is used to do btf encoding for
>> each cu. The function will try to find an "int" type for the cu
>> if it is available, otherwise, it will create a special type
>> with name __ARRAY_SIZE_TYPE__. For example,
>>    file1: yes 'int' type
>>    file2: no 'int' type
>>
>> In serial mode, file1 is processed first, followed by file2.
>> both will have 'int' type as the array index type since file2
>> will inherit the index type from file1.
>>
>> In parallel mode though, arrays in file1 will have index type 'int',
>> and arrays in file2 wil have index type '__ARRAY_SIZE_TYPE__'.
>> This will prevent some legitimate dedup and may have generated
>> vmlinux.h having compilation error.
>>
> 
> I think it is two separate problems.
> 
> 1. Maybe instead of this generating __ARRAY_SIZE_TYPE__ we should
> generate proper 'int' type?

This should work. Will post v2 with this.

> 
> 2. __builtin_va_list___2 shouldn't have happened, it's libbpf bug.
> Libbpf handles __builtin_va_list specially (see
> btf_dump_is_blacklisted()), so we need to fix libbpf to not get
> confused if there are two __builtin_va_list copies in BTF.

I checked code. the libbpf prevents generating
    typedef <...> __builtin_va_list
since __builtin_va_list is a builtin type.

Here, due to __ARRAY_SIZE_TYPE__ problem, the following are generated
in vmlinux.h.

typedef __builtin_va_list va_list;
typedef __builtin_va_list___2 va_list___2;

since __builtin_va_list appears twice in the BTF.
But due to the libbpf implementation to skip
    typedef <...> __builtin_va_list

We don't have __builtin_va_list___2 defined and this
caused compilation error.

Although we could workaround the issue in libbpf
such that if the typedef is in the format of
   typedef __builtin_va_list<...> <other_type>
we should just emit
   typedef __builtin_va_list <other_type>

But fixing the issue in pahole is much better since
we won't have va_list___2 any more.

> 
>> This patch fixed the issue by normalizing all array_index types
>> to be the first array_index type in the whole btf.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   btf_encoder.c | 24 +++++++++++++++++++++---
>>   btf_encoder.h |  2 +-
>>   pahole.c      |  2 +-
>>   3 files changed, 23 insertions(+), 5 deletions(-)
>>
> 
> [...]