Re: [PATCH bpf-next] selftests/bpf: validate eliminated global subprog is not freplaceable

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2023-11-30 at 17:30 -0800, Andrii Nakryiko wrote:
> Add selftest that establishes dead code-eliminated valid global subprog
> (global_dead) and makes sure that it's not possible to freplace it, as
> it's effectively not there. This test will fail with unexpected success
> before 2afae08c9dcb ("bpf: Validate global subprogs lazily").
> 
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c b/tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c
[...]
> +void test_global_func_dead_code(void)
> +{
[...]
> +	ASSERT_HAS_SUBSTR(log_buf, "Subprog global_dead doesn't exist", "dead_subprog_missing_msg");

Nit: the log is not printed if verbose tests execution is requested.

[...]

> index a0a5efd1caa1..7f9b21a1c5a7 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> @@ -10,25 +10,31 @@
>  
>  int arr[1];
>  int unkn_idx;
> +const volatile bool call_dead_subprog = false;
>  
> -__noinline long global_bad(void)
> +__noinline long global_bad(int x)
>  {
> -	return arr[unkn_idx]; /* BOOM */
> +	return arr[unkn_idx] + x; /* BOOM */
>  }

Nit/question:
  Why change prototype from (void) to (int) here and elsewhere?
  Does not seem necessary for test logic.

[...]