Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1444000 > unrolled thread

[PATCH 1/3] perf script python: Fix string vs byte array resolving

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa <jolsa@kernel.org> - 2016-07-15 09:40 +0200
    Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Jiri Olsa <jolsa@redhat.com> - 2016-07-15 09:40 +0200
    Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Steven Rostedt <rostedt@goodmis.org> - 2016-07-15 17:40 +0200
    Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Steven Rostedt <rostedt@goodmis.org> - 2016-07-15 18:10 +0200
      Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Jiri Olsa <jolsa@redhat.com> - 2016-07-15 18:20 +0200
        Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-07-15 18:40 +0200
        Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-07-15 18:40 +0200
        Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Steven Rostedt <rostedt@goodmis.org> - 2016-07-15 19:20 +0200
    Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Steven Rostedt <rostedt@goodmis.org> - 2016-07-15 19:20 +0200
      Re: [PATCH 1/3] perf script python: Fix string vs byte array  resolving Jiri Olsa <jolsa@redhat.com> - 2016-07-16 18:00 +0200

#1444000 — [PATCH 1/3] perf script python: Fix string vs byte array resolving

Jirka reported that python code returns all arrays as strings.
This makes impossible to get all items for byte array tracepoint
field containing 0x00 value item.

Fixing this by scanning full length of the array and returning
it as PyByteArray object in case non printable byte is found.

Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Cc: Jiri Pirko <jiri@mellanox.com>
Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../util/scripting-engines/trace-event-python.c    | 37 ++++++++++++++++++----
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 6ac6b7a33f42..1bc995de5a6d 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -386,6 +386,19 @@ exit:
 	return pylist;
 }
 
