fix some perf issues detected by ASan

AddressSanitizer (or ASan) and UndefinedBehaviorSanitizer (or UBSan) are very useful tools to detect program bugs. This series fixed some issues disclosed by ASan.

AddressSanitizer (or ASan) is a GCC feature that detects memory corruption bugs such as buffer overflows or memory leaks.

  $ cd tools/perf
  $ make DEBUG=1 EXTRA_CFLAGS='-fno-omit-frame-pointer -fsanitize=address'
  $ ASAN_OPTIONS=log_path=asan.log ./perf record -a
ASan outputs all detected issues into log file 'asan.log.<pid>'.

Note that this only fixes parts of the detected issues. There are many remaining to be fixed[1].

[1] http://104.238.181.70:8080/asan.log.32555.txt

Changbin Du (16):
perf: add doc for how to build perf with Asan and UBSan
perf: list: fix memory leak in function is_event_supported
perf: fix errors under optimization level '-Og'
perf: fix an error in config template
perf: fix a memory leak in collect_config
perf: fix memory leak in print_sdt_events()
perf: top: fix heap-use-after-free issue
perf: top: fix error handing in cmd_top()
perf: missed a map__put() in error case
perf: remove map from names tree in __maps__remove
perf: purge all maps from the names tree
perf: top: fix global-buffer-overflow issue
perf: free all counts in perf_evsel__exit
perf: fix a memory leak of cpu_map object
perf: fix memory leak by expr__find_other
perf: fix a memory leak in test__perf_evsel__tp_sched_test

 tools/lib/bpf/libbpf.c                     |  2 +-
 tools/perf/Documentation/Build.txt         | 20 +++++++++
 tools/perf/Documentation/perf-config.txt   |  2 +-
 tools/perf/bench/epoll-ctl.c               |  2 +-
 tools/perf/bench/epoll-wait.c              |  2 +-
 tools/perf/builtin-top.c                   | 47 ++++++++++------------
 tools/perf/tests/backward-ring-buffer.c    |  2 +-
 tools/perf/tests/evsel-tp-sched.c          |  1 +
 tools/perf/tests/expr.c                    |  5 ++-
 tools/perf/tests/openat-syscall-all-cpus.c |  4 +-
 tools/perf/util/build-id.c                 |  1 +
 tools/perf/util/config.c                   |  3 +-
 tools/perf/util/counts.c                   | 18 +++++++++
 tools/perf/util/counts.h                   |  4 ++
 tools/perf/util/evsel.c                    |  2 +
 tools/perf/util/hist.c                     |  4 +-
 tools/perf/util/map.c                      | 18 +++++++++
 tools/perf/util/ordered-events.c           |  2 +
 tools/perf/util/parse-events.c             |  2 +
 tools/perf/util/stat.c                     | 18 ---------
 20 files changed, 106 insertions(+), 53 deletions(-)

2.19.1

AddressSanitizer (or ASan) and UndefinedBehaviorSanitizer (or UBSan) are very useful tools to detect program bugs.
o AddressSanitizer (or ASan) is a GCC feature that detects memory corruption bugs such as buffer overflows or memory leaks.
o UndefinedBehaviorSanitizer (or UBSan) is a fast undefined behavior detector supprted by GCC. UBSan detect undefined behavior of programs at runtime.

This patch adds doc for how to use them on perf. Later patches will fix some of the issues disclosed by them.

---
 tools/perf/Documentation/Build.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

index f6fc6507ba55..cae6e1d9b901 100644
@@ -47,3 +47,23 @@ Those objects are then used in final linking:
 
 NOTE this description is omitting other libraries involved, only
      focusing on build framework outcomes
+
+3) buld and install perf
+  $ cd tools/perf
+  $ make DESTDIR=/usr
+  $ make DESTDIR=/usr install
+
+4) build with ASan or UBSan
+AddressSanitizer (or ASan) is a GCC feature that detects memory corruption bugs
+such as buffer overflows or memory leaks.
+  $ cd tools/perf
+  $ make DEBUG=1 EXTRA_CFLAGS='-fno-omit-frame-pointer -fsanitize=address'
+  $ ASAN_OPTIONS=log_path=asan.log ./perf record -a
+ASan outputs all detected issues into log file 'asan.log.<pid>'.
+
+UndefinedBehaviorSanitizer (or UBSan) is a fast undefined behavior detector
+supprted by GCC. UBSan detect undefined behavior of programs at runtime.
+  $ cd tools/perf
+  $ make DEBUG=1 EXTRA_CFLAGS='-fno-omit-frame-pointer -fsanitize=undefined'
+  $ UBSAN_OPTIONS=print_stacktrace=1 ./perf record -a
+If UBSan detects any problem at runtime, it outputs a “runtime error:” message.
\ No newline at end of file

2.19.1

Em Mon, Mar 18, 2019 at 11:07:58AM +0100, Jiri Olsa escreveu:

Did this and appart from the evsel->counts one that I fixed up, all applied as-is, I added all the Fixes: tags, please consider doing it next time.

Please take a look after I push it to my perf/core branch.

Thanks,

- Arnaldo

ok, no problem.

Cheers,
Changbin Du

Sorry for delay.

Thanks, I saw Arnaldo already taked this change with your suggestion.

Cheers,
Changbin Du

Don't forget to put the allocated thread_map.

Direct leak of 2048 byte(s) in 64 object(s) allocated from:

  6     #0 0x7f606512e370 in __interceptor_realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee370)
  7     #1 0x556b0f1d7ddd in thread_map__realloc util/thread_map.c:43
  8     #2 0x556b0f1d84c7 in thread_map__new_by_tid util/thread_map.c:85
  9     #3 0x556b0f0e045e in is_event_supported util/parse-events.c:2250
 10     #4 0x556b0f0e1aa1 in print_hwcache_events util/parse-events.c:2382
 11     #5 0x556b0f0e3231 in print_events util/parse-events.c:2514
 12     #6 0x556b0ee0a66e in cmd_list /home/changbin/work/linux/tools/perf/builtin-list.c:58
 13     #7 0x556b0f01e0ae in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
 14     #8 0x556b0f01e859 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
 15     #9 0x556b0f01edc8 in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
 16     #10 0x556b0f01f71f in main /home/changbin/work/linux/tools/perf/perf.c:520
 17     #11 0x7f6062ccf09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/util/parse-events.c | 1 +
 1 file changed, 1 insertion(+)

index 4dcc01b2532c..2e9035c4c252 100644
@@ -2271,6 +2271,7 @@ static bool is_event_supported(u8 type, unsigned config)
 		perf_evsel__delete(evsel);
 	}
 
+	thread_map__put(tmap);
 	return ret;
 }
 

2.19.1

Optimization level '-Og' offers a reasonable level of optimization while maintaining fast compilation and a good debugging experience. This patch tries to make it work.

$ make DEBUG=1 EXTRA_CFLAGS='-Og'
bench/epoll-ctl.c: In function ‘do_threads’:
bench/epoll-ctl.c:274:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return ret;
         ^~~
...

---
 tools/lib/bpf/libbpf.c                  | 2 +-
 tools/perf/bench/epoll-ctl.c            | 2 +-
 tools/perf/bench/epoll-wait.c           | 2 +-
 tools/perf/tests/backward-ring-buffer.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

index d5b830d60601..a37dac0a016e 100644
@@ -622,7 +622,7 @@ bpf_object__init_maps(struct bpf_object *obj, int flags)
 	bool strict = !(flags & MAPS_RELAX_COMPAT);
 	int i, map_idx, map_def_sz, nr_maps = 0;
 	Elf_Scn *scn;
-	Elf_Data *data;
+	Elf_Data *data = NULL;
 	Elf_Data *symbols = obj->efile.symbols;
 
 	if (obj->efile.maps_shndx < 0)

index 0c0a6e824934..2af067859966 100644
@@ -224,7 +224,7 @@ static int do_threads(struct worker *worker, struct cpu_map *cpu)
 	pthread_attr_t thread_attr, *attrp = NULL;
 	cpu_set_t cpuset;
 	unsigned int i, j;
-	int ret;
+	int ret = 0;
 
 	if (!noaffinity)
 		pthread_attr_init(&thread_attr);

index 5a11534e96a0..fe85448abd45 100644
@@ -293,7 +293,7 @@ static int do_threads(struct worker *worker, struct cpu_map *cpu)
 	pthread_attr_t thread_attr, *attrp = NULL;
 	cpu_set_t cpuset;
 	unsigned int i, j;
-	int ret, events = EPOLLIN;
+	int ret = 0, events = EPOLLIN;
 
 	if (oneshot)
 		events |= EPOLLONESHOT;

index 6d598cc071ae..1a9c3becf5ff 100644
@@ -18,7 +18,7 @@ static void testcase(void)
 	int i;
 
 	for (i = 0; i < NR_ITERS; i++) {
-		char proc_name[10];
+		char proc_name[15];
 
 		snprintf(proc_name, sizeof(proc_name), "p:%d\n", i);
 		prctl(PR_SET_NAME, proc_name);

2.19.1

The option 'sort-order' should be 'sort_order'.

---
 tools/perf/Documentation/perf-config.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 86f3dcc15f83..ba7485d86837 100644
@@ -114,7 +114,7 @@ Given a $HOME/.perfconfig like this:
 
 	[report]
 		# Defaults
-		sort-order = comm,dso,symbol
+		sort_order = comm,dso,symbol
 		percent-limit = 0
 		queue-size = 0
 		children = true

2.19.1

Direct leak of 66 byte(s) in 5 object(s) allocated from:

    #0 0x7ff3b1f32070 in __interceptor_strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3b070)
    #1 0x560c8761034d in collect_config util/config.c:597
    #2 0x560c8760d9cb in get_value util/config.c:169
    #3 0x560c8760dfd7 in perf_parse_file util/config.c:285
    #4 0x560c8760e0d2 in perf_config_from_file util/config.c:476
    #5 0x560c876108fd in perf_config_set__init util/config.c:661
    #6 0x560c87610c72 in perf_config_set__new util/config.c:709
    #7 0x560c87610d2f in perf_config__init util/config.c:718
    #8 0x560c87610e5d in perf_config util/config.c:730
    #9 0x560c875ddea0 in main /home/changbin/work/linux/tools/perf/perf.c:442
    #10 0x7ff3afb8609a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/util/config.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index fa092511c52b..7e3c1b60120c 100644
@@ -633,11 +633,10 @@ static int collect_config(const char *var, const char *value,
 	}
 
 	ret = set_value(item, value);
-	return ret;
 
 out_free:
 	free(key);
-	return -1;
+	return ret;
 }
 
 int perf_config_set__collect(struct perf_config_set *set, const char *file_name,

2.19.1

Direct leak of 4356 byte(s) in 120 object(s) allocated from:

    #0 0x7ff1a2b5a070 in __interceptor_strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3b070)
    #1 0x55719aef4814 in build_id_cache__origname util/build-id.c:215
    #2 0x55719af649b6 in print_sdt_events util/parse-events.c:2339
    #3 0x55719af66272 in print_events util/parse-events.c:2542
    #4 0x55719ad1ecaa in cmd_list /home/changbin/work/linux/tools/perf/builtin-list.c:58
    #5 0x55719aec745d in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #6 0x55719aec7d1a in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #7 0x55719aec8184 in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #8 0x55719aeca41a in main /home/changbin/work/linux/tools/perf/perf.c:520
    #9 0x7ff1a07ae09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/util/build-id.c     | 1 +
 tools/perf/util/parse-events.c | 1 +
 2 files changed, 2 insertions(+)

index bff0d17920ed..0c5517a8d0b7 100644
@@ -185,6 +185,7 @@ char *build_id_cache__linkname(const char *sbuild_id, char *bf, size_t size)
 	return bf;
 }
 
