[PATCH v2 1/2] perf maps: Move getting debug_file to verbose path

[PATCH v2 1/2] perf maps: Move getting debug_file to verbose path

[Ian Rogers]

Getting debug_file can trigger warnings if not set. Avoid getting
these warnings by pushing the use under the controlling if.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 4092211cff62..7dd6da9d1e4f 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -844,7 +844,6 @@ static int __maps__insert_sorted(struct maps *maps, unsigned int first_after_ind
 static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 {
 	int err = 0;
-	FILE *fp = debug_file();
 	unsigned int i, ni = INT_MAX; // Some gcc complain, but depends on maps_by_name...
 
 	if (!maps__maps_by_address_sorted(maps))
@@ -872,8 +871,8 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 				dso__name(map__dso(new)));
 		} else if (verbose >= 2) {
 			pr_debug("overlapping maps:\n");
-			map__fprintf(new, fp);
-			map__fprintf(pos, fp);
+			map__fprintf(new, debug_file());
+			map__fprintf(pos, debug_file());
 		}
 
 		if (maps_by_name)
@@ -894,7 +893,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			map__set_end(before, map__start(new));
 
 			if (verbose >= 2 && !use_browser)
-				map__fprintf(before, fp);
+				map__fprintf(before, debug_file());
 		}
 		if (map__end(new) < map__end(pos)) {
 			/* The new map isn't as long as the existing map. */
@@ -912,7 +911,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			       map__map_ip(after, map__end(new)));
 
 			if (verbose >= 2 && !use_browser)
