Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly

[Yonghong Song <yhs@fb.com>]

On 4/3/21 2:42 PM, Sedat Dilek wrote:
> On Sat, Apr 3, 2021 at 8:42 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> Currently, when DWARF5 is enabled in kernel, DEBUG_INFO_BTF
>> needs to be disabled. I hacked the kernel to enable DEBUG_INFO_BTF
>> like:
>>    --- a/lib/Kconfig.debug
>>    +++ b/lib/Kconfig.debug
>>    @@ -286,7 +286,6 @@ config DEBUG_INFO_DWARF5
>>            bool "Generate DWARF Version 5 debuginfo"
>>            depends on GCC_VERSION >= 50000 || CC_IS_CLANG
>>            depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS))
>>    -       depends on !DEBUG_INFO_BTF
> 
> What Linux-kernel is your base?
> For linus Git this is correct.
> [ I have currently kbuild-next stuff in my custom patchset which has
> for example CONFIG_AS_IS_XXX where XXX is "GNU" means GNU/binutils AS
> or "LLVM" means LLVM/Clang Integrated ASsembler (IAS). ]
> 
>>            help
>> and tried DWARF5 with latest trunk clang, thin-lto and no lto.
>> In both cases, I got a few additional failures like:
>>    $ ./test_progs -n 55/2
> 
> What do you mean with "trunk" clang - we have Git now no more SVN :-)?
> 
>  From where is that "test_progs" ?
> 
> I tried to build:
> 
> LLVM_TOOLCHAIN_PATH="/opt/llvm-toolchain/bin"
> if [ -d ${LLVM_TOOLCHAIN_PATH} ]; then
>    export PATH="${LLVM_TOOLCHAIN_PATH}:${PATH}"
> fi
> 
> $ echo $PATH
> /opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
> 
> $ clang --version
> dileks clang version 12.0.0 (https://github.com/llvm/llvm-project.git
> 04ba60cfe598e41084fb848daae47e0ed910fa7d)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /opt/llvm-toolchain/bin
> 
> MAKE="make V=1"
> MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> LD=ld.lld LLVM=1 LLVM_IAS=1"
> MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> 
> $ echo $MAKE $MAKE_OPTS
> make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang LD=ld.lld
> LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole
> 
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/bpf/ 2&>1 | tee ../make-log_tools-bpf.txt
> ...is OK.
> 
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> ../make-log_tools-testing-selftests-bpf.txt
> ...is broken here.
> 
> NOTE: Both make-log_tools-bpf.txt and
> make-log_tools-testing-selftests-bpf.txt are attached.

bpf selftest always tends to require latest clang.

For the error,
-idirafter /opt/llvm-toolchain/lib/clang/12.0.0/include -idirafter 
/usr/include/x86_64-linux-gnu -idirafter /usr/include 
-Wno-compare-distinct-pointer-types -DENABLE_ATOMICS_TESTS -O2 -target 
bpf -c progs/local_storage.c -o 
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/local_storage.o 
-mcpu=v3
progs/local_storage.c:41:15: error: use of undeclared identifier 
'BPF_MAP_TYPE_TASK_STORAGE'; did you mean 'BPF_MAP_TYPE_SK_STORAGE'?
         __uint(type, BPF_MAP_TYPE_TASK_STORAGE);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
                      BPF_MAP_TYPE_SK_STORAGE
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:13:39: 
note: expanded from macro '__uint'
#define __uint(name, val) int (*name)[val]
                                       ^
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include/vmlinux.h:10202:2: 
note: 'BPF_MAP_TYPE_SK_STORAGE' declared here
         BPF_MAP_TYPE_SK_STORAGE = 24,
         ^
1 error generated.

vmlinux.h does not contain BPF_MAP_TYPE_TASK_STORAGE.
vmlinux.h is generated from BTF of the vmlinux you just built,
but it looks like somehow it is generated from another vmlinux
it does not contain BPF_MAP_TYPE_TASK_STORAGE. There are some
build magic here to find correct vmlinux and maybe this just
does not work for you.

But you can use the pahole output comparison of BTF VAR encoding
between dwarf4 and dwarf5, this will have even more coverage.

> 
>>    ...
>>    libbpf: extern (var ksym) 'bpf_prog_active': failed to find BTF ID in kernel BTF(s).
>>    libbpf: failed to load object 'kfunc_call_test_subprog'
>>    libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
>>    test_subprog:FAIL:skel unexpected error: 0
>>    #55/2 subprog:FAIL
>>
>> Here, bpf_prog_active is a percpu global variable and pahole is supposed to
>> put into BTF, but it is not there.
>>
>> Further analysis shows this is due to encoding difference between
>> DWARF4 and DWARF5. In DWARF5, a new section .debug_addr
>> and several new ops, e.g. DW_OP_addrx, are introduced.
>> DW_OP_addrx is actually an index into .debug_addr section starting
>> from an offset encoded with DW_AT_addr_base in DW_TAG_compile_unit.
>>
> 
> With LLVM toolchain v12.0.0-rc4 I see "DW_OP_addrx".
> I need a test-case to hit the issue(s) and test this or any other
> (pahole) patches.
> 
> Thanks.
> 
> - Sedat -
> 
>> For the above 'bpf_prog_active' example, with DWARF4, we have
>>    0x02281a96:   DW_TAG_variable
>>                    DW_AT_name      ("bpf_prog_active")
>>                    DW_AT_decl_file ("/home/yhs/work/bpf-next/include/linux/bpf.h")
>>                    DW_AT_decl_line (1170)
>>                    DW_AT_decl_column       (0x01)
>>                    DW_AT_type      (0x0226d171 "int")
>>                    DW_AT_external  (true)
>>                    DW_AT_declaration       (true)
>>
>>    0x02292f04:   DW_TAG_variable
>>                    DW_AT_specification     (0x02281a96 "bpf_prog_active")
>>                    DW_AT_decl_file ("/home/yhs/work/bpf-next/kernel/bpf/syscall.c")
>>                    DW_AT_decl_line (45)
>>                    DW_AT_location  (DW_OP_addr 0x28940)
>> For DWARF5, we have
>>    0x0138b0a1:   DW_TAG_variable
>>                    DW_AT_name      ("bpf_prog_active")
>>                    DW_AT_type      (0x013760b9 "int")
>>                    DW_AT_external  (true)
>>                    DW_AT_decl_file ("/home/yhs/work/bpf-next/kernel/bpf/syscall.c")
>>                    DW_AT_decl_line (45)
>>                    DW_AT_location  (DW_OP_addrx 0x16)
>>
>> This patch added support for DW_OP_addrx. With the patch, the above
>> failing bpf selftest and other similar failed selftests succeeded.
>> ---
>>   dwarf_loader.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> NOTE: with this patch, at least for clang trunk, all bpf selftests
>>        are fine for DWARF5 w.r.t. compiler and pahole. Hopefully
>>        after pahole 1.21 release, we can remove DWARF5 dependence
>>        with !DEBUG_INFO_BTF.
>>
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index 82d7131..49336ac 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -401,8 +401,19 @@ static int attr_location(Dwarf_Die *die, Dwarf_Op **expr, size_t *exprlen)
>>   {
>>          Dwarf_Attribute attr;
>>          if (dwarf_attr(die, DW_AT_location, &attr) != NULL) {
>> -               if (dwarf_getlocation(&attr, expr, exprlen) == 0)
>> +               if (dwarf_getlocation(&attr, expr, exprlen) == 0) {
>> +                       /* DW_OP_addrx needs additional lookup for real addr. */
>> +                       if (*exprlen != 0 && expr[0]->atom == DW_OP_addrx) {
>> +                               Dwarf_Attribute addr_attr;
>> +                               dwarf_getlocation_attr(&attr, expr[0], &addr_attr);
>> +
>> +                               Dwarf_Addr address;
>> +                               dwarf_formaddr (&addr_attr, &address);
>> +
>> +                               expr[0]->number = address;
>> +                       }
>>                          return 0;
>> +               }
>>          }
>>
>>          return 1;
>> @@ -626,6 +637,7 @@ static enum vscope dwarf__location(Dwarf_Die *die, uint64_t *addr, struct locati
>>                  Dwarf_Op *expr = location->expr;
>>                  switch (expr->atom) {
>>                  case DW_OP_addr:
>> +               case DW_OP_addrx:
>>                          scope = VSCOPE_GLOBAL;
>>                          *addr = expr[0].number;
>>                          break;
>> --
>> 2.30.2
>>