Em Wed, Aug 16, 2017 at 03:02:01PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <[email protected]>
> 
> When there is a parse error on adding an alias the parser
> segfaults. It thinks data is a parse_events_evlist and tries
> to reference the error member. But it's really a parse_events_terms
> for this call path through parse_events_terms.
> 
> Add the error member at the same location in parse_events_terms
> as in *_evlist and set up a error structure to report errors correctly.
> 
> This can be only reproduced by adding errors to JSON aliases.

Humm, but don't we have that checked?

void parse_events_evlist_error(struct parse_events_evlist *data,
                               int idx, const char *str)
{
        struct parse_events_error *err = data->error;

        if (!err)
                return;
        err->idx = idx;
        err->str = strdup(str);
        WARN_ONCE(!err->str, "WARNING: failed to allocate error string");
}

- Arnaldo
 
> Segfault:
> 
>     #'0  0x00000000004d66d2 in parse_events_evlist_error 
> (data=0x7fffffffce20, idx=6, str=0x76cda8 "parser error") at 
> util/parse-events.c:2530
>     #'1  0x0000000000516d0a in parse_events_error (loc=0x7fffffffb210, 
> data=0x7fffffffce20, scanner=0x245b400, msg=0x76cb13 "syntax error") at 
> util/parse-events.y:692
>     #'2  0x000000000051675b in parse_events_parse (_data=0x7fffffffce20, 
> scanner=0x245b400) at /home/ak/hle/obj-perf/util/parse-events-bison.c:2213
>     #'3  0x00000000004d3fd6 in parse_events__scanner (str=0x725cff 
> "event=0,", data=0x7fffffffce20, start_token=259) at util/parse-events.c:1646
>     #'4  0x00000000004d4063 in parse_events_terms (terms=0x245b398, 
> str=0x725cff "event=0,") at util/parse-events.c:1664
>     #'5  0x00000000005179f1 in __perf_pmu__new_alias (list=0x7fffffffcf90, 
> dir=0x0, name=0x725cec "unc_cha_clockticks", desc=0x725d08 "Uncore cache 
> clock ticks. Unit: uncore_cha ",
>     val=0x725cff "event=0,", long_desc=0x0, topic=0x725d34 "uncore other", 
> unit=0x0, perpkg=0x6ca7c6 "1", metric_expr=0x0, metric_name=0x0) at 
> util/pmu.c:255
>     #'6  0x0000000000518789 in pmu_add_cpu_aliases (head=0x7fffffffcf90, 
> name=0x2450903 "uncore_cha_9") at util/pmu.c:571
>     #'7  0x00000000005188ac in pmu_lookup (name=0x2450903 "uncore_cha_9") at 
> util/pmu.c:613
>     #'8  0x0000000000518aff in perf_pmu__find (name=0x2450903 "uncore_cha_9") 
> at util/pmu.c:672
>     #'9  0x00000000005183d5 in pmu_read_sysfs () at util/pmu.c:467
>     #'10 0x0000000000518a54 in perf_pmu__scan (pmu=0x0) at util/pmu.c:651
>     #'11 0x0000000000519f26 in print_pmu_events (event_glob=0x0, 
> name_only=false, quiet_flag=false, long_desc=false, details_flag=false) at 
> util/pmu.c:1173
>     #'12 0x00000000004d5ef0 in print_events (event_glob=0x0, name_only=false, 
> quiet_flag=false, long_desc=false, details_flag=false) at 
> util/parse-events.c:2343
>     #'13 0x000000000043c7d4 in cmd_list (argc=0, argv=0x7fffffffeb90) at 
> builtin-list.c:56
>     #'14 0x00000000004ab2c8 in run_builtin (p=0xa281a0 <commands+192>, 
> argc=1, argv=0x7fffffffeb90) at perf.c:296
>     #15 0x00000000004ab535 in handle_internal_command (argc=1, 
> argv=0x7fffffffeb90) at perf.c:348
>     #16 0x00000000004ab687 in run_argv (argcp=0x7fffffffe9ec, 
> argv=0x7fffffffe9e0) at perf.c:392
>     #17 0x00000000004aba55 in main (argc=1, argv=0x7fffffffeb90) at perf.c:530
> 
> Signed-off-by: Andi Kleen <[email protected]>
> ---
>  tools/perf/util/parse-events.c | 2 ++
>  tools/perf/util/parse-events.h | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 84e301073885..15b472aec767 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1656,7 +1656,9 @@ static int parse_events__scanner(const char *str, void 
> *data, int start_token)
>   */
>  int parse_events_terms(struct list_head *terms, const char *str)
>  {
> +     struct parse_events_error err = { .idx = 0, };
>       struct parse_events_terms data = {
> +             .error = &err,
>               .terms = NULL,
>       };
>       int ret;
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index a235f4d6d5e5..de217f5b37bb 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -108,15 +108,17 @@ struct parse_events_error {
>       char *help;     /* optional help string */
>  };
>  
> +/* error field must match parse_events_terms */
>  struct parse_events_evlist {
> +     struct parse_events_error *error;
>       struct list_head           list;
>       int                        idx;
>       int                        nr_groups;
> -     struct parse_events_error *error;
>       struct perf_evlist        *evlist;
>  };
>  
>  struct parse_events_terms {
> +     struct parse_events_error *error;
>       struct list_head *terms;
>  };
>  
> -- 
> 2.9.4

Reply via email to