Re: [PATCH] perf jit: move test functionality in to a test

[Arnaldo Carvalho de Melo]

Em Wed, Nov 27, 2019 at 10:49:29AM -0800, Ian Rogers escreveu:
> On Wed, Nov 27, 2019 at 8:06 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@xxxxxxxxx> wrote:
> >
> > Em Wed, Nov 27, 2019 at 12:23:28PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Tue, Nov 26, 2019 at 03:59:13PM -0800, Ian Rogers escreveu:
> > > > Adds a test for minimal jit_write_elf functionality.
> > >
> > > Thanks, tested and applied.
> >
> > Had to apply this to have it built in systems where HAVE_JITDUMP isn't
> > defined:
> 
> Thanks for fixing this!
> Ian

This needs some more work, as it is failing in some of the cross build
containers, for arches not supported by that genelf.h header, where it
should just detect that this feature shouldn't be used, warn the user
and build what is possible to build, e.g.:

  CC       /tmp/build/perf/util/evswitch.o
In file included from tests/genelf.c:15:
tests/../util/genelf.h:42:2: error: #error "unsupported architecture"
   42 | #error "unsupported architecture"
      |  ^~~~~
tests/../util/genelf.h:51:5: error: "GEN_ELF_CLASS" is not defined, evaluates to 0 [-Werror=undef]
   51 | #if GEN_ELF_CLASS == ELFCLASS64
      |     ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
mv: cannot stat '/tmp/build/perf/tests/.genelf.o.tmp': No such file or directory
make[4]: *** [/git/linux/tools/build/Makefile.build:96: /tmp/build/perf/tests/genelf.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  CC       /tmp/build/perf/util/find_bit.o
  CC       /tmp/build/perf/util/get_current_dir_name.o


There is some wiring up to do here, related to:
[acme@quaco perf]$ vim tools/perf/Makefile.config
[acme@quaco perf]$ find . -type f| xargs grep PERF_HAVE_JITDUMP
grep: ./et: No such file or directory
grep: vi: No such file or directory
^Xgrep: ./perf.data: Permission denied
./tools/perf/arch/x86/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/arch/sparc/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/arch/arm/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/arch/arm64/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/arch/powerpc/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/arch/s390/Makefile:PERF_HAVE_JITDUMP := 1
./tools/perf/Makefile.config:ifdef PERF_HAVE_JITDUMP
^C
[acme@quaco perf]$

But I'll defer this for later, not to hold what I have too much, after
my next pull req I'll revisit this, if you haven't found a fix by then
:-)

The current patch is below, with my fixes, and looking at it again it
seems its just to replace that HAVE_LIBELF_SUPPORT with HAVE_JITDUMP,
will confirm that later, after sending the pull req to Ingo.

- Arnaldo

commit d006842faa9a26feb51b818e02c681f064b83b0a
Author: Ian Rogers <irogers@xxxxxxxxxx>
Date:   Tue Nov 26 15:59:13 2019 -0800

    perf jit: Move test functionality in to a test
    
    Adds a test for minimal jit_write_elf functionality.
    
    Committer testing:
    
      # perf test jit
      61: Test jit_write_elf                                    : Ok
      #
    
      # perf test -v jit
      61: Test jit_write_elf                                    :
      --- start ---
      test child forked, pid 10460
      Writing jit code to: /tmp/perf-test-KqxURR
      test child finished with 0
      ---- end ----
      Test jit_write_elf: Ok
      #
    
    Committer notes:
    
    Fix up the case where HAVE_JITDUMP is no defined.
    
    Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
    Tested-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
    Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
    Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
    Cc: Alexios Zavras <alexios.zavras@xxxxxxxxx>
    Cc: Allison Randal <allison@xxxxxxxxxxx>
    Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>
    Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
    Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
    Cc: Kate Stewart <kstewart@xxxxxxxxxxxxxxxxxxx>
    Cc: Leo Yan <leo.yan@xxxxxxxxxx>
    Cc: Mark Rutland <mark.rutland@xxxxxxx>
    Cc: Michael Petlan <mpetlan@xxxxxxxxxx>
    Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
    Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
    Cc: Song Liu <songliubraving@xxxxxx>
    Cc: Stephane Eranian <eranian@xxxxxxxxxx>
    Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
    Link: http://lore.kernel.org/lkml/20191126235913.41855-1-irogers@xxxxxxxxxx
    Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index a3c595fba943..1692529639b0 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -54,6 +54,7 @@ perf-y += unit_number__scnprintf.o
 perf-y += mem2node.o
 perf-y += maps.o
 perf-y += time-utils-test.o
+perf-y += genelf.o
 
 $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
 	$(call rule_mkdir)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 7115aa32a51e..970e2ecfb39f 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -296,6 +296,10 @@ static struct test generic_tests[] = {
 		.desc = "time utils",
 		.func = test__time_utils,
 	},
+	{
+		.desc = "Test jit_write_elf",
+		.func = test__jit_write_elf,
+	},
 	{
 		.desc = "maps__merge_in",
 		.func = test__maps__merge_in,
diff --git a/tools/perf/tests/genelf.c b/tools/perf/tests/genelf.c
new file mode 100644
index 000000000000..28dfd17a6b9f
--- /dev/null
+++ b/tools/perf/tests/genelf.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/compiler.h>
+
+#include "debug.h"
+#include "tests.h"
+
+
+#ifdef HAVE_LIBELF_SUPPORT
+#include <libelf.h>
+#include "../util/genelf.h"
+#endif
+
+#define TEMPL "/tmp/perf-test-XXXXXX"
+
+int test__jit_write_elf(struct test *test __maybe_unused,
+			int subtest __maybe_unused)
+{
+#ifdef HAVE_JITDUMP
+	static unsigned char x86_code[] = {
+		0xBB, 0x2A, 0x00, 0x00, 0x00, /* movl $42, %ebx */
+		0xB8, 0x01, 0x00, 0x00, 0x00, /* movl $1, %eax */
+		0xCD, 0x80            /* int $0x80 */
+	};
+	char path[PATH_MAX];
+	int fd, ret;
+
+	strcpy(path, TEMPL);
+
+	fd = mkstemp(path);
+	if (fd < 0) {
+		perror("mkstemp failed");
+		return TEST_FAIL;
+	}
+
+	pr_info("Writing jit code to: %s\n", path);
+
+	ret = jit_write_elf(fd, 0, "main", x86_code, sizeof(x86_code),
+			NULL, 0, NULL, 0, 0);
+	close(fd);
+
+	unlink(path);
+
+	return ret ? TEST_FAIL : 0;
+#else
+	return TEST_SKIP;
+#endif
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 25aea387e2bf..0e6d67910b0a 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -109,6 +109,7 @@ int test__unit_number__scnprint(struct test *test, int subtest);
 int test__mem2node(struct test *t, int subtest);
 int test__maps__merge_in(struct test *t, int subtest);
 int test__time_utils(struct test *t, int subtest);
+int test__jit_write_elf(struct test *test, int subtest);
 
 bool test__bp_signal_is_supported(void);
 bool test__bp_account_is_supported(void);
diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index f9f18b8b1df9..aed49806a09b 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -8,15 +8,12 @@
  */
 
 #include <sys/types.h>
-#include <stdio.h>
-#include <getopt.h>
 #include <stddef.h>
 #include <libelf.h>
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <inttypes.h>
-#include <limits.h>
 #include <fcntl.h>
 #include <err.h>
 #ifdef HAVE_DWARF_SUPPORT
@@ -31,8 +28,6 @@
 #define NT_GNU_BUILD_ID 3
 #endif
 
-#define JVMTI
-
 #define BUILD_ID_URANDOM /* different uuid for each run */
 
 #ifdef HAVE_LIBCRYPTO
@@ -511,44 +506,3 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 
 	return retval;
 }
-
-#ifndef JVMTI
-
-static unsigned char x86_code[] = {
-    0xBB, 0x2A, 0x00, 0x00, 0x00, /* movl $42, %ebx */
-    0xB8, 0x01, 0x00, 0x00, 0x00, /* movl $1, %eax */
-    0xCD, 0x80            /* int $0x80 */
-};
-
-static struct options options;
-
-int main(int argc, char **argv)
-{
-	int c, fd, ret;
-
-	while ((c = getopt(argc, argv, "o:h")) != -1) {
-		switch (c) {
-		case 'o':
-			options.output = optarg;
-			break;
-		case 'h':
-			printf("Usage: genelf -o output_file [-h]\n");
-			return 0;
-		default:
-			errx(1, "unknown option");
-		}
-	}
-
-	fd = open(options.output, O_CREAT|O_TRUNC|O_RDWR, 0666);
-	if (fd == -1)
-		err(1, "cannot create file %s", options.output);
-
-	ret = jit_write_elf(fd, "main", x86_code, sizeof(x86_code));
-	close(fd);
-
-	if (ret != 0)
-		unlink(options.output);
-
-	return ret;
-}
-#endif