Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang

[Sedat Dilek <sedat.dilek@gmail.com>]

On Mon, Apr 12, 2021 at 7:42 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/11/21 9:47 PM, Sedat Dilek wrote:
> > On Sun, Apr 11, 2021 at 9:08 PM Yonghong Song <yhs@fb.com> wrote:
> > [ ... ]
> >>> BTW, did you check (llvm-)objdump output?
> >>>
> >>> $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern
> >>
> >> This is what I got with g++ compiled test_cpp:
> >>
> >> $ llvm-objdump -Dr test_cpp | grep core_extern
> >>     406a80: e8 5b 01 00 00                callq   0x406be0
> >> <_ZL25test_core_extern__destroyP16test_core_extern>
> >>     406ab9: e8 22 01 00 00                callq   0x406be0
> >> <_ZL25test_core_extern__destroyP16test_core_extern>
> >> 0000000000406be0 <_ZL25test_core_extern__destroyP16test_core_extern>:
> >>     406be3: 74 1a                         je      0x406bff
> >> <_ZL25test_core_extern__destroyP16test_core_extern+0x1f>
> >>     406bef: 74 05                         je      0x406bf6
> >> <_ZL25test_core_extern__destroyP16test_core_extern+0x16>
> >>
> >
> > What is the output when compiling with clang++ in your bpf-next environment?
>
> $ llvm-objdump -Dr test_cpp | grep core_extern
> $
>
> So looks like all test_core_extern_*() functions are inlined.
> This can be confirmed by looking at assembly code.
> while for gcc, there is still the call to
>    _ZL25test_core_extern__destroyP16test_core_extern
> which is
>    test_core_extern__destroy(test_core_extern*)
>
> This is just a difference between compiler optimizations
> between gcc and clang. We don't need to worry about this.
>

( My previous comment was from my samrtphone - so I started into my desktop. )

Thanks for your analysis and hint about inlining.

My inbox is full with that different handling of inlining "GCC vs. LLVM/Clang".

When I recall correctly and I have not to care about the inlining
optimization of clang++, then we can drop
"$(OUTPUT)/test_core_extern.skel.h" from the BPF selftests Makefile:

[ tools/testing/selftests/bpf/Makefile ]

# 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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@

Note: This is with your patchset applied against Linus Git

As we have the include here:

[ tools/testing/selftests/bpf/test_cpp.cpp ]

/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
#include <iostream>
#include <bpf/libbpf.h>
#include <bpf/bpf.h>
#include <bpf/btf.h>
#include "test_core_extern.skel.h"
...

With and without keeping "test_core_extern.skel.h" I got the same
output with g++ and llvm-objdump.
Compiling with clang++ did not show that "CPP-file and C-header"
build-error when dropping "test_core_extern.skel.h" from the Makefile.

As said here all my testings with Linus Git not bpf-next.

Thanks for your precious time!

- Sedat -