[PATCH bpf-next v3 0/2] Regular expression support for test output matching

[PATCH bpf-next v3 0/2] Regular expression support for test output matching

[Cupertino Miranda]

Hi everyone,

This version fixes the patches based on Andriis review.
Looking forward to your review.

Regards,
Cupertino

Cupertino Miranda (2):
  selftests/bpf: Support checks against a regular expression.
  selftests/bpf: Match tests against regular expres

 tools/testing/selftests/bpf/progs/bpf_misc.h  |  11 +-
 .../testing/selftests/bpf/progs/dynptr_fail.c |   6 +-
 .../testing/selftests/bpf/progs/rbtree_fail.c |   8 +-
 .../bpf/progs/refcounted_kptr_fail.c          |   4 +-
 .../selftests/bpf/progs/verifier_sock.c       |   4 +-
 tools/testing/selftests/bpf/test_loader.c     | 118 +++++++++++++-----
 6 files changed, 109 insertions(+), 42 deletions(-)

-- 
2.39.2

[PATCH bpf-next v3 1/2] selftests/bpf: Support checks against a regular expression.

[Cupertino Miranda]

Add support for __regex and __regex_unpriv macros to check the test
execution output against a regular expression. This is similar to __msg
and __msg_unpriv, however those expect full text matching.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: jose.marchesi@oracle.com
Cc: david.faust@oracle.com
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
---
 tools/testing/selftests/bpf/progs/bpf_misc.h |  11 +-
 tools/testing/selftests/bpf/test_loader.c    | 119 ++++++++++++++-----
 2 files changed, 100 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
index fb2f5513e29e..c0280bd2f340 100644
--- a/tools/testing/selftests/bpf/progs/bpf_misc.h
+++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
@@ -7,9 +7,9 @@
  *
  * The test_loader sequentially loads each program in a skeleton.
  * Programs could be loaded in privileged and unprivileged modes.
- * - __success, __failure, __msg imply privileged mode;
- * - __success_unpriv, __failure_unpriv, __msg_unpriv imply
- *   unprivileged mode.
+ * - __success, __failure, __msg, __regex imply privileged mode;
+ * - __success_unpriv, __failure_unpriv, __msg_unpriv, __regex_unpriv
+ *   imply unprivileged mode.
  * If combination of privileged and unprivileged attributes is present
  * both modes are used. If none are present privileged mode is implied.
  *
@@ -24,6 +24,9 @@
  *                   Multiple __msg attributes could be specified.
  * __msg_unpriv      Same as __msg but for unprivileged mode.
  *
+ * __regex           Same as __msg, but using a regular expression.
+ * __regex_unpriv    Same as __msg_unpriv but using a regular expression.
+ *
  * __success         Expect program load success in privileged mode.
  * __success_unpriv  Expect program load success in unprivileged mode.
  *
@@ -59,10 +62,12 @@
  * __auxiliary_unpriv  Same, but load program in unprivileged mode.
  */
 #define __msg(msg)		__attribute__((btf_decl_tag("comment:test_expect_msg=" msg)))
+#define __regex(regex)		__attribute__((btf_decl_tag("comment:test_expect_regex=" regex)))
 #define __failure		__attribute__((btf_decl_tag("comment:test_expect_failure")))
 #define __success		__attribute__((btf_decl_tag("comment:test_expect_success")))
 #define __description(desc)	__attribute__((btf_decl_tag("comment:test_description=" desc)))
 #define __msg_unpriv(msg)	__attribute__((btf_decl_tag("comment:test_expect_msg_unpriv=" msg)))
+#define __regex_unpriv(regex)	__attribute__((btf_decl_tag("comment:test_expect_regex_unpriv=" regex)))
 #define __failure_unpriv	__attribute__((btf_decl_tag("comment:test_expect_failure_unpriv")))
 #define __success_unpriv	__attribute__((btf_decl_tag("comment:test_expect_success_unpriv")))
 #define __log_level(lvl)	__attribute__((btf_decl_tag("comment:test_log_level="#lvl)))
diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index 524c38e9cde4..bc79b9f6afc4 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
 #include <linux/capability.h>
 #include <stdlib.h>
+#include <regex.h>
 #include <test_progs.h>
 #include <bpf/btf.h>
 
@@ -17,9 +18,11 @@
 #define TEST_TAG_EXPECT_FAILURE "comment:test_expect_failure"
 #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"
 #define TEST_TAG_EXPECT_MSG_PFX "comment:test_expect_msg="
+#define TEST_TAG_EXPECT_REGEX_PFX "comment:test_expect_regex="
 #define TEST_TAG_EXPECT_FAILURE_UNPRIV "comment:test_expect_failure_unpriv"
 #define TEST_TAG_EXPECT_SUCCESS_UNPRIV "comment:test_expect_success_unpriv"
 #define TEST_TAG_EXPECT_MSG_PFX_UNPRIV "comment:test_expect_msg_unpriv="
+#define TEST_TAG_EXPECT_REGEX_PFX_UNPRIV "comment:test_expect_regex_unpriv="
 #define TEST_TAG_LOG_LEVEL_PFX "comment:test_log_level="
 #define TEST_TAG_PROG_FLAGS_PFX "comment:test_prog_flags="
 #define TEST_TAG_DESCRIPTION_PFX "comment:test_description="
@@ -46,10 +49,16 @@ enum mode {
 	UNPRIV = 2
 };
 
