[PATCH 00/20] perf: fix several memory leaks reported by ASan on perf-test

[PATCH 00/20] perf: fix several memory leaks reported by ASan on perf-test

[Riccardo Mancini]

This patchset fixes several memory leaks found using ASan on perf-test. 
After this patch, all memory leaks in perf-test that were found on my machine 
are fixed.
This does not mean that there are no longer memory leaks in perf (just running
perf report or perf top raises a few), but that now it is possible to run the 
perf tests with ASan to catch future memory errors.

While many of these leaks have little to no effect on the daily use of perf 
(some were inside the test itself, other were just a missing cleanup on exit), 
other were (slightly) more serious, namely:
 - nsinfo refcounting was broken since it was not decreased when a reference was
   replaced, causing many nsinfos to never being deallocated.
 - dso__new_map refcounting was wrong, missing a __put
 - lzma stream was never closed

Below are the results of perf-test before and after this patchset. Only failed
and skipped tests are reported for brevity.

The perf binary has been compiled using
make DEBUG=1 EXTRA_CFLAGS='-fno-omit-frame-pointer -fsanitize=address'
compilation using clang shows same results.

Tests are run on my laptop: 
Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz (2 cores + hyperthreading)
Fedora 34 w/ kernel 5.12.14-300.fc34.x86_64

Before this patchset:
 1: vmlinux symtab matches kallsyms                                 : FAILED!
19: 'import perf' in python                                         : FAILED!
23.1: Read Only Watchpoint                                          : Skip (missing hardware support)
31: Lookup mmap thread                                              : FAILED!
41: Session topology                                                : FAILED!
42.1: Basic BPF filtering                                           : FAILED!
42.2: BPF pinning                                                   : Skip
42.3: BPF prologue generation                                       : Skip
49: Synthesize attr update                                          : FAILED!
58: builtin clang support                                           : Skip (not compiled in)
63: Test libpfm4 support                                            : Skip (not compiled in)
65: maps__merge_in                                                  : FAILED!
73: DWARF unwind                                                    : FAILED!
78: build id cache operations                                       : FAILED!
82: Use vfs_getname probe to get syscall args filenames             : FAILED!
83: Zstd perf.data compression/decompression                        : FAILED!
86: perf stat --bpf-counters test                                   : Skip
87: Check Arm CoreSight trace data recording and synthesized samples: Skip
88: Check open filename arg using perf trace + vfs_getname          : FAILED!

After this patchset:
19: 'import perf' in python                                         : FAILED!
23.1: Read Only Watchpoint                                          : Skip (missing hardware support)
58: builtin clang support                                           : Skip (not compiled in)
63: Test libpfm4 support                                            : Skip (not compiled in)
73: DWARF unwind                                                    : FAILED!
86: perf stat --bpf-counters test                                   : Skip
87: Check Arm CoreSight trace data recording and synthesized samples: Skip

After this patchset, without ASan:
23.1: Read Only Watchpoint                                          : Skip (missing hardware support)
58: builtin clang support                                           : Skip (not compiled in)
63: Test libpfm4 support                                            : Skip (not compiled in)
86: perf stat --bpf-counters test                                   : Skip
87: Check Arm CoreSight trace data recording and synthesized samples: Skip

The two failing tests are false positives and can be skipped:
 - "19: 'import perf' in python" fails since there are memory leaks inside the 
   python library.
 - "73: DWARF unwind" fails due to stack buffer underflow, since it's unwinding
   the stack.

Riccardo

Riccardo Mancini (20):
  perf nsinfo: fix refcounting
  perf env: fix sibling_dies memory leak
  perf test: session_topology: delete session->evlist
  perf test: event_update: fix memory leak of evlist
  perf test: event_update: fix memory leak of unit
  perf dso: fix memory leak in dso__new_map
  perf test: maps__merge_in: fix memory leak of maps
  perf env: fix memory leak of cpu_pmu_caps
  perf report: free generated help strings for sort option
  perf inject: close inject.output
  perf session: cleanup trace_event
  perf script: release zstd data
  perf script: fix memory leaks in perf_script
  perf util/lzma: close lzma stream
  perf trace: free malloc'd trace fields on exit
  perf trace: free syscall->arg_fmt
  perf trace: free syscall tp fields in evsel->priv
  perf trace: free strings in trace__parse_events_option
  perf test: bpf: free obj_buf
  perf util/probe-file: delete namelist on error in del_events

 tools/perf/builtin-inject.c     | 13 ++++++---
 tools/perf/builtin-report.c     | 33 ++++++++++++++-------
 tools/perf/builtin-script.c     |  8 ++++++
 tools/perf/builtin-trace.c      | 51 +++++++++++++++++++++++++++++++--
 tools/perf/tests/bpf.c          |  1 +
 tools/perf/tests/event_update.c |  6 ++--
 tools/perf/tests/maps.c         |  2 ++
 tools/perf/tests/topology.c     |  1 +
 tools/perf/util/dso.c           |  4 ++-
 tools/perf/util/env.c           |  2 ++
 tools/perf/util/lzma.c          |  8 ++++--
 tools/perf/util/map.c           |  2 ++
 tools/perf/util/probe-event.c   |  4 ++-
 tools/perf/util/probe-file.c    |  4 +--
 tools/perf/util/session.c       |  1 +
 tools/perf/util/sort.c          |  2 +-
 tools/perf/util/sort.h          |  2 +-
 17 files changed, 116 insertions(+), 28 deletions(-)

-- 
2.31.1

[PATCH 01/20] perf nsinfo: fix refcounting

[Riccardo Mancini]

ASan reports a memory leak of nsinfo during the execution of the perf
test "31: Lookup mmap thread".
The leak is caused by a refcounted variable being replaced without
dropping the refcount.

This patch makes sure that the refcnt of nsinfo is decreased whenever
a refcounted variable is replaced with a new value.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-inject.c   | 5 +++--
 tools/perf/util/map.c         | 2 ++
 tools/perf/util/probe-event.c | 4 +++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 5d6f583e2cd35be0..ffd2b25039e36e1d 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -361,9 +361,10 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename,
 		dso = machine__findnew_dso_id(machine, filename, id);
 	}
 
-	if (dso)
+	if (dso) {
+		nsinfo__put(dso->nsinfo);
 		dso->nsinfo = nsi;
-	else
+	} else
 		nsinfo__put(nsi);
 
 	thread__put(thread);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 8af693d9678cefe0..72e7f3616157ead4 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
 			if (!(prot & PROT_EXEC))
 				dso__set_loaded(dso);
 		}
+
+		nsinfo__put(dso->nsinfo);
 		dso->nsinfo = nsi;
 
 		if (build_id__is_defined(bid))
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c14e1d228e56b1c6..e05750cea34c3a95 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -179,8 +179,10 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user)
 		struct map *map;
 
 		map = dso__new_map(target);
-		if (map && map->dso)
+		if (map && map->dso) {
+			nsinfo__put(map->dso->nsinfo);
 			map->dso->nsinfo = nsinfo__get(nsi);
+		}
 		return map;
 	} else {
 		return kernel_get_module_map(target);
-- 
2.31.1

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak of nsinfo during the execution of the perf
> test "31: Lookup mmap thread".
> The leak is caused by a refcounted variable being replaced without
> dropping the refcount.
> 
> This patch makes sure that the refcnt of nsinfo is decreased whenever
> a refcounted variable is replaced with a new value.

So, there are multiple fixes in just one patch, I'll split it into
three, no need to resend.

I'll try and check if finding Fixes: for the three is easy, that way
stable@vger.kernel.org will figure out which of the supported releases
need each of them.

- Arnaldo
 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-inject.c   | 5 +++--
>  tools/perf/util/map.c         | 2 ++
>  tools/perf/util/probe-event.c | 4 +++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 5d6f583e2cd35be0..ffd2b25039e36e1d 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -361,9 +361,10 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename,
>  		dso = machine__findnew_dso_id(machine, filename, id);
>  	}
>  
> -	if (dso)
> +	if (dso) {
> +		nsinfo__put(dso->nsinfo);
>  		dso->nsinfo = nsi;
> -	else
> +	} else
>  		nsinfo__put(nsi);
>  
>  	thread__put(thread);
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 8af693d9678cefe0..72e7f3616157ead4 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>  			if (!(prot & PROT_EXEC))
>  				dso__set_loaded(dso);
>  		}
> +
> +		nsinfo__put(dso->nsinfo);
>  		dso->nsinfo = nsi;
>  
>  		if (build_id__is_defined(bid))
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index c14e1d228e56b1c6..e05750cea34c3a95 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -179,8 +179,10 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user)
>  		struct map *map;
>  
>  		map = dso__new_map(target);
> -		if (map && map->dso)
> +		if (map && map->dso) {
> +			nsinfo__put(map->dso->nsinfo);
>  			map->dso->nsinfo = nsinfo__get(nsi);
> +		}
>  		return map;
>  	} else {
>  		return kernel_get_module_map(target);
> -- 
> 2.31.1
> 

-- 

- Arnaldo

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > ASan reports a memory leak of nsinfo during the execution of the perf
> > test "31: Lookup mmap thread".
> > The leak is caused by a refcounted variable being replaced without
> > dropping the refcount.
> > 
> > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > a refcounted variable is replaced with a new value.
> 
> So, there are multiple fixes in just one patch, I'll split it into
> three, no need to resend.
> 
> I'll try and check if finding Fixes: for the three is easy, that way
> stable@vger.kernel.org will figure out which of the supported releases
> need each of them.

The first patch becomes:

commit b36bbdefec297ee5b6e278904360ad14fa3b09de
Author: Riccardo Mancini <rickyman7@gmail.com>
Date:   Thu Jul 15 18:07:06 2021 +0200

    perf inject: Fix dso->nsinfo refcounting
    
    ASan reports a memory leak of nsinfo during the execution of the perf
    test "31: Lookup mmap thread".
    
    The leak is caused by a refcounted variable being replaced without
    dropping the refcount.
    
    This patch makes sure that the refcnt of nsinfo is decreased when a
    refcounted variable is replaced with a new value.
    
    Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
    Fixes: 27c9c3424fc217da ("perf inject: Add --buildid-all option")
    Cc: Ian Rogers <irogers@google.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
    [ Split from a larger patch ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 5d6f583e2cd35be0..ffd2b25039e36e1d 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -361,9 +361,10 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename,
 		dso = machine__findnew_dso_id(machine, filename, id);
 	}
 
-	if (dso)
+	if (dso) {
+		nsinfo__put(dso->nsinfo);
 		dso->nsinfo = nsi;
-	else
+	} else
 		nsinfo__put(nsi);
 
 	thread__put(thread);

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > ASan reports a memory leak of nsinfo during the execution of the perf
> > test "31: Lookup mmap thread".
> > The leak is caused by a refcounted variable being replaced without
> > dropping the refcount.

> > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > a refcounted variable is replaced with a new value.

> So, there are multiple fixes in just one patch, I'll split it into
> three, no need to resend.

> I'll try and check if finding Fixes: for the three is easy, that way
> stable@vger.kernel.org will figure out which of the supported releases
> need each of them.

The second is at the end of this message, and looking at it I think
there is a problem with where this nsinfo reference is stored, it should
be in the 'struct map', that is per-thread, not in the 'struct dso',
that may be shared by multiple 'struct map's in different cgroups, so we
can't set dso->nsinfo in map__new(), right? Kristen?

- Arnaldo

commit 280f3933661f9ca9563ed04ce82313fdbcbb8f75
Author: Riccardo Mancini <rickyman7@gmail.com>
Date:   Thu Jul 15 18:07:06 2021 +0200

    perf map: Fix dso->nsinfo refcounting
    
    ASan reports a memory leak of nsinfo during the execution of
    
      # perf test "31: Lookup mmap thread"
    
    The leak is caused by a refcounted variable being replaced without
    dropping the refcount.
    
    This patch makes sure that the refcnt of nsinfo is decreased whenever a
    refcounted variable is replaced with a new value.
    
    Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
    Fixes: bf2e710b3cb8445c ("perf maps: Lookup maps in both intitial mountns and inner mountns.")
    Cc: Ian Rogers <irogers@google.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Krister Johansen <kjlx@templeofstupid.com>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 8af693d9678cefe0..72e7f3616157ead4 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
 			if (!(prot & PROT_EXEC))
 				dso__set_loaded(dso);
 		}
+
+		nsinfo__put(dso->nsinfo);
 		dso->nsinfo = nsi;
 
 		if (build_id__is_defined(bid))

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Krister Johansen]

On Thu, Jul 15, 2021 at 04:20:24PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > > ASan reports a memory leak of nsinfo during the execution of the perf
> > > test "31: Lookup mmap thread".
> > > The leak is caused by a refcounted variable being replaced without
> > > dropping the refcount.
> 
> > > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > > a refcounted variable is replaced with a new value.
> 
> > So, there are multiple fixes in just one patch, I'll split it into
> > three, no need to resend.
> 
> > I'll try and check if finding Fixes: for the three is easy, that way
> > stable@vger.kernel.org will figure out which of the supported releases
> > need each of them.
> 
> The second is at the end of this message, and looking at it I think
> there is a problem with where this nsinfo reference is stored, it should
> be in the 'struct map', that is per-thread, not in the 'struct dso',
> that may be shared by multiple 'struct map's in different cgroups, so we
> can't set dso->nsinfo in map__new(), right? Kristen?

Apologies for the long delayed reply, and for the hassle that this has
caused.

You're right, Arnaldo, that namespace mapping could differ on a per
thread basis.  If the dso objects are intended to be read by multiple
threads with different views of the target process's namespaces.  With
containers, though, there might be many different versions of the same
object with the same filename, and the only ways we can disambiguate are
through the build-ids or by ensuring that the dso refers to a single
pid and mount namespace.  I may have erred in assuming that all of
perf's threads would be able to access the target namespace via the same
identifiers.  Is that what's going on here?

-K


> commit 280f3933661f9ca9563ed04ce82313fdbcbb8f75
> Author: Riccardo Mancini <rickyman7@gmail.com>
> Date:   Thu Jul 15 18:07:06 2021 +0200
> 
>     perf map: Fix dso->nsinfo refcounting
>     
>     ASan reports a memory leak of nsinfo during the execution of
>     
>       # perf test "31: Lookup mmap thread"
>     
>     The leak is caused by a refcounted variable being replaced without
>     dropping the refcount.
>     
>     This patch makes sure that the refcnt of nsinfo is decreased whenever a
>     refcounted variable is replaced with a new value.
>     
>     Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
>     Fixes: bf2e710b3cb8445c ("perf maps: Lookup maps in both intitial mountns and inner mountns.")
>     Cc: Ian Rogers <irogers@google.com>
>     Cc: Jiri Olsa <jolsa@redhat.com>
>     Cc: Krister Johansen <kjlx@templeofstupid.com>
>     Cc: Mark Rutland <mark.rutland@arm.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 8af693d9678cefe0..72e7f3616157ead4 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>  			if (!(prot & PROT_EXEC))
>  				dso__set_loaded(dso);
>  		}
> +
> +		nsinfo__put(dso->nsinfo);
>  		dso->nsinfo = nsi;
>  
>  		if (build_id__is_defined(bid))
> 

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Tue, Jan 04, 2022 at 10:19:42PM -0800, Krister Johansen escreveu:
> On Thu, Jul 15, 2021 at 04:20:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > > > ASan reports a memory leak of nsinfo during the execution of the perf
> > > > test "31: Lookup mmap thread".
> > > > The leak is caused by a refcounted variable being replaced without
> > > > dropping the refcount.
> > 
> > > > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > > > a refcounted variable is replaced with a new value.
> > 
> > > So, there are multiple fixes in just one patch, I'll split it into
> > > three, no need to resend.
> > 
> > > I'll try and check if finding Fixes: for the three is easy, that way
> > > stable@vger.kernel.org will figure out which of the supported releases
> > > need each of them.
> > 
> > The second is at the end of this message, and looking at it I think
> > there is a problem with where this nsinfo reference is stored, it should
> > be in the 'struct map', that is per-thread, not in the 'struct dso',
> > that may be shared by multiple 'struct map's in different cgroups, so we
> > can't set dso->nsinfo in map__new(), right? Kristen?
> 
> Apologies for the long delayed reply, and for the hassle that this has
> caused.
> 
> You're right, Arnaldo, that namespace mapping could differ on a per
> thread basis.  If the dso objects are intended to be read by multiple
> threads with different views of the target process's namespaces.  With
> containers, though, there might be many different versions of the same
> object with the same filename, and the only ways we can disambiguate are
> through the build-ids or by ensuring that the dso refers to a single
> pid and mount namespace.  I may have erred in assuming that all of
> perf's threads would be able to access the target namespace via the same
> identifiers.  Is that what's going on here?

yeah, I think using build-ids, when available is the way to go, and
since we can almost take it for granted (haven't checked) that it will
be present, as I think gcc/clang adds them by default, seems the way to
go.

That or somehow have the DSO list to be perf namespace.

- Arnaldo
 
> -K
> 
> 
> > commit 280f3933661f9ca9563ed04ce82313fdbcbb8f75
> > Author: Riccardo Mancini <rickyman7@gmail.com>
> > Date:   Thu Jul 15 18:07:06 2021 +0200
> > 
> >     perf map: Fix dso->nsinfo refcounting
> >     
> >     ASan reports a memory leak of nsinfo during the execution of
> >     
> >       # perf test "31: Lookup mmap thread"
> >     
> >     The leak is caused by a refcounted variable being replaced without
> >     dropping the refcount.
> >     
> >     This patch makes sure that the refcnt of nsinfo is decreased whenever a
> >     refcounted variable is replaced with a new value.
> >     
> >     Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> >     Fixes: bf2e710b3cb8445c ("perf maps: Lookup maps in both intitial mountns and inner mountns.")
> >     Cc: Ian Rogers <irogers@google.com>
> >     Cc: Jiri Olsa <jolsa@redhat.com>
> >     Cc: Krister Johansen <kjlx@templeofstupid.com>
> >     Cc: Mark Rutland <mark.rutland@arm.com>
> >     Cc: Namhyung Kim <namhyung@kernel.org>
> >     Cc: Peter Zijlstra <peterz@infradead.org>
> >     Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index 8af693d9678cefe0..72e7f3616157ead4 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> >  			if (!(prot & PROT_EXEC))
> >  				dso__set_loaded(dso);
> >  		}
> > +
> > +		nsinfo__put(dso->nsinfo);
> >  		dso->nsinfo = nsi;
> >  
> >  		if (build_id__is_defined(bid))
> > 

-- 

