From 841e3558b2de6d8c53dae4f01c7a81cd53e42e5c Mon Sep 17 00:00:00 2001 From: He Kuang Date: Fri, 6 May 2016 08:59:07 +0000 Subject: perf callchain: Recording 'dwarf' callchains do not need DWARF unwinding support There is no need to check for DWARF unwinding support when using the 'dwarf' callchain record method, as this will only ask the kernel to collect stack dumps for later DWARF CFI processing, which can be done in another machine, where the support for DWARF unwinding need to be present. Signed-off-by: He Kuang Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Andi Kleen Cc: David Ahern Cc: Ekaterina Tumanova Cc: Josh Poimboeuf Cc: Kan Liang Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Pekka Enberg Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Sukadev Bhattiprolu Cc: Wang Nan Link: http://lkml.kernel.org/r/1462525154-125656-2-git-send-email-hekuang@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/util.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index 619ba2061b62..01c9433de7ef 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -507,7 +507,6 @@ int parse_callchain_record(const char *arg, struct callchain_param *param) "needed for --call-graph fp\n"); break; -#ifdef HAVE_DWARF_UNWIND_SUPPORT /* Dwarf style */ } else if (!strncmp(name, "dwarf", sizeof("dwarf"))) { const unsigned long default_stack_dump_size = 8192; @@ -523,7 +522,6 @@ int parse_callchain_record(const char *arg, struct callchain_param *param) ret = get_stack_size(tok, &size); param->dump_size = size; } -#endif /* HAVE_DWARF_UNWIND_SUPPORT */ } else if (!strncmp(name, "lbr", sizeof("lbr"))) { if (!strtok_r(NULL, ",", &saveptr)) { param->record_mode = CALLCHAIN_LBR; -- cgit v1.2.3 From f340c5fc93bda334efd9f2b5855ef0d3746e1564 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 5 May 2016 16:04:04 -0700 Subject: perf stat: Scale values by unit before metrics Scale values by unit before passing them to the metrics printing functions. This is needed for TopDown, because it needs to scale the slots correctly by pipeline width / SMTness. For existing metrics it shouldn't make any difference, as those generally use events that don't have any units. Signed-off-by: Andi Kleen Acked-by: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1462489447-31832-8-git-send-email-andi@firstfloor.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/stat.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c index 4d9b481cf3b6..ffa1d0653861 100644 --- a/tools/perf/util/stat.c +++ b/tools/perf/util/stat.c @@ -307,6 +307,7 @@ int perf_stat_process_counter(struct perf_stat_config *config, struct perf_counts_values *aggr = &counter->counts->aggr; struct perf_stat_evsel *ps = counter->priv; u64 *count = counter->counts->aggr.values; + u64 val; int i, ret; aggr->val = aggr->ena = aggr->run = 0; @@ -346,7 +347,8 @@ int perf_stat_process_counter(struct perf_stat_config *config, /* * Save the full runtime - to allow normalization during printout: */ - perf_stat__update_shadow_stats(counter, count, 0); + val = counter->scale * *count; + perf_stat__update_shadow_stats(counter, &val, 0); return 0; } -- cgit v1.2.3 From aff633406ca2772554bad7b37f2dfbc409b6ea74 Mon Sep 17 00:00:00 2001 From: Chris Phlipot Date: Sat, 7 May 2016 02:17:00 -0700 Subject: perf script: Fix incorrect python db-export error message Fix the error message printed when attempting and failing to create the call path root incorrectly references the call return process. This change fixes the message to properly reference the failure to create the call path root. Signed-off-by: Chris Phlipot Cc: Adrian Hunter Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1462612620-25008-2-git-send-email-cphlipot0@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/scripting-engines/trace-event-python.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c index 091bce67844c..1546b749a3a3 100644 --- a/tools/perf/util/scripting-engines/trace-event-python.c +++ b/tools/perf/util/scripting-engines/trace-event-python.c @@ -1062,7 +1062,7 @@ static void set_table_handlers(struct tables *tables) tables->dbe.cpr = call_path_root__new(); if (!tables->dbe.cpr) - Py_FatalError("failed to create calls processor"); + Py_FatalError("failed to create call path root"); } tables->db_export_mode = true; -- cgit v1.2.3 From e24c7520ea9c3e5bb51592c2134aafbf75a3f88a Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Mon, 9 May 2016 01:47:50 +0000 Subject: perf tools: Support reading from backward ring buffer perf_evlist__mmap_read_backward() is introduced for reading backward ring buffer. Since direction for reading such ring buffer is different from the direction kernel writing to it, and since user need to fetch most recent record from it, a perf_evlist__mmap_read_catchup() is introduced to move the reading pointer to the end of the buffer. Signed-off-by: Wang Nan Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1462758471-89706-2-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/evlist.h | 4 ++++ 2 files changed, 54 insertions(+) (limited to 'tools') diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 17cd01421e7f..c4bfe11479a0 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -766,6 +766,56 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) return perf_mmap__read(md, evlist->overwrite, old, head, &md->prev); } +union perf_event * +perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx) +{ + struct perf_mmap *md = &evlist->mmap[idx]; + u64 head, end; + u64 start = md->prev; + + /* + * Check if event was unmapped due to a POLLHUP/POLLERR. + */ + if (!atomic_read(&md->refcnt)) + return NULL; + + head = perf_mmap__read_head(md); + if (!head) + return NULL; + + /* + * 'head' pointer starts from 0. Kernel minus sizeof(record) form + * it each time when kernel writes to it, so in fact 'head' is + * negative. 'end' pointer is made manually by adding the size of + * the ring buffer to 'head' pointer, means the validate data can + * read is the whole ring buffer. If 'end' is positive, the ring + * buffer has not fully filled, so we must adjust 'end' to 0. + * + * However, since both 'head' and 'end' is unsigned, we can't + * simply compare 'end' against 0. Here we compare '-head' and + * the size of the ring buffer, where -head is the number of bytes + * kernel write to the ring buffer. + */ + if (-head < (u64)(md->mask + 1)) + end = 0; + else + end = head + md->mask + 1; + + return perf_mmap__read(md, false, start, end, &md->prev); +} + +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx) +{ + struct perf_mmap *md = &evlist->mmap[idx]; + u64 head; + + if (!atomic_read(&md->refcnt)) + return; + + head = perf_mmap__read_head(md); + md->prev = head; +} + static bool perf_mmap__empty(struct perf_mmap *md) { return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base; diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 208897a646ca..85d1b59802e8 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -129,6 +129,10 @@ struct perf_sample_id *perf_evlist__id2sid(struct perf_evlist *evlist, u64 id); union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx); +union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, + int idx); +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx); + void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx); int perf_evlist__open(struct perf_evlist *evlist); -- cgit v1.2.3 From ee74701ed8add8ad13e642e8618b51fd75add32d Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Mon, 9 May 2016 01:47:51 +0000 Subject: perf tests: Add test to check backward ring buffer This test checks reading from backward ring buffer. Test result: # ~/perf test 'ring buffer' 45: Test backward reading from ring buffer : Ok The test case is a while loop which calls prctl(PR_SET_NAME) multiple times. Each prctl should issue 2 events: one PERF_RECORD_SAMPLE, one PERF_RECORD_COMM. The first round creates a relative large ring buffer (256 pages). It can afford all events. Read from it and check the count of each type of events. The second round creates a small ring buffer (1 page) and makes it overwritable. Check the correctness of the buffer. Signed-off-by: Wang Nan Tested-by: Arnaldo Carvalho de Melo Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1462758471-89706-3-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/Build | 1 + tools/perf/tests/backward-ring-buffer.c | 151 ++++++++++++++++++++++++++++++++ tools/perf/tests/builtin-test.c | 4 + tools/perf/tests/tests.h | 1 + 4 files changed, 157 insertions(+) create mode 100644 tools/perf/tests/backward-ring-buffer.c (limited to 'tools') diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index 449fe97a555f..66a28982547b 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -38,6 +38,7 @@ perf-y += cpumap.o perf-y += stat.o perf-y += event_update.o perf-y += event-times.o +perf-y += backward-ring-buffer.o $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build $(call rule_mkdir) diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c new file mode 100644 index 000000000000..d9ba991a9a30 --- /dev/null +++ b/tools/perf/tests/backward-ring-buffer.c @@ -0,0 +1,151 @@ +/* + * Test backward bit in event attribute, read ring buffer from end to + * beginning + */ + +#include +#include +#include +#include "tests.h" +#include "debug.h" + +#define NR_ITERS 111 + +static void testcase(void) +{ + int i; + + for (i = 0; i < NR_ITERS; i++) { + char proc_name[10]; + + snprintf(proc_name, sizeof(proc_name), "p:%d\n", i); + prctl(PR_SET_NAME, proc_name); + } +} + +static int count_samples(struct perf_evlist *evlist, int *sample_count, + int *comm_count) +{ + int i; + + for (i = 0; i < evlist->nr_mmaps; i++) { + union perf_event *event; + + perf_evlist__mmap_read_catchup(evlist, i); + while ((event = perf_evlist__mmap_read_backward(evlist, i)) != NULL) { + const u32 type = event->header.type; + + switch (type) { + case PERF_RECORD_SAMPLE: + (*sample_count)++; + break; + case PERF_RECORD_COMM: + (*comm_count)++; + break; + default: + pr_err("Unexpected record of type %d\n", type); + return TEST_FAIL; + } + } + } + return TEST_OK; +} + +static int do_test(struct perf_evlist *evlist, int mmap_pages, + int *sample_count, int *comm_count) +{ + int err; + char sbuf[STRERR_BUFSIZE]; + + err = perf_evlist__mmap(evlist, mmap_pages, true); + if (err < 0) { + pr_debug("perf_evlist__mmap: %s\n", + strerror_r(errno, sbuf, sizeof(sbuf))); + return TEST_FAIL; + } + + perf_evlist__enable(evlist); + testcase(); + perf_evlist__disable(evlist); + + err = count_samples(evlist, sample_count, comm_count); + perf_evlist__munmap(evlist); + return err; +} + + +int test__backward_ring_buffer(int subtest __maybe_unused) +{ + int ret = TEST_SKIP, err, sample_count = 0, comm_count = 0; + char pid[16], sbuf[STRERR_BUFSIZE]; + struct perf_evlist *evlist; + struct perf_evsel *evsel __maybe_unused; + struct parse_events_error parse_error; + struct record_opts opts = { + .target = { + .uid = UINT_MAX, + .uses_mmap = true, + }, + .freq = 0, + .mmap_pages = 256, + .default_interval = 1, + }; + + snprintf(pid, sizeof(pid), "%d", getpid()); + pid[sizeof(pid) - 1] = '\0'; + opts.target.tid = opts.target.pid = pid; + + evlist = perf_evlist__new(); + if (!evlist) { + pr_debug("No ehough memory to create evlist\n"); + return TEST_FAIL; + } + + err = perf_evlist__create_maps(evlist, &opts.target); + if (err < 0) { + pr_debug("Not enough memory to create thread/cpu maps\n"); + goto out_delete_evlist; + } + + bzero(&parse_error, sizeof(parse_error)); + err = parse_events(evlist, "syscalls:sys_enter_prctl", &parse_error); + if (err) { + pr_debug("Failed to parse tracepoint event, try use root\n"); + ret = TEST_SKIP; + goto out_delete_evlist; + } + + perf_evlist__config(evlist, &opts, NULL); + + /* Set backward bit, ring buffer should be writing from end */ + evlist__for_each(evlist, evsel) + evsel->attr.write_backward = 1; + + err = perf_evlist__open(evlist); + if (err < 0) { + pr_debug("perf_evlist__open: %s\n", + strerror_r(errno, sbuf, sizeof(sbuf))); + goto out_delete_evlist; + } + + ret = TEST_FAIL; + err = do_test(evlist, opts.mmap_pages, &sample_count, + &comm_count); + if (err != TEST_OK) + goto out_delete_evlist; + + if ((sample_count != NR_ITERS) || (comm_count != NR_ITERS)) { + pr_err("Unexpected counter: sample_count=%d, comm_count=%d\n", + sample_count, comm_count); + goto out_delete_evlist; + } + + err = do_test(evlist, 1, &sample_count, &comm_count); + if (err != TEST_OK) + goto out_delete_evlist; + + ret = TEST_OK; +out_delete_evlist: + perf_evlist__delete(evlist); + return ret; +} diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 93c467015e71..0e95c20ecf6e 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -207,6 +207,10 @@ static struct test generic_tests[] = { .desc = "Test events times", .func = test__event_times, }, + { + .desc = "Test backward reading from ring buffer", + .func = test__backward_ring_buffer, + }, { .func = NULL, }, diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index 0fc946989cf0..c57e72c826d2 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -86,6 +86,7 @@ int test__synthesize_stat(int subtest); int test__synthesize_stat_round(int subtest); int test__event_update(int subtest); int test__event_times(int subtest); +int test__backward_ring_buffer(int subtest); #if defined(__arm__) || defined(__aarch64__) #ifdef HAVE_DWARF_UNWIND_SUPPORT -- cgit v1.2.3 From 0a241ef4a27a00807fefdf913da104a4534c606a Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 9 May 2016 18:08:33 -0300 Subject: perf evsel: Print state of perf_event_attr.write_backward Now we can see if it is set when using verbose mode in various tools, such as 'perf test': # perf test -vv back 45: Test backward reading from ring buffer : --- start --- ------------------------------------------------------------ perf_event_attr: type 2 size 112 config 0x98 { sample_period, sample_freq } 1 sample_type IP|TID|TIME|CPU|PERIOD|RAW disabled 1 mmap 1 comm 1 task 1 sample_id_all 1 exclude_guest 1 mmap2 1 comm_exec 1 write_backward 1 ------------------------------------------------------------ sys_perf_event_open: pid 20911 cpu -1 group_fd -1 flags 0x8 ---- end ---- Test backward reading from ring buffer: Ok # Cc: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Milian Wolff Cc: Namhyung Kim Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-kxv05kv9qwl5of7rzfeiiwbv@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evsel.c | 1 + 1 file changed, 1 insertion(+) (limited to 'tools') diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 3371721a05f2..a23f54793e51 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -1316,6 +1316,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr, PRINT_ATTRf(comm_exec, p_unsigned); PRINT_ATTRf(use_clockid, p_unsigned); PRINT_ATTRf(context_switch, p_unsigned); + PRINT_ATTRf(write_backward, p_unsigned); PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned); PRINT_ATTRf(bp_type, p_unsigned); -- cgit v1.2.3 From 9c7b37cd63d0d910c531233209286f169993cbd9 Mon Sep 17 00:00:00 2001 From: Chris Phlipot Date: Sat, 7 May 2016 02:16:59 -0700 Subject: perf symbols: Fix handling of zero-length symbols. This change introduces a fix to symbols__find, so that it is able to find symbols of length zero (where start == end). The current code has the following problem: - The current implementation of symbols__find is unable to find any symbols of length zero. - The db-export framework explicitly creates zero length symbols at locations where no symbol currently exists. The combination of the two above behaviors results in behavior similar to the example below. 1. addr_location is created for a sample, but symbol is unable to be resolved. 2. db export creates an "unknown" symbol of length zero at that address and inserts it into the dso. 3. A new sample comes in at the same address, but symbol__find is unable to find the zero length symbol, so it is still unresolved. 4. db export sees the symbol is unresolved, and allocated a duplicate symbol, even though it already did this in step 2. This behavior continues every time an address without symbol information is seen, which causes a very large number of these symbols to be allocated. The effect of this fix can be observed by looking at the contents of an exported database before/after the fix (generated with scripts/python/export-to-postgresql.py) Ex. BEFORE THE CHANGE: example_db=# select count(*) from symbols; count -------- 900213 (1 row) example_db=# select count(*) from symbols where symbols.name='unknown'; count -------- 897355 (1 row) example_db=# select count(*) from symbols where symbols.name!='unknown'; count ------- 2858 (1 row) AFTER THE CHANGE: example_db=# select count(*) from symbols; count ------- 25217 (1 row) example_db=# select count(*) from symbols where name='unknown'; count ------- 22359 (1 row) example_db=# select count(*) from symbols where name!='unknown'; count ------- 2858 (1 row) Signed-off-by: Chris Phlipot Cc: Adrian Hunter Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1462612620-25008-1-git-send-email-cphlipot0@gmail.com [ Moved the test to later in the rb_tree tests, as this not the likely case ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 415c4f6d98fd..2946295ca502 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -301,7 +301,7 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip) if (ip < s->start) n = n->rb_left; - else if (ip >= s->end) + else if (ip > s->end || (ip == s->end && ip != s->start)) n = n->rb_right; else return s; -- cgit v1.2.3 From 5cea57f30a12443c05e0c5273f35d2fcef00d30a Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:46:58 +0900 Subject: perf tools: Rewrite strbuf not to die() Rewrite strbuf implementation not to use die() nor xrealloc(). Instead of die(), now most of the API returns error code or 0 if succeeded. Suggested-by: Arnaldo Carvalho de Melo Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054658.6158.24080.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/strbuf.c | 93 ++++++++++++++++++++++++++++++++++-------------- tools/perf/util/strbuf.h | 25 +++++++------ 2 files changed, 82 insertions(+), 36 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c index 8fb73295ec34..f95f682aa2b2 100644 --- a/tools/perf/util/strbuf.c +++ b/tools/perf/util/strbuf.c @@ -1,3 +1,4 @@ +#include "debug.h" #include "cache.h" #include @@ -17,12 +18,13 @@ int prefixcmp(const char *str, const char *prefix) */ char strbuf_slopbuf[1]; -void strbuf_init(struct strbuf *sb, ssize_t hint) +int strbuf_init(struct strbuf *sb, ssize_t hint) { sb->alloc = sb->len = 0; sb->buf = strbuf_slopbuf; if (hint) - strbuf_grow(sb, hint); + return strbuf_grow(sb, hint); + return 0; } void strbuf_release(struct strbuf *sb) @@ -42,67 +44,104 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz) return res; } -void strbuf_grow(struct strbuf *sb, size_t extra) +int strbuf_grow(struct strbuf *sb, size_t extra) { - if (sb->len + extra + 1 <= sb->len) - die("you want to use way too much memory"); - if (!sb->alloc) - sb->buf = NULL; - ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc); + char *buf; + size_t nr = sb->len + extra + 1; + + if (nr < sb->alloc) + return 0; + + if (nr <= sb->len) + return -E2BIG; + + if (alloc_nr(sb->alloc) > nr) + nr = alloc_nr(sb->alloc); + + /* + * Note that sb->buf == strbuf_slopbuf if sb->alloc == 0, and it is + * a static variable. Thus we have to avoid passing it to realloc. + */ + buf = realloc(sb->alloc ? sb->buf : NULL, nr * sizeof(*buf)); + if (!buf) + return -ENOMEM; + + sb->buf = buf; + sb->alloc = nr; + return 0; } -void strbuf_addch(struct strbuf *sb, int c) +int strbuf_addch(struct strbuf *sb, int c) { - strbuf_grow(sb, 1); + int ret = strbuf_grow(sb, 1); + if (ret) + return ret; + sb->buf[sb->len++] = c; sb->buf[sb->len] = '\0'; + return 0; } -void strbuf_add(struct strbuf *sb, const void *data, size_t len) +int strbuf_add(struct strbuf *sb, const void *data, size_t len) { - strbuf_grow(sb, len); + int ret = strbuf_grow(sb, len); + if (ret) + return ret; + memcpy(sb->buf + sb->len, data, len); - strbuf_setlen(sb, sb->len + len); + return strbuf_setlen(sb, sb->len + len); } -static void strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap) +static int strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap) { - int len; + int len, ret; va_list ap_saved; - if (!strbuf_avail(sb)) - strbuf_grow(sb, 64); + if (!strbuf_avail(sb)) { + ret = strbuf_grow(sb, 64); + if (ret) + return ret; + } va_copy(ap_saved, ap); len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap); if (len < 0) - die("your vsnprintf is broken"); + return len; if (len > strbuf_avail(sb)) { - strbuf_grow(sb, len); + ret = strbuf_grow(sb, len); + if (ret) + return ret; len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved); va_end(ap_saved); if (len > strbuf_avail(sb)) { - die("this should not happen, your vsnprintf is broken"); + pr_debug("this should not happen, your vsnprintf is broken"); + return -EINVAL; } } - strbuf_setlen(sb, sb->len + len); + return strbuf_setlen(sb, sb->len + len); } -void strbuf_addf(struct strbuf *sb, const char *fmt, ...) +int strbuf_addf(struct strbuf *sb, const char *fmt, ...) { va_list ap; + int ret; va_start(ap, fmt); - strbuf_addv(sb, fmt, ap); + ret = strbuf_addv(sb, fmt, ap); va_end(ap); + return ret; } ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint) { size_t oldlen = sb->len; size_t oldalloc = sb->alloc; + int ret; + + ret = strbuf_grow(sb, hint ? hint : 8192); + if (ret) + return ret; - strbuf_grow(sb, hint ? hint : 8192); for (;;) { ssize_t cnt; @@ -112,12 +151,14 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint) strbuf_release(sb); else strbuf_setlen(sb, oldlen); - return -1; + return cnt; } if (!cnt) break; sb->len += cnt; - strbuf_grow(sb, 8192); + ret = strbuf_grow(sb, 8192); + if (ret) + return ret; } sb->buf[sb->len] = '\0'; diff --git a/tools/perf/util/strbuf.h b/tools/perf/util/strbuf.h index ab9be0fbbd40..54b409297d4a 100644 --- a/tools/perf/util/strbuf.h +++ b/tools/perf/util/strbuf.h @@ -51,7 +51,7 @@ struct strbuf { #define STRBUF_INIT { 0, 0, strbuf_slopbuf } /*----- strbuf life cycle -----*/ -void strbuf_init(struct strbuf *buf, ssize_t hint); +int strbuf_init(struct strbuf *buf, ssize_t hint); void strbuf_release(struct strbuf *buf); char *strbuf_detach(struct strbuf *buf, size_t *); @@ -60,26 +60,31 @@ static inline ssize_t strbuf_avail(const struct strbuf *sb) { return sb->alloc ? sb->alloc - sb->len - 1 : 0; } -void strbuf_grow(struct strbuf *buf, size_t); +int strbuf_grow(struct strbuf *buf, size_t); -static inline void strbuf_setlen(struct strbuf *sb, size_t len) { - if (!sb->alloc) - strbuf_grow(sb, 0); +static inline int strbuf_setlen(struct strbuf *sb, size_t len) { + int ret; + if (!sb->alloc) { + ret = strbuf_grow(sb, 0); + if (ret) + return ret; + } assert(len < sb->alloc); sb->len = len; sb->buf[len] = '\0'; + return 0; } /*----- add data in your buffer -----*/ -void strbuf_addch(struct strbuf *sb, int c); +int strbuf_addch(struct strbuf *sb, int c); -void strbuf_add(struct strbuf *buf, const void *, size_t); -static inline void strbuf_addstr(struct strbuf *sb, const char *s) { - strbuf_add(sb, s, strlen(s)); +int strbuf_add(struct strbuf *buf, const void *, size_t); +static inline int strbuf_addstr(struct strbuf *sb, const char *s) { + return strbuf_add(sb, s, strlen(s)); } __attribute__((format(printf,2,3))) -void strbuf_addf(struct strbuf *sb, const char *fmt, ...); +int strbuf_addf(struct strbuf *sb, const char *fmt, ...); /* XXX: if read fails, any partial read is undone */ ssize_t strbuf_read(struct strbuf *, int fd, ssize_t hint); -- cgit v1.2.3 From bf4d5f25c90bf2eca8671f2fc4e3d15919cd7f9c Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:07 +0900 Subject: perf probe: Check the return value of strbuf APIs Check the return value of strbuf APIs in perf-probe related code, so that it can handle errors in strbuf. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054707.6158.69861.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dwarf-aux.c | 52 +++++++-------- tools/perf/util/probe-event.c | 143 +++++++++++++++++++++++++---------------- tools/perf/util/probe-finder.c | 30 +++++---- 3 files changed, 128 insertions(+), 97 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c index aea189b41cc8..a347b19c961a 100644 --- a/tools/perf/util/dwarf-aux.c +++ b/tools/perf/util/dwarf-aux.c @@ -915,8 +915,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf) tmp = "*"; else if (tag == DW_TAG_subroutine_type) { /* Function pointer */ - strbuf_add(buf, "(function_type)", 15); - return 0; + return strbuf_add(buf, "(function_type)", 15); } else { if (!dwarf_diename(&type)) return -ENOENT; @@ -927,14 +926,10 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf) else if (tag == DW_TAG_enumeration_type) tmp = "enum "; /* Write a base name */ - strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type)); - return 0; + return strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type)); } ret = die_get_typename(&type, buf); - if (ret == 0) - strbuf_addstr(buf, tmp); - - return ret; + return ret ? ret : strbuf_addstr(buf, tmp); } /** @@ -951,12 +946,10 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf) ret = die_get_typename(vr_die, buf); if (ret < 0) { pr_debug("Failed to get type, make it unknown.\n"); - strbuf_add(buf, " (unknown_type)", 14); + ret = strbuf_add(buf, " (unknown_type)", 14); } - strbuf_addf(buf, "\t%s", dwarf_diename(vr_die)); - - return 0; + return ret < 0 ? ret : strbuf_addf(buf, "\t%s", dwarf_diename(vr_die)); } #ifdef HAVE_DWARF_GETLOCATIONS @@ -999,22 +992,24 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die, } while ((offset = dwarf_ranges(&scopes[1], offset, &base, - &start, &end)) > 0) { + &start, &end)) > 0) { start -= entry; end -= entry; if (first) { - strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64, - name, start, end); + ret = strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64, + name, start, end); first = false; } else { - strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64, - start, end); + ret = strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64, + start, end); } + if (ret < 0) + goto out; } if (!first) - strbuf_add(buf, "]>", 2); + ret = strbuf_add(buf, "]>", 2); out: free(scopes); @@ -1054,31 +1049,32 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf) if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL) return -EINVAL; - while ((offset = dwarf_getlocations( - &attr, offset, &base, - &start, &end, &op, &nops)) > 0) { + while ((offset = dwarf_getlocations(&attr, offset, &base, + &start, &end, &op, &nops)) > 0) { if (start == 0) { /* Single Location Descriptions */ ret = die_get_var_innermost_scope(sp_die, vr_die, buf); - return ret; + goto out; } /* Location Lists */ start -= entry; end -= entry; if (first) { - strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64, - name, start, end); + ret = strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64, + name, start, end); first = false; } else { - strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64, - start, end); + ret = strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64, + start, end); } + if (ret < 0) + goto out; } if (!first) - strbuf_add(buf, "]>", 2); - + ret = strbuf_add(buf, "]>", 2); +out: return ret; } #else diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index c82c625395ab..74401a20106d 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -1677,28 +1677,37 @@ char *synthesize_perf_probe_arg(struct perf_probe_arg *pa) { struct perf_probe_arg_field *field = pa->field; struct strbuf buf; - char *ret; + char *ret = NULL; + int err; + + if (strbuf_init(&buf, 64) < 0) + return NULL; - strbuf_init(&buf, 64); if (pa->name && pa->var) - strbuf_addf(&buf, "%s=%s", pa->name, pa->var); + err = strbuf_addf(&buf, "%s=%s", pa->name, pa->var); else - strbuf_addstr(&buf, pa->name ?: pa->var); + err = strbuf_addstr(&buf, pa->name ?: pa->var); + if (err) + goto out; while (field) { if (field->name[0] == '[') - strbuf_addstr(&buf, field->name); + err = strbuf_addstr(&buf, field->name); else - strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".", - field->name); + err = strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".", + field->name); field = field->next; + if (err) + goto out; } if (pa->type) - strbuf_addf(&buf, ":%s", pa->type); + if (strbuf_addf(&buf, ":%s", pa->type) < 0) + goto out; ret = strbuf_detach(&buf, NULL); - +out: + strbuf_release(&buf); return ret; } @@ -1706,18 +1715,23 @@ char *synthesize_perf_probe_arg(struct perf_probe_arg *pa) static char *synthesize_perf_probe_point(struct perf_probe_point *pp) { struct strbuf buf; - char *tmp; - int len; + char *tmp, *ret = NULL; + int len, err = 0; + + if (strbuf_init(&buf, 64) < 0) + return NULL; - strbuf_init(&buf, 64); if (pp->function) { - strbuf_addstr(&buf, pp->function); + if (strbuf_addstr(&buf, pp->function) < 0) + goto out; if (pp->offset) - strbuf_addf(&buf, "+%lu", pp->offset); + err = strbuf_addf(&buf, "+%lu", pp->offset); else if (pp->line) - strbuf_addf(&buf, ":%d", pp->line); + err = strbuf_addf(&buf, ":%d", pp->line); else if (pp->retprobe) - strbuf_addstr(&buf, "%return"); + err = strbuf_addstr(&buf, "%return"); + if (err) + goto out; } if (pp->file) { tmp = pp->file; @@ -1726,12 +1740,15 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp) tmp = strchr(pp->file + len - 30, '/'); tmp = tmp ? tmp + 1 : pp->file + len - 30; } - strbuf_addf(&buf, "@%s", tmp); - if (!pp->function && pp->line) - strbuf_addf(&buf, ":%d", pp->line); + err = strbuf_addf(&buf, "@%s", tmp); + if (!err && !pp->function && pp->line) + err = strbuf_addf(&buf, ":%d", pp->line); } - - return strbuf_detach(&buf, NULL); + if (!err) + ret = strbuf_detach(&buf, NULL); +out: + strbuf_release(&buf); + return ret; } #if 0 @@ -1762,28 +1779,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev) static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref, struct strbuf *buf, int depth) { + int err; if (ref->next) { depth = __synthesize_probe_trace_arg_ref(ref->next, buf, depth + 1); if (depth < 0) - goto out; + return depth; } - strbuf_addf(buf, "%+ld(", ref->offset); -out: - return depth; + err = strbuf_addf(buf, "%+ld(", ref->offset); + return (err < 0) ? err : depth; } static int synthesize_probe_trace_arg(struct probe_trace_arg *arg, struct strbuf *buf) { struct probe_trace_arg_ref *ref = arg->ref; - int depth = 0; + int depth = 0, err; /* Argument name or separator */ if (arg->name) - strbuf_addf(buf, " %s=", arg->name); + err = strbuf_addf(buf, " %s=", arg->name); else - strbuf_addch(buf, ' '); + err = strbuf_addch(buf, ' '); + if (err) + return err; /* Special case: @XXX */ if (arg->value[0] == '@' && arg->ref) @@ -1798,18 +1817,19 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg, /* Print argument value */ if (arg->value[0] == '@' && arg->ref) - strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset); + err = strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset); else - strbuf_addstr(buf, arg->value); + err = strbuf_addstr(buf, arg->value); /* Closing */ - while (depth--) - strbuf_addch(buf, ')'); + while (!err && depth--) + err = strbuf_addch(buf, ')'); + /* Print argument type */ - if (arg->type) - strbuf_addf(buf, ":%s", arg->type); + if (!err && arg->type) + err = strbuf_addf(buf, ":%s", arg->type); - return 0; + return err; } char *synthesize_probe_trace_command(struct probe_trace_event *tev) @@ -1817,15 +1837,18 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev) struct probe_trace_point *tp = &tev->point; struct strbuf buf; char *ret = NULL; - int i; + int i, err; /* Uprobes must have tp->module */ if (tev->uprobes && !tp->module) return NULL; - strbuf_init(&buf, 32); - strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p', - tev->group, tev->event); + if (strbuf_init(&buf, 32) < 0) + return NULL; + + if (strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p', + tev->group, tev->event) < 0) + goto error; /* * If tp->address == 0, then this point must be a * absolute address uprobe. @@ -1839,14 +1862,16 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev) /* Use the tp->address for uprobes */ if (tev->uprobes) - strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address); + err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address); else if (!strncmp(tp->symbol, "0x", 2)) /* Absolute address. See try_to_find_absolute_address() */ - strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "", - tp->module ? ":" : "", tp->address); + err = strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "", + tp->module ? ":" : "", tp->address); else - strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "", - tp->module ? ":" : "", tp->symbol, tp->offset); + err = strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "", + tp->module ? ":" : "", tp->symbol, tp->offset); + if (err) + goto error; for (i = 0; i < tev->nargs; i++) if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0) @@ -1960,14 +1985,15 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev, if (tev->args[i].name) pev->args[i].name = strdup(tev->args[i].name); else { - strbuf_init(&buf, 32); + if ((ret = strbuf_init(&buf, 32)) < 0) + goto error; ret = synthesize_probe_trace_arg(&tev->args[i], &buf); pev->args[i].name = strbuf_detach(&buf, NULL); } if (pev->args[i].name == NULL && ret >= 0) ret = -ENOMEM; } - +error: if (ret < 0) clear_perf_probe_event(pev); @@ -2140,37 +2166,40 @@ static int perf_probe_event__sprintf(const char *group, const char *event, const char *module, struct strbuf *result) { - int i; + int i, ret; char *buf; if (asprintf(&buf, "%s:%s", group, event) < 0) return -errno; - strbuf_addf(result, " %-20s (on ", buf); + ret = strbuf_addf(result, " %-20s (on ", buf); free(buf); + if (ret) + return ret; /* Synthesize only event probe point */ buf = synthesize_perf_probe_point(&pev->point); if (!buf) return -ENOMEM; - strbuf_addstr(result, buf); + ret = strbuf_addstr(result, buf); free(buf); - if (module) - strbuf_addf(result, " in %s", module); + if (!ret && module) + ret = strbuf_addf(result, " in %s", module); - if (pev->nargs > 0) { - strbuf_add(result, " with", 5); - for (i = 0; i < pev->nargs; i++) { + if (!ret && pev->nargs > 0) { + ret = strbuf_add(result, " with", 5); + for (i = 0; !ret && i < pev->nargs; i++) { buf = synthesize_perf_probe_arg(&pev->args[i]); if (!buf) return -ENOMEM; - strbuf_addf(result, " %s", buf); + ret = strbuf_addf(result, " %s", buf); free(buf); } } - strbuf_addch(result, ')'); + if (!ret) + ret = strbuf_addch(result, ')'); - return 0; + return ret; } /* Show an event */ diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c index 9f688758b000..1259839dbf6d 100644 --- a/tools/perf/util/probe-finder.c +++ b/tools/perf/util/probe-finder.c @@ -1294,6 +1294,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) { struct available_var_finder *af = data; struct variable_list *vl; + struct strbuf buf = STRBUF_INIT; int tag, ret; vl = &af->vls[af->nvls - 1]; @@ -1307,25 +1308,26 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) if (ret == 0 || ret == -ERANGE) { int ret2; bool externs = !af->child; - struct strbuf buf; - strbuf_init(&buf, 64); + if (strbuf_init(&buf, 64) < 0) + goto error; if (probe_conf.show_location_range) { - if (!externs) { - if (ret) - strbuf_add(&buf, "[INV]\t", 6); - else - strbuf_add(&buf, "[VAL]\t", 6); - } else - strbuf_add(&buf, "[EXT]\t", 6); + if (!externs) + ret2 = strbuf_add(&buf, + ret ? "[INV]\t" : "[VAL]\t", 6); + else + ret2 = strbuf_add(&buf, "[EXT]\t", 6); + if (ret2) + goto error; } ret2 = die_get_varname(die_mem, &buf); if (!ret2 && probe_conf.show_location_range && !externs) { - strbuf_addch(&buf, '\t'); + if (strbuf_addch(&buf, '\t') < 0) + goto error; ret2 = die_get_var_range(&af->pf.sp_die, die_mem, &buf); } @@ -1334,8 +1336,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) if (ret2 == 0) { strlist__add(vl->vars, strbuf_detach(&buf, NULL)); - } else - strbuf_release(&buf); + } + strbuf_release(&buf); } } @@ -1343,6 +1345,10 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) return DIE_FIND_CB_CONTINUE; else return DIE_FIND_CB_SIBLING; +error: + strbuf_release(&buf); + pr_debug("Error in strbuf\n"); + return DIE_FIND_CB_END; } /* Add a found vars into available variables list */ -- cgit v1.2.3 From b72ca4039099e953f1ea2dbd58c201b14feb6605 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:17 +0900 Subject: perf help: Make check_emacsclient_version to check strbuf APIs Make check_emacsclient_version() to check the return value of strbuf APIs so that it can handle errors in strbuf. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054716.6158.11755.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-help.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c index bc1de9b8fd67..f9830c902b78 100644 --- a/tools/perf/builtin-help.c +++ b/tools/perf/builtin-help.c @@ -61,6 +61,7 @@ static int check_emacsclient_version(void) struct child_process ec_process; const char *argv_ec[] = { "emacsclient", "--version", NULL }; int version; + int ret = -1; /* emacsclient prints its version number on stderr */ memset(&ec_process, 0, sizeof(ec_process)); @@ -71,7 +72,10 @@ static int check_emacsclient_version(void) fprintf(stderr, "Failed to start emacsclient.\n"); return -1; } - strbuf_read(&buffer, ec_process.err, 20); + if (strbuf_read(&buffer, ec_process.err, 20) < 0) { + fprintf(stderr, "Failed to read emacsclient version\n"); + goto out; + } close(ec_process.err); /* @@ -82,8 +86,7 @@ static int check_emacsclient_version(void) if (prefixcmp(buffer.buf, "emacsclient")) { fprintf(stderr, "Failed to parse emacsclient version.\n"); - strbuf_release(&buffer); - return -1; + goto out; } version = atoi(buffer.buf + strlen("emacsclient")); @@ -92,12 +95,11 @@ static int check_emacsclient_version(void) fprintf(stderr, "emacsclient version '%d' too old (< 22).\n", version); - strbuf_release(&buffer); - return -1; - } - + } else + ret = 0; +out: strbuf_release(&buffer); - return 0; + return ret; } static void exec_woman_emacs(const char *path, const char *page) -- cgit v1.2.3 From 70a6898fdc11272249622f77b034f47f1e9adb35 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:26 +0900 Subject: perf tools: Make alias handler to check return value of strbuf Make alias handler and sq_quote_argv to check the return value of strbuf APIs. In sq_quote_argv() calls die(), but this fix handles strbuf failure as a special case and returns to caller, since the caller - handle_alias() also has to check the return value of other strbuf APIs and those checks can be merged to one if() statement. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054725.6158.84597.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/perf.c | 8 +++++--- tools/perf/util/quote.c | 36 ++++++++++++++++++++---------------- tools/perf/util/quote.h | 2 +- 3 files changed, 26 insertions(+), 20 deletions(-) (limited to 'tools') diff --git a/tools/perf/perf.c b/tools/perf/perf.c index 83ffe7cd7330..797000842d40 100644 --- a/tools/perf/perf.c +++ b/tools/perf/perf.c @@ -309,9 +309,11 @@ static int handle_alias(int *argcp, const char ***argv) if (*argcp > 1) { struct strbuf buf; - strbuf_init(&buf, PATH_MAX); - strbuf_addstr(&buf, alias_string); - sq_quote_argv(&buf, (*argv) + 1, PATH_MAX); + if (strbuf_init(&buf, PATH_MAX) < 0 || + strbuf_addstr(&buf, alias_string) < 0 || + sq_quote_argv(&buf, (*argv) + 1, + PATH_MAX) < 0) + die("Failed to allocate memory."); free(alias_string); alias_string = buf.buf; } diff --git a/tools/perf/util/quote.c b/tools/perf/util/quote.c index 01f03242b86a..c6d4ee2de752 100644 --- a/tools/perf/util/quote.c +++ b/tools/perf/util/quote.c @@ -17,38 +17,42 @@ static inline int need_bs_quote(char c) return (c == '\'' || c == '!'); } -static void sq_quote_buf(struct strbuf *dst, const char *src) +static int sq_quote_buf(struct strbuf *dst, const char *src) { char *to_free = NULL; + int ret; if (dst->buf == src) to_free = strbuf_detach(dst, NULL); - strbuf_addch(dst, '\''); - while (*src) { + ret = strbuf_addch(dst, '\''); + while (!ret && *src) { size_t len = strcspn(src, "'!"); - strbuf_add(dst, src, len); + ret = strbuf_add(dst, src, len); src += len; - while (need_bs_quote(*src)) { - strbuf_addstr(dst, "'\\"); - strbuf_addch(dst, *src++); - strbuf_addch(dst, '\''); - } + while (!ret && need_bs_quote(*src)) + ret = strbuf_addf(dst, "'\\%c\'", *src++); } - strbuf_addch(dst, '\''); + if (!ret) + ret = strbuf_addch(dst, '\''); free(to_free); + + return ret; } -void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen) +int sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen) { - int i; + int i, ret; /* Copy into destination buffer. */ - strbuf_grow(dst, 255); - for (i = 0; argv[i]; ++i) { - strbuf_addch(dst, ' '); - sq_quote_buf(dst, argv[i]); + ret = strbuf_grow(dst, 255); + for (i = 0; !ret && argv[i]; ++i) { + ret = strbuf_addch(dst, ' '); + if (ret) + break; + ret = sq_quote_buf(dst, argv[i]); if (maxlen && dst->len > maxlen) die("Too many or long arguments"); } + return ret; } diff --git a/tools/perf/util/quote.h b/tools/perf/util/quote.h index 3340c9c4a6ca..e1ec19146fb0 100644 --- a/tools/perf/util/quote.h +++ b/tools/perf/util/quote.h @@ -24,6 +24,6 @@ * sq_quote() in a real application. */ -void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen); +int sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen); #endif /* __PERF_QUOTE_H */ -- cgit v1.2.3 From 642aadaa320bf9466fd12e3c0903977410bcb731 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:35 +0900 Subject: perf header: Make topology checkers to check return value of strbuf Make topology checkers to check the return value of strbuf APIs so that it can detect errors in it. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054735.6158.98650.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/header.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 90680ec9f8b8..c6000d44f98c 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -1819,7 +1819,8 @@ static int process_cpu_topology(struct perf_file_section *section, ph->env.nr_sibling_cores = nr; size += sizeof(u32); - strbuf_init(&sb, 128); + if (strbuf_init(&sb, 128) < 0) + goto free_cpu; for (i = 0; i < nr; i++) { str = do_read_string(fd, ph); @@ -1827,7 +1828,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error; /* include a NULL character at the end */ - strbuf_add(&sb, str, strlen(str) + 1); + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) + goto error; size += string_size(str); free(str); } @@ -1849,7 +1851,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error; /* include a NULL character at the end */ - strbuf_add(&sb, str, strlen(str) + 1); + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) + goto error; size += string_size(str); free(str); } @@ -1912,13 +1915,14 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse /* nr nodes */ ret = readn(fd, &nr, sizeof(nr)); if (ret != sizeof(nr)) - goto error; + return -1; if (ph->needs_swap) nr = bswap_32(nr); ph->env.nr_numa_nodes = nr; - strbuf_init(&sb, 256); + if (strbuf_init(&sb, 256) < 0) + return -1; for (i = 0; i < nr; i++) { /* node number */ @@ -1940,15 +1944,17 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse mem_free = bswap_64(mem_free); } - strbuf_addf(&sb, "%u:%"PRIu64":%"PRIu64":", - node, mem_total, mem_free); + if (strbuf_addf(&sb, "%u:%"PRIu64":%"PRIu64":", + node, mem_total, mem_free) < 0) + goto error; str = do_read_string(fd, ph); if (!str) goto error; /* include a NULL character at the end */ - strbuf_add(&sb, str, strlen(str) + 1); + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) + goto error; free(str); } ph->env.numa_nodes = strbuf_detach(&sb, NULL); @@ -1982,7 +1988,8 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused } ph->env.nr_pmu_mappings = pmu_num; - strbuf_init(&sb, 128); + if (strbuf_init(&sb, 128) < 0) + return -1; while (pmu_num) { if (readn(fd, &type, sizeof(type)) != sizeof(type)) @@ -1994,9 +2001,11 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused if (!name) goto error; - strbuf_addf(&sb, "%u:%s", type, name); + if (strbuf_addf(&sb, "%u:%s", type, name) < 0) + goto error; /* include a NULL character at the end */ - strbuf_add(&sb, "", 1); + if (strbuf_add(&sb, "", 1) < 0) + goto error; if (!strcmp(name, "msr")) ph->env.msr_pmu_type = type; -- cgit v1.2.3 From 11db4e29bb50442ecef2173f325b7be4e7790025 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:44 +0900 Subject: perf pmu: Make pmu_formats_string to check return value of strbuf Make pmu_formats_string() to check return value of strbuf APIs so that it can detect errors in it. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054744.6158.37810.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/pmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index bf34468a99cb..ddb0261b2577 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -643,20 +643,20 @@ static int pmu_resolve_param_term(struct parse_events_term *term, static char *pmu_formats_string(struct list_head *formats) { struct perf_pmu_format *format; - char *str; - struct strbuf buf; + char *str = NULL; + struct strbuf buf = STRBUF_INIT; unsigned i = 0; if (!formats) return NULL; - strbuf_init(&buf, 0); /* sysfs exported terms */ list_for_each_entry(format, formats, list) - strbuf_addf(&buf, i++ ? ",%s" : "%s", - format->name); + if (strbuf_addf(&buf, i++ ? ",%s" : "%s", format->name) < 0) + goto error; str = strbuf_detach(&buf, NULL); +error: strbuf_release(&buf); return str; -- cgit v1.2.3 From 682f4f035e0fcffce511fe77a02a0f19f0996d70 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:47:53 +0900 Subject: perf help: Do not use ALLOC_GROW in add_cmd_list Replace ALLOC_GROW with normal realloc code in add_cmd_list() so that it can handle errors directly. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054752.6158.30562.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/help-unknown-cmd.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c index 43a98a4dc1e1..d62ccaeeadd6 100644 --- a/tools/perf/util/help-unknown-cmd.c +++ b/tools/perf/util/help-unknown-cmd.c @@ -27,16 +27,27 @@ static int levenshtein_compare(const void *p1, const void *p2) return l1 != l2 ? l1 - l2 : strcmp(s1, s2); } -static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) +static int add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) { - unsigned int i; - - ALLOC_GROW(cmds->names, cmds->cnt + old->cnt, cmds->alloc); - + unsigned int i, nr = cmds->cnt + old->cnt; + void *tmp; + + if (nr > cmds->alloc) { + /* Choose bigger one to alloc */ + if (alloc_nr(cmds->alloc) < nr) + cmds->alloc = nr; + else + cmds->alloc = alloc_nr(cmds->alloc); + tmp = realloc(cmds->names, cmds->alloc * sizeof(*cmds->names)); + if (!tmp) + return -1; + cmds->names = tmp; + } for (i = 0; i < old->cnt; i++) cmds->names[cmds->cnt++] = old->names[i]; zfree(&old->names); old->cnt = 0; + return 0; } const char *help_unknown_cmd(const char *cmd) @@ -52,8 +63,11 @@ const char *help_unknown_cmd(const char *cmd) load_command_list("perf-", &main_cmds, &other_cmds); - add_cmd_list(&main_cmds, &aliases); - add_cmd_list(&main_cmds, &other_cmds); + if (add_cmd_list(&main_cmds, &aliases) < 0 || + add_cmd_list(&main_cmds, &other_cmds) < 0) { + fprintf(stderr, "ERROR: Failed to allocate command list for unknown command.\n"); + goto end; + } qsort(main_cmds.names, main_cmds.cnt, sizeof(main_cmds.names), cmdname_compare); uniq(&main_cmds); @@ -99,6 +113,6 @@ const char *help_unknown_cmd(const char *cmd) for (i = 0; i < n; i++) fprintf(stderr, "\t%s\n", main_cmds.names[i]->name); } - +end: exit(1); } -- cgit v1.2.3 From 452e84012595d681f254a3a0d733fb0b18ffaf42 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 10 May 2016 14:48:01 +0900 Subject: perf tools: Remove xrealloc and ALLOC_GROW Remove unused xrealloc() and ALLOC_GROW() from libperf. Signed-off-by: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20160510054801.6158.6204.stgit@devbox Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/Build | 1 - tools/perf/util/cache.h | 19 ------------------- tools/perf/util/util.h | 6 ------ tools/perf/util/wrapper.c | 29 ----------------------------- 4 files changed, 55 deletions(-) delete mode 100644 tools/perf/util/wrapper.c (limited to 'tools') diff --git a/tools/perf/util/Build b/tools/perf/util/Build index 027bb2b89d7f..8c6c8a0ca642 100644 --- a/tools/perf/util/Build +++ b/tools/perf/util/Build @@ -27,7 +27,6 @@ libperf-y += strlist.o libperf-y += strfilter.o libperf-y += top.o libperf-y += usage.o -libperf-y += wrapper.o libperf-y += dso.o libperf-y += symbol.o libperf-y += symbol_fprintf.o diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h index 1f5a93c2c9a2..0d814bb74661 100644 --- a/tools/perf/util/cache.h +++ b/tools/perf/util/cache.h @@ -40,25 +40,6 @@ int split_cmdline(char *cmdline, const char ***argv); #define alloc_nr(x) (((x)+16)*3/2) -/* - * Realloc the buffer pointed at by variable 'x' so that it can hold - * at least 'nr' entries; the number of entries currently allocated - * is 'alloc', using the standard growing factor alloc_nr() macro. - * - * DO NOT USE any expression with side-effect for 'x' or 'alloc'. - */ -#define ALLOC_GROW(x, nr, alloc) \ - do { \ - if ((nr) > alloc) { \ - if (alloc_nr(alloc) < (nr)) \ - alloc = (nr); \ - else \ - alloc = alloc_nr(alloc); \ - x = xrealloc((x), alloc * sizeof(*(x))); \ - } \ - } while(0) - - static inline int is_absolute_path(const char *path) { return path[0] == '/'; diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index 88f607af1f47..7651633a8dc7 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -160,12 +160,6 @@ static inline char *gitstrchrnul(const char *s, int c) } #endif -/* - * Wrappers: - */ -void *xrealloc(void *ptr, size_t size) __attribute__((weak)); - - static inline void *zalloc(size_t size) { return calloc(1, size); diff --git a/tools/perf/util/wrapper.c b/tools/perf/util/wrapper.c deleted file mode 100644 index 5f1a07c4b87b..000000000000 --- a/tools/perf/util/wrapper.c +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Various trivial helper wrappers around standard functions - */ -#include "cache.h" - -/* - * There's no pack memory to release - but stay close to the Git - * version so wrap this away: - */ -static inline void release_pack_memory(size_t size __maybe_unused, - int flag __maybe_unused) -{ -} - -void *xrealloc(void *ptr, size_t size) -{ - void *ret = realloc(ptr, size); - if (!ret && !size) - ret = realloc(ptr, 1); - if (!ret) { - release_pack_memory(size, -1); - ret = realloc(ptr, size); - if (!ret && !size) - ret = realloc(ptr, 1); - if (!ret) - die("Out of memory, realloc failed"); - } - return ret; -} -- cgit v1.2.3