-				map__fprintf(after, fp);
+				map__fprintf(after, debug_file());
 		}
 		/*
 		 * If adding one entry, for `before` or `after`, we can replace
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH v2 2/2] perf maps: Fix fixup_overlap_and_insert that can break sorted by name order

[Ian Rogers]

When an entry in the address array is replaced, the corresponding name
entry is replaced. The entries names may sort differently and so it is
important that the sorted by name property be cleared on the maps.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 7dd6da9d1e4f..b44bc41f51f3 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -955,6 +955,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			if (maps_by_name) {
 				map__put(maps_by_name[ni]);
 				maps_by_name[ni] = map__get(new);
+				maps__set_maps_by_name_sorted(maps, false);
 			}
 
 			err = __maps__insert_sorted(maps, i + 1, after, NULL);
@@ -981,6 +982,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 				if (maps_by_name) {
 					map__put(maps_by_name[ni]);
 					maps_by_name[ni] = map__get(new);
+					maps__set_maps_by_name_sorted(maps, false);
 				}
 
 				check_invariants(maps);
-- 
2.53.0.1213.gd9a14994de-goog

Re: [PATCH v2 2/2] perf maps: Fix fixup_overlap_and_insert that can break sorted by name order

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [High] The maps__copy_from function corrupts the maps_by_name array by populating it in address order but flagging it as sorted by name.

--

commit bd0adc21836e1932db4199fc14353b8a08f98a0a
Author: Ian Rogers <irogers@google.com>

perf maps: Fix fixup_overlap_and_insert that can break sorted by name order

When an entry in the address array is replaced, the corresponding name
entry is replaced. The entries names may sort differently and so it is
important that the sorted by name property be cleared on the maps.

> diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
> index 7dd6da9d1e4f0..b44bc41f51f3b 100644
> --- a/tools/perf/util/maps.c
> +++ b/tools/perf/util/maps.c
[ ... ]
> @@ -955,6 +955,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
>  			if (maps_by_name) {
>  				map__put(maps_by_name[ni]);
>  				maps_by_name[ni] = map__get(new);
> +				maps__set_maps_by_name_sorted(maps, false);
>  			}
[ ... ]

This isn't a bug in the newly added lines, but while reviewing the maps
sorting logic, I noticed a regression in maps__copy_from().

In maps__copy_from(), when copying maps from a parent to a destination
(e.g., during thread fork), the dest_maps_by_name array is populated
sequentially inside a loop iterating over parent_maps_by_address:

tools/perf/util/maps.c:maps__copy_from() {
[ ... ]
		for (unsigned int i = 0; !err && i < n; i++) {
			struct map *pos = parent_maps_by_address[i];
			struct map *new = map__clone(pos);
[ ... ]
				if (!err) {
					dest_maps_by_address[i] = new;
					map__set_kmap_maps(new, dest);
					if (dest_maps_by_name)
						dest_maps_by_name[i] = map__get(new);
[ ... ]
}

Because it clones and copies the elements in address order,
dest_maps_by_name is strictly populated in address order.

However, at the end of the fast-path copy block, the code executes:

tools/perf/util/maps.c:maps__copy_from() {
[ ... ]
		if (!err) {
			RC_CHK_ACCESS(dest)->last_search_by_name_idx =
				RC_CHK_ACCESS(parent)->last_search_by_name_idx;
			maps__set_maps_by_name_sorted(dest,
						dest_maps_by_name &&
						maps__maps_by_name_sorted(parent));
		}
[ ... ]
}

If the parent's name array was sorted, does this explicitly flag the new
destination map's name array as sorted by name, even though it is physically
sorted by address?

Could this cause any subsequent lookup by name in the copied map
(such as maps__find_by_name during child thread symbol resolution)
to execute a binary search on an address-sorted array?

Would this cause the binary search to take incorrect paths and fail to find
legitimate mappings, leading to missing symbols or missing maps for
child threads?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260408001521.102140-1-irogers@google.com?part=2

Re: [PATCH v2 2/2] perf maps: Fix fixup_overlap_and_insert that can break sorted by name order

[Ian Rogers]

On Tue, Apr 7, 2026 at 5:51 PM <sashiko-bot@kernel.org> wrote:
>
> Sashiko AI review found 1 potential issue(s):
> - [High] The maps__copy_from function corrupts the maps_by_name array by populating it in address order but flagging it as sorted by name.
>
> --
>
> commit bd0adc21836e1932db4199fc14353b8a08f98a0a
> Author: Ian Rogers <irogers@google.com>
>
> perf maps: Fix fixup_overlap_and_insert that can break sorted by name order
>
> When an entry in the address array is replaced, the corresponding name
> entry is replaced. The entries names may sort differently and so it is
> important that the sorted by name property be cleared on the maps.
>
> > diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
> > index 7dd6da9d1e4f0..b44bc41f51f3b 100644
> > --- a/tools/perf/util/maps.c
> > +++ b/tools/perf/util/maps.c
> [ ... ]
> > @@ -955,6 +955,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
> >                       if (maps_by_name) {
> >                               map__put(maps_by_name[ni]);
> >                               maps_by_name[ni] = map__get(new);
> > +                             maps__set_maps_by_name_sorted(maps, false);
> >                       }
> [ ... ]
>
> This isn't a bug in the newly added lines, but while reviewing the maps
> sorting logic, I noticed a regression in maps__copy_from().
>
> In maps__copy_from(), when copying maps from a parent to a destination
> (e.g., during thread fork), the dest_maps_by_name array is populated
> sequentially inside a loop iterating over parent_maps_by_address:
>
> tools/perf/util/maps.c:maps__copy_from() {
> [ ... ]
>                 for (unsigned int i = 0; !err && i < n; i++) {
>                         struct map *pos = parent_maps_by_address[i];
>                         struct map *new = map__clone(pos);
> [ ... ]
>                                 if (!err) {
>                                         dest_maps_by_address[i] = new;
>                                         map__set_kmap_maps(new, dest);
>                                         if (dest_maps_by_name)
>                                                 dest_maps_by_name[i] = map__get(new);
> [ ... ]
> }
>
> Because it clones and copies the elements in address order,
> dest_maps_by_name is strictly populated in address order.
>
> However, at the end of the fast-path copy block, the code executes:
>
> tools/perf/util/maps.c:maps__copy_from() {
> [ ... ]
>                 if (!err) {
>                         RC_CHK_ACCESS(dest)->last_search_by_name_idx =
>                                 RC_CHK_ACCESS(parent)->last_search_by_name_idx;
>                         maps__set_maps_by_name_sorted(dest,
>                                                 dest_maps_by_name &&
>                                                 maps__maps_by_name_sorted(parent));
>                 }
> [ ... ]
> }
>
> If the parent's name array was sorted, does this explicitly flag the new
> destination map's name array as sorted by name, even though it is physically
> sorted by address?
>
> Could this cause any subsequent lookup by name in the copied map
> (such as maps__find_by_name during child thread symbol resolution)
> to execute a binary search on an address-sorted array?
>
> Would this cause the binary search to take incorrect paths and fail to find
> legitimate mappings, leading to missing symbols or missing maps for
> child threads?

This is legitimate, I'll do a v3 with a fix for this issue as well.

Thanks,
Ian

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260408001521.102140-1-irogers@google.com?part=2

[PATCH v3 1/3] perf maps: Move getting debug_file to verbose path

[Ian Rogers]

Getting debug_file can trigger warnings if not set. Avoid getting
these warnings by pushing the use under the controlling if.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 4092211cff62..7dd6da9d1e4f 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -844,7 +844,6 @@ static int __maps__insert_sorted(struct maps *maps, unsigned int first_after_ind
 static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 {
 	int err = 0;
-	FILE *fp = debug_file();
 	unsigned int i, ni = INT_MAX; // Some gcc complain, but depends on maps_by_name...
 
 	if (!maps__maps_by_address_sorted(maps))
@@ -872,8 +871,8 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 				dso__name(map__dso(new)));
 		} else if (verbose >= 2) {
 			pr_debug("overlapping maps:\n");
-			map__fprintf(new, fp);
-			map__fprintf(pos, fp);
+			map__fprintf(new, debug_file());
+			map__fprintf(pos, debug_file());
 		}
 
 		if (maps_by_name)
@@ -894,7 +893,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			map__set_end(before, map__start(new));
 
 			if (verbose >= 2 && !use_browser)
-				map__fprintf(before, fp);
+				map__fprintf(before, debug_file());
 		}
 		if (map__end(new) < map__end(pos)) {
 			/* The new map isn't as long as the existing map. */
@@ -912,7 +911,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			       map__map_ip(after, map__end(new)));
 
 			if (verbose >= 2 && !use_browser)
-				map__fprintf(after, fp);
+				map__fprintf(after, debug_file());
 		}
 		/*
 		 * If adding one entry, for `before` or `after`, we can replace
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH v3 2/3] perf maps: Fix fixup_overlap_and_insert that can break sorted by name order

[Ian Rogers]

When an entry in the address array is replaced, the corresponding name
entry is replaced. The entries names may sort differently and so it is
important that the sorted by name property be cleared on the maps.

Fixes: 0d11fab32714 ("perf maps: Fixup maps_by_name when modifying maps_by_address")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 7dd6da9d1e4f..b44bc41f51f3 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -955,6 +955,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 			if (maps_by_name) {
 				map__put(maps_by_name[ni]);
 				maps_by_name[ni] = map__get(new);
+				maps__set_maps_by_name_sorted(maps, false);
 			}
 
 			err = __maps__insert_sorted(maps, i + 1, after, NULL);
@@ -981,6 +982,7 @@ static int __maps__fixup_overlap_and_insert(struct maps *maps, struct map *new)
 				if (maps_by_name) {
 					map__put(maps_by_name[ni]);
 					maps_by_name[ni] = map__get(new);
+					maps__set_maps_by_name_sorted(maps, false);
 				}
 
 				check_invariants(maps);
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH v3 3/3] perf maps: Fix copy_from that can break sorted by name order

[Ian Rogers]

When an parent is copied into a child the name array is populated in
address not name order. Make sure the name array isn't flagged as sorted.

Fixes: 659ad3492b91 ("perf maps: Switch from rbtree to lazily sorted array for addresses")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index b44bc41f51f3..81a97ac34077 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -1081,16 +1081,9 @@ int maps__copy_from(struct maps *dest, struct maps *parent)
 				map__put(new);
 		}
 		maps__set_maps_by_address_sorted(dest, maps__maps_by_address_sorted(parent));
-		if (!err) {
-			RC_CHK_ACCESS(dest)->last_search_by_name_idx =
-				RC_CHK_ACCESS(parent)->last_search_by_name_idx;
-			maps__set_maps_by_name_sorted(dest,
-						dest_maps_by_name &&
-						maps__maps_by_name_sorted(parent));
-		} else {
-			RC_CHK_ACCESS(dest)->last_search_by_name_idx = 0;
-			maps__set_maps_by_name_sorted(dest, false);
-		}
+		RC_CHK_ACCESS(dest)->last_search_by_name_idx = 0;
+		/* Values were copied into the name array in address order. */
+		maps__set_maps_by_name_sorted(dest, false);
 	} else {
 		/* Unexpected copying to a maps containing entries. */
 		for (unsigned int i = 0; !err && i < n; i++) {
-- 
2.53.0.1213.gd9a14994de-goog

Re: [PATCH v3 1/3] perf maps: Move getting debug_file to verbose path

[Namhyung Kim]

On Tue, 07 Apr 2026 19:08:36 -0700, Ian Rogers wrote:
> Getting debug_file can trigger warnings if not set. Avoid getting
> these warnings by pushing the use under the controlling if.
> 
> 
Applied to perf-tools-next, thanks!

Best regards,
Namhyung