Em Fri, Mar 02, 2018 at 05:13:54PM +0100, Jiri Olsa escreveu:
> Currently we can crash perf record when running in pipe mode, like:
> 
>   $ perf record ls | perf report
>   # To display the perf.data header info, please use --header/--header-only 
> options.
>   #
>   perf: Segmentation fault
>   Error:
>   The - file has no samples!
> 
> The callstack of the crash is:

Thanks, reproduced the problem, applied the patch, tested, fixed, simple
enough, applied.

- Arnaldo
 
>     0x0000000000515242 in perf_event__synthesize_event_update_name
>   3513            ev = event_update_event__new(len + 1, 
> PERF_EVENT_UPDATE__NAME, evsel->id[0]);
>   (gdb) bt
>   #0  0x0000000000515242 in perf_event__synthesize_event_update_name
>   #1  0x00000000005158a4 in perf_event__synthesize_extra_attr
>   #2  0x0000000000443347 in record__synthesize
>   #3  0x00000000004438e3 in __cmd_record
>   #4  0x000000000044514e in cmd_record
>   #5  0x00000000004cbc95 in run_builtin
>   #6  0x00000000004cbf02 in handle_internal_command
>   #7  0x00000000004cc054 in run_argv
>   #8  0x00000000004cc422 in main
> 
> The reason of the crash is that the evsel does not have ids array
> allocated and the pipe's synthesize code tries to access it.
> 
> We don't force evsel ids allocation when we have single event,
> because it's not needed. However we need it when we are in pipe
> mode even for single event as a key for evsel update event.
> 
> Fixing this by forcing evsel ids allocation event for single
> event, when we are in pipe mode.
> 
> Link: http://lkml.kernel.org/n/tip-xbh5ca1azdcdqyep653vb...@git.kernel.org
> Signed-off-by: Jiri Olsa <jo...@kernel.org>
> ---
>  tools/perf/builtin-record.c | 9 +++++++++
>  tools/perf/perf.h           | 1 +
>  tools/perf/util/record.c    | 8 ++++++--
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index a60f398cbc24..eeef1143bae3 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -881,6 +881,15 @@ static int __cmd_record(struct record *rec, int argc, 
> const char **argv)
>               }
>       }
>  
> +     /*
> +      * If we have just single event and are sending data
> +      * through pipe, we need to force the ids allocation,
> +      * because we synthesize event name through the pipe
> +      * and need the id for that.
> +      */
> +     if (data->is_pipe && rec->evlist->nr_entries == 1)
> +             rec->opts.sample_id = true;
> +
>       if (record__open(rec) != 0) {
>               err = -1;
>               goto out_child;
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 007e0dfd5ce3..8fec1abd0f1f 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -62,6 +62,7 @@ struct record_opts {
>       bool         overwrite;
>       bool         ignore_missing_thread;
>       bool         strict_freq;
> +     bool         sample_id;
>       unsigned int freq;
>       unsigned int mmap_pages;
>       unsigned int auxtrace_mmap_pages;
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index 3878c503f464..867f5937ebb7 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -138,6 +138,7 @@ void perf_evlist__config(struct perf_evlist *evlist, 
> struct record_opts *opts,
>       struct perf_evsel *evsel;
>       bool use_sample_identifier = false;
>       bool use_comm_exec;
> +     bool sample_id = opts->sample_id;
>  
>       /*
>        * Set the evsel leader links before we configure attributes,
> @@ -164,8 +165,7 @@ void perf_evlist__config(struct perf_evlist *evlist, 
> struct record_opts *opts,
>                * match the id.
>                */
>               use_sample_identifier = perf_can_sample_identifier();
> -             evlist__for_each_entry(evlist, evsel)
> -                     perf_evsel__set_sample_id(evsel, use_sample_identifier);
> +             sample_id = true;
>       } else if (evlist->nr_entries > 1) {
>               struct perf_evsel *first = perf_evlist__first(evlist);
>  
> @@ -175,6 +175,10 @@ void perf_evlist__config(struct perf_evlist *evlist, 
> struct record_opts *opts,
>                       use_sample_identifier = perf_can_sample_identifier();
>                       break;
>               }
> +             sample_id = true;
> +     }
> +
> +     if (sample_id) {
>               evlist__for_each_entry(evlist, evsel)
>                       perf_evsel__set_sample_id(evsel, use_sample_identifier);
>       }
> -- 
> 2.13.6

Reply via email to