+/* The caller is responsible to free the returned buffer. */
 char *build_id_cache__origname(const char *sbuild_id)
 {
 	char *linkname;

index 2e9035c4c252..5ef4939408f2 100644
@@ -2342,6 +2342,7 @@ void print_sdt_events(const char *subsys_glob, const char *event_glob,
 				printf("  %-50s [%s]\n", buf, "SDT event");
 				free(buf);
 			}
+			free(path);
 		} else
 			printf("  %-50s [%s]\n", nd->s, "SDT event");
 		if (nd2) {

2.19.1

The evlist should be destroyed before the perf session.

=================================================================
==27350==ERROR: AddressSanitizer: heap-use-after-free on address 0x62b000002e38 at pc 0x5611da276999 bp 0x7ffce8f1d1a0 sp 0x7ffce8f1d190 WRITE of size 8 at 0x62b000002e38 thread T0

    #0 0x5611da276998 in __list_del /home/work/linux/tools/include/linux/list.h:89
    #1 0x5611da276d4a in __list_del_entry /home/work/linux/tools/include/linux/list.h:102
    #2 0x5611da276e77 in list_del_init /home/work/linux/tools/include/linux/list.h:145
    #3 0x5611da2781cd in thread__put util/thread.c:130
    #4 0x5611da2cc0a8 in __thread__zput util/thread.h:68
    #5 0x5611da2d2dcb in hist_entry__delete util/hist.c:1148
    #6 0x5611da2cdf91 in hists__delete_entry util/hist.c:337
    #7 0x5611da2ce19e in hists__delete_entries util/hist.c:365
    #8 0x5611da2db2ab in hists__delete_all_entries util/hist.c:2639
    #9 0x5611da2db325 in hists_evsel__exit util/hist.c:2651
    #10 0x5611da1c5352 in perf_evsel__exit util/evsel.c:1304
    #11 0x5611da1c5390 in perf_evsel__delete util/evsel.c:1309
    #12 0x5611da1b35f0 in perf_evlist__purge util/evlist.c:124
    #13 0x5611da1b38e2 in perf_evlist__delete util/evlist.c:148
    #14 0x5611da069781 in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1645
    #15 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #16 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #17 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #18 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #19 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #20 0x5611d9ff35c9 in _start (/home/work/linux/tools/perf/perf+0x3e95c9)

0x62b000002e38 is located 11320 bytes inside of 27448-byte region [0x62b000000200,0x62b000006d38) freed by thread T0 here:

    #0 0x7fdccb04ab70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xedb70)
    #1 0x5611da260df4 in perf_session__delete util/session.c:201
    #2 0x5611da063de5 in __cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1300
    #3 0x5611da06973c in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1642
    #4 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #5 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #6 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #7 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #8 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

previously allocated by thread T0 here:

    #0 0x7fdccb04b138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
    #1 0x5611da26010c in zalloc util/util.h:23
    #2 0x5611da260824 in perf_session__new util/session.c:118
    #3 0x5611da0633a6 in __cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1192
    #4 0x5611da06973c in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1642
    #5 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #6 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #7 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #8 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #9 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-use-after-free /home/work/linux/tools/include/linux/list.h:89 in __list_del Shadow bytes around the buggy address:
0x0c567fff8570: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff8580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff8590: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff85a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff85b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c567fff85c0: fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd
0x0c567fff85d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff85e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff85f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff8600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c567fff8610: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07

  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

==27350==ABORTING

---
 tools/perf/builtin-top.c | 42 ++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

index 231a90daa958..614f278235fa 100644
@@ -1189,23 +1189,19 @@ static int __cmd_top(struct perf_top *top)
 	pthread_t thread, thread_process;
 	int ret;
 
-	top->session = perf_session__new(NULL, false, NULL);
-	if (top->session == NULL)
-		return -1;
-
 	if (!top->annotation_opts.objdump_path) {
 		ret = perf_env__lookup_objdump(&top->session->header.env,
 					       &top->annotation_opts.objdump_path);
 		if (ret)
-			goto out_delete;
+			return ret;
 	}
 
 	ret = callchain_param__setup_sample_type(&callchain_param);
 	if (ret)
-		goto out_delete;
+		return ret;
 
 	if (perf_session__register_idle_thread(top->session) < 0)
-		goto out_delete;
+		return ret;
 
 	if (top->nr_threads_synthesize > 1)
 		perf_set_multithreaded();

@@ -1227,13 +1223,18 @@ static int __cmd_top(struct perf_top *top)
 
 	if (perf_hpp_list.socket) {
 		ret = perf_env__read_cpu_topology_map(&perf_env);
-		if (ret < 0)
-			goto out_err_cpu_topo;
+		if (ret < 0) {
+			char errbuf[BUFSIZ];
+			const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
+
+			ui__error("Could not read the CPU topology map: %s\n", err);
+			return ret;
+		}
 	}
 
 	ret = perf_top__start_counters(top);
 	if (ret)
-		goto out_delete;
+		return ret;
 
 	top->session->evlist = top->evlist;
 	perf_session__set_id_hdr_size(top->session);

@@ -1252,7 +1253,7 @@ static int __cmd_top(struct perf_top *top)
 	ret = -1;
 	if (pthread_create(&thread_process, NULL, process_thread, top)) {
 		ui__error("Could not create process thread.\n");
-		goto out_delete;
+		return ret;
 	}
 
 	if (pthread_create(&thread, NULL, (use_browser > 0 ? display_thread_tui :

@@ -1296,19 +1297,7 @@ static int __cmd_top(struct perf_top *top)
 out_join_thread:
 	pthread_cond_signal(&top->qe.cond);
 	pthread_join(thread_process, NULL);
-out_delete:
-	perf_session__delete(top->session);
-	top->session = NULL;
-
 	return ret;
-
-out_err_cpu_topo: {
-	char errbuf[BUFSIZ];
-	const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
-
-	ui__error("Could not read the CPU topology map: %s\n", err);
-	goto out_delete;
-}
 }
 
 static int

@@ -1639,10 +1628,17 @@ int cmd_top(int argc, const char **argv)
 		signal(SIGWINCH, winch_sig);
 	}
 
+	top.session = perf_session__new(NULL, false, NULL);
+	if (top.session == NULL) {
+		status = -1;
+		goto out_delete_evlist;
+	}
+
 	status = __cmd_top(&top);
 
 out_delete_evlist:
 	perf_evlist__delete(top.evlist);
+	perf_session__delete(top.session);
 
 	return status;
 }

2.19.1

ah, because thread is on the machine's list.. which is inside session looks like we could use refference counts in machine in future

thanks,
jirka

We should go to the cleanup path.

---
 tools/perf/builtin-top.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 614f278235fa..2508a7a552fa 100644
@@ -1617,8 +1617,9 @@ int cmd_top(int argc, const char **argv)
 	annotation_config__init();
 
 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
-	if (symbol__init(NULL) < 0)
-		return -1;
+	status = symbol__init(NULL);
+	if (status < 0)
+		goto out_delete_evlist;
 
 	sort__setup_elide(stdout);
 

2.19.1

We need to map__put() before returning from failure of sample__resolve_callchain().

---
 tools/perf/util/hist.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index f9eb95bf3938..ae74b82f51ea 100644
@@ -1062,8 +1062,10 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
 
 	err = sample__resolve_callchain(iter->sample, &callchain_cursor, &iter->parent,
 					iter->evsel, al, max_stack_depth);
-	if (err)
+	if (err) {
+		map__put(alm);
 		return err;
+	}
 
 	err = iter->ops->prepare_entry(iter, al);
 	if (err)

2.19.1

There two trees for each map inserted by maps__insert().

---
 tools/perf/util/map.c | 3 +++
 1 file changed, 3 insertions(+)

index fbeb0c6efaa6..64bea5eb8bf6 100644
@@ -917,6 +917,9 @@ static void __maps__remove(struct maps *maps, struct map *map)
 {
 	rb_erase_init(&map->rb_node, &maps->entries);
 	map__put(map);
+
+	rb_erase_init(&map->rb_node_name, &maps->names);
+	map__put(map);
 }
 
 void maps__remove(struct maps *maps, struct map *map)

2.19.1

Add function __maps__purge_names() to purge all maps from the names tree. We need to cleanup the names tree in maps__exit().

---
 tools/perf/util/map.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

index 64bea5eb8bf6..e32628cd20a7 100644
@@ -577,10 +577,25 @@ static void __maps__purge(struct maps *maps)
 	}
 }
 
+static void __maps__purge_names(struct maps *maps)
+{
+	struct rb_root *root = &maps->names;
+	struct rb_node *next = rb_first(root);
+
+	while (next) {
+		struct map *pos = rb_entry(next, struct map, rb_node_name);
+
+		next = rb_next(&pos->rb_node_name);
+		rb_erase_init(&pos->rb_node_name, root);
+		map__put(pos);
+	}
+}
+
 static void maps__exit(struct maps *maps)
 {
 	down_write(&maps->lock);
 	__maps__purge(maps);
+	__maps__purge_names(maps);
 	up_write(&maps->lock);
 }
 

2.19.1

The arrary str[] should have six elements.

=================================================================
==4322==ERROR: AddressSanitizer: global-buffer-overflow on address 0x56463844e300 at pc 0x564637e7ad0d bp 0x7f30c8c89d10 sp 0x7f30c8c89d00 READ of size 8 at 0x56463844e300 thread T9

    #0 0x564637e7ad0c in __ordered_events__flush util/ordered-events.c:316
    #1 0x564637e7b0e4 in ordered_events__flush util/ordered-events.c:338
    #2 0x564637c6a57d in process_thread /home/changbin/work/linux/tools/perf/builtin-top.c:1073
    #3 0x7f30d173a163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)
    #4 0x7f30cfffbdee in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x11adee)

