[PATCH 0/2] libtraceevent: Allow %pIS to work with dynamic arrays

[PATCH 0/2] libtraceevent: Allow %pIS to work with dynamic arrays

[Steven Rostedt]

Chuck Lever reported that adding the following:

#define field_sockaddr(field, len)  __dynamic_array(u8, field, len)
#define assign_sockaddr(dest, src, len)  memcpy(__get_dynamic_array(dest), src, len)
#define __get_sockaddr(field)  ((struct sockaddr *)__get_dynamic_array(field))

TRACE_EVENT(sockaddr_example,
        TP_PROTO(
                const struct sockaddr *sap,
                size_t salen
        ),  
        TP_ARGS(sap, salen),
        TP_STRUCT__entry(
                __field_sockaddr(addr, salen)
        ),  
        TP_fast_assign(
                __assign_sockaddr(addr, sap, salen);
        ),  
        TP_printk("addr=%pIS", __get_sockaddr(addr))
);

Causes trace-cmd to report:

  "addr=ARG TYPE NOT FIELD BUT 7"

Which is not only unwanted, but rather unhelpful.

Fix it to allow %pIS to work with dynamic arrays, and also allow all fields to
still work if they are typecasted.

Link: https://lore.kernel.org/all/164182978641.8391.8277203495236105391.stgit@bazille.1015granger.net/

Steven Rostedt (2):
  libtraceevent: Do not fail field parsing if field has typecast
  libtraceevent: Allow ipsa arg to use dynamic arrays

 src/event-parse.c | 70 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 16 deletions(-)

-- 
2.33.0

[PATCH 1/2] libtraceevent: Do not fail field parsing if field has typecast

[Steven Rostedt]

If a field is expected for a printk format but is typecasted, it could
fail the parsing where the error is a "helpful":

  ARG TYPE NOT FIELD BUT 7

Add a check before each of these to see if it is a typecast, and use the
arg that the typecast is for.

Link: https://lore.kernel.org/all/164182978641.8391.8277203495236105391.stgit@bazille.1015granger.net/

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 src/event-parse.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 9bd605d74b1f..5f3e2d8a6421 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -4834,6 +4834,10 @@ static int print_mac_arg(struct trace_seq *s, const char *format,
 		return 0;
 	}
 
+	/* evaluate if the arg has a type cast */
+	while (arg->type == TEP_PRINT_TYPE)
+		arg = arg->typecast.item;
+
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d",
 				 arg->type);
@@ -5042,6 +5046,10 @@ static int print_ipv4_arg(struct trace_seq *s, const char *ptr, char i,
 		return ret;
 	}
 
+	/* evaluate if the arg has a type cast */
+	while (arg->type == TEP_PRINT_TYPE)
+		arg = arg->typecast.item;
+
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
 		return ret;
@@ -5089,6 +5097,10 @@ static int print_ipv6_arg(struct trace_seq *s, const char *ptr, char i,
 		return rc;
 	}
 
+	/* evaluate if the arg has a type cast */
+	while (arg->type == TEP_PRINT_TYPE)
+		arg = arg->typecast.item;
+
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
 		return rc;
@@ -5152,6 +5164,10 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 		return rc;
 	}
 
+	/* evaluate if the arg has a type cast */
+	while (arg->type == TEP_PRINT_TYPE)
+		arg = arg->typecast.item;
+
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
 		return rc;
@@ -5267,6 +5283,10 @@ static int print_uuid_arg(struct trace_seq *s, const char *ptr,
 		return ret;
 	}
 
+	/* evaluate if the arg has a type cast */
+	while (arg->type == TEP_PRINT_TYPE)
+		arg = arg->typecast.item;
+
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
 		return ret;
