[PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Yonghong Song]

In one of upstream thread ([1]), there is a discussion about
the below inline asm code:

  if r1 == 0xdeadbeef goto +2;
  ...

In actual llvm backend, the above 0xdeadbeef will actually do
sign extension to 64bit value and then compare to register r1.

But the code itself does not imply the above semantics. It looks
like the comparision is between r1 and 0xdeadbeef. For example,
let us at a simple C code:
  $ cat t1.c
  int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
  $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
    ...
    w0 = 0x2
    r2 = 0xdeadbeef ll
    if r1 == r2 goto +0x1
    w0 = 0x3
    exit
It does try to compare r1 and 0xdeadbeef.

To address the above confusing inline asm issue, llvm backend ([2])
added some range checking for such insns and beyond. For the above
insn asm, the warning like below
  warning: immediate out of range, shall fit in int range
will be issued. If -Werror is in the compilation flags, the
error will be issued.

To avoid the above warning/error, the afore-mentioned inline asm
should be rewritten to

  if r1 == -559038737 goto +2;
  ...

Fix a few selftest cases like the above based on insn range checking
requirement in [2].

  [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
  [2] https://github.com/llvm/llvm-project/pull/142989

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../testing/selftests/bpf/progs/dummy_st_ops_success.c |  2 +-
 tools/testing/selftests/bpf/progs/dynptr_fail.c        |  2 +-
 tools/testing/selftests/bpf/progs/iters.c              |  2 +-
 tools/testing/selftests/bpf/progs/verifier_and.c       |  2 +-
 tools/testing/selftests/bpf/progs/verifier_bounds.c    |  4 ++--
 .../bpf/progs/verifier_direct_packet_access.c          |  8 ++++----
 tools/testing/selftests/bpf/progs/verifier_ldsx.c      |  2 +-
 tools/testing/selftests/bpf/progs/verifier_masking.c   |  4 ++--
 tools/testing/selftests/bpf/progs/verifier_movsx.c     |  2 +-
 .../testing/selftests/bpf/progs/verifier_or_jmp32_k.c  |  2 +-
 tools/testing/selftests/bpf/progs/verifier_raw_stack.c |  4 ++--
 .../selftests/bpf/progs/verifier_search_pruning.c      |  6 +++---
 .../testing/selftests/bpf/progs/verifier_spill_fill.c  |  6 +++---
 tools/testing/selftests/bpf/progs/verifier_stack_ptr.c | 10 +++++-----
 tools/testing/selftests/bpf/progs/verifier_subreg.c    |  6 +++---
 tools/testing/selftests/bpf/progs/verifier_unpriv.c    |  2 +-
 16 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c b/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
index ec0c595d47af..54b3d599f31a 100644
--- a/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
+++ b/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
@@ -19,7 +19,7 @@ int BPF_PROG(test_1, struct bpf_dummy_ops_state *state)
 	 */
 	asm volatile (
 		"if %[state] != 0 goto +2;"
-		"r0 = 0xf2f3f4f5;"
+		"r0 = -218893067;"
 		"exit;"
 	::[state]"p"(state));
 
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index bd8f15229f5c..7c7dac6bd3c2 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -761,7 +761,7 @@ int dynptr_pruning_type_confusion(struct __sk_buff *ctx)
 		 *(u64 *)(r2 + 0) = r9;			\
 		 r3 = r10;				\
 		 r3 += -24;				\
-		 r9 = 0xeB9FeB9F;			\
+		 r9 = -341840993;			\
 		 *(u64 *)(r10 - 16) = r9;		\
 		 *(u64 *)(r10 - 24) = r9;		\
 		 r9 = 0;				\
diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
index 7dd92a303bf6..e13023fa9593 100644
--- a/tools/testing/selftests/bpf/progs/iters.c
+++ b/tools/testing/selftests/bpf/progs/iters.c
@@ -774,7 +774,7 @@ __naked int delayed_read_mark(void)
 	"3:"
 		"r1 = r7;"
 		"r2 = 8;"
-		"r3 = 0xdeadbeef;"
+		"r3 = -559038737;"
 		"call %[bpf_probe_read_user];"
 		"goto 1b;"
 	"2:"
diff --git a/tools/testing/selftests/bpf/progs/verifier_and.c b/tools/testing/selftests/bpf/progs/verifier_and.c
index 2b4fdca162be..b53b41590b5e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_and.c
+++ b/tools/testing/selftests/bpf/progs/verifier_and.c
@@ -99,7 +99,7 @@ __naked void known_subreg_with_unknown_reg(void)
 	call %[bpf_get_prandom_u32];			\
 	r0 <<= 32;					\
 	r0 += 1;					\
-	r0 &= 0xFFFF1234;				\
+	r0 &= -60876;					\
 	/* Upper bits are unknown but AND above masks out 1 zero'ing lower bits */\
 	if w0 < 1 goto l0_%=;				\
 	r1 = *(u32*)(r1 + 512);				\
diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 30e16153fdf1..4a174f182768 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -152,7 +152,7 @@ __naked void on_sign_extended_mov_test1(void)
 	call %[bpf_map_lookup_elem];			\
 	if r0 == 0 goto l0_%=;				\
 	/* r2 = 0xffff'ffff'ffff'ffff */		\
-	r2 = 0xffffffff;				\
+	r2 = -1;					\
 	/* r2 = 0xffff'ffff */				\
 	r2 >>= 32;					\
 	/* r0 = <oob pointer> */			\
@@ -183,7 +183,7 @@ __naked void on_sign_extended_mov_test2(void)
 	call %[bpf_map_lookup_elem];			\
 	if r0 == 0 goto l0_%=;				\
 	/* r2 = 0xffff'ffff'ffff'ffff */		\
-	r2 = 0xffffffff;				\
+	r2 = -1;					\
 	/* r2 = 0xfff'ffff */				\
 	r2 >>= 36;					\
 	/* r0 = <oob pointer> */			\
diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
index 28b602ac9cbe..1213b495a0e4 100644
--- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
+++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
@@ -485,7 +485,7 @@ __naked void test20_x_pkt_ptr_1(void)
 	asm volatile ("					\
 	r2 = *(u32*)(r1 + %[__sk_buff_data]);		\
 	r3 = *(u32*)(r1 + %[__sk_buff_data_end]);	\
-	r0 = 0xffffffff;				\
+	r0 = -1;					\
 	*(u64*)(r10 - 8) = r0;				\
 	r0 = *(u64*)(r10 - 8);				\
 	r0 &= 0x7fff;					\
@@ -515,7 +515,7 @@ __naked void test21_x_pkt_ptr_2(void)
 	r0 = r2;					\
 	r0 += 8;					\
 	if r0 > r3 goto l0_%=;				\
-	r4 = 0xffffffff;				\
+	r4 = -1;					\
 	*(u64*)(r10 - 8) = r4;				\
 	r4 = *(u64*)(r10 - 8);				\
 	r4 &= 0x7fff;					\
@@ -548,7 +548,7 @@ __naked void test22_x_pkt_ptr_3(void)
 	r3 = *(u64*)(r10 - 16);				\
 	if r0 > r3 goto l0_%=;				\
 	r2 = *(u64*)(r10 - 8);				\
-	r4 = 0xffffffff;				\
+	r4 = -1;					\
 	lock *(u64 *)(r10 - 8) += r4;			\
 	r4 = *(u64*)(r10 - 8);				\
 	r4 >>= 49;					\
@@ -605,7 +605,7 @@ __naked void test24_x_pkt_ptr_5(void)
 	asm volatile ("					\
 	r2 = *(u32*)(r1 + %[__sk_buff_data]);		\
 	r3 = *(u32*)(r1 + %[__sk_buff_data_end]);	\
-	r0 = 0xffffffff;				\
+	r0 = -1;					\
 	*(u64*)(r10 - 8) = r0;				\
 	r0 = *(u64*)(r10 - 8);				\
 	r0 &= 0xff;					\
diff --git a/tools/testing/selftests/bpf/progs/verifier_ldsx.c b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
index 52edee41caf6..5fa84834cba0 100644
--- a/tools/testing/selftests/bpf/progs/verifier_ldsx.c
+++ b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
@@ -50,7 +50,7 @@ __success __success_unpriv __retval(-1)
 __naked void ldsx_s32(void)
 {
 	asm volatile (
-	"r1 = 0xfffffffe;"
+	"r1 = -2;"
 	"*(u64 *)(r10 - 8) = r1;"
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
 	"r0 = *(s32 *)(r10 - 8);"
diff --git a/tools/testing/selftests/bpf/progs/verifier_masking.c b/tools/testing/selftests/bpf/progs/verifier_masking.c
index 5732cc1b4c47..7581cd61241e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_masking.c
+++ b/tools/testing/selftests/bpf/progs/verifier_masking.c
@@ -171,7 +171,7 @@ __success __success_unpriv __retval(0)
 __naked void test_out_of_bounds_9(void)
 {
 	asm volatile ("					\
-	r1 = 0xffffffff;				\
+	r1 = -1;					\
 	w2 = %[__imm_0];				\
 	r2 -= r1;					\
 	r2 |= r1;					\
@@ -191,7 +191,7 @@ __success __success_unpriv __retval(0)
 __naked void test_out_of_bounds_10(void)
 {
 	asm volatile ("					\
-	r1 = 0xffffffff;				\
+	r1 = -1;					\
 	w2 = %[__imm_0];				\
 	r2 -= r1;					\
 	r2 |= r1;					\
diff --git a/tools/testing/selftests/bpf/progs/verifier_movsx.c b/tools/testing/selftests/bpf/progs/verifier_movsx.c
index a4d8814eb5ed..93b513ab7007 100644
--- a/tools/testing/selftests/bpf/progs/verifier_movsx.c
+++ b/tools/testing/selftests/bpf/progs/verifier_movsx.c
@@ -64,7 +64,7 @@ __success __success_unpriv __retval(-1)
 __naked void mov64sx_s32(void)
 {
 	asm volatile ("					\
-	r0 = 0xfffffffe;				\
+	r0 = -2;					\
 	r0 = (s32)r0;					\
 	r0 >>= 1;					\
 	exit;						\
diff --git a/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c b/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
index f37713a265ac..bee5363c1c08 100644
--- a/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
+++ b/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
@@ -11,7 +11,7 @@ __msg("R0 invalid mem access 'scalar'")
 __naked void or_jmp32_k(void)
 {
 	asm volatile ("					\
-	r0 = 0xffffffff;				\
+	r0 = -1;					\
 	r0 /= 1;					\
 	r1 = 0;						\
 	w1 = -1;					\
diff --git a/tools/testing/selftests/bpf/progs/verifier_raw_stack.c b/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
index c689665e07b9..0219d5890d7c 100644
--- a/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
@@ -280,9 +280,9 @@ __naked void load_bytes_invalid_access_3(void)
 	asm volatile ("					\
 	r2 = 4;						\
 	r6 = r10;					\
-	r6 += 0xffffffff;				\
+	r6 += -1;					\
 	r3 = r6;					\
-	r4 = 0xffffffff;				\
+	r4 = -1;					\
 	call %[bpf_skb_load_bytes];			\
 	r0 = *(u64*)(r6 + 0);				\
 	exit;						\
diff --git a/tools/testing/selftests/bpf/progs/verifier_search_pruning.c b/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
index f40e57251e94..def73e133930 100644
--- a/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
+++ b/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
@@ -253,14 +253,14 @@ l0_%=:	w3 /= 0;					\
 	*(u32*)(r10 - 8) = r7;				\
 	r8 = *(u64*)(r10 - 8);				\
 	/* if r8 != X goto pc+1  r8 known in fallthrough branch */\
-	if r8 != 0xffffffff goto l1_%=;			\
+	if r8 != -1 goto l1_%=;				\
 	r3 = 1;						\
 l1_%=:	/* if r8 == X goto pc+1  condition always true on first\
 	 * traversal, so starts backtracking to mark r8 as requiring\
 	 * precision. r7 marked as needing precision. r6 not marked\
 	 * since it's not tracked.			\
 	 */						\
-	if r8 == 0xffffffff goto l2_%=;			\
+	if r8 == -1 goto l2_%=;				\
 	/* fails if r8 correctly marked unknown after fill. */\
 	w3 /= 0;					\
 l2_%=:	r0 = 0;						\
@@ -324,7 +324,7 @@ __naked void and_register_parent_chain_bug(void)
 	/* if r0 > r6 goto +1 */			\
 	if r0 > r6 goto l0_%=;				\
 	/* *(u64 *)(r10 - 8) = 0xdeadbeef */		\
-	r0 = 0xdeadbeef;				\
+	r0 = -559038737;				\
 	*(u64*)(r10 - 8) = r0;				\
 l0_%=:	r1 = 42;					\
 	*(u8*)(r10 - 8) = r1;				\
diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
index 1e5a511e8494..78acd6360267 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
@@ -393,7 +393,7 @@ __naked void spill_32bit_of_64bit_fail(void)
 	call %[bpf_get_prandom_u32];			\
 	r0 &= 0x8;					\
 	/* Put a large number into r1. */		\
-	r1 = 0xffffffff;				\
+	r1 = -1;					\
 	r1 <<= 32;					\
 	r1 += r0;					\
 	/* Assign an ID to r1. */			\
@@ -827,7 +827,7 @@ __naked void spill_64bit_of_64bit_ok(void)
 	asm volatile ("					\
 	/* Roll one bit to make the register inexact. */\
 	call %[bpf_get_prandom_u32];			\
-	r0 &= 0x80000000;				\
+	r0 &= -2147483648;				\
 	r0 <<= 32;					\
 	/* 64-bit spill r0 to stack - should assign an ID. */\
 	*(u64*)(r10 - 8) = r0;				\
@@ -1057,7 +1057,7 @@ __naked void fill_32bit_after_spill_64bit_clear_id(void)
 	call %[bpf_get_prandom_u32];			\
 	r0 &= 0x8;					\
 	/* Put a large number into r1. */		\
-	r1 = 0xffffffff;				\
+	r1 = -1;					\
 	r1 <<= 32;					\
 	r1 += r0;					\
 	/* 64-bit spill r1 to stack - should assign an ID. */\
diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
index 24aabc6083fd..1d05d5fe04bc 100644
--- a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
+++ b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
@@ -28,7 +28,7 @@ __naked void ptr_to_stack_store_load(void)
 	asm volatile ("					\
 	r1 = r10;					\
 	r1 += -10;					\
-	r0 = 0xfaceb00c;				\
+	r0 = -87117812;					\
 	*(u64*)(r1 + 2) = r0;				\
 	r0 = *(u64*)(r1 + 2);				\
 	exit;						\
@@ -44,7 +44,7 @@ __naked void load_bad_alignment_on_off(void)
 	asm volatile ("					\
 	r1 = r10;					\
 	r1 += -8;					\
-	r0 = 0xfaceb00c;				\
+	r0 = -87117812;					\
 	*(u64*)(r1 + 2) = r0;				\
 	r0 = *(u64*)(r1 + 2);				\
 	exit;						\
@@ -60,7 +60,7 @@ __naked void load_bad_alignment_on_reg(void)
 	asm volatile ("					\
 	r1 = r10;					\
 	r1 += -10;					\
-	r0 = 0xfaceb00c;				\
+	r0 = -87117812;					\
 	*(u64*)(r1 + 8) = r0;				\
 	r0 = *(u64*)(r1 + 8);				\
 	exit;						\
@@ -76,7 +76,7 @@ __naked void load_out_of_bounds_low(void)
 	asm volatile ("					\
 	r1 = r10;					\
 	r1 += -80000;					\
-	r0 = 0xfaceb00c;				\
+	r0 = -87117812;					\
 	*(u64*)(r1 + 8) = r0;				\
 	r0 = *(u64*)(r1 + 8);				\
 	exit;						\
@@ -92,7 +92,7 @@ __naked void load_out_of_bounds_high(void)
 	asm volatile ("					\
 	r1 = r10;					\
 	r1 += -8;					\
-	r0 = 0xfaceb00c;				\
+	r0 = -87117812;					\
 	*(u64*)(r1 + 8) = r0;				\
 	r0 = *(u64*)(r1 + 8);				\
 	exit;						\
diff --git a/tools/testing/selftests/bpf/progs/verifier_subreg.c b/tools/testing/selftests/bpf/progs/verifier_subreg.c
index 8613ea160dcd..23584d5880a4 100644
--- a/tools/testing/selftests/bpf/progs/verifier_subreg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subreg.c
@@ -615,7 +615,7 @@ __naked void ldx_b_zero_extend_check(void)
 	asm volatile ("					\
 	r6 = r10;					\
 	r6 += -4;					\
-	r7 = 0xfaceb00c;				\
+	r7 = -87117812;					\
 	*(u32*)(r6 + 0) = r7;				\
 	call %[bpf_get_prandom_u32];			\
 	r1 = 0x1000000000 ll;				\
@@ -636,7 +636,7 @@ __naked void ldx_h_zero_extend_check(void)
 	asm volatile ("					\
 	r6 = r10;					\
 	r6 += -4;					\
-	r7 = 0xfaceb00c;				\
+	r7 = -87117812;					\
 	*(u32*)(r6 + 0) = r7;				\
 	call %[bpf_get_prandom_u32];			\
 	r1 = 0x1000000000 ll;				\
@@ -657,7 +657,7 @@ __naked void ldx_w_zero_extend_check(void)
 	asm volatile ("					\
 	r6 = r10;					\
 	r6 += -4;					\
-	r7 = 0xfaceb00c;				\
+	r7 = -87117812;					\
 	*(u32*)(r6 + 0) = r7;				\
 	call %[bpf_get_prandom_u32];			\
 	r1 = 0x1000000000 ll;				\
diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
index 4470541b5e71..cfe7c013ec2b 100644
--- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
+++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
@@ -776,7 +776,7 @@ __naked void unpriv_spec_v1_type_confusion(void)
 	r6 = r10;					\
 	r6 += -8;					\
 	/* r6: pointer to readable stack slot */	\
-	r9 = 0xffffc900;				\
+	r9 = -14080;					\
 	r9 <<= 32;					\
 	/* r9: scalar controlled by attacker */		\
 	r0 = *(u64 *)(r0 + 0); /* cache miss */		\
-- 
2.47.1

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Jose E. Marchesi]

> In one of upstream thread ([1]), there is a discussion about
> the below inline asm code:
>
>   if r1 == 0xdeadbeef goto +2;
>   ...
>
> In actual llvm backend, the above 0xdeadbeef will actually do
> sign extension to 64bit value and then compare to register r1.
>
> But the code itself does not imply the above semantics. It looks
> like the comparision is between r1 and 0xdeadbeef. For example,
> let us at a simple C code:
>   $ cat t1.c
>   int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
>   $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
>     ...
>     w0 = 0x2
>     r2 = 0xdeadbeef ll
>     if r1 == r2 goto +0x1
>     w0 = 0x3
>     exit
> It does try to compare r1 and 0xdeadbeef.
>
> To address the above confusing inline asm issue, llvm backend ([2])
> added some range checking for such insns and beyond. For the above
> insn asm, the warning like below
>   warning: immediate out of range, shall fit in int range
> will be issued. If -Werror is in the compilation flags, the
> error will be issued.

I believe in GAS we used to do exactly that, to error out in case of
overflow, then we changed it to match current llvm's behavior after some
discussion at https://lore.kernel.org/bpf/877cpwgzgh.fsf@oracle.com.

commit 5be1b787276d2adbe85ae7febc709ca517b62f08
Author: Jose E. Marchesi <jose.marchesi@oracle.com>
Date:   Thu Aug 17 09:38:37 2023 +0200

    bpf: gas: consolidate handling of immediate overflows
    
    This commit changes the BPF GAS port in order to handle immediate
    overflows the same way than the clang BPF assembler:
    
    - For an immediate field of N bits, any written number (positive or
      negative) whose two's complement encoding fit in N its is accepted.
      This means that -2 is the same than 0xffffffe.  It is up to the
      instructions to decide how to interpret the encoded value.
    
    - Immediate fields in jump instructions are no longer relaxed.
      Relaxing to jump instructions with wider range is only performed
      when expressions are involved.
    
    - The manual is updated to document this, and testsuite adapted
      accordingly.
    
    Tested in x86_64-linux-gnu host, bpf-unknown-none target.


>
> To avoid the above warning/error, the afore-mentioned inline asm
> should be rewritten to
>
>   if r1 == -559038737 goto +2;
>   ...
>
> Fix a few selftest cases like the above based on insn range checking
> requirement in [2].
>
>   [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
>   [2] https://github.com/llvm/llvm-project/pull/142989
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  .../testing/selftests/bpf/progs/dummy_st_ops_success.c |  2 +-
>  tools/testing/selftests/bpf/progs/dynptr_fail.c        |  2 +-
>  tools/testing/selftests/bpf/progs/iters.c              |  2 +-
>  tools/testing/selftests/bpf/progs/verifier_and.c       |  2 +-
>  tools/testing/selftests/bpf/progs/verifier_bounds.c    |  4 ++--
>  .../bpf/progs/verifier_direct_packet_access.c          |  8 ++++----
>  tools/testing/selftests/bpf/progs/verifier_ldsx.c      |  2 +-
>  tools/testing/selftests/bpf/progs/verifier_masking.c   |  4 ++--
>  tools/testing/selftests/bpf/progs/verifier_movsx.c     |  2 +-
>  .../testing/selftests/bpf/progs/verifier_or_jmp32_k.c  |  2 +-
>  tools/testing/selftests/bpf/progs/verifier_raw_stack.c |  4 ++--
>  .../selftests/bpf/progs/verifier_search_pruning.c      |  6 +++---
>  .../testing/selftests/bpf/progs/verifier_spill_fill.c  |  6 +++---
>  tools/testing/selftests/bpf/progs/verifier_stack_ptr.c | 10 +++++-----
>  tools/testing/selftests/bpf/progs/verifier_subreg.c    |  6 +++---
>  tools/testing/selftests/bpf/progs/verifier_unpriv.c    |  2 +-
>  16 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c b/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
> index ec0c595d47af..54b3d599f31a 100644
> --- a/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
> +++ b/tools/testing/selftests/bpf/progs/dummy_st_ops_success.c
> @@ -19,7 +19,7 @@ int BPF_PROG(test_1, struct bpf_dummy_ops_state *state)
>  	 */
>  	asm volatile (
>  		"if %[state] != 0 goto +2;"
> -		"r0 = 0xf2f3f4f5;"
> +		"r0 = -218893067;"
>  		"exit;"
>  	::[state]"p"(state));
>  
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> index bd8f15229f5c..7c7dac6bd3c2 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> @@ -761,7 +761,7 @@ int dynptr_pruning_type_confusion(struct __sk_buff *ctx)
>  		 *(u64 *)(r2 + 0) = r9;			\
>  		 r3 = r10;				\
>  		 r3 += -24;				\
> -		 r9 = 0xeB9FeB9F;			\
> +		 r9 = -341840993;			\
>  		 *(u64 *)(r10 - 16) = r9;		\
>  		 *(u64 *)(r10 - 24) = r9;		\
>  		 r9 = 0;				\
> diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
> index 7dd92a303bf6..e13023fa9593 100644
> --- a/tools/testing/selftests/bpf/progs/iters.c
> +++ b/tools/testing/selftests/bpf/progs/iters.c
> @@ -774,7 +774,7 @@ __naked int delayed_read_mark(void)
>  	"3:"
>  		"r1 = r7;"
>  		"r2 = 8;"
> -		"r3 = 0xdeadbeef;"
> +		"r3 = -559038737;"
>  		"call %[bpf_probe_read_user];"
>  		"goto 1b;"
>  	"2:"
> diff --git a/tools/testing/selftests/bpf/progs/verifier_and.c b/tools/testing/selftests/bpf/progs/verifier_and.c
> index 2b4fdca162be..b53b41590b5e 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_and.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_and.c
> @@ -99,7 +99,7 @@ __naked void known_subreg_with_unknown_reg(void)
>  	call %[bpf_get_prandom_u32];			\
>  	r0 <<= 32;					\
>  	r0 += 1;					\
> -	r0 &= 0xFFFF1234;				\
> +	r0 &= -60876;					\
>  	/* Upper bits are unknown but AND above masks out 1 zero'ing lower bits */\
>  	if w0 < 1 goto l0_%=;				\
>  	r1 = *(u32*)(r1 + 512);				\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> index 30e16153fdf1..4a174f182768 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> @@ -152,7 +152,7 @@ __naked void on_sign_extended_mov_test1(void)
>  	call %[bpf_map_lookup_elem];			\
>  	if r0 == 0 goto l0_%=;				\
>  	/* r2 = 0xffff'ffff'ffff'ffff */		\
> -	r2 = 0xffffffff;				\
> +	r2 = -1;					\
>  	/* r2 = 0xffff'ffff */				\
>  	r2 >>= 32;					\
>  	/* r0 = <oob pointer> */			\
> @@ -183,7 +183,7 @@ __naked void on_sign_extended_mov_test2(void)
>  	call %[bpf_map_lookup_elem];			\
>  	if r0 == 0 goto l0_%=;				\
>  	/* r2 = 0xffff'ffff'ffff'ffff */		\
> -	r2 = 0xffffffff;				\
> +	r2 = -1;					\
>  	/* r2 = 0xfff'ffff */				\
>  	r2 >>= 36;					\
>  	/* r0 = <oob pointer> */			\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
> index 28b602ac9cbe..1213b495a0e4 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
> @@ -485,7 +485,7 @@ __naked void test20_x_pkt_ptr_1(void)
>  	asm volatile ("					\
>  	r2 = *(u32*)(r1 + %[__sk_buff_data]);		\
>  	r3 = *(u32*)(r1 + %[__sk_buff_data_end]);	\
> -	r0 = 0xffffffff;				\
> +	r0 = -1;					\
>  	*(u64*)(r10 - 8) = r0;				\
>  	r0 = *(u64*)(r10 - 8);				\
>  	r0 &= 0x7fff;					\
> @@ -515,7 +515,7 @@ __naked void test21_x_pkt_ptr_2(void)
>  	r0 = r2;					\
>  	r0 += 8;					\
>  	if r0 > r3 goto l0_%=;				\
> -	r4 = 0xffffffff;				\
> +	r4 = -1;					\
>  	*(u64*)(r10 - 8) = r4;				\
>  	r4 = *(u64*)(r10 - 8);				\
>  	r4 &= 0x7fff;					\
> @@ -548,7 +548,7 @@ __naked void test22_x_pkt_ptr_3(void)
>  	r3 = *(u64*)(r10 - 16);				\
>  	if r0 > r3 goto l0_%=;				\
>  	r2 = *(u64*)(r10 - 8);				\
> -	r4 = 0xffffffff;				\
> +	r4 = -1;					\
>  	lock *(u64 *)(r10 - 8) += r4;			\
>  	r4 = *(u64*)(r10 - 8);				\
>  	r4 >>= 49;					\
> @@ -605,7 +605,7 @@ __naked void test24_x_pkt_ptr_5(void)
>  	asm volatile ("					\
>  	r2 = *(u32*)(r1 + %[__sk_buff_data]);		\
>  	r3 = *(u32*)(r1 + %[__sk_buff_data_end]);	\
> -	r0 = 0xffffffff;				\
> +	r0 = -1;					\
>  	*(u64*)(r10 - 8) = r0;				\
>  	r0 = *(u64*)(r10 - 8);				\
>  	r0 &= 0xff;					\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_ldsx.c b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> index 52edee41caf6..5fa84834cba0 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
> @@ -50,7 +50,7 @@ __success __success_unpriv __retval(-1)
>  __naked void ldsx_s32(void)
>  {
>  	asm volatile (
> -	"r1 = 0xfffffffe;"
> +	"r1 = -2;"
>  	"*(u64 *)(r10 - 8) = r1;"
>  #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>  	"r0 = *(s32 *)(r10 - 8);"
> diff --git a/tools/testing/selftests/bpf/progs/verifier_masking.c b/tools/testing/selftests/bpf/progs/verifier_masking.c
> index 5732cc1b4c47..7581cd61241e 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_masking.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_masking.c
> @@ -171,7 +171,7 @@ __success __success_unpriv __retval(0)
>  __naked void test_out_of_bounds_9(void)
>  {
>  	asm volatile ("					\
> -	r1 = 0xffffffff;				\
> +	r1 = -1;					\
>  	w2 = %[__imm_0];				\
>  	r2 -= r1;					\
>  	r2 |= r1;					\
> @@ -191,7 +191,7 @@ __success __success_unpriv __retval(0)
>  __naked void test_out_of_bounds_10(void)
>  {
>  	asm volatile ("					\
> -	r1 = 0xffffffff;				\
> +	r1 = -1;					\
>  	w2 = %[__imm_0];				\
>  	r2 -= r1;					\
>  	r2 |= r1;					\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_movsx.c b/tools/testing/selftests/bpf/progs/verifier_movsx.c
> index a4d8814eb5ed..93b513ab7007 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_movsx.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_movsx.c
> @@ -64,7 +64,7 @@ __success __success_unpriv __retval(-1)
>  __naked void mov64sx_s32(void)
>  {
>  	asm volatile ("					\
> -	r0 = 0xfffffffe;				\
> +	r0 = -2;					\
>  	r0 = (s32)r0;					\
>  	r0 >>= 1;					\
>  	exit;						\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c b/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
> index f37713a265ac..bee5363c1c08 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_or_jmp32_k.c
> @@ -11,7 +11,7 @@ __msg("R0 invalid mem access 'scalar'")
>  __naked void or_jmp32_k(void)
>  {
>  	asm volatile ("					\
> -	r0 = 0xffffffff;				\
> +	r0 = -1;					\
>  	r0 /= 1;					\
>  	r1 = 0;						\
>  	w1 = -1;					\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_raw_stack.c b/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
> index c689665e07b9..0219d5890d7c 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_raw_stack.c
> @@ -280,9 +280,9 @@ __naked void load_bytes_invalid_access_3(void)
>  	asm volatile ("					\
>  	r2 = 4;						\
>  	r6 = r10;					\
> -	r6 += 0xffffffff;				\
> +	r6 += -1;					\
>  	r3 = r6;					\
> -	r4 = 0xffffffff;				\
> +	r4 = -1;					\
>  	call %[bpf_skb_load_bytes];			\
>  	r0 = *(u64*)(r6 + 0);				\
>  	exit;						\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_search_pruning.c b/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
> index f40e57251e94..def73e133930 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_search_pruning.c
> @@ -253,14 +253,14 @@ l0_%=:	w3 /= 0;					\
>  	*(u32*)(r10 - 8) = r7;				\
>  	r8 = *(u64*)(r10 - 8);				\
>  	/* if r8 != X goto pc+1  r8 known in fallthrough branch */\
> -	if r8 != 0xffffffff goto l1_%=;			\
> +	if r8 != -1 goto l1_%=;				\
>  	r3 = 1;						\
>  l1_%=:	/* if r8 == X goto pc+1  condition always true on first\
>  	 * traversal, so starts backtracking to mark r8 as requiring\
>  	 * precision. r7 marked as needing precision. r6 not marked\
>  	 * since it's not tracked.			\
>  	 */						\
> -	if r8 == 0xffffffff goto l2_%=;			\
> +	if r8 == -1 goto l2_%=;				\
>  	/* fails if r8 correctly marked unknown after fill. */\
>  	w3 /= 0;					\
>  l2_%=:	r0 = 0;						\
> @@ -324,7 +324,7 @@ __naked void and_register_parent_chain_bug(void)
>  	/* if r0 > r6 goto +1 */			\
>  	if r0 > r6 goto l0_%=;				\
>  	/* *(u64 *)(r10 - 8) = 0xdeadbeef */		\
> -	r0 = 0xdeadbeef;				\
> +	r0 = -559038737;				\
>  	*(u64*)(r10 - 8) = r0;				\
>  l0_%=:	r1 = 42;					\
>  	*(u8*)(r10 - 8) = r1;				\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> index 1e5a511e8494..78acd6360267 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> @@ -393,7 +393,7 @@ __naked void spill_32bit_of_64bit_fail(void)
>  	call %[bpf_get_prandom_u32];			\
>  	r0 &= 0x8;					\
>  	/* Put a large number into r1. */		\
> -	r1 = 0xffffffff;				\
> +	r1 = -1;					\
>  	r1 <<= 32;					\
>  	r1 += r0;					\
>  	/* Assign an ID to r1. */			\
> @@ -827,7 +827,7 @@ __naked void spill_64bit_of_64bit_ok(void)
>  	asm volatile ("					\
>  	/* Roll one bit to make the register inexact. */\
>  	call %[bpf_get_prandom_u32];			\
> -	r0 &= 0x80000000;				\
> +	r0 &= -2147483648;				\
>  	r0 <<= 32;					\
>  	/* 64-bit spill r0 to stack - should assign an ID. */\
>  	*(u64*)(r10 - 8) = r0;				\
> @@ -1057,7 +1057,7 @@ __naked void fill_32bit_after_spill_64bit_clear_id(void)
>  	call %[bpf_get_prandom_u32];			\
>  	r0 &= 0x8;					\
>  	/* Put a large number into r1. */		\
> -	r1 = 0xffffffff;				\
> +	r1 = -1;					\
>  	r1 <<= 32;					\
>  	r1 += r0;					\
>  	/* 64-bit spill r1 to stack - should assign an ID. */\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
> index 24aabc6083fd..1d05d5fe04bc 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_ptr.c
> @@ -28,7 +28,7 @@ __naked void ptr_to_stack_store_load(void)
>  	asm volatile ("					\
>  	r1 = r10;					\
>  	r1 += -10;					\
> -	r0 = 0xfaceb00c;				\
> +	r0 = -87117812;					\
>  	*(u64*)(r1 + 2) = r0;				\
>  	r0 = *(u64*)(r1 + 2);				\
>  	exit;						\
> @@ -44,7 +44,7 @@ __naked void load_bad_alignment_on_off(void)
>  	asm volatile ("					\
>  	r1 = r10;					\
>  	r1 += -8;					\
> -	r0 = 0xfaceb00c;				\
> +	r0 = -87117812;					\
>  	*(u64*)(r1 + 2) = r0;				\
>  	r0 = *(u64*)(r1 + 2);				\
>  	exit;						\
> @@ -60,7 +60,7 @@ __naked void load_bad_alignment_on_reg(void)
>  	asm volatile ("					\
>  	r1 = r10;					\
>  	r1 += -10;					\
> -	r0 = 0xfaceb00c;				\
> +	r0 = -87117812;					\
>  	*(u64*)(r1 + 8) = r0;				\
>  	r0 = *(u64*)(r1 + 8);				\
>  	exit;						\
> @@ -76,7 +76,7 @@ __naked void load_out_of_bounds_low(void)
>  	asm volatile ("					\
>  	r1 = r10;					\
>  	r1 += -80000;					\
> -	r0 = 0xfaceb00c;				\
> +	r0 = -87117812;					\
>  	*(u64*)(r1 + 8) = r0;				\
>  	r0 = *(u64*)(r1 + 8);				\
>  	exit;						\
> @@ -92,7 +92,7 @@ __naked void load_out_of_bounds_high(void)
>  	asm volatile ("					\
>  	r1 = r10;					\
>  	r1 += -8;					\
> -	r0 = 0xfaceb00c;				\
> +	r0 = -87117812;					\
>  	*(u64*)(r1 + 8) = r0;				\
>  	r0 = *(u64*)(r1 + 8);				\
>  	exit;						\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_subreg.c b/tools/testing/selftests/bpf/progs/verifier_subreg.c
> index 8613ea160dcd..23584d5880a4 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_subreg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_subreg.c
> @@ -615,7 +615,7 @@ __naked void ldx_b_zero_extend_check(void)
>  	asm volatile ("					\
>  	r6 = r10;					\
>  	r6 += -4;					\
> -	r7 = 0xfaceb00c;				\
> +	r7 = -87117812;					\
>  	*(u32*)(r6 + 0) = r7;				\
>  	call %[bpf_get_prandom_u32];			\
>  	r1 = 0x1000000000 ll;				\
> @@ -636,7 +636,7 @@ __naked void ldx_h_zero_extend_check(void)
>  	asm volatile ("					\
>  	r6 = r10;					\
>  	r6 += -4;					\
> -	r7 = 0xfaceb00c;				\
> +	r7 = -87117812;					\
>  	*(u32*)(r6 + 0) = r7;				\
>  	call %[bpf_get_prandom_u32];			\
>  	r1 = 0x1000000000 ll;				\
> @@ -657,7 +657,7 @@ __naked void ldx_w_zero_extend_check(void)
>  	asm volatile ("					\
>  	r6 = r10;					\
>  	r6 += -4;					\
> -	r7 = 0xfaceb00c;				\
> +	r7 = -87117812;					\
>  	*(u32*)(r6 + 0) = r7;				\
>  	call %[bpf_get_prandom_u32];			\
>  	r1 = 0x1000000000 ll;				\
> diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> index 4470541b5e71..cfe7c013ec2b 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
> @@ -776,7 +776,7 @@ __naked void unpriv_spec_v1_type_confusion(void)
>  	r6 = r10;					\
>  	r6 += -8;					\
>  	/* r6: pointer to readable stack slot */	\
> -	r9 = 0xffffc900;				\
> +	r9 = -14080;					\
>  	r9 <<= 32;					\
>  	/* r9: scalar controlled by attacker */		\
>  	r0 = *(u64 *)(r0 + 0); /* cache miss */		\

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Eduard Zingerman]

On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
> In one of upstream thread ([1]), there is a discussion about
> the below inline asm code:
> 
>   if r1 == 0xdeadbeef goto +2;
>   ...
> 
> In actual llvm backend, the above 0xdeadbeef will actually do
> sign extension to 64bit value and then compare to register r1.
> 
> But the code itself does not imply the above semantics. It looks
> like the comparision is between r1 and 0xdeadbeef. For example,
> let us at a simple C code:
>   $ cat t1.c
>   int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
>   $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
>     ...
>     w0 = 0x2
>     r2 = 0xdeadbeef ll
>     if r1 == r2 goto +0x1
>     w0 = 0x3
>     exit
> It does try to compare r1 and 0xdeadbeef.
> 
> To address the above confusing inline asm issue, llvm backend ([2])
> added some range checking for such insns and beyond. For the above
> insn asm, the warning like below
>   warning: immediate out of range, shall fit in int range
> will be issued. If -Werror is in the compilation flags, the
> error will be issued.
> 
> To avoid the above warning/error, the afore-mentioned inline asm
> should be rewritten to
> 
>   if r1 == -559038737 goto +2;
>   ...
> 
> Fix a few selftest cases like the above based on insn range checking
> requirement in [2].
> 
>   [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
>   [2] https://github.com/llvm/llvm-project/pull/142989
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
maybe we need to reconsider.

[...]

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Alexei Starovoitov]

On Thu, Jun 12, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
> > In one of upstream thread ([1]), there is a discussion about
> > the below inline asm code:
> >
> >   if r1 == 0xdeadbeef goto +2;
> >   ...
> >
> > In actual llvm backend, the above 0xdeadbeef will actually do
> > sign extension to 64bit value and then compare to register r1.
> >
> > But the code itself does not imply the above semantics. It looks
> > like the comparision is between r1 and 0xdeadbeef. For example,
> > let us at a simple C code:
> >   $ cat t1.c
> >   int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
> >   $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
> >     ...
> >     w0 = 0x2
> >     r2 = 0xdeadbeef ll
> >     if r1 == r2 goto +0x1
> >     w0 = 0x3
> >     exit
> > It does try to compare r1 and 0xdeadbeef.
> >
> > To address the above confusing inline asm issue, llvm backend ([2])
> > added some range checking for such insns and beyond. For the above
> > insn asm, the warning like below
> >   warning: immediate out of range, shall fit in int range
> > will be issued. If -Werror is in the compilation flags, the
> > error will be issued.
> >
> > To avoid the above warning/error, the afore-mentioned inline asm
> > should be rewritten to
> >
> >   if r1 == -559038737 goto +2;
> >   ...
> >
> > Fix a few selftest cases like the above based on insn range checking
> > requirement in [2].
> >
> >   [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
> >   [2] https://github.com/llvm/llvm-project/pull/142989
> >
> > Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> > ---
>
> Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
> but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
> maybe we need to reconsider.

I have to agree.
I didn't expect there to be so many warnings.
I thought it would be good to warn for
r3 = 0xdeadbeef

since r3 will have 0xffffFFFFdeadbeef value after assignment,
but warn on
r0 &= 0xFFFF1234
and replacement with -60876 is taking the warning too far.

Also considering Jose's point.

Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
llvm should probably accept 0xffffFFFFdeadbeef as imm32.
But that is a separate discussion.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Yonghong Song]

On 6/12/25 12:18 PM, Alexei Starovoitov wrote:
> On Thu, Jun 12, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>> On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
>>> In one of upstream thread ([1]), there is a discussion about
>>> the below inline asm code:
>>>
>>>    if r1 == 0xdeadbeef goto +2;
>>>    ...
>>>
>>> In actual llvm backend, the above 0xdeadbeef will actually do
>>> sign extension to 64bit value and then compare to register r1.
>>>
>>> But the code itself does not imply the above semantics. It looks
>>> like the comparision is between r1 and 0xdeadbeef. For example,
>>> let us at a simple C code:
>>>    $ cat t1.c
>>>    int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
>>>    $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
>>>      ...
>>>      w0 = 0x2
>>>      r2 = 0xdeadbeef ll
>>>      if r1 == r2 goto +0x1
>>>      w0 = 0x3
>>>      exit
>>> It does try to compare r1 and 0xdeadbeef.
>>>
>>> To address the above confusing inline asm issue, llvm backend ([2])
>>> added some range checking for such insns and beyond. For the above
>>> insn asm, the warning like below
>>>    warning: immediate out of range, shall fit in int range
>>> will be issued. If -Werror is in the compilation flags, the
>>> error will be issued.
>>>
>>> To avoid the above warning/error, the afore-mentioned inline asm
>>> should be rewritten to
>>>
>>>    if r1 == -559038737 goto +2;
>>>    ...
>>>
>>> Fix a few selftest cases like the above based on insn range checking
>>> requirement in [2].
>>>
>>>    [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
>>>    [2] https://github.com/llvm/llvm-project/pull/142989
>>>
>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>> ---
>> Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
>> but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
>> maybe we need to reconsider.
> I have to agree.
> I didn't expect there to be so many warnings.
> I thought it would be good to warn for
> r3 = 0xdeadbeef
>
> since r3 will have 0xffffFFFFdeadbeef value after assignment,
> but warn on
> r0 &= 0xFFFF1234
> and replacement with -60876 is taking the warning too far.

Agree this -60876 is bad.

>
> Also considering Jose's point.
>
> Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> llvm should probably accept 0xffffFFFFdeadbeef as imm32.

In llvm, the value is represented as an int64, we probably
can just check the upper 32bit must be 0 or 0xffffFFFF.
Otherwise, the value is out of range.

> But that is a separate discussion.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Eduard Zingerman]

On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:

[...]

> > Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> > llvm should probably accept 0xffffFFFFdeadbeef as imm32.
> 
> In llvm, the value is represented as an int64, we probably
> can just check the upper 32bit must be 0 or 0xffffFFFF.
> Otherwise, the value is out of range.

I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
rejecting things like 0x8000FFFFdeadbeef would require changes to the
assembly parser to behave differently for literals of length 8 (signe
extend them) and >8 (zero extend them), which might be surprising in
some other ways.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Alexei Starovoitov]

On Thu, Jun 12, 2025 at 12:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:
>
> [...]
>
> > > Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> > > llvm should probably accept 0xffffFFFFdeadbeef as imm32.
> >
> > In llvm, the value is represented as an int64, we probably
> > can just check the upper 32bit must be 0 or 0xffffFFFF.
> > Otherwise, the value is out of range.
>
> I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
> rejecting things like 0x8000FFFFdeadbeef would require changes to the
> assembly parser to behave differently for literals of length 8 (signe
> extend them) and >8 (zero extend them), which might be surprising in
> some other ways.

Ok. So what's the summary?
No selftest changes needed and we add a check to llvm
to warn when upper 32 bits !=0 and != 0xffffFFFF ?

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Eduard Zingerman]

On Thu, 2025-06-12 at 14:15 -0700, Alexei Starovoitov wrote:
> On Thu, Jun 12, 2025 at 12:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:
> > 
> > [...]
> > 
> > > > Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> > > > llvm should probably accept 0xffffFFFFdeadbeef as imm32.
> > > 
> > > In llvm, the value is represented as an int64, we probably
> > > can just check the upper 32bit must be 0 or 0xffffFFFF.
> > > Otherwise, the value is out of range.
> > 
> > I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
> > rejecting things like 0x8000FFFFdeadbeef would require changes to the
> > assembly parser to behave differently for literals of length 8 (signe
> > extend them) and >8 (zero extend them), which might be surprising in
> > some other ways.
> 
> Ok. So what's the summary?
> No selftest changes needed and we add a check to llvm
> to warn when upper 32 bits !=0 and != 0xffffFFFF ?

I'd say yes.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Yonghong Song]

On 6/12/25 2:15 PM, Alexei Starovoitov wrote:
> On Thu, Jun 12, 2025 at 12:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>> On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:
>>
>> [...]
>>
>>>> Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
>>>> llvm should probably accept 0xffffFFFFdeadbeef as imm32.
>>> In llvm, the value is represented as an int64, we probably
>>> can just check the upper 32bit must be 0 or 0xffffFFFF.
>>> Otherwise, the value is out of range.
>> I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
>> rejecting things like 0x8000FFFFdeadbeef would require changes to the
>> assembly parser to behave differently for literals of length 8 (signe
>> extend them) and >8 (zero extend them), which might be surprising in
>> some other ways.
> Ok. So what's the summary?
> No selftest changes needed and we add a check to llvm
> to warn when upper 32 bits !=0 and != 0xffffFFFF ?

I did a little more checking, I think the value range
in [INT_MIN, UINT_MAX] is what we want. This is also my v1 of
llvm patch.

Support we have 64bit value, 0xffffFFFF00000001,
truncating the top 32bit, it becomes 1 and this value 1
won't be able to sign extension properly to 0xffffFFFF00000001.

But for any 64bit value in [INT_MIN, UINT_MAX],
if truncated to 32bit, it can still do proper sign
extenstion to get the original value.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Alexei Starovoitov]

On Thu, Jun 12, 2025 at 2:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> On 6/12/25 2:15 PM, Alexei Starovoitov wrote:
> > On Thu, Jun 12, 2025 at 12:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >> On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:
> >>
> >> [...]
> >>
> >>>> Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
> >>>> llvm should probably accept 0xffffFFFFdeadbeef as imm32.
> >>> In llvm, the value is represented as an int64, we probably
> >>> can just check the upper 32bit must be 0 or 0xffffFFFF.
> >>> Otherwise, the value is out of range.
> >> I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
> >> rejecting things like 0x8000FFFFdeadbeef would require changes to the
> >> assembly parser to behave differently for literals of length 8 (signe
> >> extend them) and >8 (zero extend them), which might be surprising in
> >> some other ways.
> > Ok. So what's the summary?
> > No selftest changes needed and we add a check to llvm
> > to warn when upper 32 bits !=0 and != 0xffffFFFF ?
>
> I did a little more checking, I think the value range
> in [INT_MIN, UINT_MAX] is what we want. This is also my v1 of
> llvm patch.

and that's buggy because it will reject 0xffffFFFFdeadbeef

> Support we have 64bit value, 0xffffFFFF00000001,
> truncating the top 32bit, it becomes 1 and this value 1
> won't be able to sign extension properly to 0xffffFFFF00000001.

well, yeah, 0xfff.. case should match 31-bit, of course.

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Yonghong Song]

On 6/12/25 2:42 PM, Alexei Starovoitov wrote:
> On Thu, Jun 12, 2025 at 2:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>>
>> On 6/12/25 2:15 PM, Alexei Starovoitov wrote:
>>> On Thu, Jun 12, 2025 at 12:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>>>> On Thu, 2025-06-12 at 12:29 -0700, Yonghong Song wrote:
>>>>
>>>> [...]
>>>>
>>>>>> Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
>>>>>> llvm should probably accept 0xffffFFFFdeadbeef as imm32.
>>>>> In llvm, the value is represented as an int64, we probably
>>>>> can just check the upper 32bit must be 0 or 0xffffFFFF.
>>>>> Otherwise, the value is out of range.
>>>> I agree with Yonghong, supporting things like 0xffffFFFFdeadbeef and
>>>> rejecting things like 0x8000FFFFdeadbeef would require changes to the
>>>> assembly parser to behave differently for literals of length 8 (signe
>>>> extend them) and >8 (zero extend them), which might be surprising in
>>>> some other ways.
>>> Ok. So what's the summary?
>>> No selftest changes needed and we add a check to llvm
>>> to warn when upper 32 bits !=0 and != 0xffffFFFF ?
>> I did a little more checking, I think the value range
>> in [INT_MIN, UINT_MAX] is what we want. This is also my v1 of
>> llvm patch.
> and that's buggy because it will reject 0xffffFFFFdeadbeef
>
>> Support we have 64bit value, 0xffffFFFF00000001,
>> truncating the top 32bit, it becomes 1 and this value 1
>> won't be able to sign extension properly to 0xffffFFFF00000001.
> well, yeah, 0xfff.. case should match 31-bit, of course.

Right. Just uploaded a new llvm patch for this issue:
    https://github.com/llvm/llvm-project/pull/142989

Re: [PATCH bpf-next] selftests/bpf: Fix some incorrect inline asm codes

[Yonghong Song]

On 6/12/25 12:10 PM, Eduard Zingerman wrote:
> On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
>> In one of upstream thread ([1]), there is a discussion about
>> the below inline asm code:
>>
>>    if r1 == 0xdeadbeef goto +2;
>>    ...
>>
>> In actual llvm backend, the above 0xdeadbeef will actually do
>> sign extension to 64bit value and then compare to register r1.
>>
>> But the code itself does not imply the above semantics. It looks
>> like the comparision is between r1 and 0xdeadbeef. For example,
>> let us at a simple C code:
>>    $ cat t1.c
>>    int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
>>    $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
>>      ...
>>      w0 = 0x2
>>      r2 = 0xdeadbeef ll
>>      if r1 == r2 goto +0x1
>>      w0 = 0x3
>>      exit
>> It does try to compare r1 and 0xdeadbeef.
>>
>> To address the above confusing inline asm issue, llvm backend ([2])
>> added some range checking for such insns and beyond. For the above
>> insn asm, the warning like below
>>    warning: immediate out of range, shall fit in int range
>> will be issued. If -Werror is in the compilation flags, the
>> error will be issued.
>>
>> To avoid the above warning/error, the afore-mentioned inline asm
>> should be rewritten to
>>
>>    if r1 == -559038737 goto +2;
>>    ...
>>
>> Fix a few selftest cases like the above based on insn range checking
>> requirement in [2].
>>
>>    [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@linux.dev/
>>    [2] https://github.com/llvm/llvm-project/pull/142989
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
> Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
> but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
> maybe we need to reconsider.

Another option is to generate below in inline asm:
     r2 = 0xffffFFFFffff1234  /* or r2 = 0xffff1234, depending on the actual user expectation */
     if (r1 == r2) ...

>
> [...]