* [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test
@ 2026-10-18 16:37 Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Namhyung Kim @ 2026-10-18 16:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users, Sashiko
The get_argv_exec_path() returns a dynamically allocated string so it
should be freed. Let's check the return value first so that it
doesn't crash snprintf().
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-168781@kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/tests/pe-file-parsing.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
index 190520ffc3ad6d0c..ed95aa5d8ca2af92 100644
--- a/tools/perf/tests/pe-file-parsing.c
+++ b/tools/perf/tests/pe-file-parsing.c
@@ -74,19 +74,26 @@ static int test__pe_file_parsing(struct test_suite *test __maybe_unused,
{
struct stat st;
char path_dir[PATH_MAX];
- const char * const tests_dirs[] = {
+ static const char * const tests_dirs[] = {
"./tools/perf/tests",
"./tests",
"./source/tests",
- path_dir,
};
- snprintf(path_dir, PATH_MAX, "%s/tests", get_argv_exec_path());
+ char *exec_path;
for (size_t i = 0; i < ARRAY_SIZE(tests_dirs); ++i) {
if (!lstat(tests_dirs[i], &st))
return run_dir(tests_dirs[i]);
}
+ exec_path = get_argv_exec_path();
+ TEST_ASSERT_VAL("Failed to allocate exec_path", exec_path);
+ snprintf(path_dir, PATH_MAX, "%s/tests", exec_path);
+ free(exec_path);
+
+ if (!lstat(path_dir, &st))
+ return run_dir(path_dir);
+
pr_debug("cannot find the tests directory\n");
return TEST_SKIP;
}
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 2/5] perf test: Make sure maps to have a machine
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
@ 2026-10-18 16:37 ` Namhyung Kim
2026-10-18 16:43 ` sashiko-bot
2026-10-18 20:33 ` Ian Rogers
2026-10-18 16:37 ` [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine() Namhyung Kim
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Namhyung Kim @ 2026-10-18 16:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
It's be easier if we could guarantee that maps always have a machine.
IIUC this is true for all real use cases, but only the test code has
an exception. Let's get rid of it and make sure they have machines.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/tests/maps.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
index 2c05d62f40dcdf72..f40b68f5fa5fe2c1 100644
--- a/tools/perf/tests/maps.c
+++ b/tools/perf/tests/maps.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <inttypes.h>
+#include <unistd.h>
#include <linux/compiler.h>
#include <linux/kernel.h>
#include "tests.h"
@@ -100,8 +101,12 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
};
struct map *map_kcore1, *map_kcore2, *map_kcore3;
int ret;
- struct maps *maps = maps__new(NULL);
+ struct machine *machine = machine__new_live(/*host_env=*/NULL,
+ /*kernel_maps=*/false,
+ getpid());
+ struct maps *maps = maps__new(machine);
+ TEST_ASSERT_VAL("failed to create machine", machine);
TEST_ASSERT_VAL("failed to create maps", maps);
for (i = 0; i < ARRAY_SIZE(bpf_progs); i++) {
@@ -159,6 +164,7 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
map__zput(map_kcore1);
map__zput(map_kcore2);
map__zput(map_kcore3);
+ machine__delete(machine);
return TEST_OK;
}
@@ -189,8 +195,12 @@ static int test__maps__fixup_overlap_and_insert(struct test_suite *t __maybe_unu
struct map *map_split, *map_eclipse;
int ret;
unsigned int i;
- struct maps *maps = maps__new(NULL);
+ struct machine *machine = machine__new_live(/*host_env=*/NULL,
+ /*kernel_maps=*/false,
+ getpid());
+ struct maps *maps = maps__new(machine);
+ TEST_ASSERT_VAL("failed to create machine", machine);
TEST_ASSERT_VAL("failed to create maps", maps);
for (i = 0; i < ARRAY_SIZE(initial_maps); i++) {
@@ -230,6 +240,7 @@ static int test__maps__fixup_overlap_and_insert(struct test_suite *t __maybe_unu
TEST_ASSERT_VAL("eclipse check failed", !ret);
maps__zput(maps);
+ machine__delete(machine);
return TEST_OK;
}
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine()
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
@ 2026-10-18 16:37 ` Namhyung Kim
2026-10-18 16:42 ` sashiko-bot
2026-10-18 16:37 ` [PATCH v1 4/5] perf symbol: Check result when reading section headers Namhyung Kim
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2026-10-18 16:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
Now maps should have a machine, let's get rid of checks for the return
value. There are already many places to assume it returns non-NULL,
so having this kind of check could give confusion that it can return
NULL (which is not true anymore).
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dlfilter.c | 3 +--
tools/perf/util/event.c | 3 ---
tools/perf/util/map.c | 9 +++------
tools/perf/util/python.c | 2 +-
4 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c
index e11e144af62bb979..7e179d1d84d78358 100644
--- a/tools/perf/util/dlfilter.c
+++ b/tools/perf/util/dlfilter.c
@@ -233,8 +233,7 @@ static const __u8 *dlfilter__insn(void *ctx, __u32 *len)
if (thread__maps(al->thread)) {
struct machine *machine = maps__machine(thread__maps(al->thread));
- if (machine)
- perf_sample__fetch_insn(d->sample, al->thread, machine);
+ perf_sample__fetch_insn(d->sample, al->thread, machine);
}
}
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index ea75816d126a14be..2f80e364659fea71 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -706,9 +706,6 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
al->cpumode = cpumode;
al->filtered = 0;
- if (machine == NULL)
- return NULL;
-
if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
al->level = 'k';
maps = machine__kernel_maps(machine);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 41cdddc987eee710..d5319ffa88a7477c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -538,13 +538,10 @@ u64 map__rip_2objdump(const struct map *map, u64 rip)
*/
if (kmap && is_entry_trampoline(kmap->name) && kmap->kmaps) {
struct machine *machine = maps__machine(kmap->kmaps);
+ struct map *kernel_map = machine__kernel_map(machine);
- if (machine) {
- struct map *kernel_map = machine__kernel_map(machine);
-
- if (kernel_map)
- map = kernel_map;
- }
+ if (kernel_map)
+ map = kernel_map;
}
if (!dso__adjust_symbols(dso))
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 60dbe2dad5eb8073..42c587cd54a527a4 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -4536,7 +4536,7 @@ static PyObject *pyrf_call_return__new(struct call_return *cr)
if (cr->thread) {
struct maps *maps = thread__maps(cr->thread);
- pevent->machine_pid = maps && maps__machine(maps) ? maps__machine(maps)->pid : 0;
+ pevent->machine_pid = maps ? maps__machine(maps)->pid : 0;
pevent->pid = thread__pid(cr->thread);
pevent->tid = thread__tid(cr->thread);
} else {
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 4/5] perf symbol: Check result when reading section headers
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine() Namhyung Kim
@ 2026-10-18 16:37 ` Namhyung Kim
2026-10-18 16:40 ` sashiko-bot
2026-10-18 16:37 ` [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd Namhyung Kim
2026-10-18 16:40 ` [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test sashiko-bot
4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2026-10-18 16:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users, Sashiko
Otherwise it may access uninitialized section header struct.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-170233@kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/symbol-elf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e2716c6af45712a..00ea0e19020fb256 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -196,13 +196,15 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
while ((sec = elf_nextscn(elf, sec)) != NULL) {
char *str;
- gelf_getshdr(sec, shp);
+ if (!gelf_getshdr(sec, shp))
+ goto next;
str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name);
if (str && !strcmp(name, str)) {
if (idx)
*idx = cnt;
return sec;
}
+next:
++cnt;
}
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
` (2 preceding siblings ...)
2026-10-18 16:37 ` [PATCH v1 4/5] perf symbol: Check result when reading section headers Namhyung Kim
@ 2026-10-18 16:37 ` Namhyung Kim
2026-10-18 16:44 ` sashiko-bot
2026-10-18 16:40 ` [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test sashiko-bot
4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2026-10-18 16:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users, Sashiko
Otherwise it can confuse cross-platform analysis.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-168776@kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/libbfd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index a4dcb406c79e9cee..9cc6216ce3adbcd0 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -462,6 +462,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
symbols__fixup_end(dso__symbols(dso), false);
symbols__fixup_duplicate(dso__symbols(dso));
dso__set_adjust_symbols(dso, true);
+ dso__set_is_64_bit(dso, bfd_get_arch_size(abfd) == 64);
err = dso__has_symbols(dso);
out_free:
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 4/5] perf symbol: Check result when reading section headers
2026-10-18 16:37 ` [PATCH v1 4/5] perf symbol: Check result when reading section headers Namhyung Kim
@ 2026-10-18 16:40 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-18 16:40 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
> Otherwise it may access uninitialized section header struct.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-170233@kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
` (3 preceding siblings ...)
2026-10-18 16:37 ` [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd Namhyung Kim
@ 2026-10-18 16:40 ` sashiko-bot
4 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-18 16:40 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
> The get_argv_exec_path() returns a dynamically allocated string so it
> should be freed. Let's check the return value first so that it
> doesn't crash snprintf().
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-168781@kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine()
2026-10-18 16:37 ` [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine() Namhyung Kim
@ 2026-10-18 16:42 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-18 16:42 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
> Now maps should have a machine, let's get rid of checks for the return
> value. There are already many places to assume it returns non-NULL,
> so having this kind of check could give confusion that it can return
> NULL (which is not true anymore).
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] perf test: Make sure maps to have a machine
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
@ 2026-10-18 16:43 ` sashiko-bot
2026-10-18 20:33 ` Ian Rogers
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-18 16:43 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
> It's be easier if we could guarantee that maps always have a machine.
> IIUC this is true for all real use cases, but only the test code has
> an exception. Let's get rid of it and make sure they have machines.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd
2026-10-18 16:37 ` [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd Namhyung Kim
@ 2026-10-18 16:44 ` sashiko-bot
2026-10-19 5:38 ` Namhyung Kim
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-10-18 16:44 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf symbol: bfd_get_arch_size() always returns -1 in dso__load_bfd_symbols()
--
commit 6d3220ac244766831d73c524c0b8f5184af28a7b
Author: Namhyung Kim <namhyung@kernel.org>
perf symbol: Ensure dso__set_is_64_bit() for libbfd
This commit calls dso__set_is_64_bit() using bfd_get_arch_size() when
loading symbols via libbfd in dso__load_bfd_symbols().
> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> index a4dcb406c79e9..9cc6216ce3adb 100644
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c
> @@ -462,6 +462,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
> symbols__fixup_end(dso__symbols(dso), false);
> symbols__fixup_duplicate(dso__symbols(dso));
> dso__set_adjust_symbols(dso, true);
> + dso__set_is_64_bit(dso, bfd_get_arch_size(abfd) == 64);
[Severity: Medium]
Will bfd_get_arch_size(abfd) always return -1 here?
Earlier in tools/perf/util/libbfd.c:dso__load_bfd_symbols(), ELF binaries
are rejected before reaching this point:
if (bfd_get_flavour(abfd) == bfd_target_elf_flavour)
goto out_close;
In GNU binutils libbfd, bfd_get_arch_size() only checks ELF targets and
returns -1 for any other BFD flavour:
if (abfd->xvec->flavour == bfd_target_elf_flavour)
return get_elf_backend_data (abfd)->s->arch_size;
return -1;
Because abfd is guaranteed to be non-ELF here (such as a 64-bit PE/COFF
binary), bfd_get_arch_size(abfd) == 64 always evaluates to false and
unconditionally sets dso->is_64_bit to false.
Should this use bfd_arch_bits_per_address(abfd) == 64 instead so that the
address size is queried across all BFD flavours?
>
> err = dso__has_symbols(dso);
> out_free:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] perf test: Make sure maps to have a machine
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
2026-10-18 16:43 ` sashiko-bot
@ 2026-10-18 20:33 ` Ian Rogers
2026-10-19 5:36 ` Namhyung Kim
1 sibling, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2026-10-18 20:33 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Sun, Oct 18, 2026 at 9:37 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It's be easier if we could guarantee that maps always have a machine.
> IIUC this is true for all real use cases, but only the test code has
> an exception. Let's get rid of it and make sure they have machines.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/tests/maps.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
> index 2c05d62f40dcdf72..f40b68f5fa5fe2c1 100644
> --- a/tools/perf/tests/maps.c
> +++ b/tools/perf/tests/maps.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <inttypes.h>
> +#include <unistd.h>
> #include <linux/compiler.h>
> #include <linux/kernel.h>
> #include "tests.h"
> @@ -100,8 +101,12 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
> };
> struct map *map_kcore1, *map_kcore2, *map_kcore3;
> int ret;
> - struct maps *maps = maps__new(NULL);
> + struct machine *machine = machine__new_live(/*host_env=*/NULL,
> + /*kernel_maps=*/false,
> + getpid());
> + struct maps *maps = maps__new(machine);
>
> + TEST_ASSERT_VAL("failed to create machine", machine);
> TEST_ASSERT_VAL("failed to create maps", maps);
>
> for (i = 0; i < ARRAY_SIZE(bpf_progs); i++) {
> @@ -159,6 +164,7 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
> map__zput(map_kcore1);
> map__zput(map_kcore2);
> map__zput(map_kcore3);
> + machine__delete(machine);
> return TEST_OK;
> }
>
> @@ -189,8 +195,12 @@ static int test__maps__fixup_overlap_and_insert(struct test_suite *t __maybe_unu
> struct map *map_split, *map_eclipse;
> int ret;
> unsigned int i;
> - struct maps *maps = maps__new(NULL);
> + struct machine *machine = machine__new_live(/*host_env=*/NULL,
> + /*kernel_maps=*/false,
> + getpid());
For completeness should we pass in a host_env like in builtin-top.c?
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-top.c?h=perf-tools-next#n1661
Thanks,
Ian
> + struct maps *maps = maps__new(machine);
>
> + TEST_ASSERT_VAL("failed to create machine", machine);
> TEST_ASSERT_VAL("failed to create maps", maps);
>
> for (i = 0; i < ARRAY_SIZE(initial_maps); i++) {
> @@ -230,6 +240,7 @@ static int test__maps__fixup_overlap_and_insert(struct test_suite *t __maybe_unu
> TEST_ASSERT_VAL("eclipse check failed", !ret);
>
> maps__zput(maps);
> + machine__delete(machine);
> return TEST_OK;
> }
>
> --
> 2.56.0.176.g3575e65682-goog
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] perf test: Make sure maps to have a machine
2026-10-18 20:33 ` Ian Rogers
@ 2026-10-19 5:36 ` Namhyung Kim
0 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2026-10-19 5:36 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Sun, Oct 18, 2026 at 01:33:45PM -0700, Ian Rogers wrote:
> On Sun, Oct 18, 2026 at 9:37 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > It's be easier if we could guarantee that maps always have a machine.
> > IIUC this is true for all real use cases, but only the test code has
> > an exception. Let's get rid of it and make sure they have machines.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/tests/maps.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
> > index 2c05d62f40dcdf72..f40b68f5fa5fe2c1 100644
> > --- a/tools/perf/tests/maps.c
> > +++ b/tools/perf/tests/maps.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0
> > #include <inttypes.h>
> > +#include <unistd.h>
> > #include <linux/compiler.h>
> > #include <linux/kernel.h>
> > #include "tests.h"
> > @@ -100,8 +101,12 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
> > };
> > struct map *map_kcore1, *map_kcore2, *map_kcore3;
> > int ret;
> > - struct maps *maps = maps__new(NULL);
> > + struct machine *machine = machine__new_live(/*host_env=*/NULL,
> > + /*kernel_maps=*/false,
> > + getpid());
> > + struct maps *maps = maps__new(machine);
> >
> > + TEST_ASSERT_VAL("failed to create machine", machine);
> > TEST_ASSERT_VAL("failed to create maps", maps);
> >
> > for (i = 0; i < ARRAY_SIZE(bpf_progs); i++) {
> > @@ -159,6 +164,7 @@ static int test__maps__merge_in(struct test_suite *t __maybe_unused, int subtest
> > map__zput(map_kcore1);
> > map__zput(map_kcore2);
> > map__zput(map_kcore3);
> > + machine__delete(machine);
> > return TEST_OK;
> > }
> >
> > @@ -189,8 +195,12 @@ static int test__maps__fixup_overlap_and_insert(struct test_suite *t __maybe_unu
> > struct map *map_split, *map_eclipse;
> > int ret;
> > unsigned int i;
> > - struct maps *maps = maps__new(NULL);
> > + struct machine *machine = machine__new_live(/*host_env=*/NULL,
> > + /*kernel_maps=*/false,
> > + getpid());
>
> For completeness should we pass in a host_env like in builtin-top.c?
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-top.c?h=perf-tools-next#n1661
Sure, will add in v2.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd
2026-10-18 16:44 ` sashiko-bot
@ 2026-10-19 5:38 ` Namhyung Kim
0 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2026-10-19 5:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users
On Sun, Oct 18, 2026 at 04:44:26PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] perf symbol: bfd_get_arch_size() always returns -1 in dso__load_bfd_symbols()
> --
>
> commit 6d3220ac244766831d73c524c0b8f5184af28a7b
> Author: Namhyung Kim <namhyung@kernel.org>
>
> perf symbol: Ensure dso__set_is_64_bit() for libbfd
>
> This commit calls dso__set_is_64_bit() using bfd_get_arch_size() when
> loading symbols via libbfd in dso__load_bfd_symbols().
>
> > diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> > index a4dcb406c79e9..9cc6216ce3adb 100644
> > --- a/tools/perf/util/libbfd.c
> > +++ b/tools/perf/util/libbfd.c
> > @@ -462,6 +462,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
> > symbols__fixup_end(dso__symbols(dso), false);
> > symbols__fixup_duplicate(dso__symbols(dso));
> > dso__set_adjust_symbols(dso, true);
> > + dso__set_is_64_bit(dso, bfd_get_arch_size(abfd) == 64);
>
> [Severity: Medium]
> Will bfd_get_arch_size(abfd) always return -1 here?
>
> Earlier in tools/perf/util/libbfd.c:dso__load_bfd_symbols(), ELF binaries
> are rejected before reaching this point:
>
> if (bfd_get_flavour(abfd) == bfd_target_elf_flavour)
> goto out_close;
>
> In GNU binutils libbfd, bfd_get_arch_size() only checks ELF targets and
> returns -1 for any other BFD flavour:
>
> if (abfd->xvec->flavour == bfd_target_elf_flavour)
> return get_elf_backend_data (abfd)->s->arch_size;
> return -1;
>
> Because abfd is guaranteed to be non-ELF here (such as a 64-bit PE/COFF
> binary), bfd_get_arch_size(abfd) == 64 always evaluates to false and
> unconditionally sets dso->is_64_bit to false.
>
> Should this use bfd_arch_bits_per_address(abfd) == 64 instead so that the
> address size is queried across all BFD flavours?
Looks good, will fix.
Thanks,
Namhyung
>
> >
> > err = dso__has_symbols(dso);
> > out_free:
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-19 5:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 16:37 [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 2/5] perf test: Make sure maps to have a machine Namhyung Kim
2026-10-18 16:43 ` sashiko-bot
2026-10-18 20:33 ` Ian Rogers
2026-10-19 5:36 ` Namhyung Kim
2026-10-18 16:37 ` [PATCH v1 3/5] perf tools: Remove NULL check for maps__machine() Namhyung Kim
2026-10-18 16:42 ` sashiko-bot
2026-10-18 16:37 ` [PATCH v1 4/5] perf symbol: Check result when reading section headers Namhyung Kim
2026-10-18 16:40 ` sashiko-bot
2026-10-18 16:37 ` [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd Namhyung Kim
2026-10-18 16:44 ` sashiko-bot
2026-10-19 5:38 ` Namhyung Kim
2026-10-18 16:40 ` [PATCH v1 1/5] perf test: Fix a memory leak in PE file parsing test sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.