Re: [PATCH v2] perf scripts python: Add Python 3 support to check-perf-trace.py

From: Tony Jones <tonyj@suse.de>
To: Jiri Olsa <jolsa@redhat.com>,
	Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	alexander.shishkin@linux.intel.com, namhyung@kernel.org,
	linux-kernel@vger.kernel.org,
	Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Subject: Re: [PATCH v2] perf scripts python: Add Python 3 support to check-perf-trace.py
Date: Fri, 18 Jan 2019 18:05:45 -0800	[thread overview]
Message-ID: <7d5e8be1-b2dc-7654-744b-5ddbe2faf5b2@suse.de> (raw)
In-Reply-To: <0d8a6ddc-9755-2b50-4023-26f6c84a33d1@suse.de>

On 1/18/19 4:29 PM, Tony Jones wrote:

> I'd been simultaneously working on a patch set to fix up Python3.  
> 
> It's actually already in our Factory and SLE15-SP1 releases as we had a deadline to kill Python2 usage for internal rpms. 
> 
> I was going to post once I'd fixed the last remaining issue ('import perf' is still failing [test #18]).   
> 
> I guess "you snooze you lose" :-)
> 
> https://build.opensuse.org/package/view_file/devel:tools/perf/perf.changes?expand=1

Seeteena, I'm than happy to forward my patches via email. Alternatively, as I said in another post,  the full series is in SLE15-SP1/beta2 (should apply to tip) which IBM has access to and they've been backported to v4.19 (Factory) at https://build.opensuse.org/package/show/openSUSE:Factory/perf.  Everything has been tested against python2 and python3.

Also, in this series there are patches to: 
- port tests/attr.py to Python3
- remove shebangs from the .py scripts and change to mode 644 as IMO it makes no sense to explicitly have #!/usr/bin/python since per pep-0394 this refers to version2 and the system may only have python3 installed
- remove the shebang from setup.py since it's explicitly invoked via call to ${PYTHON_WORD}

Also I found in testing that the following fix is also needed. I'm not 100% sure on it and was going to revisit before posting but _PyUnicode_FromStringAndSize() is definitely unsafe to use on attr.

----

Fixes: 66dfdff03d196e51322c6a85c0d8db8bb2bdd655

With Python3.  PyUnicode_FromStringAndSize is unsafe to call on attr and will
return NULL.  Use _PyBytes_FromStringAndSize (as with raw_buf).

$ perf script -s perf-script.py -i perf.data
in trace_begin
Segmentation fault (core dumped)
---
 tools/perf/util/scripting-engines/trace-event-python.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -733,8 +733,7 @@ static PyObject *get_perf_sample_dict(st
                Py_FatalError("couldn't create Python dictionary");

        pydict_set_item_string_decref(dict, "ev_name", _PyUnicode_FromString(perf_evsel__name(evsel)));
-       pydict_set_item_string_decref(dict, "attr", _PyUnicode_FromStringAndSize(
-                       (const char *)&evsel->attr, sizeof(evsel->attr)));
+       pydict_set_item_string_decref(dict, "attr", _PyBytes_FromStringAndSize((const char *)&evsel->attr, sizeof(evsel->attr)));

        pydict_set_item_string_decref(dict_sample, "pid",
                        _PyLong_FromLong(sample->pid));
