On Mon, 2023-11-27 at 22:15 -0500, Andrei Matei wrote: > On Mon, Nov 27, 2023 at 8:23 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > On Sat, 2023-11-25 at 20:50 -0500, Andrei Matei wrote: > > > This patch adds tests for the previous patch, checking the tracking of > > > the maximum stack size and checking that accesses to uninit stack memory > > > are allowed. > > > > > > They are a separate patch for review purposes; whoever merges them can > > > consider squashing. > > > > > > Signed-off-by: Andrei Matei <andreimatei1@xxxxxxxxx> > > > --- > > > > I think the strategy now is to add new tests using inline assembly, > > e.g. as a part of verifier_* tests in test_progs. > > Thanks, I'll try that. I see in some of the verifier tests that you > have converted to test_progs hints that they were converted > "automatically". I'm curious what tool you've used, if any. I wrote a small tree-sitter based tool for this: https://github.com/eddyz87/verifier-tests-migrator > Do you have any thoughts on how a test could assert the maximum stack > depth? test_verifier has access to the verifier verifier log and > parses it out of there; do you know how I could achieve the same in > test_progs? Could be done like in the patch below (set log level so that stack depth is printed, match stack depth message): diff --git a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c index 069c3f91705c..e85ac95c8dd3 100644 --- a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c +++ b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c @@ -27,7 +27,7 @@ __naked void stack_out_of_bounds(void) SEC("socket") __description("uninitialized stack1") -__success +__success __log_level(4) __msg("stack depth 8") __failure_unpriv __msg_unpriv("invalid indirect read from stack") __naked void uninitialized_stack1(void) { > For the curiosity of someone who doesn't know much about this code > base - how come we moved from test_verifier, which seems a bit more > unit-test-y, do the higher level test_progs? Is it because of the > nicer assembly syntax? Yes, ability to use assembly syntax is the main (sole?) driver. In fact, tests that use annotations from bpf_misc.h and are registered in tools/testing/selftests/bpf/prog_tests/verifier.c provide almost the same functionality as test_verifier: - interface: - select test to run using filter, e.g.: ./test_progs -a 'verifier_basic_stack/uninitialized stack1' ./test_progs -a 'verifier_basic_stack/uninitialized stack1 @unpriv' - see verifier log for the test, e.g.: ./test_progs -vvv -a 'verifier_basic_stack/uninitialized stack1' - test expectations: - use __success / __failure to mark expected test outcome; - use __msg to search for expected messages in the log; - use __log_level to specify log level when program is loaded; - use __flags to enable additional knobs, e.g. BPF_F_TEST_STATE_FREQ; - use __retval if test program has to be executed; - use _unpriv suffix to specify any of the above for when test is executed in unprivileged mode. See comments in tools/testing/selftests/bpf/progs/bpf_misc.h for details. - plus clang generates BTF and CO-RE relocations for us, with test_verifier one has to write these things "by hand" (which is only truly needed in tests for CO-RE/BTF). The only drawback is compilation time, because all test_progs/*.c files depend on all *.bpf.o files. Locally I mitigate this by using ccache: make CC='ccache clang' LLVM=1 -j14 test_progs . I probably need to look at what could be done in the makefile one day. Unfortunately neither test_verifier nor test_progs are true unit tests.
