All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/5] support build selftests/bpf with clang
@ 2021-04-10 16:49 Yonghong Song
  2021-04-10 16:49 ` [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set Yonghong Song
                   ` (6 more replies)
  0 siblings, 7 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

To build kernel with clang, people typically use
  make -j60 LLVM=1 LLVM_IAS=1
LLVM_IAS=1 is not required for non-LTO build but
is required for LTO build. In my environment,
I am always having LLVM_IAS=1 regardless of
whether LTO is enabled or not.

After kernel is build with clang, the following command
can be used to build selftests with clang:
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1

But currently, some compilations still use gcc
and there are also compilation errors and warnings.
This patch set intends to fix these issues.
Patch #1 and #2 fixed the issue so clang/clang++ is
used instead of gcc/g++. Patch #3 fixed a compilation
failure. Patch #4 and #5 fixed various compiler warnings.

Yonghong Song (5):
  selftests: set CC to clang in lib.mk if LLVM is set
  tools: allow proper CC/CXX/... override with LLVM=1 in
    Makefile.include
  selftests/bpf: fix test_cpp compilation failure with clang
  selftests/bpf: silence clang compilation warnings
  bpftool: fix a clang compilation warning

 tools/bpf/bpftool/net.c              |  2 +-
 tools/scripts/Makefile.include       | 12 ++++++++++--
 tools/testing/selftests/bpf/Makefile |  4 +++-
 tools/testing/selftests/lib.mk       |  4 ++++
 4 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.30.2


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

* [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
@ 2021-04-10 16:49 ` Yonghong Song
  2021-04-11 10:22   ` Sedat Dilek
  2021-04-10 16:49 ` [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include Yonghong Song
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

selftests/bpf/Makefile includes lib.mk. With the following command
  make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
some files are still compiled with gcc. This patch
fixed lib.mk issue which sets CC to gcc in all cases.

Cc: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/lib.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index a5ce26d548e4..9a41d8bb9ff1 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -1,6 +1,10 @@
 # This mimics the top-level Makefile. We do it explicitly here so that this
 # Makefile can operate with or without the kbuild infrastructure.
+ifneq ($(LLVM),)
+CC := clang
+else
 CC := $(CROSS_COMPILE)gcc
+endif
 
 ifeq (0,$(MAKELEVEL))
     ifeq ($(OUTPUT),)
-- 
2.30.2


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

* [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
  2021-04-10 16:49 ` [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set Yonghong Song
@ 2021-04-10 16:49 ` Yonghong Song
  2021-04-11 10:24   ` Sedat Dilek
  2021-04-10 16:49 ` [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang Yonghong Song
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

selftests/bpf/Makefile includes tools/scripts/Makefile.include.
With the following command
  make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
some files are still compiled with gcc. This patch
fixed the case if CC/AR/LD/CXX/STRIP is allowed to be
overridden, it will be written to clang/llvm-ar/..., instead of
gcc binaries. The definition of CC_NO_CLANG is also relocated
to the place after the above CC is defined.

Cc: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/scripts/Makefile.include | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index a402f32a145c..91130648d8e6 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -39,8 +39,6 @@ EXTRA_WARNINGS += -Wundef
 EXTRA_WARNINGS += -Wwrite-strings
 EXTRA_WARNINGS += -Wformat
 
-CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
-
 # Makefiles suck: This macro sets a default value of $(2) for the
 # variable named by $(1), unless the variable has been set by
 # environment or command line. This is necessary for CC and AR
@@ -52,12 +50,22 @@ define allow-override
     $(eval $(1) = $(2)))
 endef
 
+ifneq ($(LLVM),)
+$(call allow-override,CC,clang)
+$(call allow-override,AR,llvm-ar)
+$(call allow-override,LD,ld.lld)
+$(call allow-override,CXX,clang++)
+$(call allow-override,STRIP,llvm-strip)
+else
 # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
 $(call allow-override,CC,$(CROSS_COMPILE)gcc)
 $(call allow-override,AR,$(CROSS_COMPILE)ar)
 $(call allow-override,LD,$(CROSS_COMPILE)ld)
 $(call allow-override,CXX,$(CROSS_COMPILE)g++)
 $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
+endif
+
+CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
 
 ifneq ($(LLVM),)
 HOSTAR  ?= llvm-ar
-- 
2.30.2


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

* [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
  2021-04-10 16:49 ` [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set Yonghong Song
  2021-04-10 16:49 ` [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include Yonghong Song
@ 2021-04-10 16:49 ` Yonghong Song
  2021-04-11 10:47   ` Sedat Dilek
  2021-04-13  4:32   ` Andrii Nakryiko
  2021-04-10 16:49 ` [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings Yonghong Song
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

With clang compiler:
  make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
the test_cpp build failed due to the failure:
  warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
  clang-13: warning: cannot specify -o when generating multiple output files

test_cpp compilation flag looks like:
  clang++ -g -Og -rdynamic -Wall -I<...> ... \
  -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
  test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
  -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp

The clang++ compiler complains the header file in the command line.
Let us remove the header file from the command line which is not intended
any way, and this fixed the problem.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 6448c626498f..bbd61cc3889b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
 
 # Benchmark runner
 $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h
-- 
2.30.2


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

* [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
                   ` (2 preceding siblings ...)
  2021-04-10 16:49 ` [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang Yonghong Song
@ 2021-04-10 16:49 ` Yonghong Song
  2021-04-11 11:12   ` Sedat Dilek
  2021-04-10 16:49 ` [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning Yonghong Song
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

With clang compiler:
  make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
Some linker flags are not used/effective for some binaries and
we have warnings like:
  warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]

We also have warnings like:
  .../selftests/bpf/prog_tests/ns_current_pid_tgid.c:74:57: note: treat the string as an argument to avoid this
        if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", strerror(errno)))
                                                               ^
                                                               "%s",
  .../selftests/bpf/test_progs.h:129:35: note: expanded from macro 'CHECK'
        _CHECK(condition, tag, duration, format)
                                         ^
  .../selftests/bpf/test_progs.h:108:21: note: expanded from macro '_CHECK'
                fprintf(stdout, ##format);                              \
                                  ^
Let us add proper compilation flags to silence the above two kinds of warnings.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index bbd61cc3889b..a9c0a64a4c49 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -24,6 +24,8 @@ SAN_CFLAGS	?=
 CFLAGS += -g -Og -rdynamic -Wall $(GENFLAGS) $(SAN_CFLAGS)		\
 	  -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)		\
 	  -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)			\
+	  -Wno-unused-command-line-argument				\
+	  -Wno-format-security						\
 	  -Dbpf_prog_load=bpf_prog_test_load				\
 	  -Dbpf_load_program=bpf_test_load_program
 LDLIBS += -lcap -lelf -lz -lrt -lpthread
-- 
2.30.2


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

* [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
                   ` (3 preceding siblings ...)
  2021-04-10 16:49 ` [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings Yonghong Song
@ 2021-04-10 16:49 ` Yonghong Song
  2021-04-11 11:05   ` Sedat Dilek
  2021-04-10 17:23 ` [PATCH bpf-next 0/5] support build selftests/bpf with clang Alexei Starovoitov
  2021-04-10 19:19 ` Sedat Dilek
  6 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 16:49 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

With clang compiler:
  make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
  # build selftests/bpf or bpftool
  make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
  make -j60 -C tools/bpf/bpftool LLVM=1 LLVM_IAS=1
the following compilation warning showed up,
  net.c:160:37: warning: comparison of integers of different signs: '__u32' (aka 'unsigned int') and 'int' [-Wsign-compare]
                for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
                                                  ^~~~~~~~~~~~~~~~~
  .../tools/include/uapi/linux/netlink.h:99:24: note: expanded from macro 'NLMSG_OK'
                           (nlh)->nlmsg_len <= (len))
                           ~~~~~~~~~~~~~~~~ ^   ~~~

In this particular case, "len" is defined as "int" and (nlh)->nlmsg_len is "unsigned int".
The macro NLMSG_OK is defined as below in uapi/linux/netlink.h.
  #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
                             (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
                             (nlh)->nlmsg_len <= (len))

The clang compiler complains the comparision "(nlh)->nlmsg_len <= (len))",
but in bpftool/net.c, it is already ensured that "len > 0" must be true.
So let us add an explicit type conversion (from "int" to "unsigned int")
for "len" in NLMSG_OK to silence this warning.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/bpf/bpftool/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index ff3aa0cf3997..f836d115d7d6 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -157,7 +157,7 @@ static int netlink_recv(int sock, __u32 nl_pid, __u32 seq,
 		if (len == 0)
 			break;
 
-		for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+		for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, (unsigned int)len);
 		     nh = NLMSG_NEXT(nh, len)) {
 			if (nh->nlmsg_pid != nl_pid) {
 				ret = -LIBBPF_ERRNO__WRNGPID;
-- 
2.30.2


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

* Re: [PATCH bpf-next 0/5] support build selftests/bpf with clang
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
                   ` (4 preceding siblings ...)
  2021-04-10 16:49 ` [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning Yonghong Song
@ 2021-04-10 17:23 ` Alexei Starovoitov
  2021-04-10 17:38   ` Yonghong Song
  2021-04-10 19:19 ` Sedat Dilek
  6 siblings, 1 reply; 30+ messages in thread
From: Alexei Starovoitov @ 2021-04-10 17:23 UTC (permalink / raw)
  To: Yonghong Song, bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek

On 4/10/21 9:49 AM, Yonghong Song wrote:
> To build kernel with clang, people typically use
>    make -j60 LLVM=1 LLVM_IAS=1
> LLVM_IAS=1 is not required for non-LTO build but
> is required for LTO build. In my environment,
> I am always having LLVM_IAS=1 regardless of
> whether LTO is enabled or not.
> 
> After kernel is build with clang, the following command
> can be used to build selftests with clang:
>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1

Will it use clang to compile libbpf and bpftool as well?

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

* Re: [PATCH bpf-next 0/5] support build selftests/bpf with clang
  2021-04-10 17:23 ` [PATCH bpf-next 0/5] support build selftests/bpf with clang Alexei Starovoitov
@ 2021-04-10 17:38   ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-10 17:38 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers, Sedat Dilek



On 4/10/21 10:23 AM, Alexei Starovoitov wrote:
> On 4/10/21 9:49 AM, Yonghong Song wrote:
>> To build kernel with clang, people typically use
>>    make -j60 LLVM=1 LLVM_IAS=1
>> LLVM_IAS=1 is not required for non-LTO build but
>> is required for LTO build. In my environment,
>> I am always having LLVM_IAS=1 regardless of
>> whether LTO is enabled or not.
>>
>> After kernel is build with clang, the following command
>> can be used to build selftests with clang:
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> 
> Will it use clang to compile libbpf and bpftool as well?

Yes. selftests, libbpf and bpftool will be all built with
clang.


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

* Re: [PATCH bpf-next 0/5] support build selftests/bpf with clang
  2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
                   ` (5 preceding siblings ...)
  2021-04-10 17:23 ` [PATCH bpf-next 0/5] support build selftests/bpf with clang Alexei Starovoitov
@ 2021-04-10 19:19 ` Sedat Dilek
  2021-04-11 16:46   ` Yonghong Song
  6 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-10 19:19 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> To build kernel with clang, people typically use
>   make -j60 LLVM=1 LLVM_IAS=1
> LLVM_IAS=1 is not required for non-LTO build but
> is required for LTO build. In my environment,
> I am always having LLVM_IAS=1 regardless of
> whether LTO is enabled or not.
>
> After kernel is build with clang, the following command
> can be used to build selftests with clang:
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>
> But currently, some compilations still use gcc
> and there are also compilation errors and warnings.
> This patch set intends to fix these issues.
> Patch #1 and #2 fixed the issue so clang/clang++ is
> used instead of gcc/g++. Patch #3 fixed a compilation
> failure. Patch #4 and #5 fixed various compiler warnings.
>
> Yonghong Song (5):
>   selftests: set CC to clang in lib.mk if LLVM is set
>   tools: allow proper CC/CXX/... override with LLVM=1 in
>     Makefile.include
>   selftests/bpf: fix test_cpp compilation failure with clang
>   selftests/bpf: silence clang compilation warnings
>   bpftool: fix a clang compilation warning
>
>  tools/bpf/bpftool/net.c              |  2 +-
>  tools/scripts/Makefile.include       | 12 ++++++++++--
>  tools/testing/selftests/bpf/Makefile |  4 +++-
>  tools/testing/selftests/lib.mk       |  4 ++++
>  4 files changed, 18 insertions(+), 4 deletions(-)
>
> --
> 2.30.2
>

Thanks for CCing me and taking care to clean BPF selftests with clang.

I applied (adapted 4/5) the 5 patches to fit latest Linus Git.

As I had a fresh compiled Clang-CFI kernel without enabling BTF
debug-info KConfig this fails at some point.
I am not sure what the situation is with Clang-CFI + BTF thus I will
do another Clang-LTO build with BTF enabled.
So, I was not able to build test_cpp.

I am missing some comments that LLVM=1 misses to set CXX=clang++ if
people want that explicitly as CXX.
Did you try with this?

AFAICS LC_ALL=C was not the culprit.
Did you try with and without LC_ALL=C - I have this in all my build-scripts.
Here I have German localisation as default.

Wil report later... (might be Monday when Linux v5.12-rc7 is released).

- Sedat -

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

* Re: [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set
  2021-04-10 16:49 ` [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set Yonghong Song
@ 2021-04-11 10:22   ` Sedat Dilek
  2021-04-11 16:51     ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 10:22 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> selftests/bpf/Makefile includes lib.mk. With the following command
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
> some files are still compiled with gcc. This patch
> fixed lib.mk issue which sets CC to gcc in all cases.
>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/lib.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index a5ce26d548e4..9a41d8bb9ff1 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -1,6 +1,10 @@
>  # This mimics the top-level Makefile. We do it explicitly here so that this
>  # Makefile can operate with or without the kbuild infrastructure.
> +ifneq ($(LLVM),)
> +CC := clang
> +else
>  CC := $(CROSS_COMPILE)gcc
> +endif
>

Why not use include "include ../../../scripts/Makefile.include" here
and include CC and GNU or LLVM (bin)utils from there?

Should the CC line have a $(CROSS_COMPILE) for people doing cross-compilation?

CC := $(CROSS_COMPILE)clang

- Sedat -


>  ifeq (0,$(MAKELEVEL))
>      ifeq ($(OUTPUT),)
> --
> 2.30.2
>

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

* Re: [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include
  2021-04-10 16:49 ` [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include Yonghong Song
@ 2021-04-11 10:24   ` Sedat Dilek
  2021-04-11 16:52     ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 10:24 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> selftests/bpf/Makefile includes tools/scripts/Makefile.include.
> With the following command
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
> some files are still compiled with gcc. This patch
> fixed the case if CC/AR/LD/CXX/STRIP is allowed to be
> overridden, it will be written to clang/llvm-ar/..., instead of
> gcc binaries. The definition of CC_NO_CLANG is also relocated
> to the place after the above CC is defined.
>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/scripts/Makefile.include | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index a402f32a145c..91130648d8e6 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -39,8 +39,6 @@ EXTRA_WARNINGS += -Wundef
>  EXTRA_WARNINGS += -Wwrite-strings
>  EXTRA_WARNINGS += -Wformat
>
> -CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
> -
>  # Makefiles suck: This macro sets a default value of $(2) for the
>  # variable named by $(1), unless the variable has been set by
>  # environment or command line. This is necessary for CC and AR
> @@ -52,12 +50,22 @@ define allow-override
>      $(eval $(1) = $(2)))
>  endef
>
> +ifneq ($(LLVM),)
> +$(call allow-override,CC,clang)
> +$(call allow-override,AR,llvm-ar)
> +$(call allow-override,LD,ld.lld)
> +$(call allow-override,CXX,clang++)
> +$(call allow-override,STRIP,llvm-strip)

Use here $(CROSS_COMPILE) prefix like below for people using an LLVM
cross-toolchain?

- Sedat -

> +else
>  # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
>  $(call allow-override,CC,$(CROSS_COMPILE)gcc)
>  $(call allow-override,AR,$(CROSS_COMPILE)ar)
>  $(call allow-override,LD,$(CROSS_COMPILE)ld)
>  $(call allow-override,CXX,$(CROSS_COMPILE)g++)
>  $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
> +endif
> +
> +CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
>
>  ifneq ($(LLVM),)
>  HOSTAR  ?= llvm-ar
> --
> 2.30.2
>

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-10 16:49 ` [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang Yonghong Song
@ 2021-04-11 10:47   ` Sedat Dilek
  2021-04-11 17:20     ` Yonghong Song
  2021-04-13  4:32   ` Andrii Nakryiko
  1 sibling, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 10:47 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> With clang compiler:
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> the test_cpp build failed due to the failure:
>   warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>   clang-13: warning: cannot specify -o when generating multiple output files
>
> test_cpp compilation flag looks like:
>   clang++ -g -Og -rdynamic -Wall -I<...> ... \
>   -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
>   test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
>   -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
>
> The clang++ compiler complains the header file in the command line.
> Let us remove the header file from the command line which is not intended
> any way, and this fixed the problem.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 6448c626498f..bbd61cc3889b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
>
>  # Benchmark runner
>  $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h
> --
> 2.30.2
>

NOTE: bpf-next might be different from my build-environment.

Yonghong, can you please re-test by adding explicitly CXX=g++ to your make line?

Here I have:

$ grep test_cpp make-log_tools-testing-selftests-bpf_clang_clang++.txt
1907: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
-Wno-unused-command-line-argument -Wno-format-security
-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/tools/build/libbpf/libbpf.a
-lcap -lelf -lz -lrt -lpthread -o
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp

This clang++ line does not include <...>/test_core_extern.skel.h ^^^

$ grep test_core_extern.skel.h
make-log_tools-testing-selftests-bpf_clang_clang++.txt
704:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_co
re_extern.o > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
1592:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu
32/test_core_extern.o >
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu32/test_core_extern.skel.h

Checking test_cpp:

$ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep extern
0000000000417e50 <cmp_externs>:
 417e54: 75 22                         jne     0x417e78 <cmp_externs+0x28>
 417e59: 75 10                         jne     0x417e6b <cmp_externs+0x1b>
 417e61: 75 21                         jne     0x417e84 <cmp_externs+0x34>
 417e69: 75 1e                         jne     0x417e89 <cmp_externs+0x39>
 417e87: eb f2                         jmp     0x417e7b <cmp_externs+0x2b>
 417e8c: eb ed                         jmp     0x417e7b <cmp_externs+0x2b>

$ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern
[ EMPTY ]

When compiled with g++ in an earlier setup this contained "core_extern".

With this version of your patchser it breaks *here* when using CXX=g++
(and uses /usr/bin/ld as linker):

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
-Wno-unused-command-line-argument -Wno-format-security
-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/tools/build/libbpf/libbpf.a
-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:457:
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
Error 1

$ grep test_cpp ../make-log_tools-testing-selftests-bpf_clang_g++.txt
| grep test_core_extern.skel.h
[ EMPTY ]

As said I do NOT use bpf-next.

- Sedat -



- Sedat -

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

* Re: [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning
  2021-04-10 16:49 ` [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning Yonghong Song
@ 2021-04-11 11:05   ` Sedat Dilek
  2021-04-11 17:24     ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 11:05 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> With clang compiler:
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   # build selftests/bpf or bpftool
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>   make -j60 -C tools/bpf/bpftool LLVM=1 LLVM_IAS=1
> the following compilation warning showed up,
>   net.c:160:37: warning: comparison of integers of different signs: '__u32' (aka 'unsigned int') and 'int' [-Wsign-compare]
>                 for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
>                                                   ^~~~~~~~~~~~~~~~~
>   .../tools/include/uapi/linux/netlink.h:99:24: note: expanded from macro 'NLMSG_OK'
>                            (nlh)->nlmsg_len <= (len))
>                            ~~~~~~~~~~~~~~~~ ^   ~~~
>
> In this particular case, "len" is defined as "int" and (nlh)->nlmsg_len is "unsigned int".
> The macro NLMSG_OK is defined as below in uapi/linux/netlink.h.
>   #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
>                              (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
>                              (nlh)->nlmsg_len <= (len))
>
> The clang compiler complains the comparision "(nlh)->nlmsg_len <= (len))",
> but in bpftool/net.c, it is already ensured that "len > 0" must be true.
> So let us add an explicit type conversion (from "int" to "unsigned int")
> for "len" in NLMSG_OK to silence this warning.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/bpf/bpftool/net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> index ff3aa0cf3997..f836d115d7d6 100644
> --- a/tools/bpf/bpftool/net.c
> +++ b/tools/bpf/bpftool/net.c
> @@ -157,7 +157,7 @@ static int netlink_recv(int sock, __u32 nl_pid, __u32 seq,
>                 if (len == 0)
>                         break;
>
> -               for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
> +               for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, (unsigned int)len);
>                      nh = NLMSG_NEXT(nh, len)) {
>                         if (nh->nlmsg_pid != nl_pid) {
>                                 ret = -LIBBPF_ERRNO__WRNGPID;
> --
> 2.30.2
>

Thanks for the patch.

I remember darkly I have seen this, too.

The only warning I see remaining *here* is fixed by this patch from bpf-next:

commit 7519c387e69d367075bf493de8a9ea427c9d2a1b
"selftests: xsk: Remove unused function"

- Sedat -

[1] https://git.kernel.org/bpf/bpf-next/c/7519c387e69d367075bf493de8a9ea427c9d2a1b

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

* Re: [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings
  2021-04-10 16:49 ` [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings Yonghong Song
@ 2021-04-11 11:12   ` Sedat Dilek
  2021-04-11 17:40     ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 11:12 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>
> With clang compiler:
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> Some linker flags are not used/effective for some binaries and
> we have warnings like:
>   warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]
>
> We also have warnings like:
>   .../selftests/bpf/prog_tests/ns_current_pid_tgid.c:74:57: note: treat the string as an argument to avoid this
>         if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", strerror(errno)))
>                                                                ^
>                                                                "%s",
>   .../selftests/bpf/test_progs.h:129:35: note: expanded from macro 'CHECK'
>         _CHECK(condition, tag, duration, format)
>                                          ^
>   .../selftests/bpf/test_progs.h:108:21: note: expanded from macro '_CHECK'
>                 fprintf(stdout, ##format);                              \
>                                   ^
> Let us add proper compilation flags to silence the above two kinds of warnings.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index bbd61cc3889b..a9c0a64a4c49 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -24,6 +24,8 @@ SAN_CFLAGS    ?=
>  CFLAGS += -g -Og -rdynamic -Wall $(GENFLAGS) $(SAN_CFLAGS)             \
>           -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)          \
>           -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)                      \
> +         -Wno-unused-command-line-argument                             \
> +         -Wno-format-security                                          \

Are both compliler flags available for GCC (I simply don't know or
have checked)?

- Sedat -

>           -Dbpf_prog_load=bpf_prog_test_load                            \
>           -Dbpf_load_program=bpf_test_load_program
>  LDLIBS += -lcap -lelf -lz -lrt -lpthread
> --
> 2.30.2
>

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

* Re: [PATCH bpf-next 0/5] support build selftests/bpf with clang
  2021-04-10 19:19 ` Sedat Dilek
@ 2021-04-11 16:46   ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 16:46 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/10/21 12:19 PM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> To build kernel with clang, people typically use
>>    make -j60 LLVM=1 LLVM_IAS=1
>> LLVM_IAS=1 is not required for non-LTO build but
>> is required for LTO build. In my environment,
>> I am always having LLVM_IAS=1 regardless of
>> whether LTO is enabled or not.
>>
>> After kernel is build with clang, the following command
>> can be used to build selftests with clang:
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>>
>> But currently, some compilations still use gcc
>> and there are also compilation errors and warnings.
>> This patch set intends to fix these issues.
>> Patch #1 and #2 fixed the issue so clang/clang++ is
>> used instead of gcc/g++. Patch #3 fixed a compilation
>> failure. Patch #4 and #5 fixed various compiler warnings.
>>
>> Yonghong Song (5):
>>    selftests: set CC to clang in lib.mk if LLVM is set
>>    tools: allow proper CC/CXX/... override with LLVM=1 in
>>      Makefile.include
>>    selftests/bpf: fix test_cpp compilation failure with clang
>>    selftests/bpf: silence clang compilation warnings
>>    bpftool: fix a clang compilation warning
>>
>>   tools/bpf/bpftool/net.c              |  2 +-
>>   tools/scripts/Makefile.include       | 12 ++++++++++--
>>   tools/testing/selftests/bpf/Makefile |  4 +++-
>>   tools/testing/selftests/lib.mk       |  4 ++++
>>   4 files changed, 18 insertions(+), 4 deletions(-)
>>
>> --
>> 2.30.2
>>
> 
> Thanks for CCing me and taking care to clean BPF selftests with clang.
> 
> I applied (adapted 4/5) the 5 patches to fit latest Linus Git.
> 
> As I had a fresh compiled Clang-CFI kernel without enabling BTF
> debug-info KConfig this fails at some point.
> I am not sure what the situation is with Clang-CFI + BTF thus I will
> do another Clang-LTO build with BTF enabled.
> So, I was not able to build test_cpp.
> 
> I am missing some comments that LLVM=1 misses to set CXX=clang++ if
> people want that explicitly as CXX.
> Did you try with this?

Yes, this patch set should fix this issue.

> 
> AFAICS LC_ALL=C was not the culprit.
> Did you try with and without LC_ALL=C - I have this in all my build-scripts.
> Here I have German localisation as default.

Just tried. Yes, LC_ALL=C does not impact compilation any more
with this patch set.

> 
> Wil report later... (might be Monday when Linux v5.12-rc7 is released).
> 
> - Sedat -
> 

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

* Re: [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set
  2021-04-11 10:22   ` Sedat Dilek
@ 2021-04-11 16:51     ` Yonghong Song
  2021-04-11 17:10       ` Sedat Dilek
  0 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 16:51 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 3:22 AM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> selftests/bpf/Makefile includes lib.mk. With the following command
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
>> some files are still compiled with gcc. This patch
>> fixed lib.mk issue which sets CC to gcc in all cases.
>>
>> Cc: Sedat Dilek <sedat.dilek@gmail.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/lib.mk | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
>> index a5ce26d548e4..9a41d8bb9ff1 100644
>> --- a/tools/testing/selftests/lib.mk
>> +++ b/tools/testing/selftests/lib.mk
>> @@ -1,6 +1,10 @@
>>   # This mimics the top-level Makefile. We do it explicitly here so that this
>>   # Makefile can operate with or without the kbuild infrastructure.
>> +ifneq ($(LLVM),)
>> +CC := clang
>> +else
>>   CC := $(CROSS_COMPILE)gcc
>> +endif
>>
> 
> Why not use include "include ../../../scripts/Makefile.include" here
> and include CC and GNU or LLVM (bin)utils from there?

There is a comment above my change,

 >>   # This mimics the top-level Makefile. We do it explicitly here so 
that this
 >>   # Makefile can operate with or without the kbuild infrastructure.

It is intentionally not depending on kbuild 
(../../../scripts/Makefile.include).

> 
> Should the CC line have a $(CROSS_COMPILE) for people doing cross-compilation?
> 
> CC := $(CROSS_COMPILE)clang

The top linux/Makefile has

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

There is no CROSS_COMPILE prefix for llvm.
Also see here:
   https://clang.llvm.org/docs/CrossCompilation.html
for clang, cross compilation is mostly related to
tweaking compiler options than building a different
compiler.

Hence, I didn't add $(CROSS_COMPILER) prefix.

> 
> - Sedat -
> 
> 
>>   ifeq (0,$(MAKELEVEL))
>>       ifeq ($(OUTPUT),)
>> --
>> 2.30.2
>>

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

* Re: [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include
  2021-04-11 10:24   ` Sedat Dilek
@ 2021-04-11 16:52     ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 16:52 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 3:24 AM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> selftests/bpf/Makefile includes tools/scripts/Makefile.include.
>> With the following command
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
>> some files are still compiled with gcc. This patch
>> fixed the case if CC/AR/LD/CXX/STRIP is allowed to be
>> overridden, it will be written to clang/llvm-ar/..., instead of
>> gcc binaries. The definition of CC_NO_CLANG is also relocated
>> to the place after the above CC is defined.
>>
>> Cc: Sedat Dilek <sedat.dilek@gmail.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/scripts/Makefile.include | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
>> index a402f32a145c..91130648d8e6 100644
>> --- a/tools/scripts/Makefile.include
>> +++ b/tools/scripts/Makefile.include
>> @@ -39,8 +39,6 @@ EXTRA_WARNINGS += -Wundef
>>   EXTRA_WARNINGS += -Wwrite-strings
>>   EXTRA_WARNINGS += -Wformat
>>
>> -CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
>> -
>>   # Makefiles suck: This macro sets a default value of $(2) for the
>>   # variable named by $(1), unless the variable has been set by
>>   # environment or command line. This is necessary for CC and AR
>> @@ -52,12 +50,22 @@ define allow-override
>>       $(eval $(1) = $(2)))
>>   endef
>>
>> +ifneq ($(LLVM),)
>> +$(call allow-override,CC,clang)
>> +$(call allow-override,AR,llvm-ar)
>> +$(call allow-override,LD,ld.lld)
>> +$(call allow-override,CXX,clang++)
>> +$(call allow-override,STRIP,llvm-strip)
> 
> Use here $(CROSS_COMPILE) prefix like below for people using an LLVM
> cross-toolchain?

The same reason as my comment in previous patch.
There is no need to have $(CROSS_COMPILE) prefix for clang compiler.

> 
> - Sedat -
> 
>> +else
>>   # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
>>   $(call allow-override,CC,$(CROSS_COMPILE)gcc)
>>   $(call allow-override,AR,$(CROSS_COMPILE)ar)
>>   $(call allow-override,LD,$(CROSS_COMPILE)ld)
>>   $(call allow-override,CXX,$(CROSS_COMPILE)g++)
>>   $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
>> +endif
>> +
>> +CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
>>
>>   ifneq ($(LLVM),)
>>   HOSTAR  ?= llvm-ar
>> --
>> 2.30.2
>>

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

* Re: [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set
  2021-04-11 16:51     ` Yonghong Song
@ 2021-04-11 17:10       ` Sedat Dilek
  0 siblings, 0 replies; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 17:10 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sun, Apr 11, 2021 at 6:51 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/11/21 3:22 AM, Sedat Dilek wrote:
> > On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> selftests/bpf/Makefile includes lib.mk. With the following command
> >>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
> >>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1 V=1
> >> some files are still compiled with gcc. This patch
> >> fixed lib.mk issue which sets CC to gcc in all cases.
> >>
> >> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >>   tools/testing/selftests/lib.mk | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> >> index a5ce26d548e4..9a41d8bb9ff1 100644
> >> --- a/tools/testing/selftests/lib.mk
> >> +++ b/tools/testing/selftests/lib.mk
> >> @@ -1,6 +1,10 @@
> >>   # This mimics the top-level Makefile. We do it explicitly here so that this
> >>   # Makefile can operate with or without the kbuild infrastructure.
> >> +ifneq ($(LLVM),)
> >> +CC := clang
> >> +else
> >>   CC := $(CROSS_COMPILE)gcc
> >> +endif
> >>
> >
> > Why not use include "include ../../../scripts/Makefile.include" here
> > and include CC and GNU or LLVM (bin)utils from there?
>
> There is a comment above my change,
>
>  >>   # This mimics the top-level Makefile. We do it explicitly here so
> that this
>  >>   # Makefile can operate with or without the kbuild infrastructure.
>
> It is intentionally not depending on kbuild
> (../../../scripts/Makefile.include).
>
> >
> > Should the CC line have a $(CROSS_COMPILE) for people doing cross-compilation?
> >
> > CC := $(CROSS_COMPILE)clang
>
> The top linux/Makefile has
>
> 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
>
> There is no CROSS_COMPILE prefix for llvm.
> Also see here:
>    https://clang.llvm.org/docs/CrossCompilation.html
> for clang, cross compilation is mostly related to
> tweaking compiler options than building a different
> compiler.
>
> Hence, I didn't add $(CROSS_COMPILER) prefix.
>

OK, I see.

My last cross-compilation for a linux-kernel is very long ago.
I never used it with LLVM toolchain - might be $(CROSS_COMPILE) is
only necessary with GNU toolchain.

- Sedat -

> >
> > - Sedat -
> >
> >
> >>   ifeq (0,$(MAKELEVEL))
> >>       ifeq ($(OUTPUT),)
> >> --
> >> 2.30.2
> >>

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-11 10:47   ` Sedat Dilek
@ 2021-04-11 17:20     ` Yonghong Song
  2021-04-11 17:31       ` Sedat Dilek
  0 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 17:20 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 3:47 AM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> With clang compiler:
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>> the test_cpp build failed due to the failure:
>>    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>>    clang-13: warning: cannot specify -o when generating multiple output files
>>
>> test_cpp compilation flag looks like:
>>    clang++ -g -Og -rdynamic -Wall -I<...> ... \
>>    -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
>>    test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
>>    -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
>>
>> The clang++ compiler complains the header file in the command line.
>> Let us remove the header file from the command line which is not intended
>> any way, and this fixed the problem.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/bpf/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 6448c626498f..bbd61cc3889b 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
>>
>>   # Benchmark runner
>>   $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h
>> --
>> 2.30.2
>>
> 
> NOTE: bpf-next might be different from my build-environment.
> 
> Yonghong, can you please re-test by adding explicitly CXX=g++ to your make line?
> 
> Here I have:
> 
> $ grep test_cpp make-log_tools-testing-selftests-bpf_clang_clang++.txt
> 1907: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
> -Wno-unused-command-line-argument -Wno-format-security
> -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/tools/build/libbpf/libbpf.a
> -lcap -lelf -lz -lrt -lpthread -o
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> 
> This clang++ line does not include <...>/test_core_extern.skel.h ^^^
> 
> $ grep test_core_extern.skel.h
> make-log_tools-testing-selftests-bpf_clang_clang++.txt
> 704:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
> gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_co
> re_extern.o > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> 1592:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
> gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu
> 32/test_core_extern.o >
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu32/test_core_extern.skel.h
> 
> Checking test_cpp:
> 
> $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep extern
> 0000000000417e50 <cmp_externs>:
>   417e54: 75 22                         jne     0x417e78 <cmp_externs+0x28>
>   417e59: 75 10                         jne     0x417e6b <cmp_externs+0x1b>
>   417e61: 75 21                         jne     0x417e84 <cmp_externs+0x34>
>   417e69: 75 1e                         jne     0x417e89 <cmp_externs+0x39>
>   417e87: eb f2                         jmp     0x417e7b <cmp_externs+0x2b>
>   417e8c: eb ed                         jmp     0x417e7b <cmp_externs+0x2b>
> 
> $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern
> [ EMPTY ]
> 
> When compiled with g++ in an earlier setup this contained "core_extern".
> 
> With this version of your patchser it breaks *here* when using CXX=g++
> (and uses /usr/bin/ld as linker):
> 
> 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
> -Wno-unused-command-line-argument -Wno-format-security
> -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/tools/build/libbpf/libbpf.a
> -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:457:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> Error 1

I cannot reproduce the issue with g++ with bpf-next, my command line is

g++ -g -Og -rdynamic -Wall 
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf 
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include 
-I/home/yhs/work/bpf-next/include/generated 
-I/home/yhs/work/bpf-next/tools/lib 
-I/home/yhs/work/bpf-next/tools/include 
-I/home/yhs/work/bpf-next/tools/include/uapi 
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf 
-Wno-unused-command-line-argument -Wno-format-security 
-Dbpf_prog_load=bpf_prog_test_load 
-Dbpf_load_program=bpf_test_load_program test_cpp.cpp 
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a 
-lcap -lelf -lz -lrt -lpthread -o 
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_cpp

I modified to
g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR 
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf ...
and cannot reproduce the issue.
The macro HAVE_GENHDR is only effect for test_verifier.


Could you try to run the above g++ command by adding 
test_core_extern.skel.h back, something like

 > 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
 > -Wno-unused-command-line-argument -Wno-format-security
 > -Dbpf_prog_load=bpf_prog_test_load
 > -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
 > test_core_extern.skel.h
 > 
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
 > -lcap -lelf -lz -lrt -lpthread -o
 > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp

The issue could be somewhere else?

> 
> $ grep test_cpp ../make-log_tools-testing-selftests-bpf_clang_g++.txt
> | grep test_core_extern.skel.h
> [ EMPTY ]
> 
> As said I do NOT use bpf-next.
> 
> - Sedat -
> 
> 
> 
> - Sedat -
> 

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

* Re: [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning
  2021-04-11 11:05   ` Sedat Dilek
@ 2021-04-11 17:24     ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 17:24 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 4:05 AM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> With clang compiler:
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    # build selftests/bpf or bpftool
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>>    make -j60 -C tools/bpf/bpftool LLVM=1 LLVM_IAS=1
>> the following compilation warning showed up,
>>    net.c:160:37: warning: comparison of integers of different signs: '__u32' (aka 'unsigned int') and 'int' [-Wsign-compare]
>>                  for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
>>                                                    ^~~~~~~~~~~~~~~~~
>>    .../tools/include/uapi/linux/netlink.h:99:24: note: expanded from macro 'NLMSG_OK'
>>                             (nlh)->nlmsg_len <= (len))
>>                             ~~~~~~~~~~~~~~~~ ^   ~~~
>>
>> In this particular case, "len" is defined as "int" and (nlh)->nlmsg_len is "unsigned int".
>> The macro NLMSG_OK is defined as below in uapi/linux/netlink.h.
>>    #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
>>                               (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
>>                               (nlh)->nlmsg_len <= (len))
>>
>> The clang compiler complains the comparision "(nlh)->nlmsg_len <= (len))",
>> but in bpftool/net.c, it is already ensured that "len > 0" must be true.
>> So let us add an explicit type conversion (from "int" to "unsigned int")
>> for "len" in NLMSG_OK to silence this warning.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/bpf/bpftool/net.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
>> index ff3aa0cf3997..f836d115d7d6 100644
>> --- a/tools/bpf/bpftool/net.c
>> +++ b/tools/bpf/bpftool/net.c
>> @@ -157,7 +157,7 @@ static int netlink_recv(int sock, __u32 nl_pid, __u32 seq,
>>                  if (len == 0)
>>                          break;
>>
>> -               for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
>> +               for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, (unsigned int)len);
>>                       nh = NLMSG_NEXT(nh, len)) {
>>                          if (nh->nlmsg_pid != nl_pid) {
>>                                  ret = -LIBBPF_ERRNO__WRNGPID;
>> --
>> 2.30.2
>>
> 
> Thanks for the patch.
> 
> I remember darkly I have seen this, too.

In this particular case, through analysis, the compiler COULD decide
the comparison is okay as the range of "int" value for "len" is > 0.
But it really depends on when and how much analysis the compiler
did before issuing this particular warning. So working around at the 
source code is a better choice than silencing all similar warnings. Some
of such warnings may actually reveal a real issue.

> 
> The only warning I see remaining *here* is fixed by this patch from bpf-next:
> 
> commit 7519c387e69d367075bf493de8a9ea427c9d2a1b
> "selftests: xsk: Remove unused function"
> 
> - Sedat -
> 
> [1] https://git.kernel.org/bpf/bpf-next/c/7519c387e69d367075bf493de8a9ea427c9d2a1b
> 

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-11 17:20     ` Yonghong Song
@ 2021-04-11 17:31       ` Sedat Dilek
  2021-04-11 19:08         ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-11 17:31 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

On Sun, Apr 11, 2021 at 7:20 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/11/21 3:47 AM, Sedat Dilek wrote:
> > On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> With clang compiler:
> >>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
> >>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> >> the test_cpp build failed due to the failure:
> >>    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
> >>    clang-13: warning: cannot specify -o when generating multiple output files
> >>
> >> test_cpp compilation flag looks like:
> >>    clang++ -g -Og -rdynamic -Wall -I<...> ... \
> >>    -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
> >>    test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
> >>    -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
> >>
> >> The clang++ compiler complains the header file in the command line.
> >> Let us remove the header file from the command line which is not intended
> >> any way, and this fixed the problem.
> >>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >>   tools/testing/selftests/bpf/Makefile | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >> index 6448c626498f..bbd61cc3889b 100644
> >> --- a/tools/testing/selftests/bpf/Makefile
> >> +++ b/tools/testing/selftests/bpf/Makefile
> >> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
> >>
> >>   # Benchmark runner
> >>   $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h
> >> --
> >> 2.30.2
> >>
> >
> > NOTE: bpf-next might be different from my build-environment.
> >
> > Yonghong, can you please re-test by adding explicitly CXX=g++ to your make line?
> >
> > Here I have:
> >
> > $ grep test_cpp make-log_tools-testing-selftests-bpf_clang_clang++.txt
> > 1907: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
> > -Wno-unused-command-line-argument -Wno-format-security
> > -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/tools/build/libbpf/libbpf.a
> > -lcap -lelf -lz -lrt -lpthread -o
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
> >
> > This clang++ line does not include <...>/test_core_extern.skel.h ^^^
> >
> > $ grep test_core_extern.skel.h
> > make-log_tools-testing-selftests-bpf_clang_clang++.txt
> > 704:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
> > gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_co
> > re_extern.o > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
> > 1592:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
> > gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu
> > 32/test_core_extern.o >
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu32/test_core_extern.skel.h
> >
> > Checking test_cpp:
> >
> > $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep extern
> > 0000000000417e50 <cmp_externs>:
> >   417e54: 75 22                         jne     0x417e78 <cmp_externs+0x28>
> >   417e59: 75 10                         jne     0x417e6b <cmp_externs+0x1b>
> >   417e61: 75 21                         jne     0x417e84 <cmp_externs+0x34>
> >   417e69: 75 1e                         jne     0x417e89 <cmp_externs+0x39>
> >   417e87: eb f2                         jmp     0x417e7b <cmp_externs+0x2b>
> >   417e8c: eb ed                         jmp     0x417e7b <cmp_externs+0x2b>
> >
> > $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern
> > [ EMPTY ]
> >
> > When compiled with g++ in an earlier setup this contained "core_extern".
> >
> > With this version of your patchser it breaks *here* when using CXX=g++
> > (and uses /usr/bin/ld as linker):
> >
> > 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
> > -Wno-unused-command-line-argument -Wno-format-security
> > -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/tools/build/libbpf/libbpf.a
> > -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:457:
> > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
> > Error 1
>
> I cannot reproduce the issue with g++ with bpf-next, my command line is
>
> g++ -g -Og -rdynamic -Wall
> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf
> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include
> -I/home/yhs/work/bpf-next/include/generated
> -I/home/yhs/work/bpf-next/tools/lib
> -I/home/yhs/work/bpf-next/tools/include
> -I/home/yhs/work/bpf-next/tools/include/uapi
> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf
> -Wno-unused-command-line-argument -Wno-format-security
> -Dbpf_prog_load=bpf_prog_test_load
> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> /home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> -lcap -lelf -lz -lrt -lpthread -o
> /home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_cpp
>
> I modified to
> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf ...
> and cannot reproduce the issue.
> The macro HAVE_GENHDR is only effect for test_verifier.
>
>
> Could you try to run the above g++ command by adding
> test_core_extern.skel.h back, something like
>
>  > 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
>  > -Wno-unused-command-line-argument -Wno-format-security
>  > -Dbpf_prog_load=bpf_prog_test_load
>  > -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>  > test_core_extern.skel.h
>  >
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>  > -lcap -lelf -lz -lrt -lpthread -o
>  > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>
> The issue could be somewhere else?
>

I have seen all that *skel* was reworked in bpf-next, so this is an issue here.

Adding test_core_extern.skel.h:

$ cd /tools/testing/selftests/bpf/

$ file test_core_extern.skel.h
test_core_extern.skel.h: C source, ASCII text

$ 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/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
-Wno-unused-command-line-argument -Wno-format-security
-Dbpf_prog_load=bpf_prog_test_load
-Dbpf_load_program=bpf_test_load_program test_cpp.cpp
test_core_extern.skel.h
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
-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

Write that test_cpp.cpp in C :-)?

BTW, did you check (llvm-)objdump output?

$ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern

- Sedat -

> >
> > $ grep test_cpp ../make-log_tools-testing-selftests-bpf_clang_g++.txt
> > | grep test_core_extern.skel.h
> > [ EMPTY ]
> >
> > As said I do NOT use bpf-next.
> >
> > - Sedat -
> >
> >
> >
> > - Sedat -
> >

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

* Re: [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings
  2021-04-11 11:12   ` Sedat Dilek
@ 2021-04-11 17:40     ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 17:40 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 4:12 AM, Sedat Dilek wrote:
> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> With clang compiler:
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>> Some linker flags are not used/effective for some binaries and
>> we have warnings like:
>>    warning: -lelf: 'linker' input unused [-Wunused-command-line-argument]
>>
>> We also have warnings like:
>>    .../selftests/bpf/prog_tests/ns_current_pid_tgid.c:74:57: note: treat the string as an argument to avoid this
>>          if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", strerror(errno)))
>>                                                                 ^
>>                                                                 "%s",
>>    .../selftests/bpf/test_progs.h:129:35: note: expanded from macro 'CHECK'
>>          _CHECK(condition, tag, duration, format)
>>                                           ^
>>    .../selftests/bpf/test_progs.h:108:21: note: expanded from macro '_CHECK'
>>                  fprintf(stdout, ##format);                              \
>>                                    ^
>> Let us add proper compilation flags to silence the above two kinds of warnings.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/bpf/Makefile | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index bbd61cc3889b..a9c0a64a4c49 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -24,6 +24,8 @@ SAN_CFLAGS    ?=
>>   CFLAGS += -g -Og -rdynamic -Wall $(GENFLAGS) $(SAN_CFLAGS)             \
>>            -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)          \
>>            -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)                      \
>> +         -Wno-unused-command-line-argument                             \
>> +         -Wno-format-security                                          \
> 
> Are both compliler flags available for GCC (I simply don't know or
> have checked)?

Good question. gcc/g++ (8.4.1) does not complain these two flags
and I assume they are okay as well. But further digging shows
gcc only support -W[no-]format-security
   https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
and does not support -Wno-unused-command-line-argument.
gcc/g++ simply just ignored it.

I will make -Wno-unused-command-line-argument for clang
only in v2. Thanks for catching this.

> 
> - Sedat -
> 
>>            -Dbpf_prog_load=bpf_prog_test_load                            \
>>            -Dbpf_load_program=bpf_test_load_program
>>   LDLIBS += -lcap -lelf -lz -lrt -lpthread
>> --
>> 2.30.2
>>

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-11 17:31       ` Sedat Dilek
@ 2021-04-11 19:08         ` Yonghong Song
  2021-04-12  4:47           ` Sedat Dilek
  0 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-11 19:08 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 10:31 AM, Sedat Dilek wrote:
> On Sun, Apr 11, 2021 at 7:20 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 4/11/21 3:47 AM, Sedat Dilek wrote:
>>> On Sat, Apr 10, 2021 at 6:49 PM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>> With clang compiler:
>>>>     make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>>>     make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>>>> the test_cpp build failed due to the failure:
>>>>     warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>>>>     clang-13: warning: cannot specify -o when generating multiple output files
>>>>
>>>> test_cpp compilation flag looks like:
>>>>     clang++ -g -Og -rdynamic -Wall -I<...> ... \
>>>>     -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
>>>>     test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
>>>>     -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
>>>>
>>>> The clang++ compiler complains the header file in the command line.
>>>> Let us remove the header file from the command line which is not intended
>>>> any way, and this fixed the problem.
>>>>
>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>> ---
>>>>    tools/testing/selftests/bpf/Makefile | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>>>> index 6448c626498f..bbd61cc3889b 100644
>>>> --- a/tools/testing/selftests/bpf/Makefile
>>>> +++ b/tools/testing/selftests/bpf/Makefile
>>>> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
>>>>
>>>>    # Benchmark runner
>>>>    $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h
>>>> --
>>>> 2.30.2
>>>>
>>>
>>> NOTE: bpf-next might be different from my build-environment.
>>>
>>> Yonghong, can you please re-test by adding explicitly CXX=g++ to your make line?
>>>
>>> Here I have:
>>>
>>> $ grep test_cpp make-log_tools-testing-selftests-bpf_clang_clang++.txt
>>> 1907: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
>>> -Wno-unused-command-line-argument -Wno-format-security
>>> -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/tools/build/libbpf/libbpf.a
>>> -lcap -lelf -lz -lrt -lpthread -o
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>>
>>> This clang++ line does not include <...>/test_core_extern.skel.h ^^^
>>>
>>> $ grep test_core_extern.skel.h
>>> make-log_tools-testing-selftests-bpf_clang_clang++.txt
>>> 704:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
>>> gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_co
>>> re_extern.o > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
>>> 1592:/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/sbin/bpftool
>>> gen skeleton /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu
>>> 32/test_core_extern.o >
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/no_alu32/test_core_extern.skel.h
>>>
>>> Checking test_cpp:
>>>
>>> $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep extern
>>> 0000000000417e50 <cmp_externs>:
>>>    417e54: 75 22                         jne     0x417e78 <cmp_externs+0x28>
>>>    417e59: 75 10                         jne     0x417e6b <cmp_externs+0x1b>
>>>    417e61: 75 21                         jne     0x417e84 <cmp_externs+0x34>
>>>    417e69: 75 1e                         jne     0x417e89 <cmp_externs+0x39>
>>>    417e87: eb f2                         jmp     0x417e7b <cmp_externs+0x2b>
>>>    417e8c: eb ed                         jmp     0x417e7b <cmp_externs+0x2b>
>>>
>>> $ /opt/llvm-toolchain/bin/llvm-objdump-12 -Dr test_cpp | grep core_extern
>>> [ EMPTY ]
>>>
>>> When compiled with g++ in an earlier setup this contained "core_extern".
>>>
>>> With this version of your patchser it breaks *here* when using CXX=g++
>>> (and uses /usr/bin/ld as linker):
>>>
>>> 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
>>> -Wno-unused-command-line-argument -Wno-format-security
>>> -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/tools/build/libbpf/libbpf.a
>>> -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:457:
>>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
>>> Error 1
>>
>> I cannot reproduce the issue with g++ with bpf-next, my command line is
>>
>> g++ -g -Og -rdynamic -Wall
>> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf
>> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include
>> -I/home/yhs/work/bpf-next/include/generated
>> -I/home/yhs/work/bpf-next/tools/lib
>> -I/home/yhs/work/bpf-next/tools/include
>> -I/home/yhs/work/bpf-next/tools/include/uapi
>> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf
>> -Wno-unused-command-line-argument -Wno-format-security
>> -Dbpf_prog_load=bpf_prog_test_load
>> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>> /home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>> -lcap -lelf -lz -lrt -lpthread -o
>> /home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_cpp
>>
>> I modified to
>> g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
>> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf ...
>> and cannot reproduce the issue.
>> The macro HAVE_GENHDR is only effect for test_verifier.
>>
>>
>> Could you try to run the above g++ command by adding
>> test_core_extern.skel.h back, something like
>>
>>   > 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
>>   > -Wno-unused-command-line-argument -Wno-format-security
>>   > -Dbpf_prog_load=bpf_prog_test_load
>>   > -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
>>   > test_core_extern.skel.h
>>   >
>> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
>>   > -lcap -lelf -lz -lrt -lpthread -o
>>   > /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
>>
>> The issue could be somewhere else?
>>
> 
> I have seen all that *skel* was reworked in bpf-next, so this is an issue here.
> 
> Adding test_core_extern.skel.h:
> 
> $ cd /tools/testing/selftests/bpf/
> 
> $ file test_core_extern.skel.h
> test_core_extern.skel.h: C source, ASCII text
> 
> $ 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/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
> -Wno-unused-command-line-argument -Wno-format-security
> -Dbpf_prog_load=bpf_prog_test_load
> -Dbpf_load_program=bpf_test_load_program test_cpp.cpp
> test_core_extern.skel.h
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
> -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

So the issue is not related to this patch since adding 
test_core_extern.skel.h the failure is still there.

So looks like this is g++ compilation issue. So you should be
able to reproduce the issue without this patch set, right?

My gcc compiled libbpf.a also has a bunch of R_X86_64_32 relations
against the .rodata.str1.1 section:

$ readelf -r libbpf.a | less
File: libbpf.a(libbpf-in.o)

Relocation section '.rela.text' at offset 0x11a4b8 contains 3152 entries:
   Offset          Info           Type           Sym. Value    Sym. Name 
+ Addend
000000000136  00020000000b R_X86_64_32S      0000000000000000 .rodata + 0
00000000013b  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 6e
000000000141  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + c
000000000147  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 10
00000000014d  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 16
000000000153  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 1d
000000000159  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 23
00000000015f  00030000000a R_X86_64_32       0000000000000000 
.rodata.str1.1 + 28
...

and I did not see any issues.
Adding '-###' to the compilation flags you can find the flags for
each subcommand, for linker, I have

  /usr/libexec/gcc/x86_64-redhat-linux/8/collect2 -plugin 
/usr/libexec/gcc/x86_64-redhat-linux/8/liblto_plugin.so 
"-plugin-opt=/usr/libexec/gcc/x86_64-redhat-linux/8/lto-wrapper" 
"-plugin-opt=-fresolution=/tmp/ccjktcVg.res" 
"-plugin-opt=-pass-through=-lgcc_s" "-plugin-opt=-pass-through=-lgcc" 
"-plugin-opt=-pass-through=-lc" "-plugin-opt=-pass-through=-lgcc_s" 
"-plugin-opt=-pass-through=-lgcc" --build-id --no-add-needed 
--eh-frame-hdr "--hash-style=gnu" -m elf_x86_64 -export-dynamic 
-dynamic-linker /lib64/ld-linux-x86-64.so.2 -o 
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_cpp 
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o 
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crti.o 
/usr/lib/gcc/x86_64-redhat-linux/8/crtbegin.o 
-L/usr/lib/gcc/x86_64-redhat-linux/8 
-L/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64 -L/lib/../lib64 
-L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/8/../../.. 
/tmp/cc0xqkPi.o 
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a 
-lcap -lelf -lz -lrt -lpthread "-lstdc++" -lm -lgcc_s -lgcc -lc -lgcc_s 
-lgcc /usr/lib/gcc/x86_64-redhat-linux/8/crtend.o 
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crtn.o
COLLECT_GCC_OPTIONS='-g' '-rdynamic' '-Wall' '-O2' '-D' 'HAVE_GENHDR' 
'-I' '/home/yhs/work/bpf-next/tools/testing/selftests/bpf' '-I' 
'/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include' '-I' 
'/home/yhs/work/bpf-next/include/generated' '-I' 
'/home/yhs/work/bpf-next/tools/lib' '-I' 
'/home/yhs/work/bpf-next/tools/include' '-I' 
'/home/yhs/work/bpf-next/tools/include/uapi' '-I' 
'/home/yhs/work/bpf-next/tools/testing/selftests/bpf' 
'-Wno-uuunused-command-line-argument' '-Wno-format-security' '-D' 
'bpf_prog_load=bpf_prog_test_load' '-D' 
'bpf_load_program=bpf_test_load_program' '-o' 
'/home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_cpp' 
'-shared-libgcc' '-mtune=generic' '-march=x86-64'

Not sure whether you will see some different COLLECT_GCC_OPTIONS flags 
or not.

There is not much I can do here since I cannot reproduce the issue.
Maybe you can help to narrow down the reason of the failure a little
bit more.

> 
> Write that test_cpp.cpp in C :-)?

test_cpp.cpp is intentionally to test libbpf used by a c++ application.

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

> 
> - Sedat -
> 
>>>
>>> $ grep test_cpp ../make-log_tools-testing-selftests-bpf_clang_g++.txt
>>> | grep test_core_extern.skel.h
>>> [ EMPTY ]
>>>
>>> As said I do NOT use bpf-next.
>>>
>>> - Sedat -
>>>
>>>
>>>
>>> - Sedat -
>>>

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-11 19:08         ` Yonghong Song
@ 2021-04-12  4:47           ` Sedat Dilek
  2021-04-12  5:42             ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-12  4:47 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

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?

- Sedat -

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-12  4:47           ` Sedat Dilek
@ 2021-04-12  5:42             ` Yonghong Song
  2021-04-12  6:06               ` Sedat Dilek
  0 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-12  5:42 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



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.

> 
> - Sedat -
> 

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-12  5:42             ` Yonghong Song
@ 2021-04-12  6:06               ` Sedat Dilek
  2021-04-12 14:15                 ` Yonghong Song
  0 siblings, 1 reply; 30+ messages in thread
From: Sedat Dilek @ 2021-04-12  6:06 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers

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 -

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-12  6:06               ` Sedat Dilek
@ 2021-04-12 14:15                 ` Yonghong Song
  0 siblings, 0 replies; 30+ messages in thread
From: Yonghong Song @ 2021-04-12 14:15 UTC (permalink / raw)
  To: sedat.dilek
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, kernel-team,
	Nick Desaulniers



On 4/11/21 11:06 PM, Sedat Dilek wrote:
> 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.

Thanks for testing. Great to see all g++/clang++ compilations passed!

> 
> As said here all my testings with Linus Git not bpf-next.
> 
> Thanks for your precious time!
> 
> - Sedat -
> 

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-10 16:49 ` [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang Yonghong Song
  2021-04-11 10:47   ` Sedat Dilek
@ 2021-04-13  4:32   ` Andrii Nakryiko
  2021-04-13  6:12     ` Yonghong Song
  1 sibling, 1 reply; 30+ messages in thread
From: Andrii Nakryiko @ 2021-04-13  4:32 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, Kernel Team,
	Nick Desaulniers, Sedat Dilek

On Sat, Apr 10, 2021 at 9:49 AM Yonghong Song <yhs@fb.com> wrote:
>
> With clang compiler:
>   make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>   make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> the test_cpp build failed due to the failure:
>   warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>   clang-13: warning: cannot specify -o when generating multiple output files
>
> test_cpp compilation flag looks like:
>   clang++ -g -Og -rdynamic -Wall -I<...> ... \
>   -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
>   test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
>   -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
>
> The clang++ compiler complains the header file in the command line.
> Let us remove the header file from the command line which is not intended
> any way, and this fixed the problem.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 6448c626498f..bbd61cc3889b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@

see what we do for other binaries:

$(filter %.a %.o %.c,$^)

It's more generic. Add %.cpp, of course.

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

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-13  4:32   ` Andrii Nakryiko
@ 2021-04-13  6:12     ` Yonghong Song
  2021-04-13 17:50       ` Andrii Nakryiko
  0 siblings, 1 reply; 30+ messages in thread
From: Yonghong Song @ 2021-04-13  6:12 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, Kernel Team,
	Nick Desaulniers, Sedat Dilek



On 4/12/21 9:32 PM, Andrii Nakryiko wrote:
> On Sat, Apr 10, 2021 at 9:49 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> With clang compiler:
>>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
>>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
>> the test_cpp build failed due to the failure:
>>    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>>    clang-13: warning: cannot specify -o when generating multiple output files
>>
>> test_cpp compilation flag looks like:
>>    clang++ -g -Og -rdynamic -Wall -I<...> ... \
>>    -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
>>    test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
>>    -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
>>
>> The clang++ compiler complains the header file in the command line.
>> Let us remove the header file from the command line which is not intended
>> any way, and this fixed the problem.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/bpf/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 6448c626498f..bbd61cc3889b 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
> 
> see what we do for other binaries:
> 
> $(filter %.a %.o %.c,$^)
> 
> It's more generic. Add %.cpp, of course.

Okay, although I will remove %.c.

[yhs@devbig003.ftw2 ~/tmp/tmp2]$ clang++ -c t.c
clang-13: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated [-Wdeprecated]
[yhs@devbig003.ftw2 ~/tmp/tmp2]$

clang++ will warn if trying to compile .c file.
g++ 8.4.1 doesn't warn. Not sure about latest g++. But it is not
a good practice to compile .c files with c++ compiler any way.

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

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

* Re: [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang
  2021-04-13  6:12     ` Yonghong Song
@ 2021-04-13 17:50       ` Andrii Nakryiko
  0 siblings, 0 replies; 30+ messages in thread
From: Andrii Nakryiko @ 2021-04-13 17:50 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Andrii Nakryiko, Arnaldo Carvalho de Melo, Kernel Team,
	Nick Desaulniers, Sedat Dilek

On Mon, Apr 12, 2021 at 11:12 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/12/21 9:32 PM, Andrii Nakryiko wrote:
> > On Sat, Apr 10, 2021 at 9:49 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> With clang compiler:
> >>    make -j60 LLVM=1 LLVM_IAS=1  <=== compile kernel
> >>    make -j60 -C tools/testing/selftests/bpf LLVM=1 LLVM_IAS=1
> >> the test_cpp build failed due to the failure:
> >>    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated [-Wdeprecated]
> >>    clang-13: warning: cannot specify -o when generating multiple output files
> >>
> >> test_cpp compilation flag looks like:
> >>    clang++ -g -Og -rdynamic -Wall -I<...> ... \
> >>    -Dbpf_prog_load=bpf_prog_test_load -Dbpf_load_program=bpf_test_load_program \
> >>    test_cpp.cpp <...>/test_core_extern.skel.h <...>/libbpf.a <...>/test_stub.o \
> >>    -lcap -lelf -lz -lrt -lpthread -o <...>/test_cpp
> >>
> >> The clang++ compiler complains the header file in the command line.
> >> Let us remove the header file from the command line which is not intended
> >> any way, and this fixed the problem.
> >>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >>   tools/testing/selftests/bpf/Makefile | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >> index 6448c626498f..bbd61cc3889b 100644
> >> --- a/tools/testing/selftests/bpf/Makefile
> >> +++ b/tools/testing/selftests/bpf/Makefile
> >> @@ -481,7 +481,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) test_cpp.cpp $(BPFOBJ) $(LDLIBS) -o $@
> >
> > see what we do for other binaries:
> >
> > $(filter %.a %.o %.c,$^)
> >
> > It's more generic. Add %.cpp, of course.
>
> Okay, although I will remove %.c.
>
> [yhs@devbig003.ftw2 ~/tmp/tmp2]$ clang++ -c t.c
> clang-13: warning: treating 'c' input as 'c++' when in C++ mode, this
> behavior is deprecated [-Wdeprecated]
> [yhs@devbig003.ftw2 ~/tmp/tmp2]$
>
> clang++ will warn if trying to compile .c file.
> g++ 8.4.1 doesn't warn. Not sure about latest g++. But it is not
> a good practice to compile .c files with c++ compiler any way.

yeah, makes sense

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

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

end of thread, other threads:[~2021-04-13 17:51 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-10 16:49 [PATCH bpf-next 0/5] support build selftests/bpf with clang Yonghong Song
2021-04-10 16:49 ` [PATCH bpf-next 1/5] selftests: set CC to clang in lib.mk if LLVM is set Yonghong Song
2021-04-11 10:22   ` Sedat Dilek
2021-04-11 16:51     ` Yonghong Song
2021-04-11 17:10       ` Sedat Dilek
2021-04-10 16:49 ` [PATCH bpf-next 2/5] tools: allow proper CC/CXX/... override with LLVM=1 in Makefile.include Yonghong Song
2021-04-11 10:24   ` Sedat Dilek
2021-04-11 16:52     ` Yonghong Song
2021-04-10 16:49 ` [PATCH bpf-next 3/5] selftests/bpf: fix test_cpp compilation failure with clang Yonghong Song
2021-04-11 10:47   ` Sedat Dilek
2021-04-11 17:20     ` Yonghong Song
2021-04-11 17:31       ` Sedat Dilek
2021-04-11 19:08         ` Yonghong Song
2021-04-12  4:47           ` Sedat Dilek
2021-04-12  5:42             ` Yonghong Song
2021-04-12  6:06               ` Sedat Dilek
2021-04-12 14:15                 ` Yonghong Song
2021-04-13  4:32   ` Andrii Nakryiko
2021-04-13  6:12     ` Yonghong Song
2021-04-13 17:50       ` Andrii Nakryiko
2021-04-10 16:49 ` [PATCH bpf-next 4/5] selftests/bpf: silence clang compilation warnings Yonghong Song
2021-04-11 11:12   ` Sedat Dilek
2021-04-11 17:40     ` Yonghong Song
2021-04-10 16:49 ` [PATCH bpf-next 5/5] bpftool: fix a clang compilation warning Yonghong Song
2021-04-11 11:05   ` Sedat Dilek
2021-04-11 17:24     ` Yonghong Song
2021-04-10 17:23 ` [PATCH bpf-next 0/5] support build selftests/bpf with clang Alexei Starovoitov
2021-04-10 17:38   ` Yonghong Song
2021-04-10 19:19 ` Sedat Dilek
2021-04-11 16:46   ` Yonghong Song

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.