Messages in this thread Patch in this message | | | Date | Wed, 19 Oct 2016 15:26:01 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent |
| |
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
| |