- Arnaldo

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Thu, Jan 06, 2022 at 05:27:43PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 04, 2022 at 10:19:42PM -0800, Krister Johansen escreveu:
> > On Thu, Jul 15, 2021 at 04:20:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > > > > ASan reports a memory leak of nsinfo during the execution of the perf
> > > > > test "31: Lookup mmap thread".
> > > > > The leak is caused by a refcounted variable being replaced without
> > > > > dropping the refcount.
> > > 
> > > > > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > > > > a refcounted variable is replaced with a new value.
> > > 
> > > > So, there are multiple fixes in just one patch, I'll split it into
> > > > three, no need to resend.
> > > 
> > > > I'll try and check if finding Fixes: for the three is easy, that way
> > > > stable@vger.kernel.org will figure out which of the supported releases
> > > > need each of them.
> > > 
> > > The second is at the end of this message, and looking at it I think
> > > there is a problem with where this nsinfo reference is stored, it should
> > > be in the 'struct map', that is per-thread, not in the 'struct dso',
> > > that may be shared by multiple 'struct map's in different cgroups, so we
> > > can't set dso->nsinfo in map__new(), right? Kristen?
> > 
> > Apologies for the long delayed reply, and for the hassle that this has
> > caused.
> > 
> > You're right, Arnaldo, that namespace mapping could differ on a per
> > thread basis.  If the dso objects are intended to be read by multiple
> > threads with different views of the target process's namespaces.  With
> > containers, though, there might be many different versions of the same
> > object with the same filename, and the only ways we can disambiguate are
> > through the build-ids or by ensuring that the dso refers to a single
> > pid and mount namespace.  I may have erred in assuming that all of
> > perf's threads would be able to access the target namespace via the same
> > identifiers.  Is that what's going on here?
> 
> yeah, I think using build-ids, when available is the way to go, and
> since we can almost take it for granted (haven't checked) that it will
> be present, as I think gcc/clang adds them by default, seems the way to
> go.
> 
> That or somehow have the DSO list to be perf namespace.

To be by namespace, sorry.

But having it by build-id seems the more sensible way to go, so that we
can avoid having multiple instances of the same DSO in multiple
namespaces, which should be common.

I.e. continue to have a perf 'struct machine' DSO list instead of moving
to having the namespace hierarchy represented under 'struct machine'
since we have a way to uniquify DSOs, the build id.

One way or the other, nsinfo shouldn't be stored in DSOs, its a map
property, which in turn is a thread property.

I.e. please consider thinking about moving it to 'struct map', would you
try it, please?

- Arnaldo
 
> - Arnaldo
>  
> > -K
> > 
> > 
> > > commit 280f3933661f9ca9563ed04ce82313fdbcbb8f75
> > > Author: Riccardo Mancini <rickyman7@gmail.com>
> > > Date:   Thu Jul 15 18:07:06 2021 +0200
> > > 
> > >     perf map: Fix dso->nsinfo refcounting
> > >     
> > >     ASan reports a memory leak of nsinfo during the execution of
> > >     
> > >       # perf test "31: Lookup mmap thread"
> > >     
> > >     The leak is caused by a refcounted variable being replaced without
> > >     dropping the refcount.
> > >     
> > >     This patch makes sure that the refcnt of nsinfo is decreased whenever a
> > >     refcounted variable is replaced with a new value.
> > >     
> > >     Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> > >     Fixes: bf2e710b3cb8445c ("perf maps: Lookup maps in both intitial mountns and inner mountns.")
> > >     Cc: Ian Rogers <irogers@google.com>
> > >     Cc: Jiri Olsa <jolsa@redhat.com>
> > >     Cc: Krister Johansen <kjlx@templeofstupid.com>
> > >     Cc: Mark Rutland <mark.rutland@arm.com>
> > >     Cc: Namhyung Kim <namhyung@kernel.org>
> > >     Cc: Peter Zijlstra <peterz@infradead.org>
> > >     Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
> > >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > 
> > > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > > index 8af693d9678cefe0..72e7f3616157ead4 100644
> > > --- a/tools/perf/util/map.c
> > > +++ b/tools/perf/util/map.c
> > > @@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> > >  			if (!(prot & PROT_EXEC))
> > >  				dso__set_loaded(dso);
> > >  		}
> > > +
> > > +		nsinfo__put(dso->nsinfo);
> > >  		dso->nsinfo = nsi;
> > >  
> > >  		if (build_id__is_defined(bid))
> > > 
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

Re: [PATCH 01/20] perf nsinfo: fix refcounting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > ASan reports a memory leak of nsinfo during the execution of the perf
> > test "31: Lookup mmap thread".
> > The leak is caused by a refcounted variable being replaced without
> > dropping the refcount.
> > 
> > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > a refcounted variable is replaced with a new value.
> 
> So, there are multiple fixes in just one patch, I'll split it into
> three, no need to resend.
> 
> I'll try and check if finding Fixes: for the three is easy, that way
> stable@vger.kernel.org will figure out which of the supported releases
> need each of them.

3rd:

commit 7416120498f83963f4c5662847a1a0556e771b89
Author: Riccardo Mancini <rickyman7@gmail.com>
Date:   Thu Jul 15 18:07:06 2021 +0200

    perf probe: Fix dso->nsinfo refcounting
    
    ASan reports a memory leak of nsinfo during the execution of:
    
     # perf test "31: Lookup mmap thread".
    
    The leak is caused by a refcounted variable being replaced without
    dropping the refcount.
    
    This patch makes sure that the refcnt of nsinfo is decreased whenever
    a refcounted variable is replaced with a new value.
    
    Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
    Fixes: 544abd44c7064c8a ("perf probe: Allow placing uprobes in alternate namespaces.")
    Cc: Ian Rogers <irogers@google.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Krister Johansen <kjlx@templeofstupid.com>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
    [ Split from a larger patch ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c14e1d228e56b1c6..e05750cea34c3a95 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -179,8 +179,10 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user)
 		struct map *map;
 
 		map = dso__new_map(target);
-		if (map && map->dso)
+		if (map && map->dso) {
+			nsinfo__put(map->dso->nsinfo);
 			map->dso->nsinfo = nsinfo__get(nsi);
+		}
 		return map;
 	} else {
 		return kernel_get_module_map(target);

[PATCH 02/20] perf env: fix sibling_dies memory leak

[Riccardo Mancini]

ASan reports a memory leak in perf_env while running the perf test
"41: Session topology", caused by sibling_dies not being freed.

This patch adds the required free.

Fixes: acae8b36cded0ee6 ("perf header: Add die information in CPU topology")
Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/env.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index ebc5e9ad35db21d1..6c765946ef6f591c 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -186,6 +186,7 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->cpuid);
 	zfree(&env->cmdline);
 	zfree(&env->cmdline_argv);
+	zfree(&env->sibling_dies);
 	zfree(&env->sibling_cores);
 	zfree(&env->sibling_threads);
 	zfree(&env->pmu_mappings);
-- 
2.31.1

Re: [PATCH 02/20] perf env: fix sibling_dies memory leak

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:07PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak in perf_env while running the perf test
> "41: Session topology", caused by sibling_dies not being freed.
> 
> This patch adds the required free.
 
> Fixes: acae8b36cded0ee6 ("perf header: Add die information in CPU topology")

So, I CC the author of the patch and its committer, using this shell
script:

  $ cat ~/bin/fixes
  #!/bin/bash
  
  cset=$1
  if [ $# -eq 0 ] ; then
  	read cset
  fi
  git show --pretty=fuller $cset | grep '^\(Author\|Commit\): ' | sed -r 's/.*: +/Cc: /g' | grep -v "Arnaldo Carvalho de Melo" | sort -u
  echo Fixes: `git one $cset`
  $
  $ grep one -B1 ~/.gitconfig
  [alias]
	one = show -s --pretty='format:%h (\"%s\")'
  $
  $ fixes acae8b36cded0ee6
  Cc: Kan Liang <kan.liang@linux.intel.com>
  Fixes: acae8b36cded0ee6 ("perf header: Add die information in CPU topology")
  $

Normally I copy the cset SHA and, in vim, select it and run !fixes, i.e.

:'<,'>!fixes

Thanks, applied.

- Arnaldo

> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/env.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index ebc5e9ad35db21d1..6c765946ef6f591c 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -186,6 +186,7 @@ void perf_env__exit(struct perf_env *env)
>  	zfree(&env->cpuid);
>  	zfree(&env->cmdline);
>  	zfree(&env->cmdline_argv);
> +	zfree(&env->sibling_dies);
>  	zfree(&env->sibling_cores);
>  	zfree(&env->sibling_threads);
>  	zfree(&env->pmu_mappings);
> -- 
> 2.31.1
> 

-- 

- Arnaldo

Re: [PATCH 02/20] perf env: fix sibling_dies memory leak

[Riccardo Mancini]

Hi Arnaldo,

thanks for reviewing this patchset and, more importantly, for manually adding
all Fixes: tags. I'm so sorry for that.

On Thu, 2021-07-15 at 16:49 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 15, 2021 at 06:07:07PM +0200, Riccardo Mancini escreveu:
> > ASan reports a memory leak in perf_env while running the perf test
> > "41: Session topology", caused by sibling_dies not being freed.
> > 
> > This patch adds the required free.
>  
> > Fixes: acae8b36cded0ee6 ("perf header: Add die information in CPU topology")
> 
> So, I CC the author of the patch and its committer, using this shell
> script:
> 
>   $ cat ~/bin/fixes
>   #!/bin/bash
>   
>   cset=$1
>   if [ $# -eq 0 ] ; then
>         read cset
>   fi
>   git show --pretty=fuller $cset | grep '^\(Author\|Commit\): ' | sed -r
> 's/.*: +/Cc: /g' | grep -v "Arnaldo Carvalho de Melo" | sort -u
>   echo Fixes: `git one $cset`
>   $
>   $ grep one -B1 ~/.gitconfig
>   [alias]
>         one = show -s --pretty='format:%h (\"%s\")'
>   $
>   $ fixes acae8b36cded0ee6
>   Cc: Kan Liang <kan.liang@linux.intel.com>
>   Fixes: acae8b36cded0ee6 ("perf header: Add die information in CPU topology")
>   $
> 
> Normally I copy the cset SHA and, in vim, select it and run !fixes, i.e.
> 
> :'<,'>!fixes

Thanks! I've added this nice script to my PATH. :)

Thanks,
Riccardo

> 
> Thanks, applied.
> 
> - Arnaldo
> 
> > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> > ---
> >  tools/perf/util/env.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> > index ebc5e9ad35db21d1..6c765946ef6f591c 100644
> > --- a/tools/perf/util/env.c
> > +++ b/tools/perf/util/env.c
> > @@ -186,6 +186,7 @@ void perf_env__exit(struct perf_env *env)
> >         zfree(&env->cpuid);
> >         zfree(&env->cmdline);
> >         zfree(&env->cmdline_argv);
> > +       zfree(&env->sibling_dies);
> >         zfree(&env->sibling_cores);
> >         zfree(&env->sibling_threads);
> >         zfree(&env->pmu_mappings);
> > -- 
> > 2.31.1
> > 
> 

[PATCH 03/20] perf test: session_topology: delete session->evlist

[Riccardo Mancini]

ASan reports a memory leak related to session->evlist while running
the perf test "41: Session topology".

When perf_data is in write mode, session->evlist is owned by the
caller, which should also take care of deleting it.

This patch adds the missing evlist__delete.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/tests/topology.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
index ec4e3b21b8311f57..b5efe675b321746e 100644
--- a/tools/perf/tests/topology.c
+++ b/tools/perf/tests/topology.c
@@ -61,6 +61,7 @@ static int session_write_header(char *path)
 	TEST_ASSERT_VAL("failed to write header",
 			!perf_session__write_header(session, session->evlist, data.file.fd, true));
 
+	evlist__delete(session->evlist);
 	perf_session__delete(session);
 
 	return 0;
-- 
2.31.1

Re: [PATCH 03/20] perf test: session_topology: delete session->evlist

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:08PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak related to session->evlist while running
> the perf test "41: Session topology".
> 
> When perf_data is in write mode, session->evlist is owned by the
> caller, which should also take care of deleting it.
> 
> This patch adds the missing evlist__delete.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/topology.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
> index ec4e3b21b8311f57..b5efe675b321746e 100644
> --- a/tools/perf/tests/topology.c
> +++ b/tools/perf/tests/topology.c
> @@ -61,6 +61,7 @@ static int session_write_header(char *path)
>  	TEST_ASSERT_VAL("failed to write header",
>  			!perf_session__write_header(session, session->evlist, data.file.fd, true));
>  
> +	evlist__delete(session->evlist);
>  	perf_session__delete(session);
>  
>  	return 0;
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 04/20] perf test: event_update: fix memory leak of evlist

[Riccardo Mancini]

ASan reports a memory leak when running the perf test
"49: Synthesize attr update" caused by evlist not being deleted.

This patch adds the missing evlist__delete and removes the
perf_cpu_map__put since it's already being deleted by evlist__delete.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/tests/event_update.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
index 656218179222cc58..932ab0740d11ced6 100644
--- a/tools/perf/tests/event_update.c
+++ b/tools/perf/tests/event_update.c
@@ -118,6 +118,6 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
 	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
 			!perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus));
 
