Re: [PATCH v2 bpf-next 6/6] selftests/bpf: Remove invariant violation flags

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

Paul Chaignon <paul.chaignon@gmail.com> writes:

> With the changes to the verifier in previous commits, we're not
> expecting any invariant violations anymore. We should therefore always
> enable BPF_F_TEST_REG_INVARIANTS to fail on invariant violations. Turns
> out that's already the case and we've been explicitly setting this flag
> in selftests when it wasn't necessary. This commit removes those flags
> from selftests, which should hopefully make clearer that it's always
> enabled.
>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
BPF_F_TEST_REG_INVARIANTS is included in testing_prog_flags() from
testing_helpers.c which is used for loading verifier test programs, the
change looks good.
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
>  .../selftests/bpf/progs/verifier_bounds.c     | 24 ++++++-------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> index 818efa08404d..1ebc99d75862 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> @@ -1066,7 +1066,6 @@ l0_%=:	r0 = 0;						\
>  SEC("xdp")
>  __description("bound check with JMP_JSLT for crossing 64-bit signed boundary")
>  __success __retval(0)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void crossing_64_bit_signed_boundary_2(void)
>  {
>  	asm volatile ("					\
> @@ -1148,7 +1147,6 @@ l0_%=:	r0 = 0;						\
>  SEC("xdp")
>  __description("bound check with JMP32_JSLT for crossing 32-bit signed boundary")
>  __success __retval(0)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void crossing_32_bit_signed_boundary_2(void)
>  {
>  	asm volatile ("					\
> @@ -1536,7 +1534,7 @@ __naked void sub32_partial_overflow(void)
>  SEC("socket")
>  __description("dead branch on jset, does not result in invariants violation error")
>  __success __log_level(2)
> -__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__retval(0)
>  __naked void jset_range_analysis(void)
>  {
>  	asm volatile ("			\
> @@ -1572,7 +1570,7 @@ l0_%=:	r0 = 0;				\
>   */
>  SEC("socket")
>  __description("bounds deduction cross sign boundary, negative overlap")
> -__success __log_level(2) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__success __log_level(2)
>  __msg("7: (1f) r0 -= r6 {{.*}} R0=scalar(smin=smin32=-655,smax=smax32=-146,umin=0xfffffffffffffd71,umax=0xffffffffffffff6e,umin32=0xfffffd71,umax32=0xffffff6e,var_off=(0xfffffffffffffc00; 0x3ff))")
>  __retval(0)
>  __naked void bounds_deduct_negative_overlap(void)
> @@ -1616,7 +1614,7 @@ l0_%=:	r0 = 0;				\
>   */
>  SEC("socket")
>  __description("bounds deduction cross sign boundary, positive overlap")
> -__success __log_level(2) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__success __log_level(2)
>  __msg("3: (2d) if r0 > r1 {{.*}} R0=scalar(smin=smin32=0,smax=umax=smax32=umax32=127,var_off=(0x0; 0x7f))")
>  __retval(0)
>  __naked void bounds_deduct_positive_overlap(void)
> @@ -1649,7 +1647,7 @@ l0_%=:	r0 = 0;				\
>   */
>  SEC("socket")
>  __description("bounds deduction cross sign boundary, two overlaps")
> -__failure __flag(BPF_F_TEST_REG_INVARIANTS)
> +__failure
>  __msg("3: (2d) if r0 > r1 {{.*}} R0=scalar(smin=smin32=-128,smax=smax32=127,umax=0xffffffffffffff80)")
>  __msg("frame pointer is read only")
>  __naked void bounds_deduct_two_overlaps(void)
> @@ -1713,7 +1711,7 @@ SEC("socket")
>  __description("conditional jump on same register, branch taken")
>  __not_msg("20: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__retval(0)
>  __naked void condition_jump_on_same_register(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1748,7 +1746,7 @@ SEC("socket")
>  __description("jset on same register, constant value branch taken")
>  __not_msg("7: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__retval(0)
>  __naked void jset_on_same_register_1(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1770,7 +1768,7 @@ SEC("socket")
>  __description("jset on same register, scalar value branch taken")
>  __not_msg("12: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__retval(0) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__retval(0)
>  __naked void jset_on_same_register_2(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1800,7 +1798,6 @@ __description("jset on same register, scalar value unknown branch 1")
>  __msg("3: (b7) r0 = 0 {{.*}} R0=0")
>  __msg("5: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void jset_on_same_register_3(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1822,7 +1819,6 @@ __description("jset on same register, scalar value unknown branch 2")
>  __msg("4: (b7) r0 = 0 {{.*}} R0=0")
>  __msg("6: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void jset_on_same_register_4(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1845,7 +1841,6 @@ __description("jset on same register, scalar value unknown branch 3")
>  __msg("4: (b7) r0 = 0 {{.*}} R0=0")
>  __msg("6: (b7) r0 = 1 {{.*}} R0=1")
>  __success __log_level(2)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void jset_on_same_register_5(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1877,7 +1872,6 @@ SEC("socket")
>  __description("bounds refinement with single-value tnum on umax")
>  __msg("3: (15) if r0 == 0xe0 {{.*}} R0=240")
>  __success __log_level(2)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void bounds_refinement_tnum_umax(void *ctx)
>  {
>  	asm volatile("			\
> @@ -1907,7 +1901,6 @@ SEC("socket")
>  __description("bounds refinement with single-value tnum on umin")
>  __msg("3: (15) if r0 == 0xf0 {{.*}} R0=224")
>  __success __log_level(2)
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void bounds_refinement_tnum_umin(void *ctx)
>  {
>  	asm volatile("			\
> @@ -2002,7 +1995,6 @@ __naked void bounds_refinement_multiple_overlaps(void *ctx)
>  
>  SEC("socket")
>  __success
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void signed_unsigned_intersection32_case1(void *ctx)
>  {
>  	asm volatile("									\
> @@ -2020,7 +2012,6 @@ __naked void signed_unsigned_intersection32_case1(void *ctx)
>  
>  SEC("socket")
>  __success
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void signed_unsigned_intersection32_case2(void *ctx)
>  {
>  	asm volatile("									\
> @@ -2076,7 +2067,6 @@ __naked void refinement_32bounds_not_overwriting_64bounds(void *ctx)
>  SEC("socket")
>  __description("dead branch: tnums give impossible constant if equal")
>  __success
> -__flag(BPF_F_TEST_REG_INVARIANTS)
>  __naked void tnums_equal_impossible_constant(void *ctx)
>  {
>  	asm volatile("										\
> -- 
> 2.43.0