Skip to contentSkip to conversation

selftests/bpf: do not ignore clang failures

PATCH Applied Ilya Leoshkevich 3 participants 4 messages Jul 11, 2019 – Jul 12, 2019

Subsystems (5)
Networking Network drivers Kselftest BPF +1 more subsystems
lore
Share & tools
lore

Ilya Leoshkevich

author
#0 lore raw

PATCH v3 1/1 selftests/bpf: do not ignore clang failures

When compiling an eBPF prog fails, make still returns 0, because failing clang command's output is piped to llc and therefore its exit status is ignored.

When clang fails, pipe the string "clang failed" to llc. This will make llc fail with an informative error message. This solution was chosen over using pipefail, having separate targets or getting rid of llc invocation due to its simplicity.

In addition, pull Kbuild.include in order to get .DELETE_ON_ERROR target, which would cause partial .o files to be removed.

Signed-off-by Ilya Leoshkevich <[email protected]>

---
v1->v2: use intermediate targets instead of pipefail v2->v3: pipe "clang failed" instead of using intermediate targets

tools/testing/selftests/bpf/Makefile | 13 +++++++------

 1 file changed, 7 insertions(+), 6 deletions(-)
tools/testing/selftests/bpf/Makefile +7 -6
index e36356e2377e..e375f399b7a6 100644
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+include ../../../../scripts/Kbuild.include
 
 LIBDIR := ../../../lib
 BPFDIR := $(LIBDIR)/bpf

@@ -185,8 +186,8 @@ $(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c
 
 $(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR) \
 					$(ALU32_BUILD_DIR)/test_progs_32
-	$(CLANG) $(CLANG_FLAGS) \
-		 -O2 -target bpf -emit-llvm -c $< -o - |      \
+	($(CLANG) $(CLANG_FLAGS) -O2 -target bpf -emit-llvm -c $< -o - || \
+		echo "clang failed") | \
 	$(LLC) -march=bpf -mattr=+alu32 -mcpu=$(CPU) $(LLC_FLAGS) \
 		-filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)

@@ -197,16 +198,16 @@ endif
 # Have one program compiled without "-target bpf" to test whether libbpf loads
 # it successfully
 $(OUTPUT)/test_xdp.o: progs/test_xdp.c
-	$(CLANG) $(CLANG_FLAGS) \
-		-O2 -emit-llvm -c $< -o - | \
+	($(CLANG) $(CLANG_FLAGS) -O2 -emit-llvm -c $< -o - || \
+		echo "clang failed") | \
 	$(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)
 	$(BTF_PAHOLE) -J $@
 endif
 
 $(OUTPUT)/%.o: progs/%.c
-	$(CLANG) $(CLANG_FLAGS) \
-		 -O2 -target bpf -emit-llvm -c $< -o - |      \
+	($(CLANG) $(CLANG_FLAGS) -O2 -target bpf -emit-llvm -c $< -o - || \
+		echo "clang failed") | \
 	$(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)
 	$(BTF_PAHOLE) -J $@
Signature · 1 line
2.21.0
Acked-by on v3 1/1
#1 lore raw
Quoted 11 lines · Ilya Leoshkevich

When compiling an eBPF prog fails, make still returns 0, because failing clang command's output is piped to llc and therefore its exit status is ignored.

When clang fails, pipe the string "clang failed" to llc. This will make llc fail with an informative error message. This solution was chosen over using pipefail, having separate targets or getting rid of llc invocation due to its simplicity.

In addition, pull Kbuild.include in order to get .DELETE_ON_ERROR target,

In your original patch you explicitly declared .DELETE_ON_ERROR, but in this one you just include Kbuild.include.
Is it enough to just include that file to get desired behavior or your forgot to add .DELETE_ON_ERROR?

Quoted 4 lines

which would cause partial .o files to be removed.

Signed-off-by Ilya Leoshkevich <[email protected]>

---

Thanks!

Acked-by Andrii Nakryiko <[email protected]>
Quoted 51 lines

v1->v2: use intermediate targets instead of pipefail v2->v3: pipe "clang failed" instead of using intermediate targets

tools/testing/selftests/bpf/Makefile | 13 +++++++------

 1 file changed, 7 insertions(+), 6 deletions(-)
tools/testing/selftests/bpf/Makefile +7 -7
index e36356e2377e..e375f399b7a6 100644
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+include ../../../../scripts/Kbuild.include
 
 LIBDIR := ../../../lib
 BPFDIR := $(LIBDIR)/bpf

@@ -185,8 +186,8 @@ $(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c
 
 $(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR) \
                                        $(ALU32_BUILD_DIR)/test_progs_32
-       $(CLANG) $(CLANG_FLAGS) \
-                -O2 -target bpf -emit-llvm -c $< -o - |      \
+       ($(CLANG) $(CLANG_FLAGS) -O2 -target bpf -emit-llvm -c $< -o - || \
+               echo "clang failed") | \
        $(LLC) -march=bpf -mattr=+alu32 -mcpu=$(CPU) $(LLC_FLAGS) \
                -filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)

@@ -197,16 +198,16 @@ endif
 # Have one program compiled without "-target bpf" to test whether libbpf loads
 # it successfully
 $(OUTPUT)/test_xdp.o: progs/test_xdp.c
-       $(CLANG) $(CLANG_FLAGS) \
-               -O2 -emit-llvm -c $< -o - | \
+       ($(CLANG) $(CLANG_FLAGS) -O2 -emit-llvm -c $< -o - || \
+               echo "clang failed") | \
        $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)
        $(BTF_PAHOLE) -J $@
 endif
 
 $(OUTPUT)/%.o: progs/%.c
-       $(CLANG) $(CLANG_FLAGS) \
-                -O2 -target bpf -emit-llvm -c $< -o - |      \
+       ($(CLANG) $(CLANG_FLAGS) -O2 -target bpf -emit-llvm -c $< -o - || \
+               echo "clang failed") | \
        $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
 ifeq ($(DWARF2BTF),y)
        $(BTF_PAHOLE) -J $@
--

2.21.0

author
#2 lore raw
Quoted 11 lines

Am 11.07.2019 um 16:55 schrieb Andrii Nakryiko <[email protected]>:

Quoted 3 lines · Ilya Leoshkevich

In addition, pull Kbuild.include in order to get .DELETE_ON_ERROR target,

In your original patch you explicitly declared .DELETE_ON_ERROR, but in this one you just include Kbuild.include.
Is it enough to just include that file to get desired behavior or your forgot to add .DELETE_ON_ERROR?

It’s enough to just include Kbuild.include. I grepped the source tree and found that no one else declares .DELETE_ON_ERROR explicitly, so I've decided to avoid doing this as well.

maintainer Applied
#3 lore raw
In reply to Ilya Leoshkevich
Quoted 9 lines · Ilya Leoshkevich

When compiling an eBPF prog fails, make still returns 0, because failing clang command's output is piped to llc and therefore its exit status is ignored.

When clang fails, pipe the string "clang failed" to llc. This will make llc fail with an informative error message. This solution was chosen over using pipefail, having separate targets or getting rid of llc invocation due to its simplicity.

Quoted 4 lines

In addition, pull Kbuild.include in order to get .DELETE_ON_ERROR target, which would cause partial .o files to be removed.

Signed-off-by Ilya Leoshkevich <[email protected]>

Applied, thanks!