-	perf_cpu_map__put(evsel->core.own_cpus);
+	evlist__delete(evlist);
 	return 0;
 }
-- 
2.31.1

Re: [PATCH 04/20] perf test: event_update: fix memory leak of evlist

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:09PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak when running the perf test
> "49: Synthesize attr update" caused by evlist not being deleted.
> 
> This patch adds the missing evlist__delete and removes the
> perf_cpu_map__put since it's already being deleted by evlist__delete.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/event_update.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> index 656218179222cc58..932ab0740d11ced6 100644
> --- a/tools/perf/tests/event_update.c
> +++ b/tools/perf/tests/event_update.c
> @@ -118,6 +118,6 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
>  	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
>  			!perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus));
>  
> -	perf_cpu_map__put(evsel->core.own_cpus);
> +	evlist__delete(evlist);
>  	return 0;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 05/20] perf test: event_update: fix memory leak of unit

[Riccardo Mancini]

ASan reports a memory leak while running the perf test
"49: Synthesize attr update" caused by a string being duplicated but
never freed.

This patch adds the missing free().

Note that evsel->unit is not deallocated together with evsel since it
is supposed to be a constant string.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/tests/event_update.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
index 932ab0740d11ced6..44a50527f9d95170 100644
--- a/tools/perf/tests/event_update.c
+++ b/tools/perf/tests/event_update.c
@@ -88,6 +88,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
 	struct evsel *evsel;
 	struct event_name tmp;
 	struct evlist *evlist = evlist__new_default();
+	char *unit = strdup("KRAVA");
 
 	TEST_ASSERT_VAL("failed to get evlist", evlist);
 
@@ -98,7 +99,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
 
 	perf_evlist__id_add(&evlist->core, &evsel->core, 0, 0, 123);
 
-	evsel->unit = strdup("KRAVA");
+	evsel->unit = unit;
 
 	TEST_ASSERT_VAL("failed to synthesize attr update unit",
 			!perf_event__synthesize_event_update_unit(NULL, evsel, process_event_unit));
@@ -118,6 +119,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
 	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
 			!perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus));
 
+	free(unit);
 	evlist__delete(evlist);
 	return 0;
 }
-- 
2.31.1

Re: [PATCH 05/20] perf test: event_update: fix memory leak of unit

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:10PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak while running the perf test
> "49: Synthesize attr update" caused by a string being duplicated but
> never freed.
> 
> This patch adds the missing free().
> 
> Note that evsel->unit is not deallocated together with evsel since it
> is supposed to be a constant string.

Thanks, applied and added the Fixes tag, which I'm doing to all the
patches in the series.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/event_update.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> index 932ab0740d11ced6..44a50527f9d95170 100644
> --- a/tools/perf/tests/event_update.c
> +++ b/tools/perf/tests/event_update.c
> @@ -88,6 +88,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
>  	struct evsel *evsel;
>  	struct event_name tmp;
>  	struct evlist *evlist = evlist__new_default();
> +	char *unit = strdup("KRAVA");
>  
>  	TEST_ASSERT_VAL("failed to get evlist", evlist);
>  
> @@ -98,7 +99,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
>  
>  	perf_evlist__id_add(&evlist->core, &evsel->core, 0, 0, 123);
>  
> -	evsel->unit = strdup("KRAVA");
> +	evsel->unit = unit;
>  
>  	TEST_ASSERT_VAL("failed to synthesize attr update unit",
>  			!perf_event__synthesize_event_update_unit(NULL, evsel, process_event_unit));
> @@ -118,6 +119,7 @@ int test__event_update(struct test *test __maybe_unused, int subtest __maybe_unu
>  	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
>  			!perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus));
>  
> +	free(unit);
>  	evlist__delete(evlist);
>  	return 0;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 06/20] perf dso: fix memory leak in dso__new_map

[Riccardo Mancini]

ASan reports a memory leak when running the perf test
"65: maps__merge_in". The causes of the leaks are two, this patch
addresses only the first one, which is related to dso__new_map.
The bug is that dso__new_map creates a new dso but never decreases
the refcount it gets from creating it.

This patch adds the missing dso__put.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/dso.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index d786cf6b0cfa65f2..ee15db2be2f43403 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1154,8 +1154,10 @@ struct map *dso__new_map(const char *name)
 	struct map *map = NULL;
 	struct dso *dso = dso__new(name);
 
-	if (dso)
+	if (dso) {
 		map = map__new2(0, dso);
+		dso__put(dso);
+	}
 
 	return map;
 }
-- 
2.31.1

Re: [PATCH 06/20] perf dso: fix memory leak in dso__new_map

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:11PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak when running the perf test
> "65: maps__merge_in". The causes of the leaks are two, this patch
> addresses only the first one, which is related to dso__new_map.
> The bug is that dso__new_map creates a new dso but never decreases
> the refcount it gets from creating it.
> 
> This patch adds the missing dso__put.

Thanks, applied.

This needs some extra thinking for some unrelated issue, namely that
this dso isn't linked in the machine dso list, I think this is only used
by 'perf probe' :-\

I guess this won't be a problem since using the machine dso list is
important just for other tools, like when traversing to get the
build-ids, etc.

- Arnaldo
 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/dso.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index d786cf6b0cfa65f2..ee15db2be2f43403 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1154,8 +1154,10 @@ struct map *dso__new_map(const char *name)
>  	struct map *map = NULL;
>  	struct dso *dso = dso__new(name);
>  
> -	if (dso)
> +	if (dso) {
>  		map = map__new2(0, dso);
> +		dso__put(dso);
> +	}
>  
>  	return map;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

Re: [PATCH 06/20] perf dso: fix memory leak in dso__new_map

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:11PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak when running the perf test
> "65: maps__merge_in". The causes of the leaks are two, this patch
> addresses only the first one, which is related to dso__new_map.
> The bug is that dso__new_map creates a new dso but never decreases
> the refcount it gets from creating it.
> 
> This patch adds the missing dso__put.

Fixes: d3a7c489c7fd2463 ("perf tools: Reference count struct dso")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/dso.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index d786cf6b0cfa65f2..ee15db2be2f43403 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1154,8 +1154,10 @@ struct map *dso__new_map(const char *name)
>  	struct map *map = NULL;
>  	struct dso *dso = dso__new(name);
>  
> -	if (dso)
> +	if (dso) {
>  		map = map__new2(0, dso);
> +		dso__put(dso);
> +	}
>  
>  	return map;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 07/20] perf test: maps__merge_in: fix memory leak of maps

[Riccardo Mancini]

ASan reports a memory leak when running the perf test
"65: maps__merge_in". This is the second and final patch addressing
these memory leaks.
This time, the problem is simply that the maps object is never
destructed.

This patch adds the missing maps__exit call.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/tests/maps.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
index edcbc70ff9d66e22..1ac72919fa358601 100644
--- a/tools/perf/tests/maps.c
+++ b/tools/perf/tests/maps.c
@@ -116,5 +116,7 @@ int test__maps__merge_in(struct test *t __maybe_unused, int subtest __maybe_unus
 
 	ret = check_maps(merged3, ARRAY_SIZE(merged3), &maps);
 	TEST_ASSERT_VAL("merge check failed", !ret);
+
+	maps__exit(&maps);
 	return TEST_OK;
 }
-- 
2.31.1

Re: [PATCH 07/20] perf test: maps__merge_in: fix memory leak of maps

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:12PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak when running the perf test
> "65: maps__merge_in". This is the second and final patch addressing
> these memory leaks.
> This time, the problem is simply that the maps object is never
> destructed.
> 
> This patch adds the missing maps__exit call.

Not really this one, but at least the patch should apply more easily:

Fixes: 79b6bb73f888933c ("perf maps: Merge 'struct maps' with 'struct map_groups'")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/maps.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
> index edcbc70ff9d66e22..1ac72919fa358601 100644
> --- a/tools/perf/tests/maps.c
> +++ b/tools/perf/tests/maps.c
> @@ -116,5 +116,7 @@ int test__maps__merge_in(struct test *t __maybe_unused, int subtest __maybe_unus
>  
>  	ret = check_maps(merged3, ARRAY_SIZE(merged3), &maps);
>  	TEST_ASSERT_VAL("merge check failed", !ret);
> +
> +	maps__exit(&maps);
>  	return TEST_OK;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 08/20] perf env: fix memory leak of cpu_pmu_caps

[Riccardo Mancini]

ASan reports memory leaks while running the perf test
"83: Zstd perf.data compression/decompression".
The first of the leaks is caused by env->cpu_pmu_caps not being freed.

This patch adds the missing (z)free inside perf_env__exit.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/env.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 6c765946ef6f591c..cec2e6cad8aabfe7 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -191,6 +191,7 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->sibling_threads);
 	zfree(&env->pmu_mappings);
 	zfree(&env->cpu);
+	zfree(&env->cpu_pmu_caps);
 	zfree(&env->numa_map);
 
 	for (i = 0; i < env->nr_numa_nodes; i++)
-- 
2.31.1

Re: [PATCH 08/20] perf env: fix memory leak of cpu_pmu_caps

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:13PM +0200, Riccardo Mancini escreveu:
> ASan reports memory leaks while running the perf test
> "83: Zstd perf.data compression/decompression".
> The first of the leaks is caused by env->cpu_pmu_caps not being freed.
> 
> This patch adds the missing (z)free inside perf_env__exit.

Fixes: 6f91ea283a1ed23e ("perf header: Support CPU PMU capabilities")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/env.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 6c765946ef6f591c..cec2e6cad8aabfe7 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -191,6 +191,7 @@ void perf_env__exit(struct perf_env *env)
>  	zfree(&env->sibling_threads);
>  	zfree(&env->pmu_mappings);
>  	zfree(&env->cpu);
> +	zfree(&env->cpu_pmu_caps);
>  	zfree(&env->numa_map);
>  
>  	for (i = 0; i < env->nr_numa_nodes; i++)
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 09/20] perf report: free generated help strings for sort option

[Riccardo Mancini]

ASan reports the memory leak of the strings allocated by sort_help when
running perf report.

This patch changes the returned pointer to char* (instead of const char*),
saves it in a temporary variable, and finally deallocates it at function
exit.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-report.c | 33 ++++++++++++++++++++++-----------
 tools/perf/util/sort.c      |  2 +-
 tools/perf/util/sort.h      |  2 +-
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6386af6a2612367c..dc0364f671b97d26 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1175,6 +1175,8 @@ int cmd_report(int argc, const char **argv)
 		.annotation_opts	 = annotation__default_options,
 		.skip_empty		 = true,
 	};
+	char *sort_order_help = sort_help("sort by key(s):");
+	char *field_order_help = sort_help("output field(s): overhead period sample ");
 	const struct option options[] = {
 	OPT_STRING('i', "input", &input_name, "file",
 		    "input file name"),
@@ -1209,9 +1211,9 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN(0, "header-only", &report.header_only,
 		    "Show only data header."),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   sort_help("sort by key(s):")),
+		   sort_order_help),
 	OPT_STRING('F', "fields", &field_order, "key[,keys...]",
-		   sort_help("output field(s): overhead period sample ")),
+		   field_order_help),
 	OPT_BOOLEAN(0, "show-cpu-utilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_BOOLEAN_FLAG(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
@@ -1344,11 +1346,11 @@ int cmd_report(int argc, const char **argv)
 	char sort_tmp[128];
 
 	if (ret < 0)
-		return ret;
+		goto exit;
 
 	ret = perf_config(report__config, &report);
 	if (ret)
-		return ret;
+		goto exit;
 
 	argc = parse_options(argc, argv, options, report_usage, 0);
 	if (argc) {
@@ -1362,8 +1364,10 @@ int cmd_report(int argc, const char **argv)
 		report.symbol_filter_str = argv[0];
 	}
 
-	if (annotate_check_args(&report.annotation_opts) < 0)
-		return -EINVAL;
+	if (annotate_check_args(&report.annotation_opts) < 0) {
+		ret = -EINVAL;
+		goto exit;
+	}
 
 	if (report.mmaps_mode)
 		report.tasks_mode = true;
@@ -1377,12 +1381,14 @@ int cmd_report(int argc, const char **argv)
 	if (symbol_conf.vmlinux_name &&
 	    access(symbol_conf.vmlinux_name, R_OK)) {
 		pr_err("Invalid file: %s\n", symbol_conf.vmlinux_name);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto exit;
 	}
 	if (symbol_conf.kallsyms_name &&
 	    access(symbol_conf.kallsyms_name, R_OK)) {
 		pr_err("Invalid file: %s\n", symbol_conf.kallsyms_name);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto exit;
 	}
 
 	if (report.inverted_callchain)
@@ -1406,12 +1412,14 @@ int cmd_report(int argc, const char **argv)
 
 repeat:
 	session = perf_session__new(&data, false, &report.tool);
-	if (IS_ERR(session))
-		return PTR_ERR(session);
+	if (IS_ERR(session)) {
+		ret = PTR_ERR(session);
+		goto exit;
+	}
 
 	ret = evswitch__init(&report.evswitch, session->evlist, stderr);
 	if (ret)
-		return ret;
+		goto exit;
 
 	if (zstd_init(&(session->zstd_data), 0) < 0)
 		pr_warning("Decompression initialization failed. Reported data may be incomplete.\n");
@@ -1646,5 +1654,8 @@ int cmd_report(int argc, const char **argv)
 
 	zstd_fini(&(session->zstd_data));
 	perf_session__delete(session);
+exit:
+	free(sort_order_help);
+	free(field_order_help);
 	return ret;
 }
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 88ce47f2547e3558..568a88c001c6cb5a 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -3370,7 +3370,7 @@ static void add_hpp_sort_string(struct strbuf *sb, struct hpp_dimension *s, int
 		add_key(sb, s[i].name, llen);
 }
 
-const char *sort_help(const char *prefix)
+char *sort_help(const char *prefix)
 {
 	struct strbuf sb;
 	char *s;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 87a092645aa72e41..b67c469aba79587f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -302,7 +302,7 @@ void reset_output_field(void);
 void sort__setup_elide(FILE *fp);
 void perf_hpp__set_elide(int idx, bool elide);
 
-const char *sort_help(const char *prefix);
+char *sort_help(const char *prefix);
 
 int report_parse_ignore_callees_opt(const struct option *opt, const char *arg, int unset);
 
-- 
2.31.1

Re: [PATCH 09/20] perf report: free generated help strings for sort option

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:14PM +0200, Riccardo Mancini escreveu:
> ASan reports the memory leak of the strings allocated by sort_help when
> running perf report.
> 
> This patch changes the returned pointer to char* (instead of const char*),
> saves it in a temporary variable, and finally deallocates it at function
> exit.

Fixes: 702fb9b415e7c99b ("perf report: Show all sort keys in help output")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-report.c | 33 ++++++++++++++++++++++-----------
>  tools/perf/util/sort.c      |  2 +-
>  tools/perf/util/sort.h      |  2 +-
>  3 files changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 6386af6a2612367c..dc0364f671b97d26 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1175,6 +1175,8 @@ int cmd_report(int argc, const char **argv)
>  		.annotation_opts	 = annotation__default_options,
>  		.skip_empty		 = true,
>  	};
> +	char *sort_order_help = sort_help("sort by key(s):");
> +	char *field_order_help = sort_help("output field(s): overhead period sample ");
>  	const struct option options[] = {
>  	OPT_STRING('i', "input", &input_name, "file",
>  		    "input file name"),
> @@ -1209,9 +1211,9 @@ int cmd_report(int argc, const char **argv)
>  	OPT_BOOLEAN(0, "header-only", &report.header_only,
>  		    "Show only data header."),
>  	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
> -		   sort_help("sort by key(s):")),
> +		   sort_order_help),
>  	OPT_STRING('F', "fields", &field_order, "key[,keys...]",
> -		   sort_help("output field(s): overhead period sample ")),
> +		   field_order_help),
>  	OPT_BOOLEAN(0, "show-cpu-utilization", &symbol_conf.show_cpu_utilization,
>  		    "Show sample percentage for different cpu modes"),
>  	OPT_BOOLEAN_FLAG(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
> @@ -1344,11 +1346,11 @@ int cmd_report(int argc, const char **argv)
>  	char sort_tmp[128];
>  
>  	if (ret < 0)
> -		return ret;
> +		goto exit;
>  
>  	ret = perf_config(report__config, &report);
>  	if (ret)
> -		return ret;
> +		goto exit;
>  
>  	argc = parse_options(argc, argv, options, report_usage, 0);
>  	if (argc) {
> @@ -1362,8 +1364,10 @@ int cmd_report(int argc, const char **argv)
>  		report.symbol_filter_str = argv[0];
>  	}
>  
> -	if (annotate_check_args(&report.annotation_opts) < 0)
> -		return -EINVAL;
> +	if (annotate_check_args(&report.annotation_opts) < 0) {
> +		ret = -EINVAL;
> +		goto exit;
> +	}
>  
>  	if (report.mmaps_mode)
>  		report.tasks_mode = true;
> @@ -1377,12 +1381,14 @@ int cmd_report(int argc, const char **argv)
>  	if (symbol_conf.vmlinux_name &&
>  	    access(symbol_conf.vmlinux_name, R_OK)) {
>  		pr_err("Invalid file: %s\n", symbol_conf.vmlinux_name);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto exit;
>  	}
>  	if (symbol_conf.kallsyms_name &&
>  	    access(symbol_conf.kallsyms_name, R_OK)) {
>  		pr_err("Invalid file: %s\n", symbol_conf.kallsyms_name);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto exit;
>  	}
>  
>  	if (report.inverted_callchain)
> @@ -1406,12 +1412,14 @@ int cmd_report(int argc, const char **argv)
>  
>  repeat:
>  	session = perf_session__new(&data, false, &report.tool);
> -	if (IS_ERR(session))
> -		return PTR_ERR(session);
> +	if (IS_ERR(session)) {
> +		ret = PTR_ERR(session);
> +		goto exit;
> +	}
>  
>  	ret = evswitch__init(&report.evswitch, session->evlist, stderr);
>  	if (ret)
> -		return ret;
> +		goto exit;
>  
>  	if (zstd_init(&(session->zstd_data), 0) < 0)
>  		pr_warning("Decompression initialization failed. Reported data may be incomplete.\n");
> @@ -1646,5 +1654,8 @@ int cmd_report(int argc, const char **argv)
>  
>  	zstd_fini(&(session->zstd_data));
>  	perf_session__delete(session);
> +exit:
> +	free(sort_order_help);
> +	free(field_order_help);
>  	return ret;
>  }
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 88ce47f2547e3558..568a88c001c6cb5a 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -3370,7 +3370,7 @@ static void add_hpp_sort_string(struct strbuf *sb, struct hpp_dimension *s, int
>  		add_key(sb, s[i].name, llen);
>  }
>  
> -const char *sort_help(const char *prefix)
> +char *sort_help(const char *prefix)
>  {
>  	struct strbuf sb;
>  	char *s;
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 87a092645aa72e41..b67c469aba79587f 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -302,7 +302,7 @@ void reset_output_field(void);
>  void sort__setup_elide(FILE *fp);
>  void perf_hpp__set_elide(int idx, bool elide);
>  
> -const char *sort_help(const char *prefix);
> +char *sort_help(const char *prefix);
>  
>  int report_parse_ignore_callees_opt(const struct option *opt, const char *arg, int unset);
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 10/20] perf inject: close inject.output

