From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 00A27CA9EAF for ; Thu, 24 Oct 2019 19:02:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B031921A4A for ; Thu, 24 Oct 2019 19:02:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Ken2R02u" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2504251AbfJXTCN (ORCPT ); Thu, 24 Oct 2019 15:02:13 -0400 Received: from mail-pg1-f202.google.com ([209.85.215.202]:50942 "EHLO mail-pg1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2504245AbfJXTCM (ORCPT ); Thu, 24 Oct 2019 15:02:12 -0400 Received: by mail-pg1-f202.google.com with SMTP id r24so18488973pgj.17 for ; Thu, 24 Oct 2019 12:02:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=LatC+wbpvgZY15QWxZ6yx67BUGWlegtQyyvDTHYnuYc=; b=Ken2R02urqeEoMlzX0yqZLTfdINHC0BDtLNlTdu39stau9C4ovbhYo9mPq+geZq0/J WYJ6ok8cPh7Dz8kB2V71vN0ZMhkBxokrTUe4lC+jWQbopaMsaZKi5iDIvkUPfHqiSNe1 ARsTFZtiB+e5gVE03XDQaLtXrKeFZcm2rKnluxXlbWEQ0UizpXJ3WooqwiFlKAuJeUd2 XzSKe2VF8ApA/OnyGm+B2RqIzmlq0JyT7OHySBkDMuYNM+htOn3jr89Tflt24CFwrM8X 0OK47ChwO8WzC3u+wLmaDQ3cevjC5AnT8sUTEjQQV5DKKeR1x8oSlT1OZK0hn5xJW+8a n08Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc:content-transfer-encoding; bh=LatC+wbpvgZY15QWxZ6yx67BUGWlegtQyyvDTHYnuYc=; b=hMH4zC+h+J2oHeJEyktpb6HEn/NvnbWEjuE4A4IlrZGS5wmTEDmSn9TgTP/CFBy2jc iz5J/P3IOT54QFvPu7TuhFU4tMprV9tzD6RYCLGqhgIVixkLpVn6IjBPZND2aG/pt1kl n4nzvTbLOwcxwkaYmlDjs7X6O8X6IWwtdNlQVGfAcmiC05SEKWV/67FsblJYzl1FrNG7 RqjTdfgS35zEfuVSV2lu+6hhsU8cOBet6n3XhlHmuwHr6hYI5uZHKrmP38QFEtlz38Nj zJb3KEEvdZBNdPiJVbX3jTnc5X0K+3f9uh2/Gy53vVhEAIUgedtoZwLzKNo+LbdgOklo H3Gg== X-Gm-Message-State: APjAAAWP7Awiu71G7pxkZDgX2LNm6UcB+19lcxlIEY3h9B53u7zWE2pV dTZEN/Vd7W+CxVQY5xCixr1R3965VqRV X-Google-Smtp-Source: APXvYqxpFajHdTKYdBV3wrVXxlleTV2qG6rZ0qAgAGxO8Xa5lBQa2Wj5+r6Ff3a02+Zl0JnL1mS/QNHCc/mT X-Received: by 2002:a63:cf4a:: with SMTP id b10mr17497860pgj.86.1571943729980; Thu, 24 Oct 2019 12:02:09 -0700 (PDT) Date: Thu, 24 Oct 2019 12:01:54 -0700 In-Reply-To: <20191024190202.109403-1-irogers@google.com> Message-Id: <20191024190202.109403-2-irogers@google.com> Mime-Version: 1.0 References: <20191023005337.196160-1-irogers@google.com> <20191024190202.109403-1-irogers@google.com> X-Mailer: git-send-email 2.23.0.866.gb869b98d4c-goog Subject: [PATCH v3 1/9] perf tools: add parse events append error From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andi Kleen , Jin Yao , Adrian Hunter , Kan Liang , John Garry , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, clang-built-linux@googlegroups.com Cc: Stephane Eranian , Ian Rogers Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Parse event error handling may overwrite one error string with another creating memory leaks and masking errors. Introduce a helper routine that appends error messages and avoids the memory leak. A reproduction of this problem can be seen with: perf stat -e c/c/ After this change this produces: event syntax error: 'c/c/' \___ unknown term (previous error: unknown term (pre= vious error: unknown term (previous error: unknown term (previous error: un= known term (previous error: unknown term (previous error: unknown term (pre= vious error: unknown term (previous error: unknown term (previous error: un= known term (previous error: unknown term (previous error: unknown term (pre= vious error: unknown term (previous error: unknown term (previous error: un= known term (previous error: unknown term (previous error: unknown term (pre= vious error: unknown term (previous error: unknown term (previous error: un= known term (previous error: unknown term (previous error: Cannot find PMU `= c'. Missing kernel support?)(help: valid terms: event,filter_rem,filter_opc= 0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umask,filter_opc1,ti= d_en,thresh,filter_all_op,filter_not_nm,filter_state,filter_nm,config,confi= g1,config2,name,period,percore))(help: valid terms: event,filter_rem,filter= _opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umask,filter_opc= 1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filter_nm,config,c= onfig1,config2,name,period,percore))(help: valid terms: event,filter_rem,fi= lter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umask,filter= _opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filter_nm,conf= ig,config1,config2,name,period,percore))(help: valid terms: event,filter_re= m,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umask,fi= lter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filter_nm,= config,config1,config2,name,period,percore))(help: valid terms: event,filte= r_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umas= k,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filter= _nm,config,config1,config2,name,period,percore))(help: valid terms: event,f= ilter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,= umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,fi= lter_nm,config,config1,config2,name,period,percore))(help: valid terms: eve= nt,pc,in_tx,edge,any,offcore_rsp,in_tx_cp,ldlat,inv,umask,frontend,cmask,co= nfig,config1,config2,name,period,percore))(help: valid terms: event,filter_= rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,umask,= filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filter_n= m,config,config1,config2,name,period,percore))(help: valid terms: event,fil= ter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,um= ask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filt= er_nm,config,config1,config2,name,period,percore))(help: valid terms: event= ,config,config1,config2,name,period,percore))(help: valid terms: event,filt= er_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv,uma= sk,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,filte= r_nm,config,config1,config2,name,period,percore))(help: valid terms: event,= filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc,inv= ,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_state,f= ilter_nm,config,config1,config2,name,period,percore))(help: valid terms: ev= ent,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter_nc= ,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_sta= te,filter_nm,config,config1,config2,name,period,percore))(help: valid terms= : event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filte= r_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter= _state,filter_nm,config,config1,config2,name,period,percore))(help: valid t= erms: event,config,config1,config2,name,period,percore))(help: valid terms:= event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,filter= _nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,filter_= state,filter_nm,config,config1,config2,name,period,percore))(help: valid te= rms: event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_loc,fi= lter_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm,fil= ter_state,filter_nm,config,config1,config2,name,period,percore))(help: vali= d terms: event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filter_lo= c,filter_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_not_nm= ,filter_state,filter_nm,config,config1,config2,name,period,percore))(help: = valid terms: event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filte= r_loc,filter_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_no= t_nm,filter_state,filter_nm,config,config1,config2,name,period,percore))(he= lp: valid terms: event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,f= ilter_loc,filter_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filte= r_not_nm,filter_state,filter_nm,config,config1,config2,name,period,percore)= ) valid terms: event,filter_rem,filter_opc0,edge,filter_isoc,filter_tid,filte= r_loc,filter_nc,inv,umask,filter_opc1,tid_en,thresh,filter_all_op,filter_no= t_nm,filter_state,filter_nm,config,config1,config2,name,period,percore Run 'perf list' for a list of valid events Usage: perf stat [] [] -e, --event event selector. use 'perf list' to list available= events Signed-off-by: Ian Rogers --- tools/perf/util/parse-events.c | 100 +++++++++++++++++++++++---------- tools/perf/util/parse-events.h | 2 + tools/perf/util/pmu.c | 30 ++++++---- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.= c index db882f630f7e..edb3ae76777d 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -182,6 +182,38 @@ static int tp_event_has_id(const char *dir_path, struc= t dirent *evt_dir) =20 #define MAX_EVENT_LENGTH 512 =20 +void parse_events__append_error(struct parse_events_error *err, int idx, + char *str, char *help) +{ + char *new_str =3D NULL; + + if (WARN(!str, "WARNING: failed to provide error string")) { + free(help); + return; + } + if (err->str) { + int ret; + + if (err->help) { + ret =3D asprintf(&new_str, + "%s (previous error: %s(help: %s))", + str, err->str, err->help); + } else { + ret =3D asprintf(&new_str, + "%s (previous error: %s)", + str, err->str); + } + if (ret < 0) + new_str =3D NULL; + else + zfree(&str); + } + err->idx =3D idx; + free(err->str); + err->str =3D new_str ?: str; + free(err->help); + err->help =3D help; +} =20 struct tracepoint_path *tracepoint_id_to_path(u64 config) { @@ -932,11 +964,11 @@ static int check_type_val(struct parse_events_term *t= erm, return 0; =20 if (err) { - err->idx =3D term->err_val; - if (type =3D=3D PARSE_EVENTS__TERM_TYPE_NUM) - err->str =3D strdup("expected numeric value"); - else - err->str =3D strdup("expected string value"); + parse_events__append_error(err, term->err_val, + type =3D=3D PARSE_EVENTS__TERM_TYPE_NUM + ? strdup("expected numeric value") + : strdup("expected string value"), + NULL); } return -EINVAL; } @@ -972,8 +1004,11 @@ static bool config_term_shrinked; static bool config_term_avail(int term_type, struct parse_events_error *err) { + char *err_str; + if (term_type < 0 || term_type >=3D __PARSE_EVENTS__TERM_TYPE_NR) { - err->str =3D strdup("Invalid term_type"); + parse_events__append_error(err, -1, + strdup("Invalid term_type"), NULL); return false; } if (!config_term_shrinked) @@ -992,9 +1027,9 @@ config_term_avail(int term_type, struct parse_events_e= rror *err) return false; =20 /* term_type is validated so indexing is safe */ - if (asprintf(&err->str, "'%s' is not usable in 'perf stat'", - config_term_names[term_type]) < 0) - err->str =3D NULL; + if (asprintf(&err_str, "'%s' is not usable in 'perf stat'", + config_term_names[term_type]) >=3D 0) + parse_events__append_error(err, -1, err_str, NULL); return false; } } @@ -1036,17 +1071,20 @@ do { \ case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE: CHECK_TYPE_VAL(STR); if (strcmp(term->val.str, "no") && - parse_branch_str(term->val.str, &attr->branch_sample_type)) { - err->str =3D strdup("invalid branch sample type"); - err->idx =3D term->err_val; + parse_branch_str(term->val.str, + &attr->branch_sample_type)) { + parse_events__append_error(err, term->err_val, + strdup("invalid branch sample type"), + NULL); return -EINVAL; } break; case PARSE_EVENTS__TERM_TYPE_TIME: CHECK_TYPE_VAL(NUM); if (term->val.num > 1) { - err->str =3D strdup("expected 0 or 1"); - err->idx =3D term->err_val; + parse_events__append_error(err, term->err_val, + strdup("expected 0 or 1"), + NULL); return -EINVAL; } break; @@ -1080,8 +1118,9 @@ do { \ case PARSE_EVENTS__TERM_TYPE_PERCORE: CHECK_TYPE_VAL(NUM); if ((unsigned int)term->val.num > 1) { - err->str =3D strdup("expected 0 or 1"); - err->idx =3D term->err_val; + parse_events__append_error(err, term->err_val, + strdup("expected 0 or 1"), + NULL); return -EINVAL; } break; @@ -1089,9 +1128,9 @@ do { \ CHECK_TYPE_VAL(NUM); break; default: - err->str =3D strdup("unknown term"); - err->idx =3D term->err_term; - err->help =3D parse_events_formats_error_string(NULL); + parse_events__append_error(err, term->err_term, + strdup("unknown term"), + parse_events_formats_error_string(NULL)); return -EINVAL; } =20 @@ -1142,9 +1181,9 @@ static int config_term_tracepoint(struct perf_event_a= ttr *attr, return config_term_common(attr, term, err); default: if (err) { - err->idx =3D term->err_term; - err->str =3D strdup("unknown term"); - err->help =3D strdup("valid terms: call-graph,stack-size\n"); + parse_events__append_error(err, term->err_term, + strdup("unknown term"), + strdup("valid terms: call-graph,stack-size\n")); } return -EINVAL; } @@ -1323,10 +1362,12 @@ int parse_events_add_pmu(struct parse_events_state = *parse_state, =20 pmu =3D perf_pmu__find(name); if (!pmu) { - if (asprintf(&err->str, + char *err_str; + + if (asprintf(&err_str, "Cannot find PMU `%s'. Missing kernel support?", - name) < 0) - err->str =3D NULL; + name) >=3D 0) + parse_events__append_error(err, -1, err_str, NULL); return -EINVAL; } =20 @@ -2797,13 +2838,10 @@ void parse_events__clear_array(struct parse_events_= array *a) void parse_events_evlist_error(struct parse_events_state *parse_state, int idx, const char *str) { - struct parse_events_error *err =3D parse_state->error; - - if (!err) + if (!parse_state->error) return; - err->idx =3D idx; - err->str =3D strdup(str); - WARN_ONCE(!err->str, "WARNING: failed to allocate error string"); + + parse_events__append_error(parse_state->error, idx, strdup(str), NULL); } =20 static void config_terms_list(char *buf, size_t buf_sz) diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.= h index 769e07cddaa2..a7d42faeab0c 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -124,6 +124,8 @@ struct parse_events_state { struct list_head *terms; }; =20 +void parse_events__append_error(struct parse_events_error *err, int idx, + char *str, char *help); void parse_events__shrink_config_terms(void); int parse_events__is_hardcoded_term(struct parse_events_term *term); int parse_events_term__num(struct parse_events_term **term, diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index adbe97e941dd..4015ec11944a 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1050,9 +1050,9 @@ static int pmu_config_term(struct list_head *formats, if (err) { char *pmu_term =3D pmu_formats_string(formats); =20 - err->idx =3D term->err_term; - err->str =3D strdup("unknown term"); - err->help =3D parse_events_formats_error_string(pmu_term); + parse_events__append_error(err, term->err_term, + strdup("unknown term"), + parse_events_formats_error_string(pmu_term)); free(pmu_term); } return -EINVAL; @@ -1080,8 +1080,9 @@ static int pmu_config_term(struct list_head *formats, if (term->no_value && bitmap_weight(format->bits, PERF_PMU_FORMAT_BITS) > 1) { if (err) { - err->idx =3D term->err_val; - err->str =3D strdup("no value assigned for term"); + parse_events__append_error(err, term->err_val, + strdup("no value assigned for term"), + NULL); } return -EINVAL; } @@ -1094,8 +1095,9 @@ static int pmu_config_term(struct list_head *formats, term->config, term->val.str); } if (err) { - err->idx =3D term->err_val; - err->str =3D strdup("expected numeric value"); + parse_events__append_error(err, term->err_val, + strdup("expected numeric value"), + NULL); } return -EINVAL; } @@ -1108,11 +1110,15 @@ static int pmu_config_term(struct list_head *format= s, max_val =3D pmu_format_max_value(format->bits); if (val > max_val) { if (err) { - err->idx =3D term->err_val; - if (asprintf(&err->str, - "value too big for format, maximum is %llu", - (unsigned long long)max_val) < 0) - err->str =3D strdup("value too big for format"); + char *err_str; + + parse_events__append_error(err, term->err_val, + asprintf(&err_str, + "value too big for format, maximum is %llu", + (unsigned long long)max_val) < 0 + ? strdup("value too big for format") + : err_str, + NULL); return -EINVAL; } /* --=20 2.23.0.866.gb869b98d4c-goog