* [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 @ 2023-06-12 15:04 James Clark 2023-06-12 16:32 ` Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: James Clark @ 2023-06-12 15:04 UTC (permalink / raw) To: linux-perf-users Cc: James Clark, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Ian Rogers, Adrian Hunter, linux-kernel When quitting after running a perf report, the refcount checker finds some double frees. The issue is that map__put() is called on a function argument so it removes the refcount wrapper that someone else was using. Fix it by only calling map__put() on a reference that is owned by this function. Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/util/symbol-elf.c | 9 +++++---- tools/perf/util/symbol.c | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 63882a4db5c7..ec0d7810bbb0 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1365,6 +1365,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, struct dso *curr_dso = *curr_dsop; struct map *curr_map; char dso_name[PATH_MAX]; + struct map *map_ref; /* Adjust symbol to map to file offset */ if (adjust_kernel_syms) @@ -1390,10 +1391,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, if (kmaps) { int err; - map__get(map); - maps__remove(kmaps, map); - err = maps__insert(kmaps, map); - map__put(map); + map_ref = map__get(map); + maps__remove(kmaps, map_ref); + err = maps__insert(kmaps, map_ref); + map__put(map_ref); if (err) return err; } diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 6b9c55784b56..b3034fd5c0af 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1368,6 +1368,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, int err, fd; char kcore_filename[PATH_MAX]; u64 stext; + struct map *map_ref; if (!kmaps) return -EINVAL; @@ -1464,10 +1465,10 @@ static int dso__load_kcore(struct dso *dso, struct map *map, map__set_map_ip(map, map__map_ip_ptr(new_map)); map__set_unmap_ip(map, map__unmap_ip_ptr(new_map)); /* Ensure maps are correctly ordered */ - map__get(map); - maps__remove(kmaps, map); - err = maps__insert(kmaps, map); - map__put(map); + map_ref = map__get(map); + maps__remove(kmaps, map_ref); + err = maps__insert(kmaps, map_ref); + map__put(map_ref); map__put(new_map); if (err) goto out_err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 2023-06-12 15:04 [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 James Clark @ 2023-06-12 16:32 ` Ian Rogers 2023-06-12 17:29 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Ian Rogers @ 2023-06-12 16:32 UTC (permalink / raw) To: James Clark Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel On Mon, Jun 12, 2023 at 8:05 AM James Clark <james.clark@arm.com> wrote: > > When quitting after running a perf report, the refcount checker finds > some double frees. The issue is that map__put() is called on a function > argument so it removes the refcount wrapper that someone else was using. > > Fix it by only calling map__put() on a reference that is owned by this > function. > > Signed-off-by: James Clark <james.clark@arm.com> Acked-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/symbol-elf.c | 9 +++++---- > tools/perf/util/symbol.c | 9 +++++---- > 2 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 63882a4db5c7..ec0d7810bbb0 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -1365,6 +1365,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > struct dso *curr_dso = *curr_dsop; > struct map *curr_map; > char dso_name[PATH_MAX]; > + struct map *map_ref; nit: can we narrow the scope of this by moving it to the scope where it is used. > > /* Adjust symbol to map to file offset */ > if (adjust_kernel_syms) > @@ -1390,10 +1391,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > if (kmaps) { > int err; > > - map__get(map); > - maps__remove(kmaps, map); > - err = maps__insert(kmaps, map); > - map__put(map); > + map_ref = map__get(map); > + maps__remove(kmaps, map_ref); > + err = maps__insert(kmaps, map_ref); > + map__put(map_ref); > if (err) > return err; > } > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 6b9c55784b56..b3034fd5c0af 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1368,6 +1368,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > int err, fd; > char kcore_filename[PATH_MAX]; > u64 stext; > + struct map *map_ref; nit: can we narrow the scope of this by moving it to the scope where it is used. Thanks, Ian > > if (!kmaps) > return -EINVAL; > @@ -1464,10 +1465,10 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > map__set_map_ip(map, map__map_ip_ptr(new_map)); > map__set_unmap_ip(map, map__unmap_ip_ptr(new_map)); > /* Ensure maps are correctly ordered */ > - map__get(map); > - maps__remove(kmaps, map); > - err = maps__insert(kmaps, map); > - map__put(map); > + map_ref = map__get(map); > + maps__remove(kmaps, map_ref); > + err = maps__insert(kmaps, map_ref); > + map__put(map_ref); > map__put(new_map); > if (err) > goto out_err; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 2023-06-12 16:32 ` Ian Rogers @ 2023-06-12 17:29 ` Arnaldo Carvalho de Melo 2023-06-12 17:40 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-06-12 17:29 UTC (permalink / raw) To: Ian Rogers Cc: James Clark, linux-perf-users, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel Em Mon, Jun 12, 2023 at 09:32:30AM -0700, Ian Rogers escreveu: > On Mon, Jun 12, 2023 at 8:05 AM James Clark <james.clark@arm.com> wrote: > > > > When quitting after running a perf report, the refcount checker finds > > some double frees. The issue is that map__put() is called on a function > > argument so it removes the refcount wrapper that someone else was using. > > > > Fix it by only calling map__put() on a reference that is owned by this > > function. > > > > Signed-off-by: James Clark <james.clark@arm.com> > > Acked-by: Ian Rogers <irogers@google.com> > > > --- > > tools/perf/util/symbol-elf.c | 9 +++++---- > > tools/perf/util/symbol.c | 9 +++++---- > > 2 files changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > > index 63882a4db5c7..ec0d7810bbb0 100644 > > --- a/tools/perf/util/symbol-elf.c > > +++ b/tools/perf/util/symbol-elf.c > > @@ -1365,6 +1365,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > > struct dso *curr_dso = *curr_dsop; > > struct map *curr_map; > > char dso_name[PATH_MAX]; > > + struct map *map_ref; > > nit: can we narrow the scope of this by moving it to the scope where it is used. Which is what you did in a patch I already processed, its only in tmp.perf-tools-next as I was going thru the other patches, but this one is there already. I'm checking the tools/perf/util/symbol.c part. - Arnaldo > > > > /* Adjust symbol to map to file offset */ > > if (adjust_kernel_syms) > > @@ -1390,10 +1391,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > > if (kmaps) { > > int err; > > > > - map__get(map); > > - maps__remove(kmaps, map); > > - err = maps__insert(kmaps, map); > > - map__put(map); > > + map_ref = map__get(map); > > + maps__remove(kmaps, map_ref); > > + err = maps__insert(kmaps, map_ref); > > + map__put(map_ref); > > if (err) > > return err; > > } > > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > > index 6b9c55784b56..b3034fd5c0af 100644 > > --- a/tools/perf/util/symbol.c > > +++ b/tools/perf/util/symbol.c > > @@ -1368,6 +1368,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > > int err, fd; > > char kcore_filename[PATH_MAX]; > > u64 stext; > > + struct map *map_ref; > > nit: can we narrow the scope of this by moving it to the scope where it is used. > > Thanks, > Ian > > > > > if (!kmaps) > > return -EINVAL; > > @@ -1464,10 +1465,10 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > > map__set_map_ip(map, map__map_ip_ptr(new_map)); > > map__set_unmap_ip(map, map__unmap_ip_ptr(new_map)); > > /* Ensure maps are correctly ordered */ > > - map__get(map); > > - maps__remove(kmaps, map); > > - err = maps__insert(kmaps, map); > > - map__put(map); > > + map_ref = map__get(map); > > + maps__remove(kmaps, map_ref); > > + err = maps__insert(kmaps, map_ref); > > + map__put(map_ref); > > map__put(new_map); > > if (err) > > goto out_err; > > -- > > 2.34.1 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 2023-06-12 17:29 ` Arnaldo Carvalho de Melo @ 2023-06-12 17:40 ` Arnaldo Carvalho de Melo 2023-06-12 17:59 ` Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-06-12 17:40 UTC (permalink / raw) To: Ian Rogers Cc: James Clark, linux-perf-users, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel Em Mon, Jun 12, 2023 at 02:29:42PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Mon, Jun 12, 2023 at 09:32:30AM -0700, Ian Rogers escreveu: > > On Mon, Jun 12, 2023 at 8:05 AM James Clark <james.clark@arm.com> wrote: > > > > > > When quitting after running a perf report, the refcount checker finds > > > some double frees. The issue is that map__put() is called on a function > > > argument so it removes the refcount wrapper that someone else was using. > > > > > > Fix it by only calling map__put() on a reference that is owned by this > > > function. > > > > > > Signed-off-by: James Clark <james.clark@arm.com> > > > > Acked-by: Ian Rogers <irogers@google.com> > > > > > --- > > > tools/perf/util/symbol-elf.c | 9 +++++---- > > > tools/perf/util/symbol.c | 9 +++++---- > > > 2 files changed, 10 insertions(+), 8 deletions(-) > > > > > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > > > index 63882a4db5c7..ec0d7810bbb0 100644 > > > --- a/tools/perf/util/symbol-elf.c > > > +++ b/tools/perf/util/symbol-elf.c > > > @@ -1365,6 +1365,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > > > struct dso *curr_dso = *curr_dsop; > > > struct map *curr_map; > > > char dso_name[PATH_MAX]; > > > + struct map *map_ref; > > > > nit: can we narrow the scope of this by moving it to the scope where it is used. > > Which is what you did in a patch I already processed, its only in > tmp.perf-tools-next as I was going thru the other patches, but this one > is there already. > > I'm checking the tools/perf/util/symbol.c part. I narrowed the scope and removed the symbol-elf.c part, end result: From 6fd34445b8c94aa7f519fb0b1ed45c7ef9f6cc4e Mon Sep 17 00:00:00 2001 From: James Clark <james.clark@arm.com> Date: Mon, 12 Jun 2023 16:04:24 +0100 Subject: [PATCH 1/1] perf map: Fix double 'struct map' reference free found with -DREFCNT_CHECKING=1 When quitting after running a 'perf report', the refcount checker finds some double frees. The issue is that map__put() is called on a function argument so it removes the refcount wrapper that someone else was using. Fix it by only calling map__put() on a reference that is owned by this function. Committer notes: Narrowed the map_ref scope as suggested by Ian, removed the symbol-elf part as it was already fixed by another patch, from Ian. Signed-off-by: James Clark <james.clark@arm.com> Acked-by: Ian Rogers <irogers@google.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: https://lore.kernel.org/r/20230612150424.198914-1-james.clark@arm.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/symbol.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 6b9c55784b56a4be..d275d3bef7d54a40 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1458,16 +1458,18 @@ static int dso__load_kcore(struct dso *dso, struct map *map, list_del_init(&new_node->node); if (RC_CHK_ACCESS(new_map) == RC_CHK_ACCESS(replacement_map)) { + struct map *map_ref; + map__set_start(map, map__start(new_map)); map__set_end(map, map__end(new_map)); map__set_pgoff(map, map__pgoff(new_map)); map__set_map_ip(map, map__map_ip_ptr(new_map)); map__set_unmap_ip(map, map__unmap_ip_ptr(new_map)); /* Ensure maps are correctly ordered */ - map__get(map); - maps__remove(kmaps, map); - err = maps__insert(kmaps, map); - map__put(map); + map_ref = map__get(map); + maps__remove(kmaps, map_ref); + err = maps__insert(kmaps, map_ref); + map__put(map_ref); map__put(new_map); if (err) goto out_err; -- 2.37.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 2023-06-12 17:40 ` Arnaldo Carvalho de Melo @ 2023-06-12 17:59 ` Ian Rogers 0 siblings, 0 replies; 5+ messages in thread From: Ian Rogers @ 2023-06-12 17:59 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: James Clark, linux-perf-users, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel On Mon, Jun 12, 2023 at 10:40 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Mon, Jun 12, 2023 at 02:29:42PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Mon, Jun 12, 2023 at 09:32:30AM -0700, Ian Rogers escreveu: > > > On Mon, Jun 12, 2023 at 8:05 AM James Clark <james.clark@arm.com> wrote: > > > > > > > > When quitting after running a perf report, the refcount checker finds > > > > some double frees. The issue is that map__put() is called on a function > > > > argument so it removes the refcount wrapper that someone else was using. > > > > > > > > Fix it by only calling map__put() on a reference that is owned by this > > > > function. > > > > > > > > Signed-off-by: James Clark <james.clark@arm.com> > > > > > > Acked-by: Ian Rogers <irogers@google.com> > > > > > > > --- > > > > tools/perf/util/symbol-elf.c | 9 +++++---- > > > > tools/perf/util/symbol.c | 9 +++++---- > > > > 2 files changed, 10 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > > > > index 63882a4db5c7..ec0d7810bbb0 100644 > > > > --- a/tools/perf/util/symbol-elf.c > > > > +++ b/tools/perf/util/symbol-elf.c > > > > @@ -1365,6 +1365,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, > > > > struct dso *curr_dso = *curr_dsop; > > > > struct map *curr_map; > > > > char dso_name[PATH_MAX]; > > > > + struct map *map_ref; > > > > > > nit: can we narrow the scope of this by moving it to the scope where it is used. > > > > Which is what you did in a patch I already processed, its only in > > tmp.perf-tools-next as I was going thru the other patches, but this one > > is there already. > > > > I'm checking the tools/perf/util/symbol.c part. > > I narrowed the scope and removed the symbol-elf.c part, end result: > > From 6fd34445b8c94aa7f519fb0b1ed45c7ef9f6cc4e Mon Sep 17 00:00:00 2001 > From: James Clark <james.clark@arm.com> > Date: Mon, 12 Jun 2023 16:04:24 +0100 > Subject: [PATCH 1/1] perf map: Fix double 'struct map' reference free found > with -DREFCNT_CHECKING=1 > > When quitting after running a 'perf report', the refcount checker finds > some double frees. The issue is that map__put() is called on a function > argument so it removes the refcount wrapper that someone else was using. > > Fix it by only calling map__put() on a reference that is owned by this > function. > > Committer notes: > > Narrowed the map_ref scope as suggested by Ian, removed the symbol-elf > part as it was already fixed by another patch, from Ian. > > Signed-off-by: James Clark <james.clark@arm.com> > Acked-by: Ian Rogers <irogers@google.com> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Link: https://lore.kernel.org/r/20230612150424.198914-1-james.clark@arm.com > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> Thanks Arnaldo! I think we should be able to automate finding these issues with the warn_unused_result function attribute: ``` diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index 66a87b3d9965..2c77c28ff000 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -172,7 +172,7 @@ struct map *map__new2(u64 start, struct dso *dso); void map__delete(struct map *map); struct map *map__clone(struct map *map); -static inline struct map *map__get(struct map *map) +__attribute__ ((warn_unused_result)) static inline struct map *map__get(struct map *map) { struct map *result; diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h index 83144e0645ed..5b74465316dd 100644 --- a/tools/perf/util/maps.h +++ b/tools/perf/util/maps.h @@ -60,7 +60,7 @@ struct maps *maps__new(struct machine *machine); bool maps__empty(struct maps *maps); int maps__clone(struct thread *thread, struct maps *parent); -struct maps *maps__get(struct maps *maps); +struct maps *maps__get(struct maps *maps) __attribute__ ((warn_unused_result)); void maps__put(struct maps *maps); static inline void __maps__zput(struct maps **map) diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h index 8c0731c6cbb7..04e1878b9551 100644 --- a/tools/perf/util/namespaces.h +++ b/tools/perf/util/namespaces.h @@ -50,7 +50,7 @@ int nsinfo__init(struct nsinfo *nsi); struct nsinfo *nsinfo__new(pid_t pid); struct nsinfo *nsinfo__copy(const struct nsinfo *nsi); -struct nsinfo *nsinfo__get(struct nsinfo *nsi); +struct nsinfo *nsinfo__get(struct nsinfo *nsi) __attribute__ ((warn_unused_result)); void nsinfo__put(struct nsinfo *nsi); bool nsinfo__need_setns(const struct nsinfo *nsi); diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index 9068a21ce0fa..c6228252b093 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -71,7 +71,7 @@ struct thread *thread__new(pid_t pid, pid_t tid); irogers@irogers-glaptop0:~/kernel.org$ git diff diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index 66a87b3d9965..2c77c28ff000 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -172,7 +172,7 @@ struct map *map__new2(u64 start, struct dso *dso); void map__delete(struct map *map); struct map *map__clone(struct map *map); -static inline struct map *map__get(struct map *map) +__attribute__ ((warn_unused_result)) static inline struct map *map__get(struct map *map) { struct map *result; diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h index 83144e0645ed..5b74465316dd 100644 --- a/tools/perf/util/maps.h +++ b/tools/perf/util/maps.h @@ -60,7 +60,7 @@ struct maps *maps__new(struct machine *machine); bool maps__empty(struct maps *maps); int maps__clone(struct thread *thread, struct maps *parent); -struct maps *maps__get(struct maps *maps); +struct maps *maps__get(struct maps *maps) __attribute__ ((warn_unused_result)); void maps__put(struct maps *maps); static inline void __maps__zput(struct maps **map) diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h index 8c0731c6cbb7..04e1878b9551 100644 --- a/tools/perf/util/namespaces.h +++ b/tools/perf/util/namespaces.h @@ -50,7 +50,7 @@ int nsinfo__init(struct nsinfo *nsi); struct nsinfo *nsinfo__new(pid_t pid); struct nsinfo *nsinfo__copy(const struct nsinfo *nsi); -struct nsinfo *nsinfo__get(struct nsinfo *nsi); +struct nsinfo *nsinfo__get(struct nsinfo *nsi) __attribute__ ((warn_unused_result)); void nsinfo__put(struct nsinfo *nsi); bool nsinfo__need_setns(const struct nsinfo *nsi); diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index 9068a21ce0fa..c6228252b093 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -71,7 +71,7 @@ struct thread *thread__new(pid_t pid, pid_t tid); int thread__init_maps(struct thread *thread, struct machine *machine); void thread__delete(struct thread *thread); -struct thread *thread__get(struct thread *thread); +struct thread *thread__get(struct thread *thread) __attribute__ ((warn_unused_result)); void thread__put(struct thread *thread); static inline void __thread__zput(struct thread **thread) ``` This shows the problem like: ``` util/symbol.c: In function ‘dso__load_kcore’: util/symbol.c:1467:25: error: ignoring return value of ‘map__get’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result] 1467 | map__get(map); | ``` I double checked and the symbol.c issue was the only one in my build environment. Using warn_unused_result should be done via compiler.h which is a bit more than the patch above. Thanks, Ian > --- > tools/perf/util/symbol.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 6b9c55784b56a4be..d275d3bef7d54a40 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1458,16 +1458,18 @@ static int dso__load_kcore(struct dso *dso, struct map *map, > list_del_init(&new_node->node); > > if (RC_CHK_ACCESS(new_map) == RC_CHK_ACCESS(replacement_map)) { > + struct map *map_ref; > + > map__set_start(map, map__start(new_map)); > map__set_end(map, map__end(new_map)); > map__set_pgoff(map, map__pgoff(new_map)); > map__set_map_ip(map, map__map_ip_ptr(new_map)); > map__set_unmap_ip(map, map__unmap_ip_ptr(new_map)); > /* Ensure maps are correctly ordered */ > - map__get(map); > - maps__remove(kmaps, map); > - err = maps__insert(kmaps, map); > - map__put(map); > + map_ref = map__get(map); > + maps__remove(kmaps, map_ref); > + err = maps__insert(kmaps, map_ref); > + map__put(map_ref); > map__put(new_map); > if (err) > goto out_err; > -- > 2.37.1 > ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-12 18:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-12 15:04 [PATCH] perf map: Fix refcount errors on Arm with -DREFCNT_CHECKING=1 James Clark 2023-06-12 16:32 ` Ian Rogers 2023-06-12 17:29 ` Arnaldo Carvalho de Melo 2023-06-12 17:40 ` Arnaldo Carvalho de Melo 2023-06-12 17:59 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox