All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
@ 2021-04-03 18:41 Yonghong Song
  2021-04-03 18:52 ` David Blaikie
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Yonghong Song @ 2021-04-03 18:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Bill Wendling, bpf, David Blaikie,
	kernel-team, Nick Desaulniers, Sedat Dilek

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
          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
  ...
  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.

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-03 18:41 [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
@ 2021-04-03 18:52 ` David Blaikie
  2021-04-03 20:20   ` Yonghong Song
       [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: David Blaikie @ 2021-04-03 18:52 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, kernel-team, Nick Desaulniers, Sedat Dilek

On Sat, Apr 3, 2021 at 11:42 AM 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
>           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
>   ...
>   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.
>
> 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) {


^ this change is probably overly narrow. DW_OP_addrx could appear at
other locations in a DWARF expression. So whatever code is responsible
for doing general evaluation of DWARF expressions should probably be
the place to handle this so it can deal with the full generality of
expressions, not only this specific case of a DW_OP_addrx as being the
first operation in the expression?

>
> +                               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
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-03 18:52 ` David Blaikie
@ 2021-04-03 20:20   ` Yonghong Song
  2021-04-03 23:31     ` David Blaikie
  0 siblings, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-03 20:20 UTC (permalink / raw)
  To: David Blaikie
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, kernel-team, Nick Desaulniers, Sedat Dilek



On 4/3/21 11:52 AM, David Blaikie wrote:
> On Sat, Apr 3, 2021 at 11:42 AM 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
>>            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
>>    ...
>>    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.
>>
>> 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) {
> 
> 
> ^ this change is probably overly narrow. DW_OP_addrx could appear at
> other locations in a DWARF expression. So whatever code is responsible
> for doing general evaluation of DWARF expressions should probably be
> the place to handle this so it can deal with the full generality of
> expressions, not only this specific case of a DW_OP_addrx as being the
> first operation in the expression?

We really have a narrow usage here. This code path (DW_OP_addr and 
DW_OP_addrx) is triggered and later on only used when converting
dwarf to BTF for certain category of global variables and that is
why address is needed. The above code is only called for
DW_TAG_variable.

The previous code to handle DW_OP_addr also only took the first expression.

         if (attr_location(die, &location->expr, &location->exprlen) != 0)
                 scope = VSCOPE_OPTIMIZED;
         else if (location->exprlen != 0) {
                 Dwarf_Op *expr = location->expr;
                 switch (expr->atom) {
                 case DW_OP_addr:
                         scope = VSCOPE_GLOBAL;
                         *addr = expr[0].number;
                         break;
	......

Do you think we could have multiple OPs for an *external* variable location?

> 
>>
>> +                               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
>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
@ 2021-04-03 23:13   ` Yonghong Song
  2021-04-03 23:27   ` Yonghong Song
  1 sibling, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2021-04-03 23:13 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



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?

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
latest one, essentially with top commit:
   89d69c5d0fbc Merge branch 'sockmap: introduce BPF_SK_SKB_VERDICT and 
support UDP'

> 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 :-)?

git: https://github.com/llvm/llvm-project.git
top commit:
    commit aaab4441796909e1b9cf4279906a56350c8fade7
    Author: Jianzhou Zhao <jianzhouzh@google.com>
    Date:   Mon Mar 29 00:14:16 2021 +0000

     [dfsan] Ignore dfsan origin wrappers when instrumenting code
> 
>  From where is that "test_progs" ?

Some information here.
 
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/README.rst

Typically I do
   $ make -C tools/testing/selftests/bpf -j60 LLVM=1 LLVM_IAS=1
and then boot up a qemu with the bpf-next vmlinux and then run
"test_progs" binary which is located as
     tools/test/selftests/bpf/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.
> 
>>    ...
>>    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.

I am using llvm-project repo "main" branch which is the development
branch.

To test this, you can do:
    . build vmlinux (say bpf-next branch) with dwarf4
    . pahole -JV vmlinux >& log.pahole
    . grep "bpf_prog_active" log.pahole and you will see
$ grep bpf_prog_active log
Found per-CPU symbol 'bpf_prog_active' at address 0x1f8350
Variable 'bpf_prog_active' from CU 
'/home/yhs/work/bpf-next/kernel/bpf/syscall.c' at address 0x1f8350 encoded
[601922] VAR bpf_prog_active type=598691 linkage=1
$ grep " VAR " log.pahole | wc -l
299

totally 299 BTF variables are encoded.

     . build vmlinux with dwarf5 (without this patch)
     . pahole -JV vmlinux >& log.pahole
     . grep "bpf_prog_active" log.pahole and you will see
$ grep "bpf_prog_active" log.pahole
Found per-CPU symbol 'bpf_prog_active' at address 0x1f8350
$ grep " VAR " log.pahole | wc -l
0

There are o BTF variables are encoded.

With this patch, for dwarf5 again,
$ pahole -JV vmlinux >& log.pahole2
$ grep "bpf_prog_active" log.pahole2
Found per-CPU symbol 'bpf_prog_active' at address 0x1f8350
Variable 'bpf_prog_active' from CU 
'/home/yhs/work/bpf-next/kernel/bpf/syscall.c' at address 0x1f8350 encoded
[602735] VAR bpf_prog_active type=599504 linkage=1
$ grep " VAR " log.pahole2 | wc -l
299

I guess this could be fold into some kind of regression test to ensure
dwarf4 and dwarf5 does not change at lease variable encoding for BTF.
If they are, we need to investigate.

> 
> 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
>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
  2021-04-03 23:13   ` Yonghong Song
@ 2021-04-03 23:27   ` Yonghong Song
       [not found]     ` <CA+icZUV4fw5GNXFnyOjvajkVFdPhkOrhr3rn5OrAKGujpSrmgQ@mail.gmail.com>
  1 sibling, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-03 23:27 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



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
>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-03 20:20   ` Yonghong Song
@ 2021-04-03 23:31     ` David Blaikie
  0 siblings, 0 replies; 23+ messages in thread
From: David Blaikie @ 2021-04-03 23:31 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, kernel-team, Nick Desaulniers, Sedat Dilek

On Sat, Apr 3, 2021 at 1:20 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/3/21 11:52 AM, David Blaikie wrote:
> > On Sat, Apr 3, 2021 at 11:42 AM 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
> >>            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
> >>    ...
> >>    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.
> >>
> >> 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) {
> >
> >
> > ^ this change is probably overly narrow. DW_OP_addrx could appear at
> > other locations in a DWARF expression. So whatever code is responsible
> > for doing general evaluation of DWARF expressions should probably be
> > the place to handle this so it can deal with the full generality of
> > expressions, not only this specific case of a DW_OP_addrx as being the
> > first operation in the expression?
>
> We really have a narrow usage here. This code path (DW_OP_addr and
> DW_OP_addrx) is triggered and later on only used when converting
> dwarf to BTF for certain category of global variables and that is
> why address is needed. The above code is only called for
> DW_TAG_variable.
>
> The previous code to handle DW_OP_addr also only took the first expression.
>
>          if (attr_location(die, &location->expr, &location->exprlen) != 0)
>                  scope = VSCOPE_OPTIMIZED;
>          else if (location->exprlen != 0) {
>                  Dwarf_Op *expr = location->expr;
>                  switch (expr->atom) {
>                  case DW_OP_addr:
>                          scope = VSCOPE_GLOBAL;
>                          *addr = expr[0].number;
>                          break;
>         ......

Fair enough - if it's that limited, as you say - this is nothing new then.

> Do you think we could have multiple OPs for an *external* variable location?

With LTO I believe Clang can merge globals, but looks like only
internal linkage variables & I can't quite tickle the codepath at a
quick glance right now.




>
> >
> >>
> >> +                               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
> >>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found]     ` <CA+icZUV4fw5GNXFnyOjvajkVFdPhkOrhr3rn5OrAKGujpSrmgQ@mail.gmail.com>
@ 2021-04-04 12:46       ` Sedat Dilek
  2021-04-04 16:39         ` Yonghong Song
  0 siblings, 1 reply; 23+ messages in thread
From: Sedat Dilek @ 2021-04-04 12:46 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

[-- Attachment #1: Type: text/plain, Size: 7456 bytes --]

> This shows a new build-error:
>
> clang  -g -D__TARGET_ARCH_x86 -mlittle-endian
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> -I/home/dileks/src/linux-kernel/git/tools/t
> esting/selftests/bpf
> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/usr/include
> -idirafter /usr/loc
> al/include -idirafter /opt/llvm-toolchain/lib/clang/12.0.0/include
> -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include
> -Wno-compare-distinct-pointer-type
> s -DENABLE_ATOMICS_TESTS -O2 -target bpf -c
> progs/test_sk_storage_tracing.c -o
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o-mcpu=v3
> progs/test_sk_storage_tracing.c:38:18: error: use of undeclared
> identifier 'BPF_TCP_CLOSE'
>        if (newstate == BPF_TCP_CLOSE)
>                        ^
> 1 error generated.
> make: *** [Makefile:414:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o]
> Error 1
>

I was able to fix this by adding appropriate enums from <linux/bpf.h>.

$ git diff
diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
index 8e94e5c080aa..3c7508f48ce0 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
@@ -6,6 +6,28 @@
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_helpers.h>

+/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
+ * changes between the TCP and BPF versions. Ideally this should never happen.
+ * If it does, we need to add code to convert them before calling
+ * the BPF sock_ops function.
+ */
+enum {
+       BPF_TCP_ESTABLISHED = 1,
+       BPF_TCP_SYN_SENT,
+       BPF_TCP_SYN_RECV,
+       BPF_TCP_FIN_WAIT1,
+       BPF_TCP_FIN_WAIT2,
+       BPF_TCP_TIME_WAIT,
+       BPF_TCP_CLOSE,
+       BPF_TCP_CLOSE_WAIT,
+       BPF_TCP_LAST_ACK,
+       BPF_TCP_LISTEN,
+       BPF_TCP_CLOSING,        /* Now a valid state */
+       BPF_TCP_NEW_SYN_RECV,
+
+       BPF_TCP_MAX_STATES      /* Leave at the end! */
+};
+
struct sk_stg {
       __u32 pid;
       __u32 last_notclose_state;

NOTE: Attached as a diff as Gmail might truncate it.

[ Q ] Should these enums be in vmlinux.h - if so why are they missing?

Next build-error:

g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
-I/home/dileks/src/linux-kernel/git/tools/lib
-I/home/dileks/src/linux-kernel/git/tools/include
-I/home/dileks/src/linux-kernel/git/tools/include/uapi
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-Dbpf_prog_load=bpf_prog_test_load
-Dbpf_load_program=bpf_test_load_program test_cpp.cpp
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
-lcap -lelf -lz -lrt -lpthread -o
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
/usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
relocation R_X86_64_32 against `.rodata.str1.1' ca
n not be used when making a PIE object; recompile with -fPIE
collect2: error: ld returned 1 exit status
make: *** [Makefile:455:
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
Error 1
make: Leaving directory
'/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'

LOL, I was not aware that there is usage of *** CXX*** in tools
directory (see g++ line and /usr/bin/ld ?).

So, I changed my $MAKE_OPTS to use "CXX=clang++".

$ echo $PATH
/opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

$ echo $MAKE $MAKE_OPTS
make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole

$ 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
$ ld.lld --version
LLD 12.0.0 (https://github.com/llvm/llvm-project.git
04ba60cfe598e41084fb848daae47e0ed910fa7d) (compatible with GNU
linkers)

$ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/

This breaks like this:

clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-I/home/dileks/src/linux-kernel/git/tools/testing/selftes
ts/bpf/tools/include
-I/home/dileks/src/linux-kernel/git/include/generated
-I/home/dileks/src/linux-kernel/git/tools/lib
-I/home/dileks/src/linux-kernel/git/tools/incl
ude -I/home/dileks/src/linux-kernel/git/tools/include/uapi
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-Dbpf_prog_load=bpf_prog_test_load -Dbpf_loa
d_program=bpf_test_load_program test_cpp.cpp
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
/home/dileks/src/linux-kernel/git/to
ols/testing/selftests/bpf/tools/build/libbpf/libbpf.a
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
-lcap -lelf -lz -lrt -lpthread -o /home
/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
clang-12: warning: treating 'c-header' input as 'c++-header' when in
C++ mode, this behavior is deprecated [-Wdeprecated]
clang-12: error: cannot specify -o when generating multiple output files
make: *** [Makefile:455:
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
Error 1

OK, I see in bpf-next includes several fixes like:

commit a0964f526df6facd4e12a4c416185013026eecf9
"selftests/bpf: Add multi-file statically linked BPF object file test"

...and to "selftests: xsk".

Finally, I was able to build by suppressing the build of "test_cpp"
and "xdpxceiver":

$ git diff tools/testing/selftests/bpf/Makefile
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d9b19524b2d4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
# Compile but not part of 'make run_tests'
TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
       flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
-       xdpxceiver
+       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
+       # test_cpp xdpxceiver

TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read

This diff is also attached before Gmail eats it.

Yonghong Song as you described your build-environment and checking
requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
to upgrade LLVM toolchain to v13-git and use bpf-next as the new
kernel base.
Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
<apt.llvm.org>.

- Sedat -

[1] https://git.kernel.org/bpf/bpf-next/c/2ba4badca9977b64c966b0177920daadbd5501fe
[2] https://git.kernel.org/bpf/bpf-next/c/a0964f526df6facd4e12a4c416185013026eecf9

[-- Attachment #2: selftests-bpf-Makefile.diff --]
[-- Type: text/x-patch, Size: 677 bytes --]

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d9b19524b2d4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 # Compile but not part of 'make run_tests'
 TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
-	xdpxceiver
+	test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
+	# test_cpp xdpxceiver
 
 TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
 

[-- Attachment #3: selftests-bpf-progs-test_sk_storage_tracing_c.diff --]
[-- Type: text/x-patch, Size: 1040 bytes --]

diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
index 8e94e5c080aa..3c7508f48ce0 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
@@ -6,6 +6,28 @@
 #include <bpf/bpf_core_read.h>
 #include <bpf/bpf_helpers.h>
 
+/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
+ * changes between the TCP and BPF versions. Ideally this should never happen.
+ * If it does, we need to add code to convert them before calling
+ * the BPF sock_ops function.
+ */
+enum {
+	BPF_TCP_ESTABLISHED = 1,
+	BPF_TCP_SYN_SENT,
+	BPF_TCP_SYN_RECV,
+	BPF_TCP_FIN_WAIT1,
+	BPF_TCP_FIN_WAIT2,
+	BPF_TCP_TIME_WAIT,
+	BPF_TCP_CLOSE,
+	BPF_TCP_CLOSE_WAIT,
+	BPF_TCP_LAST_ACK,
+	BPF_TCP_LISTEN,
+	BPF_TCP_CLOSING,	/* Now a valid state */
+	BPF_TCP_NEW_SYN_RECV,
+
+	BPF_TCP_MAX_STATES	/* Leave at the end! */
+};
+
 struct sk_stg {
 	__u32 pid;
 	__u32 last_notclose_state;

[-- Attachment #4: make-log_tools-testing-selftests-bpf_5.12.0-rc5-12-amd64-clang12-lto_test_sk_storage_tracing_c-CXX-test_cpp.txt.gz --]
[-- Type: application/gzip, Size: 39210 bytes --]

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Usage of CXX in tools directory
  2021-04-03 18:41 [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
  2021-04-03 18:52 ` David Blaikie
       [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
@ 2021-04-04 14:59 ` Sedat Dilek
  2021-04-04 15:19   ` Sedat Dilek
  2021-04-06 18:39   ` Nick Desaulniers
  2021-04-04 16:45 ` [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
  3 siblings, 2 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-04 14:59 UTC (permalink / raw)
  To: Masahiro Yamada, Yonghong Song, Arnaldo Carvalho de Melo,
	David Blaikie, Bill Wendling, Nick Desaulniers,
	Nathan Chancellor, Alexei Starovoitov, Peter Zijlstra,
	Mark Rutland, Alexander Shishkin, Ingo Molnar, Jiri Olsa,
	Namhyung Kim
  Cc: dwarves, bpf, kernel-team, linux-kbuild, linux-kernel, x86,
	Clang-Built-Linux ML

[ Please CC me I am not subscribed to all mailing-lists ]
[ Please CC some more folks if you like ]

Hi,

when dealing/experimenting with BPF together with pahole/dwarves and
dwarf-v5 and clang-lto I fell over that there is usage of CXX in tools
directory.
Especially,  I wanted to build and run test_progs from BPF selftests.
One BPF selftest called "test_cpp" used GNU/g++ (and even /usr/bin/ld)
and NOT LLVM/clang++.

For details see the linux-bpf/dwarves thread "[PATCH dwarves]
dwarf_loader: handle DWARF5 DW_OP_addrx properly" in [1].

Lemme check:

$ git grep CXX tools/
tools/build/Build.include:cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@
$(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXXFLAGS_$(basetarget).o)
$(CXXFLAGS_$(obj))
tools/build/Makefile.build:quiet_cmd_cxx_o_c = CXX      $@
tools/build/Makefile.build:      cmd_cxx_o_c = $(CXX) $(cxx_flags) -c -o $@ $<
tools/build/Makefile.feature:  feature-$(1) := $(shell $(MAKE)
OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)"
CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))"
CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))"
LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir)
$(OUTPUT_FEATURES)test-$1.bin >/dev/nu
ll 2>/dev/null && echo 1 || echo 0)
tools/build/feature/Makefile:__BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall
-Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)
...
tools/perf/Makefile.config:USE_CXX = 0
tools/perf/Makefile.config:        CXXFLAGS +=
-DHAVE_LIBCLANGLLVM_SUPPORT -I$(shell $(LLVM_CONFIG) --includedir)
tools/perf/Makefile.config:        $(call detected,CONFIG_CXX)
tools/perf/Makefile.config:     USE_CXX = 1
tools/perf/Makefile.perf:export srctree OUTPUT RM CC CXX LD AR CFLAGS
CXXFLAGS V BISON FLEX AWK
tools/perf/Makefile.perf:ifeq ($(USE_CXX), 1)
tools/perf/util/Build:perf-$(CONFIG_CXX) += c++/
...
tools/scripts/Makefile.include:$(call allow-override,CXX,$(CROSS_COMPILE)g++)
...
tools/testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
tools/testing/selftests/bpf/Makefile:   $(call msg,CXX,,$@)
tools/testing/selftests/bpf/Makefile:   $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o $@