0x56463844e300 is located 32 bytes to the left of global variable 'flags' defined in 'util/trace-event-parse.c:229:26' (0x56463844e320) of size 192 0x56463844e300 is located 0 bytes to the right of global variable 'str' defined in 'util/ordered-events.c:268:28' (0x56463844e2e0) of size 32
SUMMARY: AddressSanitizer: global-buffer-overflow util/ordered-events.c:316 in __ordered_events__flush
Shadow bytes around the buggy address:
0x0ac947081c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c50: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
=>0x0ac947081c60:[f9]f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c70: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
0x0ac947081c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081c90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081ca0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0ac947081cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07

  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T9 created by T0 here:
    #0 0x7f30d179de5f in __interceptor_pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x4ae5f)
    #1 0x564637c6b954 in __cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1253
    #2 0x564637c7173c in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1642
    #3 0x564637d85038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #4 0x564637d85577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #5 0x564637d8597b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #6 0x564637d860e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #7 0x7f30cff0509a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/util/ordered-events.c | 2 ++
 1 file changed, 2 insertions(+)

index ea523d3b248f..989fed6f43b5 100644
@@ -270,6 +270,8 @@ static int __ordered_events__flush(struct ordered_events *oe, enum oe_flush how,
 		"FINAL",
 		"ROUND",
 		"HALF ",
+		"TOP  ",
+		"TIME ",
 	};
 	int err;
 	bool show_progress = false;

2.19.1

Ensure that we have freed all allocated counts for struct perf_evsel.

=================================================================
==7494==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 48 byte(s) in 1 object(s) allocated from:

    #0 0x7f0333a89138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
    #1 0x5625e5330a5e in zalloc util/util.h:23
    #2 0x5625e5330a9b in perf_counts__new util/counts.c:10
    #3 0x5625e5330ca0 in perf_evsel__alloc_counts util/counts.c:47
    #4 0x5625e520d8e5 in __perf_evsel__read_on_cpu util/evsel.c:1505
    #5 0x5625e517a985 in perf_evsel__read_on_cpu /home/work/linux/tools/perf/util/evsel.h:347
    #6 0x5625e517ad1a in test__openat_syscall_event tests/openat-syscall.c:47
    #7 0x5625e51528e6 in run_test tests/builtin-test.c:358
    #8 0x5625e5152baf in test_and_print tests/builtin-test.c:388
    #9 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
    #10 0x5625e515572f in cmd_test tests/builtin-test.c:722
    #11 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #12 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #13 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #14 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #15 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Indirect leak of 72 byte(s) in 1 object(s) allocated from:

    #0 0x7f0333a89138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
    #1 0x5625e532560d in zalloc util/util.h:23
    #2 0x5625e532566b in xyarray__new util/xyarray.c:10
    #3 0x5625e5330aba in perf_counts__new util/counts.c:15
    #4 0x5625e5330ca0 in perf_evsel__alloc_counts util/counts.c:47
    #5 0x5625e520d8e5 in __perf_evsel__read_on_cpu util/evsel.c:1505
    #6 0x5625e517a985 in perf_evsel__read_on_cpu /home/work/linux/tools/perf/util/evsel.h:347
    #7 0x5625e517ad1a in test__openat_syscall_event tests/openat-syscall.c:47
    #8 0x5625e51528e6 in run_test tests/builtin-test.c:358
    #9 0x5625e5152baf in test_and_print tests/builtin-test.c:388
    #10 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
    #11 0x5625e515572f in cmd_test tests/builtin-test.c:722
    #12 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #13 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #14 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #15 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #16 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/util/counts.c | 18 ++++++++++++++++++
 tools/perf/util/counts.h |  4 ++++
 tools/perf/util/evsel.c  |  2 ++
 tools/perf/util/stat.c   | 18 ------------------
 4 files changed, 24 insertions(+), 18 deletions(-)

