lkml.org 
[lkml]   [2016]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH perf/core v3 2/8] perf probe: Check the return value of strbuf APIs
    Date
    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 <mhiramat@kernel.org>
    ---
    Changes in v3:
    - Cleanup code path according to Arnaldo's review.
    Changes in V2:
    - Rebased on the latest perf/core.
    ---
    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(-)

    diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
    index aea189b..a347b19 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 c82c625..74401a2 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 9f68875..1259839 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 */
    \
     
     \ /
      Last update: 2016-05-10 08:01    [W:4.276 / U:0.468 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site