The problem is if you pass LLVM=1 there is no clang(++) assigned to
CXX automagically.

[2] says:

LLVM has substitutes for GNU binutils utilities. Kbuild supports
LLVM=1 to enable them.

make LLVM=1
They can be enabled individually. The full list of the parameters:

make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
  OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
  HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld

[ EndOfQuote  ]

So you need to pass CXX=clang++ manually when playing in tools directory:

MAKE="make V=1
MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1"
MAKE_OPTS="MAKE_OPTS $PAHOLE=/opt/pahole/bin/pahole"

$ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ clean
$ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/

Unsure, if tools needs a special treatment in things of CXX or LLVM=1
needs to be enhanced with CCX=clang++.
If we have HOSTCXX why not have a CXX in toplevel Makefile?

In "tools: Factor Clang, LLC and LLVM utils definitions" (see [3]) I
did some factor-ing.

For the records: Here Linus Git is my base.

Ideas?

Thanks.

Regards,
- Sedat -

P.S.: Just a small note: I know there is less usage of CXX code in the
linux-kernel.

[1] https://lore.kernel.org/bpf/CA+icZUWh6YOkCKG72SndqUbQNwG+iottO4=cPyRRVjaHD2=0qw@mail.gmail.com/T/#m22907f838d2d27be24e8959a53473a62f21cecea
[2] https://www.kernel.org/doc/html/latest/kbuild/llvm.html#llvm-utilities
[3] https://git.kernel.org/linus/211a741cd3e124bffdc13ee82e7e65f204e53f60

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Usage of CXX in tools directory
  2021-04-04 14:59 ` Usage of CXX in tools directory Sedat Dilek
@ 2021-04-04 15:19   ` Sedat Dilek
  2021-04-06 18:39   ` Nick Desaulniers
  1 sibling, 0 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-04 15:19 UTC (permalink / raw)
  To: Masahiro Yamada, Yonghong Song, Arnaldo Carvalho de Melo,
	David Blaikie, Bill Wendling, Nick Desaulniers,
	Nathan Chancellor, Alexei Starovoitov, Peter Zijlstra,
	Mark Rutland, Alexander Shishkin, Ingo Molnar, Jiri Olsa,
	Namhyung Kim
  Cc: dwarves, bpf, kernel-team, linux-kbuild, linux-kernel, x86,
	Clang-Built-Linux ML

> So you need to pass CXX=clang++ manually when playing in tools directory:
>
> MAKE="make V=1
> MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1"
> MAKE_OPTS="MAKE_OPTS $PAHOLE=/opt/pahole/bin/pahole"
>
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ clean
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/
>

Correct:

MAKE="make V=1"
MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1"
MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"

- Sedat -

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-04 12:46       ` Sedat Dilek
@ 2021-04-04 16:39         ` Yonghong Song
  2021-04-04 17:25           ` Sedat Dilek
  0 siblings, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-04 16:39 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