index 03032b410c29..c5715d08b1f9 100644
@@ -53,3 +53,21 @@ void perf_evsel__free_counts(struct perf_evsel *evsel)
 	perf_counts__delete(evsel->counts);
 	evsel->counts = NULL;
 }
+
+int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
+				      int ncpus, int nthreads)
+{
+	struct perf_counts *counts;
+
+	counts = perf_counts__new(ncpus, nthreads);
+	if (counts)
+		evsel->prev_raw_counts = counts;
+
+	return counts ? 0 : -ENOMEM;
+}
+
+void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
+{
+	perf_counts__delete(evsel->prev_raw_counts);
+	evsel->prev_raw_counts = NULL;
+}

index 0d1050ccc586..b8bdbff586bb 100644
@@ -36,4 +36,8 @@ void perf_evsel__reset_counts(struct perf_evsel *evsel);
 int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus, int nthreads);
 void perf_evsel__free_counts(struct perf_evsel *evsel);
 
+int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
+				      int ncpus, int nthreads);
+void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel);
+
 #endif /* __PERF_COUNTS_H */

index 3bbf73e979c0..8e0fbe34e5d9 100644
@@ -1294,6 +1294,8 @@ void perf_evsel__exit(struct perf_evsel *evsel)
 	assert(evsel->evlist == NULL);
 	perf_evsel__free_fd(evsel);
 	perf_evsel__free_id(evsel);
+	perf_evsel__free_counts(evsel);
+	perf_evsel__free_prev_raw_counts(evsel);
 	perf_evsel__free_config_terms(evsel);
 	cgroup__put(evsel->cgrp);
 	cpu_map__put(evsel->cpus);

index 4d40515307b8..6a22842f76d2 100644
@@ -136,24 +136,6 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 	zfree(&evsel->stats);
 }
 
