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

[Jiri Olsa]

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