On 4/4/21 5:46 AM, Sedat Dilek wrote:
>> This shows a new build-error:
>>
>> clang  -g -D__TARGET_ARCH_x86 -mlittle-endian
>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
>> -I/home/dileks/src/linux-kernel/git/tools/t
>> esting/selftests/bpf
>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/usr/include
>> -idirafter /usr/loc
>> al/include -idirafter /opt/llvm-toolchain/lib/clang/12.0.0/include
>> -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include
>> -Wno-compare-distinct-pointer-type
>> s -DENABLE_ATOMICS_TESTS -O2 -target bpf -c
>> progs/test_sk_storage_tracing.c -o
>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o-mcpu=v3
>> progs/test_sk_storage_tracing.c:38:18: error: use of undeclared
>> identifier 'BPF_TCP_CLOSE'
>>         if (newstate == BPF_TCP_CLOSE)
>>                         ^
>> 1 error generated.
>> make: *** [Makefile:414:
>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o]
>> Error 1
>>
> 
> I was able to fix this by adding appropriate enums from <linux/bpf.h>.
> 
> $ git diff
> diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> index 8e94e5c080aa..3c7508f48ce0 100644
> --- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> +++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> @@ -6,6 +6,28 @@
> #include <bpf/bpf_core_read.h>
> #include <bpf/bpf_helpers.h>
> 
> +/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> + * changes between the TCP and BPF versions. Ideally this should never happen.
> + * If it does, we need to add code to convert them before calling
> + * the BPF sock_ops function.
> + */
> +enum {
> +       BPF_TCP_ESTABLISHED = 1,
> +       BPF_TCP_SYN_SENT,
> +       BPF_TCP_SYN_RECV,
> +       BPF_TCP_FIN_WAIT1,
> +       BPF_TCP_FIN_WAIT2,
> +       BPF_TCP_TIME_WAIT,
> +       BPF_TCP_CLOSE,
> +       BPF_TCP_CLOSE_WAIT,
> +       BPF_TCP_LAST_ACK,
> +       BPF_TCP_LISTEN,
> +       BPF_TCP_CLOSING,        /* Now a valid state */
> +       BPF_TCP_NEW_SYN_RECV,
> +
> +       BPF_TCP_MAX_STATES      /* Leave at the end! */
> +};
> +
> struct sk_stg {
>         __u32 pid;
>         __u32 last_notclose_state;
> 
> NOTE: Attached as a diff as Gmail might truncate it.

This bpf-next commit:
   commit 97a19caf1b1f6a9d4f620a9d51405a1973bd4641
   Author: Yonghong Song <yhs@fb.com>
   Date:   Wed Mar 17 10:41:32 2021 -0700

     bpf: net: Emit anonymous enum with BPF_TCP_CLOSE value explicitly

fixed the issue.

> 
> [ Q ] Should these enums be in vmlinux.h - if so why are they missing?
> 
> Next build-error:
> 
> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
> -I/home/dileks/src/linux-kernel/git/tools/lib
> -I/home/dileks/src/linux-kernel/git/tools/include
> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -Dbpf_prog_load=bpf_prog_test_load
> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> -lcap -lelf -lz -lrt -lpthread -o
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
> relocation R_X86_64_32 against `.rodata.str1.1' ca
> n not be used when making a PIE object; recompile with -fPIE
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:455:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> Error 1
> make: Leaving directory
> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> 
> LOL, I was not aware that there is usage of *** CXX*** in tools
> directory (see g++ line and /usr/bin/ld ?).
> 
> So, I changed my $MAKE_OPTS to use "CXX=clang++".

In kernel, if LLVM=1 is set, we have:

ifneq ($(LLVM),)
HOSTCC  = clang
HOSTCXX = clang++
else
HOSTCC  = gcc
HOSTCXX = g++
endif

ifneq ($(LLVM),)
CC              = clang
LD              = ld.lld
AR              = llvm-ar
NM              = llvm-nm
OBJCOPY         = llvm-objcopy
OBJDUMP         = llvm-objdump
READELF         = llvm-readelf
STRIP           = llvm-strip
else
CC              = $(CROSS_COMPILE)gcc
LD              = $(CROSS_COMPILE)ld
AR              = $(CROSS_COMPILE)ar
NM              = $(CROSS_COMPILE)nm
OBJCOPY         = $(CROSS_COMPILE)objcopy
OBJDUMP         = $(CROSS_COMPILE)objdump
READELF         = $(CROSS_COMPILE)readelf
STRIP           = $(CROSS_COMPILE)strip
endif

So if you have right path, you don't need to set HOSTCC and HOSTCXX 
explicitly.

> 
> $ echo $PATH
> /opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
> 
> $ echo $MAKE $MAKE_OPTS
> make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole
> 
> $ 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
> $ ld.lld --version
> LLD 12.0.0 (https://github.com/llvm/llvm-project.git
> 04ba60cfe598e41084fb848daae47e0ed910fa7d) (compatible with GNU
> linkers)
> 
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/
> 
> This breaks like this:
> 
> clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftes
> ts/bpf/tools/include
> -I/home/dileks/src/linux-kernel/git/include/generated
> -I/home/dileks/src/linux-kernel/git/tools/lib
> -I/home/dileks/src/linux-kernel/git/tools/incl
> ude -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -Dbpf_prog_load=bpf_prog_test_load -Dbpf_loa
> d_program=bpf_test_load_program test_cpp.cpp
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> /home/dileks/src/linux-kernel/git/to
> ols/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> -lcap -lelf -lz -lrt -lpthread -o /home
> /dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> clang-12: warning: treating 'c-header' input as 'c++-header' when in
> C++ mode, this behavior is deprecated [-Wdeprecated]
> clang-12: error: cannot specify -o when generating multiple output files
> make: *** [Makefile:455:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> Error 1
> 
> OK, I see in bpf-next includes several fixes like:
> 
> commit a0964f526df6facd4e12a4c416185013026eecf9
> "selftests/bpf: Add multi-file statically linked BPF object file test"
> 
> ...and to "selftests: xsk".
> 
> Finally, I was able to build by suppressing the build of "test_cpp"
> and "xdpxceiver":
> 
> $ git diff tools/testing/selftests/bpf/Makefile
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 044bfdcf5b74..d9b19524b2d4 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
> # Compile but not part of 'make run_tests'
> TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
>         flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> -       xdpxceiver
> +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
> +       # test_cpp xdpxceiver
> 
> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> 
> This diff is also attached before Gmail eats it.
> 
> Yonghong Song as you described your build-environment and checking
> requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
> to upgrade LLVM toolchain to v13-git and use bpf-next as the new
> kernel base.
> Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
> <apt.llvm.org>.

If you want to run bpf-next, clang v13 definitely recommended.
But I think if you use clang v13 to run linus linux, you may
hit DWARF5 DW_OP_addrx as well. But unfortunately you will
may hit a few selftest issues (e.g., BPF_TCP_CLOSE issue).

> 
> - Sedat -
> 
> [1] https://git.kernel.org/bpf/bpf-next/c/2ba4badca9977b64c966b0177920daadbd5501fe
> [2] https://git.kernel.org/bpf/bpf-next/c/a0964f526df6facd4e12a4c416185013026eecf9
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-03 18:41 [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
                   ` (2 preceding siblings ...)
  2021-04-04 14:59 ` Usage of CXX in tools directory Sedat Dilek
@ 2021-04-04 16:45 ` Yonghong Song
  2021-04-04 17:29   ` Sedat Dilek
  3 siblings, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-04 16:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Bill Wendling, bpf, David Blaikie,
	kernel-team, Nick Desaulniers, Sedat Dilek



On 4/3/21 11:41 AM, Yonghong Song 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
>            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
>    ...
>    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.
> 
> 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.

Arnaldo, sorry, I just found that I forgot signed-off. Here it is,

Signed-off-by: Yonghong Song <yhs@fb.com>

If no further change is needed for this patch, maybe you can help add 
it? Otherwise, I can add it in v2. Thanks!

> ---
>   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;
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-04 16:39         ` Yonghong Song
@ 2021-04-04 17:25           ` Sedat Dilek
  2021-04-05  2:24             ` Yonghong Song
       [not found]             ` <CA+icZUVp3UTPUS-ZjCOnHbNXxaA7DN=4x_08jc8BExFe4Nf2ZQ@mail.gmail.com>
  0 siblings, 2 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-04 17:25 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/4/21 5:46 AM, Sedat Dilek wrote:
> >> This shows a new build-error:
> >>
> >> clang  -g -D__TARGET_ARCH_x86 -mlittle-endian
> >> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> >> -I/home/dileks/src/linux-kernel/git/tools/t
> >> esting/selftests/bpf
> >> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> >> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/usr/include
> >> -idirafter /usr/loc
> >> al/include -idirafter /opt/llvm-toolchain/lib/clang/12.0.0/include
> >> -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include
> >> -Wno-compare-distinct-pointer-type
> >> s -DENABLE_ATOMICS_TESTS -O2 -target bpf -c
> >> progs/test_sk_storage_tracing.c -o
> >> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o-mcpu=v3
> >> progs/test_sk_storage_tracing.c:38:18: error: use of undeclared
> >> identifier 'BPF_TCP_CLOSE'
> >>         if (newstate == BPF_TCP_CLOSE)
> >>                         ^
> >> 1 error generated.
> >> make: *** [Makefile:414:
> >> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o]
> >> Error 1
> >>
> >
> > I was able to fix this by adding appropriate enums from <linux/bpf.h>.
> >
> > $ git diff
> > diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> > b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> > index 8e94e5c080aa..3c7508f48ce0 100644
> > --- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> > +++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
> > @@ -6,6 +6,28 @@
> > #include <bpf/bpf_core_read.h>
> > #include <bpf/bpf_helpers.h>
> >
> > +/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> > + * changes between the TCP and BPF versions. Ideally this should never happen.
> > + * If it does, we need to add code to convert them before calling
> > + * the BPF sock_ops function.
> > + */
> > +enum {
> > +       BPF_TCP_ESTABLISHED = 1,
> > +       BPF_TCP_SYN_SENT,
> > +       BPF_TCP_SYN_RECV,
> > +       BPF_TCP_FIN_WAIT1,
> > +       BPF_TCP_FIN_WAIT2,
> > +       BPF_TCP_TIME_WAIT,
> > +       BPF_TCP_CLOSE,
> > +       BPF_TCP_CLOSE_WAIT,
> > +       BPF_TCP_LAST_ACK,
> > +       BPF_TCP_LISTEN,
> > +       BPF_TCP_CLOSING,        /* Now a valid state */
> > +       BPF_TCP_NEW_SYN_RECV,
> > +
> > +       BPF_TCP_MAX_STATES      /* Leave at the end! */
> > +};
> > +
> > struct sk_stg {
> >         __u32 pid;
> >         __u32 last_notclose_state;
> >
> > NOTE: Attached as a diff as Gmail might truncate it.
>
> This bpf-next commit:
>    commit 97a19caf1b1f6a9d4f620a9d51405a1973bd4641
>    Author: Yonghong Song <yhs@fb.com>
>    Date:   Wed Mar 17 10:41:32 2021 -0700
>
>      bpf: net: Emit anonymous enum with BPF_TCP_CLOSE value explicitly
>
> fixed the issue.
>

Cool, looks like the correct fix.

> >
> > [ Q ] Should these enums be in vmlinux.h - if so why are they missing?
> >
> > Next build-error:
> >
> > g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
> > pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
> > -I/home/dileks/src/linux-kernel/git/tools/lib
> > -I/home/dileks/src/linux-kernel/git/tools/include
> > -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -Dbpf_prog_load=bpf_prog_test_load
> > -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> > -lcap -lelf -lz -lrt -lpthread -o
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> > /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
> > relocation R_X86_64_32 against `.rodata.str1.1' ca
> > n not be used when making a PIE object; recompile with -fPIE
> > collect2: error: ld returned 1 exit status
> > make: *** [Makefile:455:
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> > Error 1
> > make: Leaving directory
> > '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> >
> > LOL, I was not aware that there is usage of *** CXX*** in tools
> > directory (see g++ line and /usr/bin/ld ?).
> >
> > So, I changed my $MAKE_OPTS to use "CXX=clang++".
>
> In kernel, if LLVM=1 is set, we have:
>
> ifneq ($(LLVM),)
> HOSTCC  = clang
> HOSTCXX = clang++
> else
> HOSTCC  = gcc
> HOSTCXX = g++
> endif
>
> ifneq ($(LLVM),)
> CC              = clang
> LD              = ld.lld
> AR              = llvm-ar
> NM              = llvm-nm
> OBJCOPY         = llvm-objcopy
> OBJDUMP         = llvm-objdump
> READELF         = llvm-readelf
> STRIP           = llvm-strip
> else
> CC              = $(CROSS_COMPILE)gcc
> LD              = $(CROSS_COMPILE)ld
> AR              = $(CROSS_COMPILE)ar
> NM              = $(CROSS_COMPILE)nm
> OBJCOPY         = $(CROSS_COMPILE)objcopy
> OBJDUMP         = $(CROSS_COMPILE)objdump
> READELF         = $(CROSS_COMPILE)readelf
> STRIP           = $(CROSS_COMPILE)strip
> endif
>
> So if you have right path, you don't need to set HOSTCC and HOSTCXX
> explicitly.
>

That is all correct with HOSTCXX but there is no CXX=... assignment
otherwise test_cpp will use g++ as demonstrated.

> >
> > $ echo $PATH
> > /opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
> >
> > $ echo $MAKE $MAKE_OPTS
> > make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> > CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole
> >
> > $ 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
> > $ ld.lld --version
> > LLD 12.0.0 (https://github.com/llvm/llvm-project.git
> > 04ba60cfe598e41084fb848daae47e0ed910fa7d) (compatible with GNU
> > linkers)
> >
> > $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/
> >
> > This breaks like this:
> >
> > clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftes
> > ts/bpf/tools/include
> > -I/home/dileks/src/linux-kernel/git/include/generated
> > -I/home/dileks/src/linux-kernel/git/tools/lib
> > -I/home/dileks/src/linux-kernel/git/tools/incl
> > ude -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -Dbpf_prog_load=bpf_prog_test_load -Dbpf_loa
> > d_program=bpf_test_load_program test_cpp.cpp
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> > /home/dileks/src/linux-kernel/git/to
> > ols/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> > -lcap -lelf -lz -lrt -lpthread -o /home
> > /dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> > clang-12: warning: treating 'c-header' input as 'c++-header' when in
> > C++ mode, this behavior is deprecated [-Wdeprecated]
> > clang-12: error: cannot specify -o when generating multiple output files
> > make: *** [Makefile:455:
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> > Error 1
> >
> > OK, I see in bpf-next includes several fixes like:
> >
> > commit a0964f526df6facd4e12a4c416185013026eecf9
> > "selftests/bpf: Add multi-file statically linked BPF object file test"
> >
> > ...and to "selftests: xsk".
> >
> > Finally, I was able to build by suppressing the build of "test_cpp"
> > and "xdpxceiver":
> >
> > $ git diff tools/testing/selftests/bpf/Makefile
> > diff --git a/tools/testing/selftests/bpf/Makefile
> > b/tools/testing/selftests/bpf/Makefile
> > index 044bfdcf5b74..d9b19524b2d4 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
> > # Compile but not part of 'make run_tests'
> > TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> >         flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> > -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> > -       xdpxceiver
> > +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
> > +       # test_cpp xdpxceiver
> >
> > TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> >
> > This diff is also attached before Gmail eats it.
> >
> > Yonghong Song as you described your build-environment and checking
> > requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
> > to upgrade LLVM toolchain to v13-git and use bpf-next as the new
> > kernel base.
> > Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
> > <apt.llvm.org>.
>
> If you want to run bpf-next, clang v13 definitely recommended.
> But I think if you use clang v13 to run linus linux, you may
> hit DWARF5 DW_OP_addrx as well. But unfortunately you will
> may hit a few selftest issues (e.g., BPF_TCP_CLOSE issue).
>

OK, I started a fresh build with LLVM/Clang v13-git from <apt.llvm.org>...

$ /usr/lib/llvm-13/bin/clang --version
Debian clang version
13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-13/bin

...with latest bpf-next as new base.

I applied your/this pahole patch "[PATCH dwarves] dwarf_loader: handle
DWARF5 DW_OP_addrx properly".

Will report later...

- Sedat -

> >
> > [1] https://git.kernel.org/bpf/bpf-next/c/2ba4badca9977b64c966b0177920daadbd5501fe
> > [2] https://git.kernel.org/bpf/bpf-next/c/a0964f526df6facd4e12a4c416185013026eecf9
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-04 16:45 ` [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
@ 2021-04-04 17:29   ` Sedat Dilek
  0 siblings, 0 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-04 17:29 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Sun, Apr 4, 2021 at 6:45 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/3/21 11:41 AM, Yonghong Song 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
> >            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
> >    ...
> >    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.
> >
> > 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.
>
> Arnaldo, sorry, I just found that I forgot signed-off. Here it is,
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
>
> If no further change is needed for this patch, maybe you can help add
> it? Otherwise, I can add it in v2. Thanks!
>

I see a S-o-b when using b4 tool:

link="https://lore.kernel.org/bpf/20210403184158.2834387-1-yhs@fb.com"
b4 -d am $link

$ grep Signed-off-by:
20210403_yhs_dwarf_loader_handle_dwarf5_dw_op_addrx_properly.mbx
71:Signed-off-by: Yonghong Song <yhs@fb.com>

- Sedat -
> > ---
> >   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;
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-04 17:25           ` Sedat Dilek
@ 2021-04-05  2:24             ` Yonghong Song
       [not found]               ` <CA+icZUVcQ+vQjc0VavetA3s6jzNhC20dU4Sa9ApBLNXbY=w5wA@mail.gmail.com>
       [not found]             ` <CA+icZUVp3UTPUS-ZjCOnHbNXxaA7DN=4x_08jc8BExFe4Nf2ZQ@mail.gmail.com>
  1 sibling, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-05  2:24 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



On 4/4/21 10:25 AM, Sedat Dilek wrote:
> On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 4/4/21 5:46 AM, Sedat Dilek wrote:
[...]
>>> Next build-error:
>>>
>>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
>>> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
>>> -I/home/dileks/src/linux-kernel/git/tools/lib
>>> -I/home/dileks/src/linux-kernel/git/tools/include
>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -Dbpf_prog_load=bpf_prog_test_load
>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
>>> -lcap -lelf -lz -lrt -lpthread -o
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
>>> relocation R_X86_64_32 against `.rodata.str1.1' ca
>>> n not be used when making a PIE object; recompile with -fPIE
>>> collect2: error: ld returned 1 exit status
>>> make: *** [Makefile:455:
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>> Error 1
>>> make: Leaving directory
>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
>>>
>>> LOL, I was not aware that there is usage of *** CXX*** in tools
>>> directory (see g++ line and /usr/bin/ld ?).
>>>
>>> So, I changed my $MAKE_OPTS to use "CXX=clang++".
>>
>> In kernel, if LLVM=1 is set, we have:
>>
>> ifneq ($(LLVM),)
>> HOSTCC  = clang
>> HOSTCXX = clang++
>> else
>> HOSTCC  = gcc
>> HOSTCXX = g++
>> endif
>>
>> ifneq ($(LLVM),)
>> CC              = clang
>> LD              = ld.lld
>> AR              = llvm-ar
>> NM              = llvm-nm
>> OBJCOPY         = llvm-objcopy
>> OBJDUMP         = llvm-objdump
>> READELF         = llvm-readelf
>> STRIP           = llvm-strip
>> else
>> CC              = $(CROSS_COMPILE)gcc
>> LD              = $(CROSS_COMPILE)ld
>> AR              = $(CROSS_COMPILE)ar
>> NM              = $(CROSS_COMPILE)nm
>> OBJCOPY         = $(CROSS_COMPILE)objcopy
>> OBJDUMP         = $(CROSS_COMPILE)objdump
>> READELF         = $(CROSS_COMPILE)readelf
>> STRIP           = $(CROSS_COMPILE)strip
>> endif
>>
>> So if you have right path, you don't need to set HOSTCC and HOSTCXX
>> explicitly.
>>
> 
> That is all correct with HOSTCXX but there is no CXX=... assignment
> otherwise test_cpp will use g++ as demonstrated.

This is not a kernel Makefile issue.

We have:
testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++

So you need to explicit add CXX=clang++ when compiling
bpf selftests with LLVM=1 LLVM_IAS=1. 


> 
>>>
>>> $ echo $PATH
>>> /opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
>>>
>>> $ echo $MAKE $MAKE_OPTS
>>> make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
>>> CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole
>>>
>>> $ 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
>>> $ ld.lld --version
>>> LLD 12.0.0 (https://github.com/llvm/llvm-project.git
>>> 04ba60cfe598e41084fb848daae47e0ed910fa7d) (compatible with GNU
>>> linkers)
>>>
>>> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/
>>>
>>> This breaks like this:
>>>
>>> clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftes
>>> ts/bpf/tools/include
>>> -I/home/dileks/src/linux-kernel/git/include/generated
>>> -I/home/dileks/src/linux-kernel/git/tools/lib
>>> -I/home/dileks/src/linux-kernel/git/tools/incl
>>> ude -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -Dbpf_prog_load=bpf_prog_test_load -Dbpf_loa
>>> d_program=bpf_test_load_program test_cpp.cpp
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>> /home/dileks/src/linux-kernel/git/to
>>> ols/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
>>> -lcap -lelf -lz -lrt -lpthread -o /home
>>> /dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>> clang-12: warning: treating 'c-header' input as 'c++-header' when in
>>> C++ mode, this behavior is deprecated [-Wdeprecated]
>>> clang-12: error: cannot specify -o when generating multiple output files
>>> make: *** [Makefile:455:
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>> Error 1
>>>
>>> OK, I see in bpf-next includes several fixes like:
>>>
>>> commit a0964f526df6facd4e12a4c416185013026eecf9
>>> "selftests/bpf: Add multi-file statically linked BPF object file test"
>>>
>>> ...and to "selftests: xsk".
>>>
>>> Finally, I was able to build by suppressing the build of "test_cpp"
>>> and "xdpxceiver":
>>>
[...]
> 
> OK, I started a fresh build with LLVM/Clang v13-git from <apt.llvm.org>...
> 
> $ /usr/lib/llvm-13/bin/clang --version
> Debian clang version
> 13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> InstalledDir: /usr/lib/llvm-13/bin
> 
> ...with latest bpf-next as new base.
> 
> I applied your/this pahole patch "[PATCH dwarves] dwarf_loader: handle
> DWARF5 DW_OP_addrx properly".
> 
> Will report later...
> 
> - Sedat -
> 
>>>
>>> [1] https://git.kernel.org/bpf/bpf-next/c/2ba4badca9977b64c966b0177920daadbd5501fe
>>> [2] https://git.kernel.org/bpf/bpf-next/c/a0964f526df6facd4e12a4c416185013026eecf9
>>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found]             ` <CA+icZUVp3UTPUS-ZjCOnHbNXxaA7DN=4x_08jc8BExFe4Nf2ZQ@mail.gmail.com>
@ 2021-04-05  2:30               ` Yonghong Song
       [not found]                 ` <CA+icZUVtzXNxuVtEUwfULa7nivV0VFfJznsRnSZtEh+V=C=RPg@mail.gmail.com>
  0 siblings, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-05  2:30 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



On 4/4/21 5:20 PM, Sedat Dilek wrote:
> On Sun, Apr 4, 2021 at 7:25 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> [ ... ]
>>>> Yonghong Song as you described your build-environment and checking
>>>> requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
>>>> to upgrade LLVM toolchain to v13-git and use bpf-next as the new
>>>> kernel base.
>>>> Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
>>>> <apt.llvm.org>.
>>>
>>> If you want to run bpf-next, clang v13 definitely recommended.
>>> But I think if you use clang v13 to run linus linux, you may
>>> hit DWARF5 DW_OP_addrx as well. But unfortunately you will
>>> may hit a few selftest issues (e.g., BPF_TCP_CLOSE issue).
>>>
>>
>> OK, I started a fresh build with LLVM/Clang v13-git from <apt.llvm.org>...
>>
>> $ /usr/lib/llvm-13/bin/clang --version
>> Debian clang version
>> 13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891
>> Target: x86_64-pc-linux-gnu
>> Thread model: posix
>> InstalledDir: /usr/lib/llvm-13/bin
>>
>> ...with latest bpf-next as new base.
>>
>> I applied your/this pahole patch "[PATCH dwarves] dwarf_loader: handle
>> DWARF5 DW_OP_addrx properly".
>>
>> Will report later...
>>
> 
> Yupp, works.
> 
> $ cat /proc/version
> Linux version 5.12.0-rc5-13-amd64-clang13-lto
> (sedat.dilek@gmail.com@iniza) (Debian clang version
> 13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891, LLD
> 13.0.0) #13~bullseye+dileks1 SMP 2021-04-04
> 
> MAKE="make V=1"
> MAKE_OPTS="LLVM=1 LLVM_IAS=1"
> MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> 
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1.txt
> 
> dileks@iniza:~/src/linux-kernel/git/tools/testing/selftests/bpf$ sudo
> ./test_progs -n 55/2
> #55/2 subprog:OK
> #55 kfunc_call:OK
> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
> 
> My linux-config and
> make-log_tools-testing-selftests-bpf_llvm-1_llvm_ias-1.txt.gz files
> are attached.
> 
> Feel free to add my:
> 
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13-git (x86-64)

Great! Thanks for the help to test the pahole/kernel patches.

> 
> - Sedat -
> 
> P.S.: List of some relevant Linux Kconfigs
> 
> $ grep 'LTO_|_BTF|DWARF' /boot/config-5.12.0-rc5-13-amd64-clang13-lto
> | grep ^CONFIG | sort
> CONFIG_ARCH_SUPPORTS_LTO_CLANG_THIN=y
> CONFIG_ARCH_SUPPORTS_LTO_CLANG=y
> CONFIG_DEBUG_INFO_BTF_MODULES=y
> CONFIG_DEBUG_INFO_BTF=y
> CONFIG_DEBUG_INFO_DWARF5=y
> CONFIG_HAS_LTO_CLANG=y
> CONFIG_LTO_CLANG_THIN=y
> CONFIG_LTO_CLANG=y
> CONFIG_PAHOLE_HAS_SPLIT_BTF=y
> 
> - EOT -
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found]                 ` <CA+icZUVtzXNxuVtEUwfULa7nivV0VFfJznsRnSZtEh+V=C=RPg@mail.gmail.com>
@ 2021-04-05  6:56                   ` Sedat Dilek
  0 siblings, 0 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-05  6:56 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Mon, Apr 5, 2021 at 8:17 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Apr 5, 2021 at 4:31 AM Yonghong Song <yhs@fb.com> wrote:
> >
> >
> >
> > On 4/4/21 5:20 PM, Sedat Dilek wrote:
> > > On Sun, Apr 4, 2021 at 7:25 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > > [ ... ]
> > >>>> Yonghong Song as you described your build-environment and checking
> > >>>> requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
> > >>>> to upgrade LLVM toolchain to v13-git and use bpf-next as the new
> > >>>> kernel base.
> > >>>> Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
> > >>>> <apt.llvm.org>.
> > >>>
> > >>> If you want to run bpf-next, clang v13 definitely recommended.
> > >>> But I think if you use clang v13 to run linus linux, you may
> > >>> hit DWARF5 DW_OP_addrx as well. But unfortunately you will
> > >>> may hit a few selftest issues (e.g., BPF_TCP_CLOSE issue).
> > >>>
> > >>
> > >> OK, I started a fresh build with LLVM/Clang v13-git from <apt.llvm.org>...
> > >>
> > >> $ /usr/lib/llvm-13/bin/clang --version
> > >> Debian clang version
> > >> 13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891
> > >> Target: x86_64-pc-linux-gnu
> > >> Thread model: posix
> > >> InstalledDir: /usr/lib/llvm-13/bin
> > >>
> > >> ...with latest bpf-next as new base.
> > >>
> > >> I applied your/this pahole patch "[PATCH dwarves] dwarf_loader: handle
> > >> DWARF5 DW_OP_addrx properly".
> > >>
> > >> Will report later...
> > >>
> > >
> > > Yupp, works.
> > >
> > > $ cat /proc/version
> > > Linux version 5.12.0-rc5-13-amd64-clang13-lto
> > > (sedat.dilek@gmail.com@iniza) (Debian clang version
> > > 13.0.0-++20210404092853+c4c511337247-1~exp1~20210404073605.3891, LLD
> > > 13.0.0) #13~bullseye+dileks1 SMP 2021-04-04
> > >
> > > MAKE="make V=1"
> > > MAKE_OPTS="LLVM=1 LLVM_IAS=1"
> > > MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> > >
> > > $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> > > ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1.txt
> > >
> > > dileks@iniza:~/src/linux-kernel/git/tools/testing/selftests/bpf$ sudo
> > > ./test_progs -n 55/2
> > > #55/2 subprog:OK
> > > #55 kfunc_call:OK
> > > Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
> > >
> > > My linux-config and
> > > make-log_tools-testing-selftests-bpf_llvm-1_llvm_ias-1.txt.gz files
> > > are attached.
> > >
> > > Feel free to add my:
> > >
> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13-git (x86-64)
> >
> > Great! Thanks for the help to test the pahole/kernel patches.
> >
>
> Thank you Yonghong!
>
> I was able to do so with Linux v5.12-rc6 and some custom patches using
> LLVM/clang v12.0.0-rc4.
>
> Please see attachments.
>
> I applied this commit from bpf-next:
>
> commit 97a19caf1b1f6a9d4f620a9d51405a1973bd4641
> "bpf: net: Emit anonymous enum with BPF_TCP_CLOSE value explicitly"
>
> Link: https://git.kernel.org/bpf/bpf-next/c/97a19caf1b1f6a9d4f620a9d51405a1973bd4641
>
> Checking vmlinux.h:
>
> $ grep 'BPF_TCP_CLOSE ' tools/testing/selftests/bpf/tools/include/vmlinux.h
> 91929:  BPF_TCP_CLOSE = 7,
>
> Checking with test_progs:
>
> dileks@iniza:~/src/linux-kernel/git/tools/testing/selftests/bpf$ sudo
> ./test_progs -n 55/2
> #55/2 null_check:OK
> #55 ksyms_btf:OK
> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>
> Can we have this patch from bpf-next for Linus Git?
>

I am awfully sorry to have sent the make-log file uncompressed
(~1.1MiB) to the list.
Mea culpa.

- Sedat -

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found]               ` <CA+icZUVcQ+vQjc0VavetA3s6jzNhC20dU4Sa9ApBLNXbY=w5wA@mail.gmail.com>
@ 2021-04-05 11:04                 ` Sedat Dilek
  2021-04-05 11:46                   ` Sedat Dilek
  2021-04-05 16:17                 ` Yonghong Song
  1 sibling, 1 reply; 23+ messages in thread
From: Sedat Dilek @ 2021-04-05 11:04 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

I hoped to drop "test_core_extern.skel.h"
tools/testing/selftests/bpf/Makefile as test_cpp.cpp includes it:

$ git grep include tools/testing/selftests/bpf/test_cpp.cpp
tools/testing/selftests/bpf/test_cpp.cpp:#include "test_core_extern.skel.h"

$ git diff
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..a93e4d6ff93c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -450,7 +450,7 @@ $(OUTPUT)/test_verifier: test_verifier.c
verifier/tests.h $(BPFOBJ) | $(OUTPUT)
       $(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@

# Make sure we are able to include and link libbpf against c++.
-$(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ)
+$(OUTPUT)/test_cpp: test_cpp.cpp $(BPFOBJ)
       $(call msg,CXX,,$@)
       $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o $@

When using g++:

$ llvm-objdump-12 -Dr test_cpp | grep test_core_extern
   77dd: e8 be 01 00 00                callq   0x79a0
<_ZL25test_core_extern__destroyP16test_core_extern>
   7842: e8 59 01 00 00                callq   0x79a0
<_ZL25test_core_extern__destroyP16test_core_extern>
00000000000079a0 <_ZL25test_core_extern__destroyP16test_core_extern>:
   79a3: 74 1a                         je      0x79bf
<_ZL25test_core_extern__destroyP16test_core_extern+0x1f>
   79af: 74 05                         je      0x79b6
<_ZL25test_core_extern__destroyP16test_core_extern+0x16>
   799e: 74 06                         je      0x79a6
<_ZL25test_core_extern__destroyP16test_core_extern+0x6>
   7942: 73 61                         jae     0x79a5
<_ZL25test_core_extern__destroyP16test_core_extern+0x5>
   7945: 70 6c                         jo      0x79b3
<_ZL25test_core_extern__destroyP16test_core_extern+0x13>
   794b: 70 65                         jo      0x79b2
<_ZL25test_core_extern__destroyP16test_core_extern+0x12>
   7954: 73 5f                         jae     0x79b5
<_ZL25test_core_extern__destroyP16test_core_extern+0x15>
   79aa: 79 00                         jns     0x79ac
<_ZL25test_core_extern__destroyP16test_core_extern+0xc>

When using clang++-12:

$ llvm-objdump-12 -Dr test_cpp | grep test_core_extern
[ empty ]

Last I tried:

selftests-bpf-Makefile-EXTRA_CXXFLAGS-x-c-header.diff
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..df07fd9325d0 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -27,6 +27,7 @@ CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS)
$(SAN_CFLAGS)            \
         -Dbpf_prog_load=bpf_prog_test_load                            \
         -Dbpf_load_program=bpf_test_load_program
LDLIBS += -lcap -lelf -lz -lrt -lpthread
+EXTRA_CXXFLAGS := -x c-header

# Order correspond to 'make run_tests' order
TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map
test_lpm_map test_progs \
@@ -452,7 +453,7 @@ $(OUTPUT)/test_verifier: test_verifier.c
verifier/tests.h $(BPFOBJ) | $(OUTPUT)
# Make sure we are able to include and link libbpf against c++.
$(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ)
       $(call msg,CXX,,$@)
-       $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o $@
+       $(Q)$(CXX) $(CFLAGS) $(EXTRA_CXXFLAGS) $^ $(LDLIBS) -o $@

# Benchmark runner
$(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h

NOPE.

- Sedat -

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-05 11:04                 ` Sedat Dilek
@ 2021-04-05 11:46                   ` Sedat Dilek
  0 siblings, 0 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-05 11:46 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Mon, Apr 5, 2021 at 1:04 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> I hoped to drop "test_core_extern.skel.h"
> tools/testing/selftests/bpf/Makefile as test_cpp.cpp includes it:
>
> $ git grep include tools/testing/selftests/bpf/test_cpp.cpp
> tools/testing/selftests/bpf/test_cpp.cpp:#include "test_core_extern.skel.h"
>
> $ git diff
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 044bfdcf5b74..a93e4d6ff93c 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -450,7 +450,7 @@ $(OUTPUT)/test_verifier: test_verifier.c
> verifier/tests.h $(BPFOBJ) | $(OUTPUT)
>        $(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
>
> # Make sure we are able to include and link libbpf against c++.
> -$(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ)
> +$(OUTPUT)/test_cpp: test_cpp.cpp $(BPFOBJ)
>        $(call msg,CXX,,$@)
>        $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o $@
>
> When using g++:
>
> $ llvm-objdump-12 -Dr test_cpp | grep test_core_extern
>    77dd: e8 be 01 00 00                callq   0x79a0
> <_ZL25test_core_extern__destroyP16test_core_extern>
>    7842: e8 59 01 00 00                callq   0x79a0
> <_ZL25test_core_extern__destroyP16test_core_extern>
> 00000000000079a0 <_ZL25test_core_extern__destroyP16test_core_extern>:
>    79a3: 74 1a                         je      0x79bf
> <_ZL25test_core_extern__destroyP16test_core_extern+0x1f>
>    79af: 74 05                         je      0x79b6
> <_ZL25test_core_extern__destroyP16test_core_extern+0x16>
>    799e: 74 06                         je      0x79a6
> <_ZL25test_core_extern__destroyP16test_core_extern+0x6>
>    7942: 73 61                         jae     0x79a5
> <_ZL25test_core_extern__destroyP16test_core_extern+0x5>
>    7945: 70 6c                         jo      0x79b3
> <_ZL25test_core_extern__destroyP16test_core_extern+0x13>
>    794b: 70 65                         jo      0x79b2
> <_ZL25test_core_extern__destroyP16test_core_extern+0x12>
>    7954: 73 5f                         jae     0x79b5
> <_ZL25test_core_extern__destroyP16test_core_extern+0x15>
>    79aa: 79 00                         jns     0x79ac
> <_ZL25test_core_extern__destroyP16test_core_extern+0xc>
>

Just to clarify:
Using g++ results in the same llvm-objdump output - independent of
keeping or dropping "$(OUTPUT)/test_core_extern.skel.h".

- Sedat -

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
       [not found]               ` <CA+icZUVcQ+vQjc0VavetA3s6jzNhC20dU4Sa9ApBLNXbY=w5wA@mail.gmail.com>
  2021-04-05 11:04                 ` Sedat Dilek
@ 2021-04-05 16:17                 ` Yonghong Song
  2021-04-05 18:32                   ` Sedat Dilek
  1 sibling, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-05 16:17 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



On 4/4/21 11:55 PM, Sedat Dilek wrote:
> On Mon, Apr 5, 2021 at 4:24 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 4/4/21 10:25 AM, Sedat Dilek wrote:
>>> On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/4/21 5:46 AM, Sedat Dilek wrote:
>> [...]
>>>>> Next build-error:
>>>>>
>>>>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
>>>>> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
>>>>> -I/home/dileks/src/linux-kernel/git/tools/lib
>>>>> -I/home/dileks/src/linux-kernel/git/tools/include
>>>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>>>> -Dbpf_prog_load=bpf_prog_test_load
>>>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
>>>>> -lcap -lelf -lz -lrt -lpthread -o
>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>>>> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
>>>>> relocation R_X86_64_32 against `.rodata.str1.1' ca
>>>>> n not be used when making a PIE object; recompile with -fPIE
>>>>> collect2: error: ld returned 1 exit status
>>>>> make: *** [Makefile:455:
>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>>>> Error 1
>>>>> make: Leaving directory
>>>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
>>>>>
>>>>> LOL, I was not aware that there is usage of *** CXX*** in tools
>>>>> directory (see g++ line and /usr/bin/ld ?).
>>>>>
>>>>> So, I changed my $MAKE_OPTS to use "CXX=clang++".
>>>>
>>>> In kernel, if LLVM=1 is set, we have:
>>>>
>>>> ifneq ($(LLVM),)
>>>> HOSTCC  = clang
>>>> HOSTCXX = clang++
>>>> else
>>>> HOSTCC  = gcc
>>>> HOSTCXX = g++
>>>> endif
>>>>
>>>> ifneq ($(LLVM),)
>>>> CC              = clang
>>>> LD              = ld.lld
>>>> AR              = llvm-ar
>>>> NM              = llvm-nm
>>>> OBJCOPY         = llvm-objcopy
>>>> OBJDUMP         = llvm-objdump
>>>> READELF         = llvm-readelf
>>>> STRIP           = llvm-strip
>>>> else
>>>> CC              = $(CROSS_COMPILE)gcc
>>>> LD              = $(CROSS_COMPILE)ld
>>>> AR              = $(CROSS_COMPILE)ar
>>>> NM              = $(CROSS_COMPILE)nm
>>>> OBJCOPY         = $(CROSS_COMPILE)objcopy
>>>> OBJDUMP         = $(CROSS_COMPILE)objdump
>>>> READELF         = $(CROSS_COMPILE)readelf
>>>> STRIP           = $(CROSS_COMPILE)strip
>>>> endif
>>>>
>>>> So if you have right path, you don't need to set HOSTCC and HOSTCXX
>>>> explicitly.
>>>>
>>>
>>> That is all correct with HOSTCXX but there is no CXX=... assignment
>>> otherwise test_cpp will use g++ as demonstrated.
>>
>> This is not a kernel Makefile issue.
>>
>> We have:
>> testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
>>
>> So you need to explicit add CXX=clang++ when compiling
>> bpf selftests with LLVM=1 LLVM_IAS=1.
>>
> 
> NOPE.
> 
> $ echo $MAKE $MAKE_OPTS
> make V=1 LLVM=1 LLVM_IAS=1 CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
> 
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1_cxx-clang.txt
> 
> This breaks again like reported before:
> 
> clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> -I/home/dileks/src/linux-kernel/git/include/generated
> -I/home/dileks/src/linux-kernel/git/tools/lib
> -I/home/dileks/src/linux-kernel/git/tools/include
> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> -Dbpf_prog_load=bpf_prog_test_load
> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> -lcap -lelf -lz -lrt -lpthread -o
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> 
> clang-12: warning: treating 'c-header' input as 'c++-header' when in
> C++ mode, this behavior is deprecated [-Wdeprecated]
> clang-12: error: cannot specify -o when generating multiple output files
> make: *** [Makefile:455:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> Error 1
> make: Leaving directory
> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> 
> Do you know some magic CXX flags to be passed?

I tested in my environment. The reason is LC_ALL=C.
Without LC_ALL=C, make succeeded and with it, test_cpp
compilation failed. Is it possible for you to drop
LC_ALL=C for bpf selftests?

The following command succeeded for me:
    make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang


> 
> The only solution is to suppress the build of test_cpp (see
> TEST_GEN_PROGS_EXTENDED):
> 
> $ git diff tools/testing/selftests/bpf/Makefile
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 044bfdcf5b74..cf7c7c8f72cf 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
> # Compile but not part of 'make run_tests'
> TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
>         flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> -       xdpxceiver
> +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko xdpxceiver
> +       # test_cpp # Suppress the build when CXX=clang++ is used
> 
> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> 
> I have attached both make-logs with and without suppressing the build
> of test_cpp and the diff.
> 
> - Sedat -
> 
>>
>>>
[...]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-05 16:17                 ` Yonghong Song
@ 2021-04-05 18:32                   ` Sedat Dilek
  2021-04-05 18:56                     ` Yonghong Song
  0 siblings, 1 reply; 23+ messages in thread
From: Sedat Dilek @ 2021-04-05 18:32 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Mon, Apr 5, 2021 at 6:17 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/4/21 11:55 PM, Sedat Dilek wrote:
> > On Mon, Apr 5, 2021 at 4:24 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >>
> >> On 4/4/21 10:25 AM, Sedat Dilek wrote:
> >>> On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 4/4/21 5:46 AM, Sedat Dilek wrote:
> >> [...]
> >>>>> Next build-error:
> >>>>>
> >>>>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
> >>>>> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/lib
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/include
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> >>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>>>> -Dbpf_prog_load=bpf_prog_test_load
> >>>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> >>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> >>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> >>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> >>>>> -lcap -lelf -lz -lrt -lpthread -o
> >>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> >>>>> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
> >>>>> relocation R_X86_64_32 against `.rodata.str1.1' ca
> >>>>> n not be used when making a PIE object; recompile with -fPIE
> >>>>> collect2: error: ld returned 1 exit status
> >>>>> make: *** [Makefile:455:
> >>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> >>>>> Error 1
> >>>>> make: Leaving directory
> >>>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> >>>>>
> >>>>> LOL, I was not aware that there is usage of *** CXX*** in tools
> >>>>> directory (see g++ line and /usr/bin/ld ?).
> >>>>>
> >>>>> So, I changed my $MAKE_OPTS to use "CXX=clang++".
> >>>>
> >>>> In kernel, if LLVM=1 is set, we have:
> >>>>
> >>>> ifneq ($(LLVM),)
> >>>> HOSTCC  = clang
> >>>> HOSTCXX = clang++
> >>>> else
> >>>> HOSTCC  = gcc
> >>>> HOSTCXX = g++
> >>>> endif
> >>>>
> >>>> ifneq ($(LLVM),)
> >>>> CC              = clang
> >>>> LD              = ld.lld
> >>>> AR              = llvm-ar
> >>>> NM              = llvm-nm
> >>>> OBJCOPY         = llvm-objcopy
> >>>> OBJDUMP         = llvm-objdump
> >>>> READELF         = llvm-readelf
> >>>> STRIP           = llvm-strip
> >>>> else
> >>>> CC              = $(CROSS_COMPILE)gcc
> >>>> LD              = $(CROSS_COMPILE)ld
> >>>> AR              = $(CROSS_COMPILE)ar
> >>>> NM              = $(CROSS_COMPILE)nm
> >>>> OBJCOPY         = $(CROSS_COMPILE)objcopy
> >>>> OBJDUMP         = $(CROSS_COMPILE)objdump
> >>>> READELF         = $(CROSS_COMPILE)readelf
> >>>> STRIP           = $(CROSS_COMPILE)strip
> >>>> endif
> >>>>
> >>>> So if you have right path, you don't need to set HOSTCC and HOSTCXX
> >>>> explicitly.
> >>>>
> >>>
> >>> That is all correct with HOSTCXX but there is no CXX=... assignment
> >>> otherwise test_cpp will use g++ as demonstrated.
> >>
> >> This is not a kernel Makefile issue.
> >>
> >> We have:
> >> testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
> >>
> >> So you need to explicit add CXX=clang++ when compiling
> >> bpf selftests with LLVM=1 LLVM_IAS=1.
> >>
> >
> > NOPE.
> >
> > $ echo $MAKE $MAKE_OPTS
> > make V=1 LLVM=1 LLVM_IAS=1 CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
> >
> > $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> > ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1_cxx-clang.txt
> >
> > This breaks again like reported before:
> >
> > clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> > -I/home/dileks/src/linux-kernel/git/include/generated
> > -I/home/dileks/src/linux-kernel/git/tools/lib
> > -I/home/dileks/src/linux-kernel/git/tools/include
> > -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> > -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> > -Dbpf_prog_load=bpf_prog_test_load
> > -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> > -lcap -lelf -lz -lrt -lpthread -o
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> >
> > clang-12: warning: treating 'c-header' input as 'c++-header' when in
> > C++ mode, this behavior is deprecated [-Wdeprecated]
> > clang-12: error: cannot specify -o when generating multiple output files
> > make: *** [Makefile:455:
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> > Error 1
> > make: Leaving directory
> > '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> >
> > Do you know some magic CXX flags to be passed?
>
> I tested in my environment. The reason is LC_ALL=C.
> Without LC_ALL=C, make succeeded and with it, test_cpp
> compilation failed. Is it possible for you to drop
> LC_ALL=C for bpf selftests?
>
> The following command succeeded for me:
>     make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
>

First, I tried the exact make invocation ^^^ in my build-environment
but that breaks with the same ERROR.

I did in a second run:

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

MAKE="make"
MAKE_OPTS="V=1 -j4 LLVM=1 CC=clang CXX=clang++"
MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"

echo $MAKE $MAKE_OPTS
make V=1 -j4 LLVM=1 CC=clang CXX=clang++ PAHOLE=/opt/pahole/bin/pahole

$MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
../make-log_tools-testing-selftests-bpf.txt

That would have been funny... Drop LC_ALL=C from make line as a fix.

Just curious: Do you see these warnings?

clang-12: warning: argument unused during compilation: '-rdynamic'
[-Wunused-command-line-argument]
clang-12: warning: -lcap: 'linker' input unused [-Wunused-command-line-argument]
clang-12: warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]
clang-12: warning: -lz: 'linker' input unused [-Wunused-command-line-argument]
clang-12: warning: -lrt: 'linker' input unused [-Wunused-command-line-argument]
clang-12: warning: -lpthread: 'linker' input unused
[-Wunused-command-line-argument]
clang-12: warning: -lm: 'linker' input unused [-Wunused-command-line-argument]

Equivalent CFLAGS for '-rdynamic' when CC=clang is used?
Missing LDFLAGS when LD=ld.lld (make LLVM=1) is used?

Last question:
Can you pass LLVM_IAS=1 (means use LLVM/Clang Integrated ASsembler) to
your make line?

Old: make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
New: make -C tools/testing/selftests/bpf -j60 LLVM=1 LLVM_IAS=1 V=1
CXX=clang++ CC=clang

Does it build successfully?

- Sedat -

> >
> > The only solution is to suppress the build of test_cpp (see
> > TEST_GEN_PROGS_EXTENDED):
> >
> > $ git diff tools/testing/selftests/bpf/Makefile
> > diff --git a/tools/testing/selftests/bpf/Makefile
> > b/tools/testing/selftests/bpf/Makefile
> > index 044bfdcf5b74..cf7c7c8f72cf 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
> > # Compile but not part of 'make run_tests'
> > TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> >         flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> > -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> > -       xdpxceiver
> > +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko xdpxceiver
> > +       # test_cpp # Suppress the build when CXX=clang++ is used
> >
> > TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> >
> > I have attached both make-logs with and without suppressing the build
> > of test_cpp and the diff.
> >
> > - Sedat -
> >
> >>
> >>>
> [...]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-05 18:32                   ` Sedat Dilek
@ 2021-04-05 18:56                     ` Yonghong Song
  2021-04-05 20:42                       ` Sedat Dilek
  0 siblings, 1 reply; 23+ messages in thread
From: Yonghong Song @ 2021-04-05 18:56 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers



On 4/5/21 11:32 AM, Sedat Dilek wrote:
> On Mon, Apr 5, 2021 at 6:17 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 4/4/21 11:55 PM, Sedat Dilek wrote:
>>> On Mon, Apr 5, 2021 at 4:24 AM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/4/21 10:25 AM, Sedat Dilek wrote:
>>>>> On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 4/4/21 5:46 AM, Sedat Dilek wrote:
>>>> [...]
>>>>>>> Next build-error:
>>>>>>>
>>>>>>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
>>>>>>> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/lib
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/include
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>>>>>> -Dbpf_prog_load=bpf_prog_test_load
>>>>>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
>>>>>>> -lcap -lelf -lz -lrt -lpthread -o
>>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>>>>>> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
>>>>>>> relocation R_X86_64_32 against `.rodata.str1.1' ca
>>>>>>> n not be used when making a PIE object; recompile with -fPIE
>>>>>>> collect2: error: ld returned 1 exit status
>>>>>>> make: *** [Makefile:455:
>>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>>>>>> Error 1
>>>>>>> make: Leaving directory
>>>>>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
>>>>>>>
>>>>>>> LOL, I was not aware that there is usage of *** CXX*** in tools
>>>>>>> directory (see g++ line and /usr/bin/ld ?).
>>>>>>>
>>>>>>> So, I changed my $MAKE_OPTS to use "CXX=clang++".
>>>>>>
>>>>>> In kernel, if LLVM=1 is set, we have:
>>>>>>
>>>>>> ifneq ($(LLVM),)
>>>>>> HOSTCC  = clang
>>>>>> HOSTCXX = clang++
>>>>>> else
>>>>>> HOSTCC  = gcc
>>>>>> HOSTCXX = g++
>>>>>> endif
>>>>>>
>>>>>> ifneq ($(LLVM),)
>>>>>> CC              = clang
>>>>>> LD              = ld.lld
>>>>>> AR              = llvm-ar
>>>>>> NM              = llvm-nm
>>>>>> OBJCOPY         = llvm-objcopy
>>>>>> OBJDUMP         = llvm-objdump
>>>>>> READELF         = llvm-readelf
>>>>>> STRIP           = llvm-strip
>>>>>> else
>>>>>> CC              = $(CROSS_COMPILE)gcc
>>>>>> LD              = $(CROSS_COMPILE)ld
>>>>>> AR              = $(CROSS_COMPILE)ar
>>>>>> NM              = $(CROSS_COMPILE)nm
>>>>>> OBJCOPY         = $(CROSS_COMPILE)objcopy
>>>>>> OBJDUMP         = $(CROSS_COMPILE)objdump
>>>>>> READELF         = $(CROSS_COMPILE)readelf
>>>>>> STRIP           = $(CROSS_COMPILE)strip
>>>>>> endif
>>>>>>
>>>>>> So if you have right path, you don't need to set HOSTCC and HOSTCXX
>>>>>> explicitly.
>>>>>>
>>>>>
>>>>> That is all correct with HOSTCXX but there is no CXX=... assignment
>>>>> otherwise test_cpp will use g++ as demonstrated.
>>>>
>>>> This is not a kernel Makefile issue.
>>>>
>>>> We have:
>>>> testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
>>>>
>>>> So you need to explicit add CXX=clang++ when compiling
>>>> bpf selftests with LLVM=1 LLVM_IAS=1.
>>>>
>>>
>>> NOPE.
>>>
>>> $ echo $MAKE $MAKE_OPTS
>>> make V=1 LLVM=1 LLVM_IAS=1 CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
>>>
>>> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
>>> ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1_cxx-clang.txt
>>>
>>> This breaks again like reported before:
>>>
>>> clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
>>> -I/home/dileks/src/linux-kernel/git/include/generated
>>> -I/home/dileks/src/linux-kernel/git/tools/lib
>>> -I/home/dileks/src/linux-kernel/git/tools/include
>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
>>> -Dbpf_prog_load=bpf_prog_test_load
>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
>>> -lcap -lelf -lz -lrt -lpthread -o
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>>
>>> clang-12: warning: treating 'c-header' input as 'c++-header' when in
>>> C++ mode, this behavior is deprecated [-Wdeprecated]
>>> clang-12: error: cannot specify -o when generating multiple output files
>>> make: *** [Makefile:455:
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>> Error 1
>>> make: Leaving directory
>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
>>>
>>> Do you know some magic CXX flags to be passed?
>>
>> I tested in my environment. The reason is LC_ALL=C.
>> Without LC_ALL=C, make succeeded and with it, test_cpp
>> compilation failed. Is it possible for you to drop
>> LC_ALL=C for bpf selftests?
>>
>> The following command succeeded for me:
>>      make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
>>
> 
> First, I tried the exact make invocation ^^^ in my build-environment
> but that breaks with the same ERROR.
> 
> I did in a second run:
> 
> 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
> 
> MAKE="make"
> MAKE_OPTS="V=1 -j4 LLVM=1 CC=clang CXX=clang++"
> MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> 
> echo $MAKE $MAKE_OPTS
> make V=1 -j4 LLVM=1 CC=clang CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
> 
> $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> ../make-log_tools-testing-selftests-bpf.txt
> 
> That would have been funny... Drop LC_ALL=C from make line as a fix.
> 
> Just curious: Do you see these warnings?
> 
> clang-12: warning: argument unused during compilation: '-rdynamic'
> [-Wunused-command-line-argument]
> clang-12: warning: -lcap: 'linker' input unused [-Wunused-command-line-argument]
> clang-12: warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]
> clang-12: warning: -lz: 'linker' input unused [-Wunused-command-line-argument]
> clang-12: warning: -lrt: 'linker' input unused [-Wunused-command-line-argument]
> clang-12: warning: -lpthread: 'linker' input unused
> [-Wunused-command-line-argument]
> clang-12: warning: -lm: 'linker' input unused [-Wunused-command-line-argument]
> 
> Equivalent CFLAGS for '-rdynamic' when CC=clang is used?
> Missing LDFLAGS when LD=ld.lld (make LLVM=1) is used?

I see this warning as well, but seems it does not hurt...
Maybe some flags need change if the CC is clang...

> 
> Last question:
> Can you pass LLVM_IAS=1 (means use LLVM/Clang Integrated ASsembler) to
> your make line?
> 
> Old: make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
> New: make -C tools/testing/selftests/bpf -j60 LLVM=1 LLVM_IAS=1 V=1
> CXX=clang++ CC=clang
> 
> Does it build successfully?

I think it is better to add LLVM_IAS=1. In my build, for non-lto, I 
didn't pass LLVM_IAS=1 to kernel build, so selftest does not need it either.

But for LTO build, LLVM_IAS=1 is required so selftest also needs
LLVM_IAS=1.

Yes, we can always have LLVM_IAS=1 if it is included in kernel build.

> 
> - Sedat -
> 
>>>
>>> The only solution is to suppress the build of test_cpp (see
>>> TEST_GEN_PROGS_EXTENDED):
>>>
>>> $ git diff tools/testing/selftests/bpf/Makefile
>>> diff --git a/tools/testing/selftests/bpf/Makefile
>>> b/tools/testing/selftests/bpf/Makefile
>>> index 044bfdcf5b74..cf7c7c8f72cf 100644
>>> --- a/tools/testing/selftests/bpf/Makefile
>>> +++ b/tools/testing/selftests/bpf/Makefile
>>> @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
>>> # Compile but not part of 'make run_tests'
>>> TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
>>>          flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
>>> -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
>>> -       xdpxceiver
>>> +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko xdpxceiver
>>> +       # test_cpp # Suppress the build when CXX=clang++ is used
>>>
>>> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
>>>
>>> I have attached both make-logs with and without suppressing the build
>>> of test_cpp and the diff.
>>>
>>> - Sedat -
>>>
>>>>
>>>>>
>> [...]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
  2021-04-05 18:56                     ` Yonghong Song
@ 2021-04-05 20:42                       ` Sedat Dilek
  0 siblings, 0 replies; 23+ messages in thread
From: Sedat Dilek @ 2021-04-05 20:42 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Bill Wendling, bpf, David Blaikie, kernel-team, Nick Desaulniers

On Mon, Apr 5, 2021 at 8:56 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/5/21 11:32 AM, Sedat Dilek wrote:
> > On Mon, Apr 5, 2021 at 6:17 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >>
> >> On 4/4/21 11:55 PM, Sedat Dilek wrote:
> >>> On Mon, Apr 5, 2021 at 4:24 AM Yonghong Song <yhs@fb.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 4/4/21 10:25 AM, Sedat Dilek wrote:
> >>>>> On Sun, Apr 4, 2021 at 6:40 PM Yonghong Song <yhs@fb.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 4/4/21 5:46 AM, Sedat Dilek wrote:
> >>>> [...]
> >>>>>>> Next build-error:
> >>>>>>>
> >>>>>>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
> >>>>>>> pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/lib
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/include
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> >>>>>>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>>>>>> -Dbpf_prog_load=bpf_prog_test_load
> >>>>>>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> >>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> >>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> >>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> >>>>>>> -lcap -lelf -lz -lrt -lpthread -o
> >>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> >>>>>>> /usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
> >>>>>>> relocation R_X86_64_32 against `.rodata.str1.1' ca
> >>>>>>> n not be used when making a PIE object; recompile with -fPIE
> >>>>>>> collect2: error: ld returned 1 exit status
> >>>>>>> make: *** [Makefile:455:
> >>>>>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> >>>>>>> Error 1
> >>>>>>> make: Leaving directory
> >>>>>>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> >>>>>>>
> >>>>>>> LOL, I was not aware that there is usage of *** CXX*** in tools
> >>>>>>> directory (see g++ line and /usr/bin/ld ?).
> >>>>>>>
> >>>>>>> So, I changed my $MAKE_OPTS to use "CXX=clang++".
> >>>>>>
> >>>>>> In kernel, if LLVM=1 is set, we have:
> >>>>>>
> >>>>>> ifneq ($(LLVM),)
> >>>>>> HOSTCC  = clang
> >>>>>> HOSTCXX = clang++
> >>>>>> else
> >>>>>> HOSTCC  = gcc
> >>>>>> HOSTCXX = g++
> >>>>>> endif
> >>>>>>
> >>>>>> ifneq ($(LLVM),)
> >>>>>> CC              = clang
> >>>>>> LD              = ld.lld
> >>>>>> AR              = llvm-ar
> >>>>>> NM              = llvm-nm
> >>>>>> OBJCOPY         = llvm-objcopy
> >>>>>> OBJDUMP         = llvm-objdump
> >>>>>> READELF         = llvm-readelf
> >>>>>> STRIP           = llvm-strip
> >>>>>> else
> >>>>>> CC              = $(CROSS_COMPILE)gcc
> >>>>>> LD              = $(CROSS_COMPILE)ld
> >>>>>> AR              = $(CROSS_COMPILE)ar
> >>>>>> NM              = $(CROSS_COMPILE)nm
> >>>>>> OBJCOPY         = $(CROSS_COMPILE)objcopy
> >>>>>> OBJDUMP         = $(CROSS_COMPILE)objdump
> >>>>>> READELF         = $(CROSS_COMPILE)readelf
> >>>>>> STRIP           = $(CROSS_COMPILE)strip
> >>>>>> endif
> >>>>>>
> >>>>>> So if you have right path, you don't need to set HOSTCC and HOSTCXX
> >>>>>> explicitly.
> >>>>>>
> >>>>>
> >>>>> That is all correct with HOSTCXX but there is no CXX=... assignment
> >>>>> otherwise test_cpp will use g++ as demonstrated.
> >>>>
> >>>> This is not a kernel Makefile issue.
> >>>>
> >>>> We have:
> >>>> testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
> >>>>
> >>>> So you need to explicit add CXX=clang++ when compiling
> >>>> bpf selftests with LLVM=1 LLVM_IAS=1.
> >>>>
> >>>
> >>> NOPE.
> >>>
> >>> $ echo $MAKE $MAKE_OPTS
> >>> make V=1 LLVM=1 LLVM_IAS=1 CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
> >>>
> >>> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> >>> ../make-log_tools-testing-selftests-bpf_llvm-1-llvm_ias-1_cxx-clang.txt
> >>>
> >>> This breaks again like reported before:
> >>>
> >>> clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> >>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> >>> -I/home/dileks/src/linux-kernel/git/include/generated
> >>> -I/home/dileks/src/linux-kernel/git/tools/lib
> >>> -I/home/dileks/src/linux-kernel/git/tools/include
> >>> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> >>> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
> >>> -Dbpf_prog_load=bpf_prog_test_load
> >>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> >>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> >>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> >>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
> >>> -lcap -lelf -lz -lrt -lpthread -o
> >>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> >>>
> >>> clang-12: warning: treating 'c-header' input as 'c++-header' when in
> >>> C++ mode, this behavior is deprecated [-Wdeprecated]
> >>> clang-12: error: cannot specify -o when generating multiple output files
> >>> make: *** [Makefile:455:
> >>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> >>> Error 1
> >>> make: Leaving directory
> >>> '/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'
> >>>
> >>> Do you know some magic CXX flags to be passed?
> >>
> >> I tested in my environment. The reason is LC_ALL=C.
> >> Without LC_ALL=C, make succeeded and with it, test_cpp
> >> compilation failed. Is it possible for you to drop
> >> LC_ALL=C for bpf selftests?
> >>
> >> The following command succeeded for me:
> >>      make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
> >>
> >
> > First, I tried the exact make invocation ^^^ in my build-environment
> > but that breaks with the same ERROR.
> >
> > I did in a second run:
> >
> > 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
> >
> > MAKE="make"
> > MAKE_OPTS="V=1 -j4 LLVM=1 CC=clang CXX=clang++"
> > MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> >
> > echo $MAKE $MAKE_OPTS
> > make V=1 -j4 LLVM=1 CC=clang CXX=clang++ PAHOLE=/opt/pahole/bin/pahole
> >
> > $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ 2>&1 | tee
> > ../make-log_tools-testing-selftests-bpf.txt
> >
> > That would have been funny... Drop LC_ALL=C from make line as a fix.
> >
> > Just curious: Do you see these warnings?
> >
> > clang-12: warning: argument unused during compilation: '-rdynamic'
> > [-Wunused-command-line-argument]
> > clang-12: warning: -lcap: 'linker' input unused [-Wunused-command-line-argument]
> > clang-12: warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]
> > clang-12: warning: -lz: 'linker' input unused [-Wunused-command-line-argument]
> > clang-12: warning: -lrt: 'linker' input unused [-Wunused-command-line-argument]
> > clang-12: warning: -lpthread: 'linker' input unused
> > [-Wunused-command-line-argument]
> > clang-12: warning: -lm: 'linker' input unused [-Wunused-command-line-argument]
> >
> > Equivalent CFLAGS for '-rdynamic' when CC=clang is used?
> > Missing LDFLAGS when LD=ld.lld (make LLVM=1) is used?
>
> I see this warning as well, but seems it does not hurt...
> Maybe some flags need change if the CC is clang...
>
> >
> > Last question:
> > Can you pass LLVM_IAS=1 (means use LLVM/Clang Integrated ASsembler) to
> > your make line?
> >
> > Old: make -C tools/testing/selftests/bpf -j60 LLVM=1 V=1 CXX=clang++ CC=clang
> > New: make -C tools/testing/selftests/bpf -j60 LLVM=1 LLVM_IAS=1 V=1
> > CXX=clang++ CC=clang
> >
> > Does it build successfully?
>
> I think it is better to add LLVM_IAS=1. In my build, for non-lto, I
> didn't pass LLVM_IAS=1 to kernel build, so selftest does not need it either.
>
> But for LTO build, LLVM_IAS=1 is required so selftest also needs
> LLVM_IAS=1.
>
> Yes, we can always have LLVM_IAS=1 if it is included in kernel build.
>

For Clang-LTO builds LLVM_IAS=1 is mandatory.

- Sedat -



> >
> > - Sedat -
> >
> >>>
> >>> The only solution is to suppress the build of test_cpp (see
> >>> TEST_GEN_PROGS_EXTENDED):
> >>>
> >>> $ git diff tools/testing/selftests/bpf/Makefile
> >>> diff --git a/tools/testing/selftests/bpf/Makefile
> >>> b/tools/testing/selftests/bpf/Makefile
> >>> index 044bfdcf5b74..cf7c7c8f72cf 100644
> >>> --- a/tools/testing/selftests/bpf/Makefile
> >>> +++ b/tools/testing/selftests/bpf/Makefile
> >>> @@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
> >>> # Compile but not part of 'make run_tests'
> >>> TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> >>>          flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> >>> -       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
> >>> -       xdpxceiver
> >>> +       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko xdpxceiver
> >>> +       # test_cpp # Suppress the build when CXX=clang++ is used
> >>>
> >>> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> >>>
> >>> I have attached both make-logs with and without suppressing the build
> >>> of test_cpp and the diff.
> >>>
> >>> - Sedat -
> >>>
> >>>>
> >>>>>
> >> [...]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Usage of CXX in tools directory
  2021-04-04 14:59 ` Usage of CXX in tools directory Sedat Dilek
  2021-04-04 15:19   ` Sedat Dilek
@ 2021-04-06 18:39   ` Nick Desaulniers
  1 sibling, 0 replies; 23+ messages in thread
From: Nick Desaulniers @ 2021-04-06 18:39 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Masahiro Yamada, Yonghong Song, Arnaldo Carvalho de Melo,
	David Blaikie, Bill Wendling, Nathan Chancellor,
	Alexei Starovoitov, Peter Zijlstra, Mark Rutland,
	Alexander Shishkin, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	dwarves, bpf, kernel-team, Linux Kbuild mailing list, LKML,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Clang-Built-Linux ML

On Sun, Apr 4, 2021 at 8:00 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> [ Please CC me I am not subscribed to all mailing-lists ]
> [ Please CC some more folks if you like ]
>
> Hi,
>
> when dealing/experimenting with BPF together with pahole/dwarves and
> dwarf-v5 and clang-lto I fell over that there is usage of CXX in tools
> directory.
> Especially,  I wanted to build and run test_progs from BPF selftests.
> One BPF selftest called "test_cpp" used GNU/g++ (and even /usr/bin/ld)
> and NOT LLVM/clang++.
>
> For details see the linux-bpf/dwarves thread "[PATCH dwarves]
> dwarf_loader: handle DWARF5 DW_OP_addrx properly" in [1].
>
> Lemme check:
>
> $ git grep CXX tools/
> tools/build/Build.include:cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@
> $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXXFLAGS_$(basetarget).o)
> $(CXXFLAGS_$(obj))
> tools/build/Makefile.build:quiet_cmd_cxx_o_c = CXX      $@
> tools/build/Makefile.build:      cmd_cxx_o_c = $(CXX) $(cxx_flags) -c -o $@ $<
> tools/build/Makefile.feature:  feature-$(1) := $(shell $(MAKE)
> OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)"
> CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))"
> CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))"
> LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir)
> $(OUTPUT_FEATURES)test-$1.bin >/dev/nu
> ll 2>/dev/null && echo 1 || echo 0)
> tools/build/feature/Makefile:__BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall
> -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)
> ...
> tools/perf/Makefile.config:USE_CXX = 0
> tools/perf/Makefile.config:        CXXFLAGS +=
> -DHAVE_LIBCLANGLLVM_SUPPORT -I$(shell $(LLVM_CONFIG) --includedir)
> tools/perf/Makefile.config:        $(call detected,CONFIG_CXX)
> tools/perf/Makefile.config:     USE_CXX = 1
> tools/perf/Makefile.perf:export srctree OUTPUT RM CC CXX LD AR CFLAGS
> CXXFLAGS V BISON FLEX AWK
> tools/perf/Makefile.perf:ifeq ($(USE_CXX), 1)
> tools/perf/util/Build:perf-$(CONFIG_CXX) += c++/
> ...
> tools/scripts/Makefile.include:$(call allow-override,CXX,$(CROSS_COMPILE)g++)
> ...
> tools/testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++
> tools/testing/selftests/bpf/Makefile:   $(call msg,CXX,,$@)
> tools/testing/selftests/bpf/Makefile:   $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o $@
>
> The problem is if you pass LLVM=1 there is no clang(++) assigned to
> CXX automagically.
>
> [2] says:
>
> LLVM has substitutes for GNU binutils utilities. Kbuild supports
> LLVM=1 to enable them.
>
> make LLVM=1
> They can be enabled individually. The full list of the parameters:
>
> make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
>   OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
>   HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>
> [ EndOfQuote  ]
>
> So you need to pass CXX=clang++ manually when playing in tools directory:

Yes, CXX is not set by LLVM=1 in the top level Makefile.  CXX is not
exported by the top level Makefile.  I suspect that
tools/scripts/Mafefile.include (and possible
testing/selftests/bpf/Makefile) needs to check for LLVM=1 and set
CXX=clang++ explicitly.

>
> MAKE="make V=1
> MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1"
> MAKE_OPTS="MAKE_OPTS $PAHOLE=/opt/pahole/bin/pahole"
>
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ clean
> $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/
>
> Unsure, if tools needs a special treatment in things of CXX or LLVM=1
> needs to be enhanced with CCX=clang++.
> If we have HOSTCXX why not have a CXX in toplevel Makefile?
>
> In "tools: Factor Clang, LLC and LLVM utils definitions" (see [3]) I
> did some factor-ing.
>
> For the records: Here Linus Git is my base.
>
> Ideas?
>
> Thanks.
>
> Regards,
> - Sedat -
>
> P.S.: Just a small note: I know there is less usage of CXX code in the
> linux-kernel.
>
> [1] https://lore.kernel.org/bpf/CA+icZUWh6YOkCKG72SndqUbQNwG+iottO4=cPyRRVjaHD2=0qw@mail.gmail.com/T/#m22907f838d2d27be24e8959a53473a62f21cecea
> [2] https://www.kernel.org/doc/html/latest/kbuild/llvm.html#llvm-utilities
> [3] https://git.kernel.org/linus/211a741cd3e124bffdc13ee82e7e65f204e53f60



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-04-06 18:40 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-03 18:41 [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
2021-04-03 18:52 ` David Blaikie
2021-04-03 20:20   ` Yonghong Song
2021-04-03 23:31     ` David Blaikie
     [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
2021-04-03 23:13   ` Yonghong Song
2021-04-03 23:27   ` Yonghong Song
     [not found]     ` <CA+icZUV4fw5GNXFnyOjvajkVFdPhkOrhr3rn5OrAKGujpSrmgQ@mail.gmail.com>
2021-04-04 12:46       ` Sedat Dilek
2021-04-04 16:39         ` Yonghong Song
2021-04-04 17:25           ` Sedat Dilek
2021-04-05  2:24             ` Yonghong Song
     [not found]               ` <CA+icZUVcQ+vQjc0VavetA3s6jzNhC20dU4Sa9ApBLNXbY=w5wA@mail.gmail.com>
2021-04-05 11:04                 ` Sedat Dilek
2021-04-05 11:46                   ` Sedat Dilek
2021-04-05 16:17                 ` Yonghong Song
2021-04-05 18:32                   ` Sedat Dilek
2021-04-05 18:56                     ` Yonghong Song
2021-04-05 20:42                       ` Sedat Dilek
     [not found]             ` <CA+icZUVp3UTPUS-ZjCOnHbNXxaA7DN=4x_08jc8BExFe4Nf2ZQ@mail.gmail.com>
2021-04-05  2:30               ` Yonghong Song
     [not found]                 ` <CA+icZUVtzXNxuVtEUwfULa7nivV0VFfJznsRnSZtEh+V=C=RPg@mail.gmail.com>
2021-04-05  6:56                   ` Sedat Dilek
2021-04-04 14:59 ` Usage of CXX in tools directory Sedat Dilek
2021-04-04 15:19   ` Sedat Dilek
2021-04-06 18:39   ` Nick Desaulniers
2021-04-04 16:45 ` [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
2021-04-04 17:29   ` Sedat Dilek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.