-static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
-					     int ncpus, int nthreads)
-{
-	struct perf_counts *counts;
-
-	counts = perf_counts__new(ncpus, nthreads);
-	if (counts)
-		evsel->prev_raw_counts = counts;
-
-	return counts ? 0 : -ENOMEM;
-}
-
-static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
-{
-	perf_counts__delete(evsel->prev_raw_counts);
-	evsel->prev_raw_counts = NULL;
-}
-
 static int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
 {
 	int ncpus = perf_evsel__nr_cpus(evsel);

2.19.1

Em Sat, Mar 16, 2019 at 04:05:53PM +0800, Changbin Du escreveu:

Does this backtrace corresponds to this patch? I don't think so, this is allocating evsel->counts, not evsel->prev_raw_counts.

The fix for this specific one is to call perf_evsel__free_counts() in perf_evsel__exit(), ok? I'm adding a patch to that effect.

The case for evsel->prev_raw_counts is different, I think this started in 'perf stat' and ended up moving from evsel->priv handled by 'perf stat' to a new evsel field, prev_raw_counts, and that was being handled by the stat code, that would call perf_evsel__free_prev_raw_counts() before calling perf_evsel__delete(), I think. But I would have to double check.

If you find some leak where evsel->prev_raw_counts leaks, or if you determine that by looking at the code, please submit a patch for that case, ok?

- Arnaldo

- Arnaldo

=================================================================
==7497==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:

    #0 0x7f0333a88f30 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xedf30)
    #1 0x5625e5326213 in cpu_map__trim_new util/cpumap.c:45
    #2 0x5625e5326703 in cpu_map__read util/cpumap.c:103
    #3 0x5625e53267ef in cpu_map__read_all_cpu_map util/cpumap.c:120
    #4 0x5625e5326915 in cpu_map__new util/cpumap.c:135
    #5 0x5625e517b355 in test__openat_syscall_event_on_all_cpus tests/openat-syscall-all-cpus.c:36
    #6 0x5625e51528e6 in run_test tests/builtin-test.c:358
    #7 0x5625e5152baf in test_and_print tests/builtin-test.c:388
    #8 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
    #9 0x5625e515572f in cmd_test tests/builtin-test.c:722
    #10 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #11 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #12 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #13 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #14 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/tests/openat-syscall-all-cpus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index c531e6deb104..493ecb611540 100644
@@ -45,7 +45,7 @@ int test__openat_syscall_event_on_all_cpus(struct test *test __maybe_unused, int
 	if (IS_ERR(evsel)) {
 		tracing_path__strerror_open_tp(errno, errbuf, sizeof(errbuf), "syscalls", "sys_enter_openat");
 		pr_debug("%s\n", errbuf);
-		goto out_thread_map_delete;
+		goto out_cpu_map_delete;
 	}
 
 	if (perf_evsel__open(evsel, cpus, threads) < 0) {

@@ -119,6 +119,8 @@ int test__openat_syscall_event_on_all_cpus(struct test *test __maybe_unused, int
 	perf_evsel__close_fd(evsel);
 out_evsel_delete:
 	perf_evsel__delete(evsel);
+out_cpu_map_delete:
+	cpu_map__put(cpus);
 out_thread_map_delete:
 	thread_map__put(threads);
 	return err;

2.19.1

=================================================================
==7506==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 13 byte(s) in 3 object(s) allocated from:

    #0 0x7f03339d6070 in __interceptor_strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3b070)
    #1 0x5625e53aaef0 in expr__find_other util/expr.y:221
    #2 0x5625e51bcd3f in test__expr tests/expr.c:52
    #3 0x5625e51528e6 in run_test tests/builtin-test.c:358
    #4 0x5625e5152baf in test_and_print tests/builtin-test.c:388
    #5 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
    #6 0x5625e515572f in cmd_test tests/builtin-test.c:722
    #7 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #8 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #9 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #10 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #11 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

---
 tools/perf/tests/expr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 01f0706995a9..9acc1e80b936 100644
@@ -19,7 +19,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
 	const char *p;
 	const char **other;
 	double val;
-	int ret;
+	int i, ret;
 	struct parse_ctx ctx;
 	int num_other;
 

@@ -56,6 +56,9 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
 	TEST_ASSERT_VAL("find other", !strcmp(other[1], "BAZ"));
 	TEST_ASSERT_VAL("find other", !strcmp(other[2], "BOZO"));
 	TEST_ASSERT_VAL("find other", other[3] == NULL);
+
+	for (i = 0; i < num_other; i++)
+		free((void *)other[i]);
 	free((void *)other);
 
 	return 0;

2.19.1

=================================================================
==20875==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1160 byte(s) in 1 object(s) allocated from:

    #0 0x7f1b6fc84138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
    #1 0x55bd50005599 in zalloc util/util.h:23
    #2 0x55bd500068f5 in perf_evsel__newtp_idx util/evsel.c:327
    #3 0x55bd4ff810fc in perf_evsel__newtp /home/work/linux/tools/perf/util/evsel.h:216
    #4 0x55bd4ff81608 in test__perf_evsel__tp_sched_test tests/evsel-tp-sched.c:69
    #5 0x55bd4ff528e6 in run_test tests/builtin-test.c:358
    #6 0x55bd4ff52baf in test_and_print tests/builtin-test.c:388
    #7 0x55bd4ff543fe in __cmd_test tests/builtin-test.c:583
    #8 0x55bd4ff5572f in cmd_test tests/builtin-test.c:722
    #9 0x55bd4ffc4087 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
    #10 0x55bd4ffc45c6 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
    #11 0x55bd4ffc49ca in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
    #12 0x55bd4ffc5138 in main /home/changbin/work/linux/tools/perf/perf.c:520
    #13 0x7f1b6e34809a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

Indirect leak of 19 byte(s) in 1 object(s) allocated from:

    #0 0x7f1b6fc83f30 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xedf30)
    #1 0x7f1b6e3ac30f in vasprintf (/lib/x86_64-linux-gnu/libc.so.6+0x8830f)

---
 tools/perf/tests/evsel-tp-sched.c | 1 +
 1 file changed, 1 insertion(+)

index ea7acf403727..71f60c0f9faa 100644
@@ -85,5 +85,6 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes
 	if (perf_evsel__test_field(evsel, "target_cpu", 4, true))
 		ret = -1;
 
+	perf_evsel__delete(evsel);
 	return ret;
 }

2.19.1

awesome.. thanks a lot

jirka

Em Mon, Mar 18, 2019 at 11:08:12AM +0100, Jiri Olsa escreveu:

Yeah, I'll go over all and apply, thanks a lot.

If you plan to work on the others, please first submit the ones that are not related to the exit path, i.e. leave the ones that only take place when exiting perf for a second batch, this way we'll get the more important ones, that take place in normal operation, first.

- Arnaldo