+static int is_printable_array(char *p, unsigned int len)
+{
+	unsigned int i;
+
+	if (p[len - 1] == 0)
+		len--;
+
+	for (i = 0; i < len - 1; i++)
+		if (!isprint(p[i]) && !isspace(p[i]))
+			return 0;
+
+	return 1;
+}
 
 static void python_process_tracepoint(struct perf_sample *sample,
 				      struct perf_evsel *evsel,
@@ -457,14 +470,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
 		pydict_set_item_string_decref(dict, "common_callchain", callchain);
 	}
 	for (field = event->format.fields; field; field = field->next) {
-		if (field->flags & FIELD_IS_STRING) {
-			int offset;
+		unsigned int offset, len;
+		unsigned long long val;
+
+		if (field->flags & FIELD_IS_ARRAY) {
+			offset = field->offset;
+			len    = field->size;
 			if (field->flags & FIELD_IS_DYNAMIC) {
-				offset = *(int *)(data + field->offset);
+				val     = pevent_read_number(scripting_context->pevent,
+							     data + offset, len);
+				offset  = val;
+				len     = offset >> 16;
 				offset &= 0xffff;
-			} else
-				offset = field->offset;
-			obj = PyString_FromString((char *)data + offset);
+			}
+			if (field->flags & FIELD_IS_STRING &&
+			    is_printable_array(data + offset, len)) {
+				obj = PyString_FromString((char *) data + offset);
+			} else {
+				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
+				field->flags &= ~FIELD_IS_STRING;
+			}
 		} else { /* FIELD_IS_NUMERIC */
 			obj = get_field_numeric_entry(event, field, data);
 		}
-- 
2.4.11

[toc] | [next] | [standalone]

#1444004 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, Jul 15, 2016 at 09:29:55AM +0200, Jiri Olsa wrote:
> Jirka reported that python code returns all arrays as strings.
> This makes impossible to get all items for byte array tracepoint
> field containing 0x00 value item.
> 
> Fixing this by scanning full length of the array and returning
> it as PyByteArray object in case non printable byte is found.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> Cc: Jiri Pirko <jiri@mellanox.com>

forgot:

Reported-by: Jiri Pirko <jiri@mellanox.com>

Jirka,
could I please also have your tested-by on this version?

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1444389 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, 15 Jul 2016 09:29:55 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> Jirka reported that python code returns all arrays as strings.
> This makes impossible to get all items for byte array tracepoint
> field containing 0x00 value item.
> 
> Fixing this by scanning full length of the array and returning
> it as PyByteArray object in case non printable byte is found.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

[toc] | [prev] | [next] | [standalone]

#1444408 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, 15 Jul 2016 17:51:10 +0200
Jiri Pirko <jiri@mellanox.com> wrote:

> Fri, Jul 15, 2016 at 09:29:55AM CEST, jolsa@kernel.org wrote:
> >Jirka reported that python code returns all arrays as strings.
> >This makes impossible to get all items for byte array tracepoint
> >field containing 0x00 value item.
> >
> >Fixing this by scanning full length of the array and returning
> >it as PyByteArray object in case non printable byte is found.
> >
> >Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
> >Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >---
> > .../util/scripting-engines/trace-event-python.c    | 37 ++++++++++++++++++----
> > 1 file changed, 31 insertions(+), 6 deletions(-)
> >
> >diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> >index 6ac6b7a33f42..1bc995de5a6d 100644
> >--- a/tools/perf/util/scripting-engines/trace-event-python.c
> >+++ b/tools/perf/util/scripting-engines/trace-event-python.c
> >@@ -386,6 +386,19 @@ exit:
> > 	return pylist;
> > }
> > 
> >+static int is_printable_array(char *p, unsigned int len)
> >+{
> >+	unsigned int i;
> >+
> >+	if (p[len - 1] == 0)
> >+		len--;
> >+
> >+	for (i = 0; i < len - 1; i++)
> >+		if (!isprint(p[i]) && !isspace(p[i]))
> >+			return 0;  
> 
> 
> for "AA\1\0" this returns "1" although that should return "0".
> 
> orig len 4
> decremented len 3
> for:
> 0 1
> 
> index 2 would not be inspected. Or am I missing something?
> 
> I think that the for check should be "i < len"

Yes it should be. I think we got the two solutions mixed up.

With the above len--, it should be i < len, but when we did the check
for zero at the end, we needed the i < len - 1

-- Steve

[toc] | [prev] | [next] | [standalone]

#1444415 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:

SNIP

> > for "AA\1\0" this returns "1" although that should return "0".
> > 
> > orig len 4
> > decremented len 3
> > for:
> > 0 1
> > 
> > index 2 would not be inspected. Or am I missing something?
> > 
> > I think that the for check should be "i < len"
> 
> Yes it should be. I think we got the two solutions mixed up.
> 
> With the above len--, it should be i < len, but when we did the check
> for zero at the end, we needed the i < len - 1

ugh right.. should be 'i < len' check in the for loop,

there's also the patch 2/3 that needs to be changed

Arnaldo,
please let me know if you need me to resend this.

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1444435 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

Em Fri, Jul 15, 2016 at 01:37:31PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 15, 2016 at 06:13:10PM +0200, Jiri Olsa escreveu:
> > On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> > 
> > SNIP
> > 
> > > > for "AA\1\0" this returns "1" although that should return "0".
> > > > 
> > > > orig len 4
> > > > decremented len 3
> > > > for:
> > > > 0 1
> > > > 
> > > > index 2 would not be inspected. Or am I missing something?
> > > > 
> > > > I think that the for check should be "i < len"
> > > 
> > > Yes it should be. I think we got the two solutions mixed up.
> > > 
> > > With the above len--, it should be i < len, but when we did the check
> > > for zero at the end, we needed the i < len - 1
> > 
> > ugh right.. should be 'i < len' check in the for loop,
> > 
> > there's also the patch 2/3 that needs to be changed
> > 
> > Arnaldo,
> > please let me know if you need me to resend this.
> 
> So I need to drop those, even with Rostedt's acked-by? Ok, please
> resend, hopefully this time with a Tested-by from the reporter?

I removed the first two, as the second needs context from the first,
kept the third one, so please resend just the first two.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1444438 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

Em Fri, Jul 15, 2016 at 06:13:10PM +0200, Jiri Olsa escreveu:
> On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> 
> SNIP
> 
> > > for "AA\1\0" this returns "1" although that should return "0".
> > > 
> > > orig len 4
> > > decremented len 3
> > > for:
> > > 0 1
> > > 
> > > index 2 would not be inspected. Or am I missing something?
> > > 
> > > I think that the for check should be "i < len"
> > 
> > Yes it should be. I think we got the two solutions mixed up.
> > 
> > With the above len--, it should be i < len, but when we did the check
> > for zero at the end, we needed the i < len - 1
> 
> ugh right.. should be 'i < len' check in the for loop,
> 
> there's also the patch 2/3 that needs to be changed
> 
> Arnaldo,
> please let me know if you need me to resend this.

So I need to drop those, even with Rostedt's acked-by? Ok, please
resend, hopefully this time with a Tested-by from the reporter?

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1444466 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, 15 Jul 2016 18:13:10 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> 
> SNIP
> 
> > > for "AA\1\0" this returns "1" although that should return "0".
> > > 
> > > orig len 4
> > > decremented len 3
> > > for:
> > > 0 1
> > > 
> > > index 2 would not be inspected. Or am I missing something?
> > > 
> > > I think that the for check should be "i < len"  
> > 
> > Yes it should be. I think we got the two solutions mixed up.
> > 
> > With the above len--, it should be i < len, but when we did the check
> > for zero at the end, we needed the i < len - 1  
> 
> ugh right.. should be 'i < len' check in the for loop,
> 
> there's also the patch 2/3 that needs to be changed
> 

I'm wondering if we should also add at the beginning:

	if (!len)
		return 0;

Otherwise we will be accessing out of bounds with the len-1.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1444467 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, 15 Jul 2016 09:29:55 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> +			if (field->flags & FIELD_IS_STRING &&
> +			    is_printable_array(data + offset, len)) {
> +				obj = PyString_FromString((char *) data + offset);

Hmm. As I stated, It is possible that strings can be non nul
terminated. But I'm looking here and thinking we need to make sure that
it is nul terminated.

Can PyString_FromString() handle a non nul terminated string?

-- Steve

> +			} else {
> +				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
> +				field->flags &= ~FIELD_IS_STRING;
> +			}
>  		} else { /* FIELD_IS_NUMERIC */
>  			obj = get_field_numeric_entry(event, field, data);
>  		}

[toc] | [prev] | [next] | [standalone]

#1444870 — Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving

On Fri, Jul 15, 2016 at 01:18:40PM -0400, Steven Rostedt wrote:
> On Fri, 15 Jul 2016 09:29:55 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > +			if (field->flags & FIELD_IS_STRING &&
> > +			    is_printable_array(data + offset, len)) {
> > +				obj = PyString_FromString((char *) data + offset);
> 
> Hmm. As I stated, It is possible that strings can be non nul
> terminated. But I'm looking here and thinking we need to make sure that
> it is nul terminated.
> 
> Can PyString_FromString() handle a non nul terminated string?

couldn't find in the doc.. but I think it's necessary,
there's no other way it could tell the end ;-)

I'll make the is_printable_array in case the final 0 is missing

thanks,
jirka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web