[Riccardo Mancini]

ASan reports a memory leak when running the perf test
"83: Zstd perf.data compression/decompression", which happens inside
perf-inject.
The bug is caused by inject.output never being closed.

This patch adds the missing perf_data__close.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-inject.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index ffd2b25039e36e1d..c88c61e7f8ccb6a7 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -993,8 +993,10 @@ int cmd_inject(int argc, const char **argv)
 
 	data.path = inject.input_name;
 	inject.session = perf_session__new(&data, inject.output.is_pipe, &inject.tool);
-	if (IS_ERR(inject.session))
-		return PTR_ERR(inject.session);
+	if (IS_ERR(inject.session)) {
+		ret = PTR_ERR(inject.session);
+		goto out_close_output;
+	}
 
 	if (zstd_init(&(inject.session->zstd_data), 0) < 0)
 		pr_warning("Decompression initialization failed.\n");
@@ -1036,6 +1038,8 @@ int cmd_inject(int argc, const char **argv)
 out_delete:
 	zstd_fini(&(inject.session->zstd_data));
 	perf_session__delete(inject.session);
+out_close_output:
+	perf_data__close(&inject.output);
 	free(inject.itrace_synth_opts.vm_tm_corr_args);
 	return ret;
 }
-- 
2.31.1

Re: [PATCH 10/20] perf inject: close inject.output

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:15PM +0200, Riccardo Mancini escreveu:
> ASan reports a memory leak when running the perf test
> "83: Zstd perf.data compression/decompression", which happens inside
> perf-inject.
> The bug is caused by inject.output never being closed.
> 
> This patch adds the missing perf_data__close.

Fixes: 6ef81c55a2b6584c ("perf session: Return error code for perf_session__new() function on failure")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-inject.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index ffd2b25039e36e1d..c88c61e7f8ccb6a7 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -993,8 +993,10 @@ int cmd_inject(int argc, const char **argv)
>  
>  	data.path = inject.input_name;
>  	inject.session = perf_session__new(&data, inject.output.is_pipe, &inject.tool);
> -	if (IS_ERR(inject.session))
> -		return PTR_ERR(inject.session);
> +	if (IS_ERR(inject.session)) {
> +		ret = PTR_ERR(inject.session);
> +		goto out_close_output;
> +	}
>  
>  	if (zstd_init(&(inject.session->zstd_data), 0) < 0)
>  		pr_warning("Decompression initialization failed.\n");
> @@ -1036,6 +1038,8 @@ int cmd_inject(int argc, const char **argv)
>  out_delete:
>  	zstd_fini(&(inject.session->zstd_data));
>  	perf_session__delete(inject.session);
> +out_close_output:
> +	perf_data__close(&inject.output);
>  	free(inject.itrace_synth_opts.vm_tm_corr_args);
>  	return ret;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 11/20] perf session: cleanup trace_event

[Riccardo Mancini]

ASan reports several memory leaks when running the perf test
"82: Use vfs_getname probe to get syscall args filenames", many of which
are related to session->tevent.
This patch will solve this problem, then next patch will fix the remaining
memory leaks in perf-script.
This bug is due to a missing deallocation of the trace_event data
strutures.

This patch adds the missing trace_event__cleanup in
perf_session__delete.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/session.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index e9c929a39973ace9..51f72740291267c5 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -306,6 +306,7 @@ void perf_session__delete(struct perf_session *session)
 			evlist__delete(session->evlist);
 		perf_data__close(session->data);
 	}
+	trace_event__cleanup(&session->tevent);
 	free(session);
 }
 
-- 
2.31.1

Re: [PATCH 11/20] perf session: cleanup trace_event

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:16PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks when running the perf test
> "82: Use vfs_getname probe to get syscall args filenames", many of which
> are related to session->tevent.
> This patch will solve this problem, then next patch will fix the remaining
> memory leaks in perf-script.
> This bug is due to a missing deallocation of the trace_event data
> strutures.
> 
> This patch adds the missing trace_event__cleanup in
> perf_session__delete.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/session.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index e9c929a39973ace9..51f72740291267c5 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -306,6 +306,7 @@ void perf_session__delete(struct perf_session *session)
>  			evlist__delete(session->evlist);
>  		perf_data__close(session->data);
>  	}
> +	trace_event__cleanup(&session->tevent);
>  	free(session);
>  }
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 12/20] perf script: release zstd data

[Riccardo Mancini]

ASan reports several memory leak while running the perf test
"82: Use vfs_getname probe to get syscall args filenames".
One of the leaks is caused by zstd data not being released on exit in
perf-script.

This patch adds the missing zstd_fini.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-script.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 8c03a9862872d495..bae0e5b72c0e6050 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -4143,6 +4143,7 @@ int cmd_script(int argc, const char **argv)
 		zfree(&script.ptime_range);
 	}
 
+	zstd_fini(&(session->zstd_data));
 	evlist__free_stats(session->evlist);
 	perf_session__delete(session);
 
-- 
2.31.1

Re: [PATCH 12/20] perf script: release zstd data

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:17PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leak while running the perf test
> "82: Use vfs_getname probe to get syscall args filenames".
> One of the leaks is caused by zstd data not being released on exit in
> perf-script.
> 
> This patch adds the missing zstd_fini.

Fixes: b13b04d9382113f7 ("perf script: Initialize zstd_data")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-script.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 8c03a9862872d495..bae0e5b72c0e6050 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -4143,6 +4143,7 @@ int cmd_script(int argc, const char **argv)
>  		zfree(&script.ptime_range);
>  	}
>  
> +	zstd_fini(&(session->zstd_data));
>  	evlist__free_stats(session->evlist);
>  	perf_session__delete(session);
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 13/20] perf script: fix memory leaks in perf_script

[Riccardo Mancini]

ASan reports several memory leaks while running the perf test
"82: Use vfs_getname probe to get syscall args filenames".
Two of these are caused by some refcounts not being decreased on
perf-script exit, namely script.threads and script.cpus.

This patch adds the missing __put calls in a new perf_script__exit
function, which is called at the end of cmd_script.

This patch concludes the fixes of all remaining memory leaks in perf
test "82: Use vfs_getname probe to get syscall args filenames".

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-script.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index bae0e5b72c0e6050..064da7f3618d39d8 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2601,6 +2601,12 @@ static void perf_script__exit_per_event_dump_stats(struct perf_script *script)
 	}
 }
 
+static void perf_script__exit(struct perf_script *script)
+{
+	perf_thread_map__put(script->threads);
+	perf_cpu_map__put(script->cpus);
+}
+
 static int __cmd_script(struct perf_script *script)
 {
 	int ret;
@@ -4146,6 +4152,7 @@ int cmd_script(int argc, const char **argv)
 	zstd_fini(&(session->zstd_data));
 	evlist__free_stats(session->evlist);
 	perf_session__delete(session);
+	perf_script__exit(&script);
 
 	if (script_started)
 		cleanup_scripting();
-- 
2.31.1

Re: [PATCH 13/20] perf script: fix memory leaks in perf_script

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:18PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks while running the perf test
> "82: Use vfs_getname probe to get syscall args filenames".
> Two of these are caused by some refcounts not being decreased on
> perf-script exit, namely script.threads and script.cpus.
> 
> This patch adds the missing __put calls in a new perf_script__exit
> function, which is called at the end of cmd_script.
> 
> This patch concludes the fixes of all remaining memory leaks in perf
> test "82: Use vfs_getname probe to get syscall args filenames".

Fixes: cfc8874a48599249 ("perf script: Process cpu/threads maps")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-script.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index bae0e5b72c0e6050..064da7f3618d39d8 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2601,6 +2601,12 @@ static void perf_script__exit_per_event_dump_stats(struct perf_script *script)
>  	}
>  }
>  
> +static void perf_script__exit(struct perf_script *script)
> +{
> +	perf_thread_map__put(script->threads);
> +	perf_cpu_map__put(script->cpus);
> +}
> +
>  static int __cmd_script(struct perf_script *script)
>  {
>  	int ret;
> @@ -4146,6 +4152,7 @@ int cmd_script(int argc, const char **argv)
>  	zstd_fini(&(session->zstd_data));
>  	evlist__free_stats(session->evlist);
>  	perf_session__delete(session);
> +	perf_script__exit(&script);
>  
>  	if (script_started)
>  		cleanup_scripting();
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 14/20] perf util/lzma: close lzma stream

