selftests/bpf: Attempt to build BPF programs with -Wsign-compare

GCC's -Wall includes -Wsign-compare while clang does not.
Since BPF programs are built with clang we need to add this flag explicitly
to catch problematic comparisons like:

  int i = -1;
  unsigned int j = 1;
  if (i < j) // this is false.

  long i = -1;
  unsigned int j = 1;
  if (i < j) // this is true.

C standard for reference:

- If either operand is unsigned long the other shall be converted to unsigned long.

- Otherwise, if one operand is a long int and the other unsigned int, then if a
long int can represent all the values of an unsigned int, the unsigned int
shall be converted to a long int; otherwise both operands shall be converted to
unsigned long int.

- Otherwise, if either operand is long, the other shall be converted to long.

- Otherwise, if either operand is unsigned, the other shall be converted to unsigned.

Unfortunately clang's -Wsign-compare is very noisy.
It complains about (s32)a == (u32)b which is safe and doen't have surprising behavior.

This patch fixes some of the issues. It needs a follow up to fix the rest.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/bpf/20231226191148.48536-2-alexei.starovoitov@gmail.com

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 617ae55c3bb5b5b0fc8bef1e73b42a4b43a528a9..fd15017ed3b12bfbbefce4578d1de947b5f308bc 100644 (file)
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -383,6 +383,7 @@ CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
 BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)    \
             -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR)                   \
             -I$(abspath $(OUTPUT)/../usr/include)
+# TODO: enable me -Wsign-compare
 
 CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
               -Wno-compare-distinct-pointer-types

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_hash_map.c b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_hash_map.c
index feaaa2b89c574fd982684de8475737979a92bafd..5014a17d6c02fdb6a9d1ec48c6fa999baeb412be 100644 (file)
--- a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_hash_map.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_hash_map.c
@@ -20,7 +20,7 @@ struct {
 } hashmap1 SEC(".maps");
 
 /* will set before prog run */
-volatile const __u32 num_cpus = 0;
+volatile const __s32 num_cpus = 0;
 
 /* will collect results during prog run */
 __u32 key_sum_a = 0, key_sum_b = 0, key_sum_c = 0;

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c b/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
index dd923dc637d5c62d187431506856265332efbad9..423b39e60b6f1cf2684188f33b5c531dacffa200 100644 (file)
--- a/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
@@ -35,7 +35,7 @@ SEC("iter/task_vma") int proc_maps(struct bpf_iter__task_vma *ctx)
                return 0;
 
        file = vma->vm_file;
-       if (task->tgid != pid) {
+       if (task->tgid != (pid_t)pid) {
                if (one_task)
                        one_task_error = 1;
                return 0;

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
index 96131b9a1caae5f4337b97f4811b3de43c5b785d..6cbb3393f2438db300a767c8f3c081262dbc7a9a 100644 (file)
--- a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c
@@ -22,7 +22,7 @@ int dump_task(struct bpf_iter__task *ctx)
                return 0;
        }
 
-       if (task->pid != tid)
+       if (task->pid != (pid_t)tid)
                num_unknown_tid++;
        else
                num_known_tid++;

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
index 400fdf8d623321ffdb88b76a766aad7c20ab64f1..dbf61c44acac6d52f8bcc19b859643467bd62b20 100644 (file)
--- a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
@@ -45,7 +45,7 @@ int dump_bpf_map(struct bpf_iter__bpf_map *ctx)
        }
 
        /* fill seq_file buffer */
-       for (i = 0; i < print_len; i++)
+       for (i = 0; i < (int)print_len; i++)
                bpf_seq_write(seq, &seq_num, sizeof(seq_num));
 
        return ret;

diff --git a/tools/testing/selftests/bpf/progs/cgroup_getset_retval_setsockopt.c b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_setsockopt.c
index b7fa8804e19de25f20f6973dd59666b836e5d408..45a0e9f492a979a82fb857275fe43126e800f49c 100644 (file)
--- a/tools/testing/selftests/bpf/progs/cgroup_getset_retval_setsockopt.c
+++ b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_setsockopt.c
@@ -11,7 +11,7 @@
 __u32 invocations = 0;
 __u32 assertion_error = 0;
 __u32 retval_value = 0;
-__u32 page_size = 0;
+__s32 page_size = 0;
 
 SEC("cgroup/setsockopt")
 int get_retval(struct bpf_sockopt *ctx)

diff --git a/tools/testing/selftests/bpf/progs/cgrp_ls_sleepable.c b/tools/testing/selftests/bpf/progs/cgrp_ls_sleepable.c
index facedd8b825007c34628f322aeb095e74eb79b96..5e282c16eadc53f8cbbe1a05b433a28432874978 100644 (file)
--- a/tools/testing/selftests/bpf/progs/cgrp_ls_sleepable.c
+++ b/tools/testing/selftests/bpf/progs/cgrp_ls_sleepable.c
@@ -15,7 +15,7 @@ struct {
        __type(value, long);
 } map_a SEC(".maps");
 
-__u32 target_pid;
+__s32 target_pid;
 __u64 cgroup_id;
 int target_hid;
 bool is_cgroup1;

diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
index fc3666edf4561a2e681b72ea570558985f6b6515..7a1e64c6c065ce48df3059332378dc91165402a2 100644 (file)
--- a/tools/testing/selftests/bpf/progs/cpumask_success.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
@@ -332,7 +332,7 @@ SEC("tp_btf/task_newtask")
 int BPF_PROG(test_copy_any_anyand, struct task_struct *task, u64 clone_flags)
 {
        struct bpf_cpumask *mask1, *mask2, *dst1, *dst2;
-       u32 cpu;
+       int cpu;
 
        if (!is_test_task())
                return 0;

diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
index 3aca3dc145b55282ce9ee0af03fa4d7685fc1a40..fe971992e635969b610be0b2d838ad7e70f54480 100644 (file)
--- a/tools/testing/selftests/bpf/progs/iters.c
+++ b/tools/testing/selftests/bpf/progs/iters.c
@@ -6,7 +6,7 @@
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(x) (int)(sizeof(x) / sizeof((x)[0]))
 
 static volatile int zero = 0;
 
@@ -676,7 +676,7 @@ static __noinline int sum(struct bpf_iter_num *it, int *arr, __u32 n)
 
        while ((t = bpf_iter_num_next(it))) {
                i = *t;
-               if (i >= n)
+               if ((__u32)i >= n)
                        break;
                sum += arr[i];
        }

diff --git a/tools/testing/selftests/bpf/progs/linked_funcs1.c b/tools/testing/selftests/bpf/progs/linked_funcs1.c
index c4b49ceea967b82a8b8588c20bc18e1271596b10..cc79dddac182c20da69a1a57fa39bc81004184f1 100644 (file)
--- a/tools/testing/selftests/bpf/progs/linked_funcs1.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs1.c
@@ -8,7 +8,7 @@
 #include "bpf_misc.h"
 
 /* weak and shared between two files */
-const volatile int my_tid __weak;
+const volatile __u32 my_tid __weak;
 long syscall_id __weak;
 
 int output_val1;

diff --git a/tools/testing/selftests/bpf/progs/linked_funcs2.c b/tools/testing/selftests/bpf/progs/linked_funcs2.c
index 013ff0645f0ccb2eb229b79bb4f8cd4151db53d0..942cc5526ddf02004bf82d1f72c74cfba6eaf486 100644 (file)
--- a/tools/testing/selftests/bpf/progs/linked_funcs2.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs2.c
@@ -68,7 +68,7 @@ int BPF_PROG(handler2, struct pt_regs *regs, long id)
 {
        static volatile int whatever;
 
-       if (my_tid != (u32)bpf_get_current_pid_tgid() || id != syscall_id)
+       if (my_tid != (s32)bpf_get_current_pid_tgid() || id != syscall_id)
                return 0;
 
        /* make sure we have CO-RE relocations in main program */

diff --git a/tools/testing/selftests/bpf/progs/linked_list.c b/tools/testing/selftests/bpf/progs/linked_list.c
index 84d1777a9e6c96b7c41f6c0ede0577a20253075a..26205ca8067968d3b58443257cedc47e2300a6dd 100644 (file)
--- a/tools/testing/selftests/bpf/progs/linked_list.c
+++ b/tools/testing/selftests/bpf/progs/linked_list.c
@@ -6,7 +6,7 @@
 #include "bpf_experimental.h"
 
 #ifndef ARRAY_SIZE
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(x) (int)(sizeof(x) / sizeof((x)[0]))
 #endif
 
 #include "linked_list.h"

diff --git a/tools/testing/selftests/bpf/progs/local_storage.c b/tools/testing/selftests/bpf/progs/local_storage.c
index bc8ea56671a16a07ddc602b34c22850310403f21..e5e3a8b8dd075845e3c62bf8bcd6ec3027647595 100644 (file)
--- a/tools/testing/selftests/bpf/progs/local_storage.c
+++ b/tools/testing/selftests/bpf/progs/local_storage.c
@@ -13,7 +13,7 @@ char _license[] SEC("license") = "GPL";
 
 #define DUMMY_STORAGE_VALUE 0xdeadbeef
 
-int monitored_pid = 0;
+__u32 monitored_pid = 0;
 int inode_storage_result = -1;
 int sk_storage_result = -1;
 int task_storage_result = -1;

diff --git a/tools/testing/selftests/bpf/progs/lsm.c b/tools/testing/selftests/bpf/progs/lsm.c
index fadfdd98707c4e6913f39d7dfe29632f7503d3e3..0c13b7409947ef2c0216ab8282a7af98408b1a5f 100644 (file)
--- a/tools/testing/selftests/bpf/progs/lsm.c
+++ b/tools/testing/selftests/bpf/progs/lsm.c
@@ -92,7 +92,7 @@ int BPF_PROG(test_int_hook, struct vm_area_struct *vma,
        if (ret != 0)
                return ret;
 
-       __u32 pid = bpf_get_current_pid_tgid() >> 32;
+       __s32 pid = bpf_get_current_pid_tgid() >> 32;
        int is_stack = 0;
 
        is_stack = (vma->vm_start <= vma->vm_mm->start_stack &&

diff --git a/tools/testing/selftests/bpf/progs/normal_map_btf.c b/tools/testing/selftests/bpf/progs/normal_map_btf.c
index 66cde82aa86d92cf8fe885acc63f70b4dfbd70f5..a45c9299552c992889cd11bdb2b628b4606e0a34 100644 (file)
--- a/tools/testing/selftests/bpf/progs/normal_map_btf.c
+++ b/tools/testing/selftests/bpf/progs/normal_map_btf.c
@@ -36,7 +36,7 @@ int add_to_list_in_array(void *ctx)
        struct node_data *new;
        int zero = 0;
 
-       if (done || (u32)bpf_get_current_pid_tgid() != pid)
+       if (done || (int)bpf_get_current_pid_tgid() != pid)
                return 0;
 
        value = bpf_map_lookup_elem(&array, &zero);

diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h
index 897061930cb76e713e7619e109f872e1e7f4137b..ba99d17dac544f74a9ca3608e3c1ae3ab41c62a1 100644 (file)
--- a/tools/testing/selftests/bpf/progs/profiler.inc.h
+++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
@@ -132,7 +132,7 @@ struct {
 } disallowed_exec_inodes SEC(".maps");
 
 #ifndef ARRAY_SIZE
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
+#define ARRAY_SIZE(arr) (int)(sizeof(arr) / sizeof(arr[0]))
 #endif
 
 static INLINE bool IS_ERR(const void* ptr)
@@ -645,7 +645,7 @@ int raw_tracepoint__sched_process_exit(void* ctx)
        for (int i = 0; i < ARRAY_SIZE(arr_struct->array); i++) {
                struct var_kill_data_t* past_kill_data = &arr_struct->array[i];
 
-               if (past_kill_data != NULL && past_kill_data->kill_target_pid == tpid) {
+               if (past_kill_data != NULL && past_kill_data->kill_target_pid == (pid_t)tpid) {
                        bpf_probe_read_kernel(kill_data, sizeof(*past_kill_data),
                                              past_kill_data);
                        void* payload = kill_data->payload;

diff --git a/tools/testing/selftests/bpf/progs/sockopt_inherit.c b/tools/testing/selftests/bpf/progs/sockopt_inherit.c
index c8f59caa46394d459e84295725e114e1abe35a83..a3434b840928173c462d809161db7776753d1240 100644 (file)
--- a/tools/testing/selftests/bpf/progs/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_inherit.c
@@ -9,7 +9,7 @@ char _license[] SEC("license") = "GPL";
 #define CUSTOM_INHERIT2                        1
 #define CUSTOM_LISTENER                        2
 
-__u32 page_size = 0;
+__s32 page_size = 0;
 
 struct sockopt_inherit {
        __u8 val;

diff --git a/tools/testing/selftests/bpf/progs/sockopt_multi.c b/tools/testing/selftests/bpf/progs/sockopt_multi.c
index 96f29fce050bcde6315606e80e4cbc4eca7675c6..db67278e12d4dbf918bc288394bc3164bd9462bd 100644 (file)
--- a/tools/testing/selftests/bpf/progs/sockopt_multi.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_multi.c
@@ -5,7 +5,7 @@
 
 char _license[] SEC("license") = "GPL";
 
-__u32 page_size = 0;
+__s32 page_size = 0;
 
 SEC("cgroup/getsockopt")
 int _getsockopt_child(struct bpf_sockopt *ctx)

diff --git a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
index dbe235ede7f3808c2918951c9497490a0442f191..83753b00a55693cb8e578fd1b85697b9ac595ba0 100644 (file)
--- a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
@@ -9,7 +9,7 @@
 
 char _license[] SEC("license") = "GPL";
 
-__u32 page_size = 0;
+__s32 page_size = 0;
 
 SEC("cgroup/setsockopt")
 int sockopt_qos_to_cc(struct bpf_sockopt *ctx)

diff --git a/tools/testing/selftests/bpf/progs/test_bpf_ma.c b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
index 069db9085e789a4b82a852e8c01f0914970836af..b78f4f702ae0f11099a078533240b3459524c704 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_bpf_ma.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
@@ -21,7 +21,7 @@ const unsigned int data_sizes[] = {16, 32, 64, 96, 128, 192, 256, 512, 1024, 204
 const volatile unsigned int data_btf_ids[ARRAY_SIZE(data_sizes)] = {};
 
 int err = 0;
-int pid = 0;
+u32 pid = 0;
 
 #define DEFINE_ARRAY_WITH_KPTR(_size) \
        struct bin_data_##_size { \

diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index a17dd83eae674f42fdc1c395d2ec000947916ae3..ee4a601dcb066fa8ac81cde823260a7d7858c9af 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -53,7 +53,7 @@ int test_core_kernel(void *ctx)
        struct task_struct *task = (void *)bpf_get_current_task();
        struct core_reloc_kernel_output *out = (void *)&data.out;
        uint64_t pid_tgid = bpf_get_current_pid_tgid();
-       uint32_t real_tgid = (uint32_t)pid_tgid;
+       int32_t real_tgid = (int32_t)pid_tgid;
        int pid, tgid;
 
        if (data.my_pid_tgid != pid_tgid)

diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
index f59f175c7bafb6b2456b12a2c6b4e967149755a5..bcb31ff92dcccc2f71b18326fdd8e7c48a632c89 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
@@ -43,8 +43,8 @@ int BPF_PROG(test_core_module_probed,
 #if __has_builtin(__builtin_preserve_enum_value)
        struct core_reloc_module_output *out = (void *)&data.out;
        __u64 pid_tgid = bpf_get_current_pid_tgid();
-       __u32 real_tgid = (__u32)(pid_tgid >> 32);
-       __u32 real_pid = (__u32)pid_tgid;
+       __s32 real_tgid = (__s32)(pid_tgid >> 32);
+       __s32 real_pid = (__s32)pid_tgid;
 
        if (data.my_pid_tgid != pid_tgid)
                return 0;
@@ -77,8 +77,8 @@ int BPF_PROG(test_core_module_direct,
 #if __has_builtin(__builtin_preserve_enum_value)
        struct core_reloc_module_output *out = (void *)&data.out;
        __u64 pid_tgid = bpf_get_current_pid_tgid();
-       __u32 real_tgid = (__u32)(pid_tgid >> 32);
-       __u32 real_pid = (__u32)pid_tgid;
+       __s32 real_tgid = (__s32)(pid_tgid >> 32);
+       __s32 real_pid = (__s32)pid_tgid;
 
        if (data.my_pid_tgid != pid_tgid)
                return 0;

diff --git a/tools/testing/selftests/bpf/progs/test_fsverity.c b/tools/testing/selftests/bpf/progs/test_fsverity.c
index 3975495b75c8df282c9acf0830208a64549564e3..9e0f73e8189ce3bb0c6aa3b8faa15eb60906e8ef 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_fsverity.c
+++ b/tools/testing/selftests/bpf/progs/test_fsverity.c
@@ -38,7 +38,7 @@ int BPF_PROG(test_file_open, struct file *f)
                return 0;
        got_fsverity = 1;
 
-       for (i = 0; i < sizeof(digest); i++) {
+       for (i = 0; i < (int)sizeof(digest); i++) {
                if (digest[i] != expected_digest[i])
                        return 0;
        }

diff --git a/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c b/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c
index eacda9fe07ebb08a8c50ff43cfe788863a5a9bb6..4cfa42aa943619c14be58024ac1628c8a0c92032 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c
+++ b/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c
@@ -29,7 +29,7 @@ int BPF_PROG(unix_listen, struct socket *sock, int backlog)
        len = unix_sk->addr->len - sizeof(short);
        path[0] = '@';
        for (i = 1; i < len; i++) {
-               if (i >= sizeof(struct sockaddr_un))
+               if (i >= (int)sizeof(struct sockaddr_un))
                        break;
 
                path[i] = unix_sk->addr->name->sun_path[i];

diff --git a/tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c b/tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c
index 5baaafed0d2decea667a16d63665be6792688c3d..3abf068b84464ce0460a671abc4dfb97e1f2be4a 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c
@@ -38,7 +38,7 @@ int xdp_redirect(struct xdp_md *xdp)
        if (payload + 1 > data_end)
                return XDP_ABORTED;
 
-       if (xdp->ingress_ifindex != ifindex_in)
+       if (xdp->ingress_ifindex != (__u32)ifindex_in)
                return XDP_ABORTED;
 
        if (metadata + 1 > data)