@@ -5440,6 +5460,7 @@ static inline void print_field(struct trace_seq *s, void *data, int size,
 	struct tep_event *event = field->event;
 	struct tep_print_parse *start_parse;
 	struct tep_print_parse *parse;
+	struct tep_print_arg *arg;
 	bool has_0x;
 
 	parse = parse_ptr ? *parse_ptr : event->print_fmt.print_cache;
@@ -5464,9 +5485,13 @@ static inline void print_field(struct trace_seq *s, void *data, int size,
 			goto next;
 		}
 
-		if (!parse->arg ||
-		    parse->arg->type != TEP_PRINT_FIELD ||
-		    parse->arg->field.field != field) {
+		arg = parse->arg;
+
+		while (arg && arg->type == TEP_PRINT_TYPE)
+			arg = arg->typecast.item;
+
+		if (!arg || arg->type != TEP_PRINT_FIELD ||
+		    arg->field.field != field) {
 			has_0x = false;
 			goto next;
 		}
-- 
2.33.0

[PATCH 2/2] libtraceevent: Allow ipsa arg to use dynamic arrays

[Steven Rostedt]

As the print format of %pIS can handle different types of sockets that may
be of different sizes, a dynamic array may be used to save the socket
instead of a static field.

Allow %pIS to be used against a dynamic array.

Link: https://lore.kernel.org/all/164182978641.8391.8277203495236105391.stgit@bazille.1015granger.net/

Reported-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 src/event-parse.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 5f3e2d8a6421..d3e43e5c11f8 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -5139,6 +5139,8 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 	unsigned char *buf;
 	struct sockaddr_storage *sa;
 	bool reverse = false;
+	unsigned int offset;
+	unsigned int len;
 	int rc = 0;
 	int ret;
 
@@ -5168,27 +5170,38 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 	while (arg->type == TEP_PRINT_TYPE)
 		arg = arg->typecast.item;
 
-	if (arg->type != TEP_PRINT_FIELD) {
-		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
-		return rc;
-	}
+	if (arg->type == TEP_PRINT_FIELD) {
 
-	if (!arg->field.field) {
-		arg->field.field =
-			tep_find_any_field(event, arg->field.name);
 		if (!arg->field.field) {
-			do_warning("%s: field %s not found",
-				   __func__, arg->field.name);
-			return rc;
+			arg->field.field =
+				tep_find_any_field(event, arg->field.name);
+			if (!arg->field.field) {
+				do_warning("%s: field %s not found",
+					   __func__, arg->field.name);
+				return rc;
+			}
 		}
+
+		offset = arg->field.field->offset;
+		len = arg->field.field->size;
+
+	} else if (arg->type == TEP_PRINT_DYNAMIC_ARRAY) {
+
+		dynamic_offset_field(event->tep, arg->dynarray.field, data,
+				     size, &offset, &len);
+
+	} else {
+		trace_seq_printf(s, "ARG NOT FIELD NOR DYNAMIC ARRAY BUT TYPE %d",
+				 arg->type);
+		return rc;
 	}
 
-	sa = (struct sockaddr_storage *) (data + arg->field.field->offset);
+	sa = (struct sockaddr_storage *)(data + offset);
 
 	if (sa->ss_family == AF_INET) {
 		struct sockaddr_in *sa4 = (struct sockaddr_in *) sa;
 
-		if (arg->field.field->size < sizeof(struct sockaddr_in)) {
+		if (len < sizeof(struct sockaddr_in)) {
 			trace_seq_printf(s, "INVALIDIPv4");
 			return rc;
 		}
@@ -5201,7 +5214,7 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 	} else if (sa->ss_family == AF_INET6) {
 		struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *) sa;
 
-		if (arg->field.field->size < sizeof(struct sockaddr_in6)) {
+		if (len < sizeof(struct sockaddr_in6)) {
 			trace_seq_printf(s, "INVALIDIPv6");
 			return rc;
 		}
-- 
2.33.0

Re: [PATCH 0/2] libtraceevent: Allow %pIS to work with dynamic arrays

[Steven Rostedt]

Chuck,

I forgot to add you to the Cc of the series, but you can download it from
here:

  https://patchwork.kernel.org/series/604304/mbox/

-- Steve


On Mon, 10 Jan 2022 19:17:03 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Chuck Lever reported that adding the following:
> 
> #define field_sockaddr(field, len)  __dynamic_array(u8, field, len)
> #define assign_sockaddr(dest, src, len)  memcpy(__get_dynamic_array(dest), src, len)
> #define __get_sockaddr(field)  ((struct sockaddr *)__get_dynamic_array(field))
> 
> TRACE_EVENT(sockaddr_example,
>         TP_PROTO(
>                 const struct sockaddr *sap,
>                 size_t salen
>         ),  
>         TP_ARGS(sap, salen),
>         TP_STRUCT__entry(
>                 __field_sockaddr(addr, salen)
>         ),  
>         TP_fast_assign(
>                 __assign_sockaddr(addr, sap, salen);
>         ),  
>         TP_printk("addr=%pIS", __get_sockaddr(addr))
> );
> 
> Causes trace-cmd to report:
> 
>   "addr=ARG TYPE NOT FIELD BUT 7"
> 
> Which is not only unwanted, but rather unhelpful.
> 
> Fix it to allow %pIS to work with dynamic arrays, and also allow all fields to
> still work if they are typecasted.
> 
> Link: https://lore.kernel.org/all/164182978641.8391.8277203495236105391.stgit@bazille.1015granger.net/
> 
> Steven Rostedt (2):
>   libtraceevent: Do not fail field parsing if field has typecast
>   libtraceevent: Allow ipsa arg to use dynamic arrays
> 
>  src/event-parse.c | 70 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 54 insertions(+), 16 deletions(-)
>