[Riccardo Mancini]

ASan reports memory leaks when running the perf test
"88: Check open filename arg using perf trace + vfs_getname".
One of these is caused by the lzma stream never being closed inside
lzma_decompress_to_file.

This patch adds the missing lzma_end.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/lzma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/lzma.c b/tools/perf/util/lzma.c
index 39062df0262915bd..51424cdc3b682c64 100644
--- a/tools/perf/util/lzma.c
+++ b/tools/perf/util/lzma.c
@@ -69,7 +69,7 @@ int lzma_decompress_to_file(const char *input, int output_fd)
 
 			if (ferror(infile)) {
 				pr_err("lzma: read error: %s\n", strerror(errno));
-				goto err_fclose;
+				goto err_lzma_end;
 			}
 
 			if (feof(infile))
@@ -83,7 +83,7 @@ int lzma_decompress_to_file(const char *input, int output_fd)
 
 			if (writen(output_fd, buf_out, write_size) != write_size) {
 				pr_err("lzma: write error: %s\n", strerror(errno));
-				goto err_fclose;
+				goto err_lzma_end;
 			}
 
 			strm.next_out  = buf_out;
@@ -95,11 +95,13 @@ int lzma_decompress_to_file(const char *input, int output_fd)
 				break;
 
 			pr_err("lzma: failed %s\n", lzma_strerror(ret));
-			goto err_fclose;
+			goto err_lzma_end;
 		}
 	}
 
 	err = 0;
+err_lzma_end:
+	lzma_end(&strm);
 err_fclose:
 	fclose(infile);
 	return err;
-- 
2.31.1

Re: [PATCH 14/20] perf util/lzma: close lzma stream

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:19PM +0200, Riccardo Mancini escreveu:
> ASan reports memory leaks when running the perf test
> "88: Check open filename arg using perf trace + vfs_getname".
> One of these is caused by the lzma stream never being closed inside
> lzma_decompress_to_file.
> 
> This patch adds the missing lzma_end.

Fixes: 80a32e5b498a7547 ("perf tools: Add lzma decompression support for kernel module")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/lzma.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/lzma.c b/tools/perf/util/lzma.c
> index 39062df0262915bd..51424cdc3b682c64 100644
> --- a/tools/perf/util/lzma.c
> +++ b/tools/perf/util/lzma.c
> @@ -69,7 +69,7 @@ int lzma_decompress_to_file(const char *input, int output_fd)
>  
>  			if (ferror(infile)) {
>  				pr_err("lzma: read error: %s\n", strerror(errno));
> -				goto err_fclose;
> +				goto err_lzma_end;
>  			}
>  
>  			if (feof(infile))
> @@ -83,7 +83,7 @@ int lzma_decompress_to_file(const char *input, int output_fd)
>  
>  			if (writen(output_fd, buf_out, write_size) != write_size) {
>  				pr_err("lzma: write error: %s\n", strerror(errno));
> -				goto err_fclose;
> +				goto err_lzma_end;
>  			}
>  
>  			strm.next_out  = buf_out;
> @@ -95,11 +95,13 @@ int lzma_decompress_to_file(const char *input, int output_fd)
>  				break;
>  
>  			pr_err("lzma: failed %s\n", lzma_strerror(ret));
> -			goto err_fclose;
> +			goto err_lzma_end;
>  		}
>  	}
>  
>  	err = 0;
> +err_lzma_end:
> +	lzma_end(&strm);
>  err_fclose:
>  	fclose(infile);
>  	return err;
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 15/20] perf trace: free malloc'd trace fields on exit

[Riccardo Mancini]

ASan reports several memory leaks running the perf test
"88: Check open filename arg using perf trace + vfs_getname".
The first of these leaks is related to struct trace fields never being
deallocated.

This patch adds the function trace__exit, which is called at the end of
cmd_trace, replacing the existing deallocation, which is now moved
inside the new function.
This function deallocates:
 - ev_qualifier
 - ev_qualifier_ids.entries
 - syscalls.table
 - sctbl
 - perfconfig_events

In order to prevent errors in case any of this field is never
initialized, this patch also adds initialization to NULL to these
fields.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-trace.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 7ec18ff57fc4ae35..fe26d87ca8ccc14d 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4701,6 +4701,15 @@ static int trace__config(const char *var, const char *value, void *arg)
 	return err;
 }
 
+static void trace__exit(struct trace *trace)
+{
+	strlist__delete(trace->ev_qualifier);
+	free(trace->ev_qualifier_ids.entries);
+	free(trace->syscalls.table);
+	syscalltbl__delete(trace->sctbl);
+	zfree(&trace->perfconfig_events);
+}
+
 int cmd_trace(int argc, const char **argv)
 {
 	const char *trace_usage[] = {
@@ -4731,6 +4740,12 @@ int cmd_trace(int argc, const char **argv)
 		.kernel_syscallchains = false,
 		.max_stack = UINT_MAX,
 		.max_events = ULONG_MAX,
+		.ev_qualifier = NULL,
+		.sctbl = NULL,
+		.ev_qualifier_ids = {
+			.entries = NULL,
+			.nr = 0,
+		},
 	};
 	const char *map_dump_str = NULL;
 	const char *output_name = NULL;
@@ -5135,6 +5150,6 @@ int cmd_trace(int argc, const char **argv)
 	if (output_name != NULL)
 		fclose(trace.output);
 out:
-	zfree(&trace.perfconfig_events);
+	trace__exit(&trace);
 	return err;
 }
-- 
2.31.1

Re: [PATCH 15/20] perf trace: free malloc'd trace fields on exit

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:20PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks running the perf test
> "88: Check open filename arg using perf trace + vfs_getname".
> The first of these leaks is related to struct trace fields never being
> deallocated.
> 
> This patch adds the function trace__exit, which is called at the end of
> cmd_trace, replacing the existing deallocation, which is now moved
> inside the new function.
> This function deallocates:
>  - ev_qualifier
>  - ev_qualifier_ids.entries
>  - syscalls.table
>  - sctbl
>  - perfconfig_events
> 
> In order to prevent errors in case any of this field is never
> initialized, this patch also adds initialization to NULL to these
> fields.
> 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-trace.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 7ec18ff57fc4ae35..fe26d87ca8ccc14d 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -4701,6 +4701,15 @@ static int trace__config(const char *var, const char *value, void *arg)
>  	return err;
>  }
>  
> +static void trace__exit(struct trace *trace)
> +{
> +	strlist__delete(trace->ev_qualifier);
> +	free(trace->ev_qualifier_ids.entries);
> +	free(trace->syscalls.table);
> +	syscalltbl__delete(trace->sctbl);
> +	zfree(&trace->perfconfig_events);
> +}
> +
>  int cmd_trace(int argc, const char **argv)
>  {
>  	const char *trace_usage[] = {
> @@ -4731,6 +4740,12 @@ int cmd_trace(int argc, const char **argv)
>  		.kernel_syscallchains = false,
>  		.max_stack = UINT_MAX,
>  		.max_events = ULONG_MAX,
> +		.ev_qualifier = NULL,
> +		.sctbl = NULL,
> +		.ev_qualifier_ids = {
> +			.entries = NULL,
> +			.nr = 0,
> +		},
>  	};
>  	const char *map_dump_str = NULL;
>  	const char *output_name = NULL;

The above hunk is not needed, as all non explicitely initialized members
will be zeroed, so to reduce patch size I'm dropping it.

Thanks, applied.

- Arnaldo


> @@ -5135,6 +5150,6 @@ int cmd_trace(int argc, const char **argv)
>  	if (output_name != NULL)
>  		fclose(trace.output);
>  out:
> -	zfree(&trace.perfconfig_events);
> +	trace__exit(&trace);
>  	return err;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 16/20] perf trace: free syscall->arg_fmt

[Riccardo Mancini]

ASan reports several memory leaks running the perf test
"88: Check open filename arg using perf trace + vfs_getname".
The second of these leaks is caused by the arg_fmt field of syscall not
being deallocated.

This patch adds a new function syscall__exit which is called on all
syscalls.table entries in trace__exit, which will free the arg_fmt
field.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-trace.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index fe26d87ca8ccc14d..d67f02d237eb0c7e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2266,6 +2266,14 @@ static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sam
 	return augmented_args;
 }
 
+static void syscall__exit(struct syscall *sc)
+{
+	if (!sc)
+		return;
+
+	free(sc->arg_fmt);
+}
+
 static int trace__sys_enter(struct trace *trace, struct evsel *evsel,
 			    union perf_event *event __maybe_unused,
 			    struct perf_sample *sample)
@@ -4703,9 +4711,15 @@ static int trace__config(const char *var, const char *value, void *arg)
 
 static void trace__exit(struct trace *trace)
 {
+	int i;
+
 	strlist__delete(trace->ev_qualifier);
 	free(trace->ev_qualifier_ids.entries);
-	free(trace->syscalls.table);
+	if (trace->syscalls.table) {
+		for (i = 0; i <= trace->sctbl->syscalls.max_id; i++)
+			syscall__exit(&trace->syscalls.table[i]);
+		free(trace->syscalls.table);
+	}
 	syscalltbl__delete(trace->sctbl);
 	zfree(&trace->perfconfig_events);
 }
-- 
2.31.1

