2021-05-24 11:17:28

by Denys Zagorui

[permalink] [raw]
Subject: [PATCH v8 2/3] perf tests: avoid storing an absolute path in perf binary

python binding test uses PYTHONPATH definition to find python/perf.so
library. This definition is an absolute path that makes perf binary
unreproducible. This path can be found during runtime execution.

Signed-off-by: Denys Zagorui <[email protected]>
---
tools/perf/tests/Build | 2 +-
tools/perf/tests/python-use.c | 25 ++++++++++++++++++++++---
2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 650aec19d490..a20098dcdbc4 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
endif

CFLAGS_attr.o += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
-CFLAGS_python-use.o += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
+CFLAGS_python-use.o += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
index 98c6d474aa6f..32af71300aa3 100644
--- a/tools/perf/tests/python-use.c
+++ b/tools/perf/tests/python-use.c
@@ -8,18 +8,37 @@
#include <linux/compiler.h>
#include "tests.h"
#include "util/debug.h"
+#include "util/util.h"
+#include <sys/stat.h>
+#include <limits.h>
+#include <libgen.h>

int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
{
char *cmd;
- int ret;
+ int ret = -1;
+ char *exec_path;
+ char buf[PATH_MAX];
+ char *pythonpath;
+ struct stat sb;
+
+ perf_exe(buf, PATH_MAX);
+ exec_path = dirname(buf);
+
+ if (asprintf(&pythonpath, "%s/python", exec_path) < 0)
+ return ret;
+
+ if (stat(pythonpath, &sb) || !S_ISDIR(sb.st_mode))
+ pythonpath[0] = 0;

if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
- PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
- return -1;
+ pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
+ goto out;

pr_debug("python usage test: \"%s\"\n", cmd);
ret = system(cmd) ? -1 : 0;
free(cmd);
+out:
+ free(pythonpath);
return ret;
}
--
2.26.2.Cisco


2021-05-27 18:59:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v8 2/3] perf tests: avoid storing an absolute path in perf binary

Em Mon, May 24, 2021 at 04:15:13AM -0700, Denys Zagorui escreveu:
> python binding test uses PYTHONPATH definition to find python/perf.so
> library. This definition is an absolute path that makes perf binary
> unreproducible. This path can be found during runtime execution.
>
> Signed-off-by: Denys Zagorui <[email protected]>
> ---
> tools/perf/tests/Build | 2 +-
> tools/perf/tests/python-use.c | 25 ++++++++++++++++++++++---
> 2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 650aec19d490..a20098dcdbc4 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> endif
>
> CFLAGS_attr.o += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> -CFLAGS_python-use.o += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> +CFLAGS_python-use.o += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
> diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
> index 98c6d474aa6f..32af71300aa3 100644
> --- a/tools/perf/tests/python-use.c
> +++ b/tools/perf/tests/python-use.c
> @@ -8,18 +8,37 @@
> #include <linux/compiler.h>
> #include "tests.h"
> #include "util/debug.h"
> +#include "util/util.h"
> +#include <sys/stat.h>
> +#include <limits.h>
> +#include <libgen.h>
>
> int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
> {
> char *cmd;
> - int ret;
> + int ret = -1;
> + char *exec_path;
> + char buf[PATH_MAX];
> + char *pythonpath;
> + struct stat sb;
> +
> + perf_exe(buf, PATH_MAX);
> + exec_path = dirname(buf);
> +
> + if (asprintf(&pythonpath, "%s/python", exec_path) < 0)
> + return ret;
> +
> + if (stat(pythonpath, &sb) || !S_ISDIR(sb.st_mode))
> + pythonpath[0] = 0;
>
> if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
> - PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> - return -1;
> + pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> + goto out;
>
> pr_debug("python usage test: \"%s\"\n", cmd);
> ret = system(cmd) ? -1 : 0;
> free(cmd);
> +out:
> + free(pythonpath);
> return ret;
> }
> --
> 2.26.2.Cisco
>

I noticed this is failing the test, nothing is being appended. I'll
investigate later. Can you try to reproduce this? I build perf with:

alias m='perf stat -e cycles:u,instructions:u make -k CORESIGHT=1 BUILD_BPF_SKEL=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin && perf test python'


⬢[acme@toolbox perf]$ perf test python
19: 'import perf' in python : FAILED!
⬢[acme@toolbox perf]$ perf test -v python
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
19: 'import perf' in python :
--- start ---
test child forked, pid 347155
python usage test: "echo "import sys ; sys.path.append(''); import perf" | '/usr/bin/python3' "
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'perf'
test child finished with -1
---- end ----
'import perf' in python: FAILED!
⬢[acme@toolbox perf]$ git log --oneline -3
60cdbfd0586e53c3 (HEAD) perf tests: Avoid storing an absolute path in perf binary
f7fc0d1c915a74ff perf inject: Do not inject BUILD_ID record if MMAP2 has it
0c3f7b38d72b9247 perf inject: Call dso__put() even if dso->hit is set
⬢[acme@toolbox perf]$

2021-05-28 12:49:45

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v8 2/3] perf tests: avoid storing an absolute path in perf binary

Em Fri, May 28, 2021 at 11:13:09AM +0000, Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco) escreveu:
>
> > I noticed this is failing the test, nothing is being appended. I'll
> > investigate later. Can you try to reproduce this? I build perf with:
>
> Maybe this test should fail.

I don't think so :-)

> Path is being appended only if perf is invoked from the build
> directory (basically if there is python dir near perf binary this path
> will be added to sys.path)

And that is ok for a developer, but I agree with you, we shouldn't have
such hardcoded build directories in a production binary.

> I'm not sure if install-bin task installs perf.so to system i mean
> before this patch python binary contains an absolute path to its build
> directory and if this build dir is deleted this test also will fail.

That is what is happening, yes.

> Maybe we should use export PYTHONPATH=<build dir>/python for such test

Agreed, can you cook up a patch that does that in the Makefile that
runs the 'perf python' in tools/perf/tests/?

One other suggestion: When the test fails, say when we run 'perf test
python' directly, it could check if PYTHONPATH is set and if not, warn
the user, something like:

⬢[acme@toolbox perf]$ perf test python
19: 'import perf' in python : FAILED! (Please set the PYTHONPATH env variable)

> Thanks,
> Denys
>
> > alias m='perf stat -e cycles:u,instructions:u make -k CORESIGHT=1 BUILD_BPF_SKEL=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin && perf test python'
> >
> >
> > ⬢[acme@toolbox perf]$ perf test python
> > 19: 'import perf' in python : FAILED!
> > ⬢[acme@toolbox perf]$ perf test -v python
> > Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> > 19: 'import perf' in python :

--

- Arnaldo

2021-05-28 13:33:43

by Denys Zagorui

[permalink] [raw]
Subject: Re: [PATCH v8 2/3] perf tests: avoid storing an absolute path in perf binary


> I noticed this is failing the test, nothing is being appended. I'll
> investigate later. Can you try to reproduce this? I build perf with:

Maybe this test should fail. Path is being appended only if perf is invoked from the build directory (basically if there is python dir near perf binary
this path will be added to sys.path)
I'm not sure if install-bin task installs perf.so to system i mean before this patch python binary contains an absolute path to its build directory
and if this build dir is deleted this test also will fail.
Maybe we should use export PYTHONPATH=<build dir>/python for such test

Thanks,
Denys

> alias m='perf stat -e cycles:u,instructions:u make -k CORESIGHT=1 BUILD_BPF_SKEL=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin && perf test python'
>
>
> ⬢[acme@toolbox perf]$ perf test python
> 19: 'import perf' in python : FAILED!
> ⬢[acme@toolbox perf]$ perf test -v python
> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> 19: 'import perf' in python :