Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1504273

Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent

Show all headers | View raw

On Wed, 19 Oct 2016 15:06:34 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Oct 19, 2016 at 03:05:48PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Oct 19, 2016 at 02:48:45PM -0300, Arnaldo Carvalho de Melo escreveu:  
> > > Em Tue, Oct 18, 2016 at 11:29:53AM -0400, Steven Rostedt escreveu:  
> > > > On Tue, 18 Oct 2016 11:01:09 +0900
> > > > Namhyung Kim <namhyung@kernel.org> wrote:
> > > >   
> > > > > Hi Honggyu,
> > > > > 
> > > > > You need to CC relevant maintainers when you send patches to LKML.
> > > > > For the libtraceevent, they are Arnaldo and Steven.  You can use
> > > > > scripts/get_maintainer.pl for this job later.  In addition running
> > > > > scripts/checkpatch.pl before sending patches is a good habit.
> > > > > 
> > > > > Arnaldo and Steve,
> > > > > 
> > > > > This is from uftrace building libtraceevent with the optimization flag
> > > > > and we want to fix the upstream as well.
> > > > >   
> > > > 
> > > > Acked-by: Steven Rostedt <rostedt@goodmis.org>  
> > > 
> > > So right after applying this patch I get these new warnings, investigating...  
> > 
> > Some are the compiler not grokking logic where the compiler gets
> > confused with logic that tests one variable to use another and thinks it
> > is using garbage (uninitialized stuff), I tried to follow the logic and
> > I think it got slightly more confused than me, as I _think_ its not a
> > problem, but the one on the case entry for
> > 
> >   OLD_RINGBUF_TYPE_TIME_EXTEND
> > 
> > in old_update_pointers() looks like a bug, unless some macro magic is
> > taking place that updates that 'lenght' variable.
> > 
> > Rostedt, that -O2 unleashed some warnings, please check, I'll defer
> > applying those patches till it doesn't show these warnings, i.e. till
> > other patches fixing these issues or simply silencing the compiler with
> > a harmless init gets submitted,  
> 
> Ah, the patch I had so far shutting off most of this is:
> 
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 664c90c8e22b..449056e96fe6 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -3490,7 +3490,7 @@ struct event_format *
>  pevent_find_event_by_name(struct pevent *pevent,
>  			  const char *sys, const char *name)
>  {
> -	struct event_format *event;
> +	struct event_format *event = NULL;
>  	int i;

Grumble. This is just bad gcc. I mean we have:

	for (i = 0; i < pevent->nr_events; i++) {
		event = pevent->events[i];

	if (i == pevent->nr_events)
		event = NULL;

How the hell can event be uninitialized after that?


>  
>  	if (pevent->last_event &&
> @@ -4843,7 +4843,7 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
>  	char format[32];
>  	int show_func;
>  	int len_as_arg;
> -	int len_arg;
> +	int len_arg = 0;

Again, silly gcc.

>  	int len;
>  	int ls;
>  
> @@ -5102,8 +5102,8 @@ void pevent_data_lat_fmt(struct pevent *pevent,
>  	static int migrate_disable_exists;
>  	unsigned int lat_flags;
>  	unsigned int pc;
> -	int lock_depth;
> -	int migrate_disable;
> +	int lock_depth = 0;
> +	int migrate_disable = 0;
>  	int hardirq;
>  	int softirq;
>  	void *data = record->data;

silly gcc.

Fine, add these.

-- Steve

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Honggyu Kim <hong.gyu.kim@lge.com> - 2016-10-17 16:20 +0200
  [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim <hong.gyu.kim@lge.com> - 2016-10-17 16:20 +0200
    Re: [PATCH 2/3] tools lib traceevent: Check the return value of  asprintf Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-18 20:00 +0200
      Re: [PATCH 2/3] tools lib traceevent: Check the return value of  asprintf Namhyung Kim <namhyung@kernel.org> - 2016-10-19 02:30 +0200
  [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables Honggyu Kim <hong.gyu.kim@lge.com> - 2016-10-17 16:20 +0200
    Re: [PATCH 3/3] tools lib traceevent: Fix to set uninitialized  variables Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-18 20:10 +0200
  Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Namhyung Kim <namhyung@kernel.org> - 2016-10-18 04:10 +0200
    Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Steven Rostedt <rostedt@goodmis.org> - 2016-10-18 17:40 +0200
      Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-19 19:50 +0200
        Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-19 20:10 +0200
          Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-19 20:10 +0200
            Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Steven Rostedt <rostedt@goodmis.org> - 2016-10-19 21:30 +0200
          Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Steven Rostedt <rostedt@goodmis.org> - 2016-10-19 21:30 +0200
        Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Steven Rostedt <rostedt@goodmis.org> - 2016-10-19 20:30 +0200

csiph-web