+struct expect_msg {
+	const char *substr; /* substring match */
+	const char *regex_str; /* regex-based match */
+	regex_t regex;
+};
+
 struct test_subspec {
 	char *name;
 	bool expect_failure;
-	const char **expect_msgs;
+	struct expect_msg *expect_msgs;
 	size_t expect_msg_cnt;
 	int retval;
 	bool execute;
@@ -89,6 +98,16 @@ void test_loader_fini(struct test_loader *tester)
 
 static void free_test_spec(struct test_spec *spec)
 {
+	int i;
+
+	/* Deallocate expect_msgs arrays. */
+	for (i = 0; i < spec->priv.expect_msg_cnt; i++)
+		if (spec->priv.expect_msgs && spec->priv.expect_msgs[i].regex_str)
+			regfree(&spec->priv.expect_msgs[i].regex);
+	for (i = 0; i < spec->unpriv.expect_msg_cnt; i++)
+		if (spec->unpriv.expect_msgs && spec->unpriv.expect_msgs[i].regex_str)
+			regfree(&spec->unpriv.expect_msgs[i].regex);
+
 	free(spec->priv.name);
 	free(spec->unpriv.name);
 	free(spec->priv.expect_msgs);
@@ -100,17 +119,38 @@ static void free_test_spec(struct test_spec *spec)
 	spec->unpriv.expect_msgs = NULL;
 }
 
-static int push_msg(const char *msg, struct test_subspec *subspec)
+static int push_msg(const char *substr, const char *regex_str, struct test_subspec *subspec)
 {
 	void *tmp;
+	int regcomp_res;
+	char error_msg[100];
+	struct expect_msg *msg;
 
-	tmp = realloc(subspec->expect_msgs, (1 + subspec->expect_msg_cnt) * sizeof(void *));
+	tmp = realloc(subspec->expect_msgs,
+		      (1 + subspec->expect_msg_cnt) * sizeof(struct expect_msg));
 	if (!tmp) {
 		ASSERT_FAIL("failed to realloc memory for messages\n");
 		return -ENOMEM;
 	}
 	subspec->expect_msgs = tmp;
-	subspec->expect_msgs[subspec->expect_msg_cnt++] = msg;
+	msg = &subspec->expect_msgs[subspec->expect_msg_cnt];
+	subspec->expect_msg_cnt += 1;
+
+	if (substr) {
+		msg->substr = substr;
+		msg->regex_str = NULL;
+	} else {
+		msg->regex_str = regex_str;
+		msg->substr = NULL;
+		regcomp_res = regcomp(&msg->regex, regex_str, REG_EXTENDED|REG_NEWLINE);
+		if (regcomp_res != 0) {
+			regerror(regcomp_res, &msg->regex, error_msg, 100);
+			fprintf(stderr, "Regexp compilation error in '%s': '%s'\n",
+				regex_str, error_msg);
+			ASSERT_FAIL("failed to compile regex\n");
+			return -EINVAL;
+		}
+	}
 
 	return 0;
 }
@@ -233,13 +273,25 @@ static int parse_test_spec(struct test_loader *tester,
 			spec->mode_mask |= UNPRIV;
 		} else if (str_has_pfx(s, TEST_TAG_EXPECT_MSG_PFX)) {
 			msg = s + sizeof(TEST_TAG_EXPECT_MSG_PFX) - 1;
-			err = push_msg(msg, &spec->priv);
+			err = push_msg(msg, NULL, &spec->priv);
 			if (err)
 				goto cleanup;
 			spec->mode_mask |= PRIV;
 		} else if (str_has_pfx(s, TEST_TAG_EXPECT_MSG_PFX_UNPRIV)) {
 			msg = s + sizeof(TEST_TAG_EXPECT_MSG_PFX_UNPRIV) - 1;
-			err = push_msg(msg, &spec->unpriv);
+			err = push_msg(msg, NULL, &spec->unpriv);
+			if (err)
+				goto cleanup;
+			spec->mode_mask |= UNPRIV;
+		} else if (str_has_pfx(s, TEST_TAG_EXPECT_REGEX_PFX)) {
+			msg = s + sizeof(TEST_TAG_EXPECT_REGEX_PFX) - 1;
+			err = push_msg(NULL, msg, &spec->priv);
+			if (err)
+				goto cleanup;
+			spec->mode_mask |= PRIV;
+		} else if (str_has_pfx(s, TEST_TAG_EXPECT_REGEX_PFX_UNPRIV)) {
+			msg = s + sizeof(TEST_TAG_EXPECT_REGEX_PFX_UNPRIV) - 1;
+			err = push_msg(NULL, msg, &spec->unpriv);
 			if (err)
 				goto cleanup;
 			spec->mode_mask |= UNPRIV;
@@ -337,16 +389,11 @@ static int parse_test_spec(struct test_loader *tester,
 		}
 
 		if (!spec->unpriv.expect_msgs) {
-			size_t sz = spec->priv.expect_msg_cnt * sizeof(void *);
+			for (i = 0; i < spec->priv.expect_msg_cnt; i++) {
+				struct expect_msg *msg = &spec->priv.expect_msgs[i];
 
-			spec->unpriv.expect_msgs = malloc(sz);
-			if (!spec->unpriv.expect_msgs) {
-				PRINT_FAIL("failed to allocate memory for unpriv.expect_msgs\n");
-				err = -ENOMEM;
-				goto cleanup;
+				push_msg(msg->substr, msg->regex_str, &spec->unpriv);
 			}
-			memcpy(spec->unpriv.expect_msgs, spec->priv.expect_msgs, sz);
-			spec->unpriv.expect_msg_cnt = spec->priv.expect_msg_cnt;
 		}
 	}
 
@@ -402,27 +449,45 @@ static void validate_case(struct test_loader *tester,
 			  struct bpf_program *prog,
 			  int load_err)
 {
-	int i, j;
+	int i, j, reg_error;
+	char *match;
+	regmatch_t reg_match[1];
 
 	for (i = 0; i < subspec->expect_msg_cnt; i++) {
-		char *match;
-		const char *expect_msg;
-
-		expect_msg = subspec->expect_msgs[i];
+		struct expect_msg *msg = &subspec->expect_msgs[i];
+
+		if (msg->substr) {
+			match = strstr(tester->log_buf + tester->next_match_pos, msg->substr);
+			tester->next_match_pos = match - tester->log_buf + strlen(msg->substr);
+		} else {
+			reg_error = regexec(&msg->regex,
+					    tester->log_buf + tester->next_match_pos,
+					    1, reg_match, 0);
+			if (reg_error == 0) {
+				match = tester->log_buf + tester->next_match_pos
+					+ reg_match[0].rm_so;
+				tester->next_match_pos += reg_match[0].rm_eo;
+			} else
+				match = NULL;
+		}
 
-		match = strstr(tester->log_buf + tester->next_match_pos, expect_msg);
 		if (!ASSERT_OK_PTR(match, "expect_msg")) {
-			/* if we are in verbose mode, we've already emitted log */
 			if (env.verbosity == VERBOSE_NONE)
 				emit_verifier_log(tester->log_buf, true /*force*/);
-			for (j = 0; j < i; j++)
-				fprintf(stderr,
-					"MATCHED  MSG: '%s'\n", subspec->expect_msgs[j]);
-			fprintf(stderr, "EXPECTED MSG: '%s'\n", expect_msg);
+			for (j = 0; j <= i; j++) {
+				const char *header = (j < i) ? "MATCHED" : "EXPECTED";
+
+				msg = &subspec->expect_msgs[j];
+
+				if (msg->substr)
+					fprintf(stderr,
+						"%s  MSG: '%s'\n", header, msg->substr);
+				else
+					fprintf(stderr,
+						"%s  REGEX: '%s'\n", header, msg->regex_str);
+			}
 			return;
 		}
-
-		tester->next_match_pos = match - tester->log_buf + strlen(expect_msg);
 	}
 }
 
-- 
2.39.2

[PATCH bpf-next v3 2/2] selftests/bpf: Match tests against regular expres

[Cupertino Miranda]

This patch changes a few tests to make use of reg
would otherwise fail when compiled with GCC.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: jose.marchesi@oracle.com
Cc: david.faust@oracle.com
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
---
 tools/testing/selftests/bpf/progs/dynptr_fail.c          | 6 +++---
 tools/testing/selftests/bpf/progs/rbtree_fail.c          | 8 ++++----
 tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c | 4 ++--
 tools/testing/selftests/bpf/progs/verifier_sock.c        | 4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index 66a60bfb5867..64cc9d936a13 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -964,7 +964,7 @@ int dynptr_invalidate_slice_reinit(void *ctx)
  * mem_or_null pointers.
  */
 SEC("?raw_tp")
-__failure __msg("R1 type=scalar expected=percpu_ptr_")
+__failure __regex("R[0-9]+ type=scalar expected=percpu_ptr_")
 int dynptr_invalidate_slice_or_null(void *ctx)
 {
 	struct bpf_dynptr ptr;
@@ -982,7 +982,7 @@ int dynptr_invalidate_slice_or_null(void *ctx)
 
 /* Destruction of dynptr should also any slices obtained from it */
 SEC("?raw_tp")
-__failure __msg("R7 invalid mem access 'scalar'")
+__failure __regex("R[0-9]+ invalid mem access 'scalar'")
 int dynptr_invalidate_slice_failure(void *ctx)
 {
 	struct bpf_dynptr ptr1;
@@ -1069,7 +1069,7 @@ int dynptr_read_into_slot(void *ctx)
 
 /* bpf_dynptr_slice()s are read-only and cannot be written to */
 SEC("?tc")
-__failure __msg("R0 cannot write into rdonly_mem")
+__failure __regex("R[0-9]+ cannot write into rdonly_mem")
 int skb_invalid_slice_write(struct __sk_buff *skb)
 {
 	struct bpf_dynptr ptr;
diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c
index 3fecf1c6dfe5..8399304eca72 100644
--- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
+++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
@@ -29,7 +29,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
 }
 
 SEC("?tc")
-__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
+__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
 long rbtree_api_nolock_add(void *ctx)
 {
 	struct node_data *n;
@@ -43,7 +43,7 @@ long rbtree_api_nolock_add(void *ctx)
 }
 
 SEC("?tc")
-__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
+__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
 long rbtree_api_nolock_remove(void *ctx)
 {
 	struct node_data *n;
@@ -61,7 +61,7 @@ long rbtree_api_nolock_remove(void *ctx)
 }
 
 SEC("?tc")
-__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
+__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
 long rbtree_api_nolock_first(void *ctx)
 {
 	bpf_rbtree_first(&groot);
@@ -105,7 +105,7 @@ long rbtree_api_remove_unadded_node(void *ctx)
 }
 
 SEC("?tc")
-__failure __msg("Unreleased reference id=3 alloc_insn=10")
+__failure __regex("Unreleased reference id=3 alloc_insn=[0-9]+")
 long rbtree_api_remove_no_drop(void *ctx)
 {
 	struct bpf_rb_node *res;
diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
index 1553b9c16aa7..f8d4b7cfcd68 100644
--- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
@@ -32,7 +32,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
 }
 
 SEC("?tc")
-__failure __msg("Unreleased reference id=4 alloc_insn=21")
+__failure __regex("Unreleased reference id=4 alloc_insn=[0-9]+")
 long rbtree_refcounted_node_ref_escapes(void *ctx)
 {
 	struct node_acquire *n, *m;
@@ -73,7 +73,7 @@ long refcount_acquire_maybe_null(void *ctx)
 }
 
 SEC("?tc")
-__failure __msg("Unreleased reference id=3 alloc_insn=9")
+__failure __regex("Unreleased reference id=3 alloc_insn=[0-9]+")
 long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx)
 {
 	struct node_acquire *n, *m;
diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
index ee76b51005ab..450b57933c79 100644
--- a/tools/testing/selftests/bpf/progs/verifier_sock.c
+++ b/tools/testing/selftests/bpf/progs/verifier_sock.c
@@ -799,7 +799,7 @@ l0_%=:	r0 = *(u32*)(r0 + %[bpf_xdp_sock_queue_id]);	\
 
 SEC("sk_skb")
 __description("bpf_map_lookup_elem(sockmap, &key)")
-__failure __msg("Unreleased reference id=2 alloc_insn=6")
+__failure __regex("Unreleased reference id=2 alloc_insn=[0-9]+")
 __naked void map_lookup_elem_sockmap_key(void)
 {
 	asm volatile ("					\
@@ -819,7 +819,7 @@ __naked void map_lookup_elem_sockmap_key(void)
 
 SEC("sk_skb")
 __description("bpf_map_lookup_elem(sockhash, &key)")
-__failure __msg("Unreleased reference id=2 alloc_insn=6")
+__failure __regex("Unreleased reference id=2 alloc_insn=[0-9]+")
 __naked void map_lookup_elem_sockhash_key(void)
 {
 	asm volatile ("					\
-- 
2.39.2

Re: [PATCH bpf-next v3 1/2] selftests/bpf: Support checks against a regular expression.

[Eduard Zingerman]

On Tue, 2024-06-11 at 18:40 +0100, Cupertino Miranda wrote:
> Add support for __regex and __regex_unpriv macros to check the test
> execution output against a regular expression. This is similar to __msg
> and __msg_unpriv, however those expect full text matching.
> 
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: jose.marchesi@oracle.com
> Cc: david.faust@oracle.com
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> ---

Overall looks good, could you please fix a few things noted below and respin?
Please add my ack on the respin.

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

[...]

> @@ -89,6 +98,16 @@ void test_loader_fini(struct test_loader *tester)
>  
>  static void free_test_spec(struct test_spec *spec)
>  {
> +	int i;
> +
> +	/* Deallocate expect_msgs arrays. */
> +	for (i = 0; i < spec->priv.expect_msg_cnt; i++)
> +		if (spec->priv.expect_msgs && spec->priv.expect_msgs[i].regex_str)

I don't think situation when spec->priv.expect_msg_cnt > 0 and
spec->priv.expect_msgs == NULL is possible, conditions above and below
could be simplified to just "if (spec->[un]priv.expect_msgs[i].regex_str)"

> +			regfree(&spec->priv.expect_msgs[i].regex);
> +	for (i = 0; i < spec->unpriv.expect_msg_cnt; i++)
> +		if (spec->unpriv.expect_msgs && spec->unpriv.expect_msgs[i].regex_str)
> +			regfree(&spec->unpriv.expect_msgs[i].regex);
> +
>  	free(spec->priv.name);
>  	free(spec->unpriv.name);
>  	free(spec->priv.expect_msgs);
> @@ -100,17 +119,38 @@ static void free_test_spec(struct test_spec *spec)
>  	spec->unpriv.expect_msgs = NULL;
>  }
>  
> -static int push_msg(const char *msg, struct test_subspec *subspec)
> +static int push_msg(const char *substr, const char *regex_str, struct test_subspec *subspec)
>  {
>  	void *tmp;
> +	int regcomp_res;
> +	char error_msg[100];
> +	struct expect_msg *msg;
>  
> -	tmp = realloc(subspec->expect_msgs, (1 + subspec->expect_msg_cnt) * sizeof(void *));
> +	tmp = realloc(subspec->expect_msgs,
> +		      (1 + subspec->expect_msg_cnt) * sizeof(struct expect_msg));
>  	if (!tmp) {
>  		ASSERT_FAIL("failed to realloc memory for messages\n");
>  		return -ENOMEM;
>  	}
>  	subspec->expect_msgs = tmp;
> -	subspec->expect_msgs[subspec->expect_msg_cnt++] = msg;
> +	msg = &subspec->expect_msgs[subspec->expect_msg_cnt];
> +	subspec->expect_msg_cnt += 1;
> +
> +	if (substr) {
> +		msg->substr = substr;
> +		msg->regex_str = NULL;
> +	} else {
> +		msg->regex_str = regex_str;
> +		msg->substr = NULL;
> +		regcomp_res = regcomp(&msg->regex, regex_str, REG_EXTENDED|REG_NEWLINE);
> +		if (regcomp_res != 0) {
> +			regerror(regcomp_res, &msg->regex, error_msg, 100);
                                                                      ^^^^
Nit:                                                      sizeof(error_msg)

> +			fprintf(stderr, "Regexp compilation error in '%s': '%s'\n",
> +				regex_str, error_msg);
> +			ASSERT_FAIL("failed to compile regex\n");

Nit:                    these two calls could be combined as a single PRINT_FAIL().

> +			return -EINVAL;
> +		}
> +	}
>  
>  	return 0;
>  }

[...]

> @@ -337,16 +389,11 @@ static int parse_test_spec(struct test_loader *tester,
>  		}
>  
>  		if (!spec->unpriv.expect_msgs) {
> -			size_t sz = spec->priv.expect_msg_cnt * sizeof(void *);
> +			for (i = 0; i < spec->priv.expect_msg_cnt; i++) {
> +				struct expect_msg *msg = &spec->priv.expect_msgs[i];
>  
> -			spec->unpriv.expect_msgs = malloc(sz);
> -			if (!spec->unpriv.expect_msgs) {
> -				PRINT_FAIL("failed to allocate memory for unpriv.expect_msgs\n");
> -				err = -ENOMEM;
> -				goto cleanup;
> +				push_msg(msg->substr, msg->regex_str, &spec->unpriv);

Need to check push_msg() return value.

>  			}
> -			memcpy(spec->unpriv.expect_msgs, spec->priv.expect_msgs, sz);
> -			spec->unpriv.expect_msg_cnt = spec->priv.expect_msg_cnt;
>  		}
>  	}
>  

[...]

Re: [PATCH bpf-next v3 2/2] selftests/bpf: Match tests against regular expres

[Eduard Zingerman]

On Tue, 2024-06-11 at 18:40 +0100, Cupertino Miranda wrote:
> This patch changes a few tests to make use of reg
> would otherwise fail when compiled with GCC.
> 
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: jose.marchesi@oracle.com
> Cc: david.faust@oracle.com
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> ---

Looks good, but I think that changes for 'off' for three cases below
are not necessary.

[...]

> diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c
> index 3fecf1c6dfe5..8399304eca72 100644
> --- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
> +++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
> @@ -29,7 +29,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
>  }
>  
>  SEC("?tc")
> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")

This error message is reported in a single place in
verifier.c:__process_kf_arg_ptr_to_graph_root():

	if (check_reg_allocation_locked(env, reg)) {
		verbose(env, "bpf_spin_lock at off=%d must be held for %s\n",
			rec->spin_lock_off, head_type_name);
		return -EINVAL;
	}

Where `rec` is a description of the BTF type, `off` is an offset
inside the structure, why do you need to change it to regex?

>  long rbtree_api_nolock_add(void *ctx)
>  {
>  	struct node_data *n;
> @@ -43,7 +43,7 @@ long rbtree_api_nolock_add(void *ctx)
>  }
>  
>  SEC("?tc")
> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>  long rbtree_api_nolock_remove(void *ctx)
>  {
>  	struct node_data *n;
> @@ -61,7 +61,7 @@ long rbtree_api_nolock_remove(void *ctx)
>  }
>  
>  SEC("?tc")
> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>  long rbtree_api_nolock_first(void *ctx)
>  {
>  	bpf_rbtree_first(&groot);

[...]

Re: [PATCH bpf-next v3 2/2] selftests/bpf: Match tests against regular expres

[Cupertino Miranda]

Eduard Zingerman writes:

> On Tue, 2024-06-11 at 18:40 +0100, Cupertino Miranda wrote:
>> This patch changes a few tests to make use of reg
>> would otherwise fail when compiled with GCC.
>>
>> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
>> Cc: jose.marchesi@oracle.com
>> Cc: david.faust@oracle.com
>> Cc: Yonghong Song <yonghong.song@linux.dev>
>> Cc: Eduard Zingerman <eddyz87@gmail.com>
>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>> ---
>
> Looks good, but I think that changes for 'off' for three cases below
> are not necessary.
>
> [...]
>
>> diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> index 3fecf1c6dfe5..8399304eca72 100644
>> --- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> +++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> @@ -29,7 +29,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>
> This error message is reported in a single place in
> verifier.c:__process_kf_arg_ptr_to_graph_root():
>
> 	if (check_reg_allocation_locked(env, reg)) {
> 		verbose(env, "bpf_spin_lock at off=%d must be held for %s\n",
> 			rec->spin_lock_off, head_type_name);
> 		return -EINVAL;
> 	}
>
> Where `rec` is a description of the BTF type, `off` is an offset
> inside the structure, why do you need to change it to regex?
>
In GCC the off value would print something else.
Judging by the message I deduced that off was refering to an instruction
location and so, tight to the compiler.
Now I see the value is rather tight to BTF content.

I will remove the offset patching from the series and later on evaluate
what is happening in GCC for the result difference.

>
>>  long rbtree_api_nolock_add(void *ctx)
>>  {
>>  	struct node_data *n;
>> @@ -43,7 +43,7 @@ long rbtree_api_nolock_add(void *ctx)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>>  long rbtree_api_nolock_remove(void *ctx)
>>  {
>>  	struct node_data *n;
>> @@ -61,7 +61,7 @@ long rbtree_api_nolock_remove(void *ctx)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>>  long rbtree_api_nolock_first(void *ctx)
>>  {
>>  	bpf_rbtree_first(&groot);
>
> [...]