Re: [PATCH 16/20] perf trace: free syscall->arg_fmt

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:21PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks running the perf test
> "88: Check open filename arg using perf trace + vfs_getname".
> The second of these leaks is caused by the arg_fmt field of syscall not
> being deallocated.
> 
> This patch adds a new function syscall__exit which is called on all
> syscalls.table entries in trace__exit, which will free the arg_fmt
> field.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-trace.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index fe26d87ca8ccc14d..d67f02d237eb0c7e 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2266,6 +2266,14 @@ static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sam
>  	return augmented_args;
>  }
>  
> +static void syscall__exit(struct syscall *sc)
> +{
> +	if (!sc)
> +		return;
> +
> +	free(sc->arg_fmt);
> +}
> +
>  static int trace__sys_enter(struct trace *trace, struct evsel *evsel,
>  			    union perf_event *event __maybe_unused,
>  			    struct perf_sample *sample)
> @@ -4703,9 +4711,15 @@ static int trace__config(const char *var, const char *value, void *arg)
>  
>  static void trace__exit(struct trace *trace)
>  {
> +	int i;
> +
>  	strlist__delete(trace->ev_qualifier);
>  	free(trace->ev_qualifier_ids.entries);
> -	free(trace->syscalls.table);
> +	if (trace->syscalls.table) {
> +		for (i = 0; i <= trace->sctbl->syscalls.max_id; i++)
> +			syscall__exit(&trace->syscalls.table[i]);
> +		free(trace->syscalls.table);
> +	}
>  	syscalltbl__delete(trace->sctbl);
>  	zfree(&trace->perfconfig_events);
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 17/20] perf trace: free syscall tp fields in evsel->priv

[Riccardo Mancini]

ASan reports several memory leaks running the perf test
"88: Check open filename arg using perf trace + vfs_getname".
The third of these leaks is related to evsel->priv fields of sycalls never
being deallocated.

This patch adds the function evlist__free_syscall_tp_fields which
iterates over all evsels in evlist, matching syscalls, and calling the
missing frees.
This new function is called at the end of trace__run, right before
calling evlist__delete.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-trace.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d67f02d237eb0c7e..d9c65d55a9ae7526 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3103,6 +3103,21 @@ static struct evsel *evsel__new_pgfault(u64 config)
 	return evsel;
 }
 
+static void evlist__free_syscall_tp_fields(struct evlist *evlist)
+{
+	struct evsel *evsel;
+
+	evlist__for_each_entry(evlist, evsel) {
+		struct evsel_trace *et = evsel->priv;
+
+		if (!et || !evsel->tp_format || strcmp(evsel->tp_format->system, "syscalls"))
+			continue;
+
+		free(et->fmt);
+		free(et);
+	}
+}
+
 static void trace__handle_event(struct trace *trace, union perf_event *event, struct perf_sample *sample)
 {
 	const u32 type = event->header.type;
@@ -4138,7 +4153,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 
 out_delete_evlist:
 	trace__symbols__exit(trace);
-
+	evlist__free_syscall_tp_fields(evlist);
 	evlist__delete(evlist);
 	cgroup__put(trace->cgroup);
 	trace->evlist = NULL;
-- 
2.31.1

Re: [PATCH 17/20] perf trace: free syscall tp fields in evsel->priv

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:22PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks running the perf test
> "88: Check open filename arg using perf trace + vfs_getname".
> The third of these leaks is related to evsel->priv fields of sycalls never
> being deallocated.
> 
> This patch adds the function evlist__free_syscall_tp_fields which
> iterates over all evsels in evlist, matching syscalls, and calling the
> missing frees.
> This new function is called at the end of trace__run, right before
> calling evlist__delete.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-trace.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index d67f02d237eb0c7e..d9c65d55a9ae7526 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -3103,6 +3103,21 @@ static struct evsel *evsel__new_pgfault(u64 config)
>  	return evsel;
>  }
>  
> +static void evlist__free_syscall_tp_fields(struct evlist *evlist)
> +{
> +	struct evsel *evsel;
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		struct evsel_trace *et = evsel->priv;
> +
> +		if (!et || !evsel->tp_format || strcmp(evsel->tp_format->system, "syscalls"))
> +			continue;
> +
> +		free(et->fmt);
> +		free(et);
> +	}
> +}
> +
>  static void trace__handle_event(struct trace *trace, union perf_event *event, struct perf_sample *sample)
>  {
>  	const u32 type = event->header.type;
> @@ -4138,7 +4153,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
>  
>  out_delete_evlist:
>  	trace__symbols__exit(trace);
> -
> +	evlist__free_syscall_tp_fields(evlist);
>  	evlist__delete(evlist);
>  	cgroup__put(trace->cgroup);
>  	trace->evlist = NULL;
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 18/20] perf trace: free strings in trace__parse_events_option

[Riccardo Mancini]

ASan reports several memory leaks running the perf test
"88: Check open filename arg using perf trace + vfs_getname".
The fourth of these leaks is related to some strings never being freed
in trace__parse_events_option.

This patch adds the missing frees.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/builtin-trace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d9c65d55a9ae7526..4a677f3ff273887a 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4659,6 +4659,9 @@ static int trace__parse_events_option(const struct option *opt, const char *str,
 		err = parse_events_option(&o, lists[0], 0);
 	}
 out:
+	free(strace_groups_dir);
+	free(lists[0]);
+	free(lists[1]);
 	if (sep)
 		*sep = ',';
 
-- 
2.31.1

Re: [PATCH 18/20] perf trace: free strings in trace__parse_events_option

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:23PM +0200, Riccardo Mancini escreveu:
> ASan reports several memory leaks running the perf test
> "88: Check open filename arg using perf trace + vfs_getname".
> The fourth of these leaks is related to some strings never being freed
> in trace__parse_events_option.
> 
> This patch adds the missing frees.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/builtin-trace.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index d9c65d55a9ae7526..4a677f3ff273887a 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -4659,6 +4659,9 @@ static int trace__parse_events_option(const struct option *opt, const char *str,
>  		err = parse_events_option(&o, lists[0], 0);
>  	}
>  out:
> +	free(strace_groups_dir);
> +	free(lists[0]);
> +	free(lists[1]);
>  	if (sep)
>  		*sep = ',';
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo

[PATCH 19/20] perf test: bpf: free obj_buf

[Riccardo Mancini]

ASan reports some memory leaks when running the perf test
"42: BPF filter".
The first of these leaks is caused by obj_buf never being deallocated in
__test__bpf.

This patch adds the missing free.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/tests/bpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 33bda9c265423547..2cda4a6297e07967 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -276,6 +276,7 @@ static int __test__bpf(int idx)
 	}
 
 out:
+	free(obj_buf);
 	bpf__clear();
 	return ret;
 }
-- 
2.31.1

Re: [PATCH 19/20] perf test: bpf: free obj_buf

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:24PM +0200, Riccardo Mancini escreveu:
> ASan reports some memory leaks when running the perf test
> "42: BPF filter".
> The first of these leaks is caused by obj_buf never being deallocated in
> __test__bpf.
> 
> This patch adds the missing free.

Fixes: ba1fae431e74bb42 ("perf test: Add 'perf test BPF'")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/bpf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 33bda9c265423547..2cda4a6297e07967 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -276,6 +276,7 @@ static int __test__bpf(int idx)
>  	}
>  
>  out:
> +	free(obj_buf);
>  	bpf__clear();
>  	return ret;
>  }
> -- 
> 2.31.1
> 

-- 

- Arnaldo

Re: [PATCH 19/20] perf test: bpf: free obj_buf

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:24PM +0200, Riccardo Mancini escreveu:
> ASan reports some memory leaks when running the perf test
> "42: BPF filter".
> The first of these leaks is caused by obj_buf never being deallocated in
> __test__bpf.
> 
> This patch adds the missing free.
> 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/tests/bpf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 33bda9c265423547..2cda4a6297e07967 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -276,6 +276,7 @@ static int __test__bpf(int idx)
>  	}
>  
>  out:
> +	free(obj_buf);
>  	bpf__clear();
>  	return ret;
>  }
> -- 

I followed the advice and added the stdlib.h include, elsewhere we're
getting included via some other include by luck.

- Arnaldo

  66     6.97 ubuntu:18.04-x-m68k           : FAIL gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)
    tests/bpf.c: In function '__test__bpf':
    tests/bpf.c:279:2: error: implicit declaration of function 'free' [-Werror=implicit-function-declaration]
      free(obj_buf);
      ^~~~
    tests/bpf.c:279:2: error: incompatible implicit declaration of built-in function 'free' [-Werror]
    tests/bpf.c:279:2: note: include '<stdlib.h>' or provide a declaration of 'free'
    cc1: all warnings being treated as errors
    /git/perf-5.14.0-rc1/tools/build/Makefile.build:139: recipe for target 'tests' failed
    make[3]: *** [tests] Error 2

[PATCH 20/20] perf util/probe-file: delete namelist on error in del_events

[Riccardo Mancini]

ASan reports some memory leaks when running the perf test
"42: BPF filter".
This second leak is caused by a strlist not being dellocated on error
inside probe_file__del_events.

This patch adds a goto label before the deallocation and makes the error
path jump to it.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/util/probe-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index f9a6cbcd641501b8..3d50de3217d50ae1 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -377,11 +377,11 @@ int probe_file__del_events(int fd, struct strfilter *filter)
 
 	ret = probe_file__get_events(fd, filter, namelist);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = probe_file__del_strlist(fd, namelist);
+out:
 	strlist__delete(namelist);
-
 	return ret;
 }
 
-- 
2.31.1

Re: [PATCH 20/20] perf util/probe-file: delete namelist on error in del_events

[Arnaldo Carvalho de Melo]

Em Thu, Jul 15, 2021 at 06:07:25PM +0200, Riccardo Mancini escreveu:
> ASan reports some memory leaks when running the perf test
> "42: BPF filter".
> This second leak is caused by a strlist not being dellocated on error
> inside probe_file__del_events.
> 
> This patch adds a goto label before the deallocation and makes the error
> path jump to it.

Fixes: e7895e422e4da63d ("perf probe: Split del_perf_probe_events()")

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/util/probe-file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index f9a6cbcd641501b8..3d50de3217d50ae1 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -377,11 +377,11 @@ int probe_file__del_events(int fd, struct strfilter *filter)
>  
>  	ret = probe_file__get_events(fd, filter, namelist);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
>  
>  	ret = probe_file__del_strlist(fd, namelist);
> +out:
>  	strlist__delete(namelist);
> -
>  	return ret;
>  }
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo