Re: [PATCH v2 3/3] perf tests: Add test for PE binary format support

From: Jiri Olsa
Date: Tue Aug 04 2020 - 17:14:24 EST


On Tue, Aug 04, 2020 at 10:57:36AM +0200, Remi Bernon wrote:
> This adds a precompiled file in PE binary format, with split debug file,
> and tries to read its build_id and .gnu_debuglink sections, as well as
> looking up the main symbol from the debug file. This should succeed if
> libbfd is supported.
>
> Signed-off-by: Remi Bernon <rbernon@xxxxxxxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Jacek Caban <jacek@xxxxxxxxxxxxxxx>
> ---
>
> v2: No changes.
>
> tools/perf/Makefile.perf | 1 +
> tools/perf/tests/Build | 1 +
> tools/perf/tests/builtin-test.c | 4 ++
> tools/perf/tests/pe-file-parsing.c | 96 +++++++++++++++++++++++++++++
> tools/perf/tests/pe-file.c | 14 +++++
> tools/perf/tests/pe-file.exe | Bin 0 -> 75595 bytes
> tools/perf/tests/pe-file.exe.debug | Bin 0 -> 141644 bytes
> tools/perf/tests/tests.h | 1 +
> 8 files changed, 117 insertions(+)
> create mode 100644 tools/perf/tests/pe-file-parsing.c
> create mode 100644 tools/perf/tests/pe-file.c
> create mode 100644 tools/perf/tests/pe-file.exe
> create mode 100644 tools/perf/tests/pe-file.exe.debug
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 86dbb51bb272..11899fdd8eee 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -951,6 +951,7 @@ install-tests: all install-gtk
> $(call QUIET_INSTALL, tests) \
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests'; \
> $(INSTALL) tests/attr.py '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests'; \
> + $(INSTALL) tests/pe-file.exe* '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests'; \
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/attr'; \
> $(INSTALL) tests/attr/* '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/attr'; \
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell'; \
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index cd00498a5dce..da6aeba3c1e1 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -59,6 +59,7 @@ perf-y += genelf.o
> perf-y += api-io.o
> perf-y += demangle-java-test.o
> perf-y += pfm.o
> +perf-y += pe-file-parsing.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 da5b6cc23f25..e54ef171c73d 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -337,6 +337,10 @@ static struct test generic_tests[] = {
> .desc = "Demangle Java",
> .func = test__demangle_java,
> },
> + {
> + .desc = "PE file support",
> + .func = test__pe_file_parsing,
> + },
> {
> .func = NULL,
> },
> diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
> new file mode 100644
> index 000000000000..f40aca016a4f
> --- /dev/null
> +++ b/tools/perf/tests/pe-file-parsing.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdbool.h>
> +#include <inttypes.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/bitops.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <subcmd/exec-cmd.h>
> +
> +#include "debug.h"
> +#include "util/build-id.h"
> +#include "util/symbol.h"
> +#include "util/dso.h"
> +
> +#include "tests.h"
> +
> +#ifdef HAVE_LIBBFD_SUPPORT
> +
> +static int run_dir(const char *d)
> +{
> + char filename[PATH_MAX];
> + char debugfile[PATH_MAX];
> + char build_id[BUILD_ID_SIZE];
> + char debuglink[PATH_MAX];
> + char expect_build_id[] = {
> + 0x5a, 0x0f, 0xd8, 0x82, 0xb5, 0x30, 0x84, 0x22,
> + 0x4b, 0xa4, 0x7b, 0x62, 0x4c, 0x55, 0xa4, 0x69,
> + };
> + char expect_debuglink[PATH_MAX] = "pe-file.exe.debug";
> + struct dso *dso;
> + struct symbol *sym;
> + int ret;
> +
> + scnprintf(filename, PATH_MAX, "%s/pe-file.exe", d);
> + ret = filename__read_build_id(filename, build_id, BUILD_ID_SIZE);
> + TEST_ASSERT_VAL("Failed to read build_id",
> + ret == sizeof(expect_build_id));
> + TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
> + sizeof(expect_build_id)));
> +
> + ret = filename__read_debuglink(filename, debuglink, PATH_MAX);
> + TEST_ASSERT_VAL("Failed to read debuglink", ret == 0);
> + TEST_ASSERT_VAL("Wrong debuglink",
> + !strcmp(debuglink, expect_debuglink));
> +
> + scnprintf(debugfile, PATH_MAX, "%s/%s", d, debuglink);
> + ret = filename__read_build_id(debugfile, build_id, BUILD_ID_SIZE);
> + TEST_ASSERT_VAL("Failed to read debug file build_id",
> + ret == sizeof(expect_build_id));
> + TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
> + sizeof(expect_build_id)));
> +
> + dso = dso__new(filename);

missing dso != NULL check

other than this all 3 paches look ok to me

Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>

thanks,
jirka