perf python: Add support to access tracepoint fields

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) <[email protected]>

Cc Jiri Pirko <[email protected]>

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) <[email protected]>

Cc Jiri Pirko <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---

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) <[email protected]>

Cc Jiri Pirko <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
.../util/scripting-engines/trace-event-python.c | 37 ++++++++++++++++++----

1 file changed, 31 insertions(+), 6 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +0 -0

index 6ac6b7a33f42..1bc995de5a6d 100644
@@ -386,6 +386,19 @@ exit:
 	return pylist;
 }
 

tools/perf/util/scripting-engines/trace-event-python.c +21 -6

+
+	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);
 		}

Signature · 1 line

2.4.11

Quoted 23 lines · Fri, Jul 15, 2016 at 09:29:55AM CEST, [email protected] 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) <[email protected]>

Cc Jiri Pirko <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
.../util/scripting-engines/trace-event-python.c | 37 ++++++++++++++++++----

1 file changed, 31 insertions(+), 6 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +0 -0

index 6ac6b7a33f42..1bc995de5a6d 100644
@@ -386,6 +386,19 @@ exit:
 	return pylist;
 }
 

tools/perf/util/scripting-engines/trace-event-python.c exit: · line 398

Quoted 10 lines

tools/perf/util/scripting-engines/trace-event-python.c +10 -0

+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"

Quoted 7 lines

for "AA\1\0" this returns "1" although that should return "0".

orig len 4
decremented len 3
for:
0 1

Quoted 3 lines

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

SNIP

Quoted 7 lines

Quoted 7 lines

for "AA\1\0" this returns "1" although that should return "0".

orig len 4
decremented len 3
for:
0 1

Quoted 8 lines

Quoted 3 lines

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.

Em Fri, Jul 15, 2016 at 06:13:10PM +0200, Jiri Olsa escreveu:

Quoted 22 lines · Steven Rostedt

SNIP

Quoted 7 lines

Quoted 7 lines

for "AA\1\0" this returns "1" although that should return "0".

orig len 4
decremented len 3
for:
0 1

Quoted 8 lines

Quoted 3 lines

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,

Quoted 4 lines

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?

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

SNIP

Quoted 11 lines

Quoted 10 lines

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"

Quoted 4 lines

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

tools/perf/util/scripting-engines/trace-event-python.c +4 -0

+			} 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);
 		}

tools/perf/util/scripting-engines/trace-event-python.c static void python_process_tracepoint(struct perf_sample *sample, · line 488

Quoted 3 lines · On Fri, 15 Jul 2016 09:29:55 +0200 Jiri Olsa <[email protected]> wrote:

tools/perf/util/scripting-engines/trace-event-python.c +3 -0

+			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?

It's used from 2 objects in perf, so it's better
to keep just one copy.

Cc Steven Rostedt (Red Hat) <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>