* [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent
@ 2016-10-17 14:17 Honggyu Kim
2016-10-17 14:17 ` [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Honggyu Kim @ 2016-10-17 14:17 UTC (permalink / raw)
To: namhyung; +Cc: linux-kernel, Honggyu Kim
Since current traceevent somehow does not have an optimization flag,
this patch just adds -O2 to optimize its code.
Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com>
---
tools/lib/traceevent/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index 7851df1..56d223b 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -124,7 +124,7 @@ else
endif
# Append required CFLAGS
-override CFLAGS += -fPIC
+override CFLAGS += -O2 -fPIC
override CFLAGS += $(CONFIG_FLAGS) $(INCLUDES) $(PLUGIN_DIR_SQ)
override CFLAGS += $(udis86-flags) -D_GNU_SOURCE
--
2.10.0.rc2.dirty
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/3] tools lib traceevent: Check the return value of asprintf 2016-10-17 14:17 [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Honggyu Kim @ 2016-10-17 14:17 ` Honggyu Kim 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 2016-10-17 14:17 ` [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables Honggyu Kim 2016-10-18 2:01 ` [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Namhyung Kim 2 siblings, 1 reply; 14+ messages in thread From: Honggyu Kim @ 2016-10-17 14:17 UTC (permalink / raw) To: namhyung; +Cc: linux-kernel, Honggyu Kim Since asprintf generates a compiler warning when its return value is not not properly handled, this patch checks that asprintf call is successful or not. Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> --- tools/lib/traceevent/parse-filter.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 7c214ce..f0fcdcd 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -2122,7 +2122,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) default: break; } - asprintf(&str, val ? "TRUE" : "FALSE"); + if (asprintf(&str, val ? "TRUE" : "FALSE") < 0) + return NULL; break; } } @@ -2140,7 +2141,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) break; } - asprintf(&str, "(%s) %s (%s)", left, op, right); + if (asprintf(&str, "(%s) %s (%s)", left, op, right) < 0) + return NULL; break; case FILTER_OP_NOT: @@ -2156,10 +2158,12 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) right_val = 0; if (right_val >= 0) { /* just return the opposite */ - asprintf(&str, right_val ? "FALSE" : "TRUE"); + if (asprintf(&str, right_val ? "FALSE" : "TRUE") < 0) + return NULL; break; } - asprintf(&str, "%s(%s)", op, right); + if (asprintf(&str, "%s(%s)", op, right) < 0) + return NULL; break; default: @@ -2175,7 +2179,8 @@ static char *val_to_str(struct event_filter *filter, struct filter_arg *arg) { char *str = NULL; - asprintf(&str, "%lld", arg->value.val); + if (asprintf(&str, "%lld", arg->value.val) < 0) + return NULL; return str; } @@ -2233,7 +2238,8 @@ static char *exp_to_str(struct event_filter *filter, struct filter_arg *arg) break; } - asprintf(&str, "%s %s %s", lstr, op, rstr); + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) + return NULL; out: free(lstr); free(rstr); @@ -2277,7 +2283,8 @@ static char *num_to_str(struct event_filter *filter, struct filter_arg *arg) if (!op) op = "<="; - asprintf(&str, "%s %s %s", lstr, op, rstr); + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) + return NULL; break; default: @@ -2312,8 +2319,9 @@ static char *str_to_str(struct event_filter *filter, struct filter_arg *arg) if (!op) op = "!~"; - asprintf(&str, "%s %s \"%s\"", - arg->str.field->name, op, arg->str.val); + if (asprintf(&str, "%s %s \"%s\"", + arg->str.field->name, op, arg->str.val) < 0) + return NULL; break; default: @@ -2329,7 +2337,8 @@ static char *arg_to_str(struct event_filter *filter, struct filter_arg *arg) switch (arg->type) { case FILTER_ARG_BOOLEAN: - asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE"); + if (asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE") < 0) + return NULL; return str; case FILTER_ARG_OP: -- 2.10.0.rc2.dirty ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf 2016-10-17 14:17 ` [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim @ 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 2016-10-19 0:25 ` Namhyung Kim 0 siblings, 1 reply; 14+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-10-18 17:59 UTC (permalink / raw) To: Steven Rostedt; +Cc: Honggyu Kim, namhyung, linux-kernel Steven, one more, please ack. - Arnaldo Em Mon, Oct 17, 2016 at 11:17:11PM +0900, Honggyu Kim escreveu: > Since asprintf generates a compiler warning when its return value is not > not properly handled, this patch checks that asprintf call is successful > or not. > > Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> > --- > tools/lib/traceevent/parse-filter.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c > index 7c214ce..f0fcdcd 100644 > --- a/tools/lib/traceevent/parse-filter.c > +++ b/tools/lib/traceevent/parse-filter.c > @@ -2122,7 +2122,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > default: > break; > } > - asprintf(&str, val ? "TRUE" : "FALSE"); > + if (asprintf(&str, val ? "TRUE" : "FALSE") < 0) > + return NULL; > break; > } > } > @@ -2140,7 +2141,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > break; > } > > - asprintf(&str, "(%s) %s (%s)", left, op, right); > + if (asprintf(&str, "(%s) %s (%s)", left, op, right) < 0) > + return NULL; > break; > > case FILTER_OP_NOT: > @@ -2156,10 +2158,12 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > right_val = 0; > if (right_val >= 0) { > /* just return the opposite */ > - asprintf(&str, right_val ? "FALSE" : "TRUE"); > + if (asprintf(&str, right_val ? "FALSE" : "TRUE") < 0) > + return NULL; > break; > } > - asprintf(&str, "%s(%s)", op, right); > + if (asprintf(&str, "%s(%s)", op, right) < 0) > + return NULL; > break; > > default: > @@ -2175,7 +2179,8 @@ static char *val_to_str(struct event_filter *filter, struct filter_arg *arg) > { > char *str = NULL; > > - asprintf(&str, "%lld", arg->value.val); > + if (asprintf(&str, "%lld", arg->value.val) < 0) > + return NULL; > > return str; > } > @@ -2233,7 +2238,8 @@ static char *exp_to_str(struct event_filter *filter, struct filter_arg *arg) > break; > } > > - asprintf(&str, "%s %s %s", lstr, op, rstr); > + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) > + return NULL; > out: > free(lstr); > free(rstr); > @@ -2277,7 +2283,8 @@ static char *num_to_str(struct event_filter *filter, struct filter_arg *arg) > if (!op) > op = "<="; > > - asprintf(&str, "%s %s %s", lstr, op, rstr); > + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) > + return NULL; > break; > > default: > @@ -2312,8 +2319,9 @@ static char *str_to_str(struct event_filter *filter, struct filter_arg *arg) > if (!op) > op = "!~"; > > - asprintf(&str, "%s %s \"%s\"", > - arg->str.field->name, op, arg->str.val); > + if (asprintf(&str, "%s %s \"%s\"", > + arg->str.field->name, op, arg->str.val) < 0) > + return NULL; > break; > > default: > @@ -2329,7 +2337,8 @@ static char *arg_to_str(struct event_filter *filter, struct filter_arg *arg) > > switch (arg->type) { > case FILTER_ARG_BOOLEAN: > - asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE"); > + if (asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE") < 0) > + return NULL; > return str; > > case FILTER_ARG_OP: > -- > 2.10.0.rc2.dirty ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf 2016-10-18 17:59 ` Arnaldo Carvalho de Melo @ 2016-10-19 0:25 ` Namhyung Kim 0 siblings, 0 replies; 14+ messages in thread From: Namhyung Kim @ 2016-10-19 0:25 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: Steven Rostedt, Honggyu Kim, linux-kernel Hi Arnaldo, You can add my Acked-by for all 3 patches. Thanks, Namhyung On Tue, Oct 18, 2016 at 02:59:17PM -0300, Arnaldo Carvalho de Melo wrote: > Steven, one more, please ack. > > - Arnaldo > > Em Mon, Oct 17, 2016 at 11:17:11PM +0900, Honggyu Kim escreveu: > > Since asprintf generates a compiler warning when its return value is not > > not properly handled, this patch checks that asprintf call is successful > > or not. > > > > Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> > > --- > > tools/lib/traceevent/parse-filter.c | 29 +++++++++++++++++++---------- > > 1 file changed, 19 insertions(+), 10 deletions(-) > > > > diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c > > index 7c214ce..f0fcdcd 100644 > > --- a/tools/lib/traceevent/parse-filter.c > > +++ b/tools/lib/traceevent/parse-filter.c > > @@ -2122,7 +2122,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > > default: > > break; > > } > > - asprintf(&str, val ? "TRUE" : "FALSE"); > > + if (asprintf(&str, val ? "TRUE" : "FALSE") < 0) > > + return NULL; > > break; > > } > > } > > @@ -2140,7 +2141,8 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > > break; > > } > > > > - asprintf(&str, "(%s) %s (%s)", left, op, right); > > + if (asprintf(&str, "(%s) %s (%s)", left, op, right) < 0) > > + return NULL; > > break; > > > > case FILTER_OP_NOT: > > @@ -2156,10 +2158,12 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg) > > right_val = 0; > > if (right_val >= 0) { > > /* just return the opposite */ > > - asprintf(&str, right_val ? "FALSE" : "TRUE"); > > + if (asprintf(&str, right_val ? "FALSE" : "TRUE") < 0) > > + return NULL; > > break; > > } > > - asprintf(&str, "%s(%s)", op, right); > > + if (asprintf(&str, "%s(%s)", op, right) < 0) > > + return NULL; > > break; > > > > default: > > @@ -2175,7 +2179,8 @@ static char *val_to_str(struct event_filter *filter, struct filter_arg *arg) > > { > > char *str = NULL; > > > > - asprintf(&str, "%lld", arg->value.val); > > + if (asprintf(&str, "%lld", arg->value.val) < 0) > > + return NULL; > > > > return str; > > } > > @@ -2233,7 +2238,8 @@ static char *exp_to_str(struct event_filter *filter, struct filter_arg *arg) > > break; > > } > > > > - asprintf(&str, "%s %s %s", lstr, op, rstr); > > + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) > > + return NULL; > > out: > > free(lstr); > > free(rstr); > > @@ -2277,7 +2283,8 @@ static char *num_to_str(struct event_filter *filter, struct filter_arg *arg) > > if (!op) > > op = "<="; > > > > - asprintf(&str, "%s %s %s", lstr, op, rstr); > > + if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0) > > + return NULL; > > break; > > > > default: > > @@ -2312,8 +2319,9 @@ static char *str_to_str(struct event_filter *filter, struct filter_arg *arg) > > if (!op) > > op = "!~"; > > > > - asprintf(&str, "%s %s \"%s\"", > > - arg->str.field->name, op, arg->str.val); > > + if (asprintf(&str, "%s %s \"%s\"", > > + arg->str.field->name, op, arg->str.val) < 0) > > + return NULL; > > break; > > > > default: > > @@ -2329,7 +2337,8 @@ static char *arg_to_str(struct event_filter *filter, struct filter_arg *arg) > > > > switch (arg->type) { > > case FILTER_ARG_BOOLEAN: > > - asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE"); > > + if (asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE") < 0) > > + return NULL; > > return str; > > > > case FILTER_ARG_OP: > > -- > > 2.10.0.rc2.dirty ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables 2016-10-17 14:17 [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Honggyu Kim 2016-10-17 14:17 ` [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim @ 2016-10-17 14:17 ` Honggyu Kim 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 2016-10-18 2:01 ` [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Namhyung Kim 2 siblings, 1 reply; 14+ messages in thread From: Honggyu Kim @ 2016-10-17 14:17 UTC (permalink / raw) To: namhyung; +Cc: linux-kernel, Honggyu Kim This patch fixes uninitialized variables to remove remaining compiler warnings as follows: event-parse.c: In function ‘pevent_find_event_by_name’: event-parse.c:3513:21: warning: ‘event’ may be used uninitialized in this function [-Wmaybe-uninitialized] pevent->last_event = event; ^ event-parse.c: In function ‘pevent_data_lat_fmt’: event-parse.c:5156:20: warning: ‘migrate_disable’ may be used uninitialized in this function [-Wmaybe-uninitialized] trace_seq_printf(s, "%d", migrate_disable); ^ event-parse.c:5163:20: warning: ‘lock_depth’ may be used uninitialized in this function [-Wmaybe-uninitialized] trace_seq_printf(s, "%d", lock_depth); ^ event-parse.c: In function ‘pevent_event_info’: event-parse.c:5060:18: warning: ‘len_arg’ may be used uninitialized in this function [-Wmaybe-uninitialized] print_str_arg(&p, data, size, event, ^ event-parse.c:4846:6: note: ‘len_arg’ was declared here int len_arg; ^ ... kbuffer-parse.c: In function ‘__old_next_event’: kbuffer-parse.c:339:27: warning: ‘length’ may be used uninitialized in this function [-Wmaybe-uninitialized] kbuf->next = kbuf->index + length; ^ kbuffer-parse.c:297:15: note: ‘length’ was declared here unsigned int length; ^ Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> --- tools/lib/traceevent/event-parse.c | 8 ++++---- tools/lib/traceevent/kbuffer-parse.c | 2 +- tools/lib/traceevent/plugin_function.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index 664c90c..2fb9338 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; 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; 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 = -1; + int migrate_disable = 0; int hardirq; int softirq; void *data = record->data; diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c index 65984f1..fc8f20c 100644 --- a/tools/lib/traceevent/kbuffer-parse.c +++ b/tools/lib/traceevent/kbuffer-parse.c @@ -294,7 +294,7 @@ static unsigned int old_update_pointers(struct kbuffer *kbuf) unsigned int type; unsigned int len; unsigned int delta; - unsigned int length; + unsigned int length = 0; void *ptr = kbuf->data + kbuf->curr; type_len_ts = read_4(kbuf, ptr); diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c index a00ec19..42dbf73 100644 --- a/tools/lib/traceevent/plugin_function.c +++ b/tools/lib/traceevent/plugin_function.c @@ -130,7 +130,7 @@ static int function_handler(struct trace_seq *s, struct pevent_record *record, unsigned long long pfunction; const char *func; const char *parent; - int index; + int index = 0; if (pevent_get_field_val(s, event, "ip", record, &function, 1)) return trace_seq_putc(s, '!'); -- 2.10.0.rc2.dirty ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables 2016-10-17 14:17 ` [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables Honggyu Kim @ 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 14+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-10-18 17:59 UTC (permalink / raw) To: Steven Rostedt; +Cc: Honggyu Kim, namhyung, linux-kernel One more. Em Mon, Oct 17, 2016 at 11:17:12PM +0900, Honggyu Kim escreveu: > This patch fixes uninitialized variables to remove remaining compiler > warnings as follows: > > event-parse.c: In function ‘pevent_find_event_by_name’: > event-parse.c:3513:21: warning: ‘event’ may be used uninitialized in this function [-Wmaybe-uninitialized] > pevent->last_event = event; > ^ > event-parse.c: In function ‘pevent_data_lat_fmt’: > event-parse.c:5156:20: warning: ‘migrate_disable’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", migrate_disable); > ^ > event-parse.c:5163:20: warning: ‘lock_depth’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", lock_depth); > ^ > event-parse.c: In function ‘pevent_event_info’: > event-parse.c:5060:18: warning: ‘len_arg’ may be used uninitialized in this function [-Wmaybe-uninitialized] > print_str_arg(&p, data, size, event, > ^ > event-parse.c:4846:6: note: ‘len_arg’ was declared here > int len_arg; > ^ > ... > kbuffer-parse.c: In function ‘__old_next_event’: > kbuffer-parse.c:339:27: warning: ‘length’ may be used uninitialized in this function [-Wmaybe-uninitialized] > kbuf->next = kbuf->index + length; > ^ > kbuffer-parse.c:297:15: note: ‘length’ was declared here > unsigned int length; > ^ > > Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> > --- > tools/lib/traceevent/event-parse.c | 8 ++++---- > tools/lib/traceevent/kbuffer-parse.c | 2 +- > tools/lib/traceevent/plugin_function.c | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c > index 664c90c..2fb9338 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; > > 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; > 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 = -1; > + int migrate_disable = 0; > int hardirq; > int softirq; > void *data = record->data; > diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c > index 65984f1..fc8f20c 100644 > --- a/tools/lib/traceevent/kbuffer-parse.c > +++ b/tools/lib/traceevent/kbuffer-parse.c > @@ -294,7 +294,7 @@ static unsigned int old_update_pointers(struct kbuffer *kbuf) > unsigned int type; > unsigned int len; > unsigned int delta; > - unsigned int length; > + unsigned int length = 0; > void *ptr = kbuf->data + kbuf->curr; > > type_len_ts = read_4(kbuf, ptr); > diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c > index a00ec19..42dbf73 100644 > --- a/tools/lib/traceevent/plugin_function.c > +++ b/tools/lib/traceevent/plugin_function.c > @@ -130,7 +130,7 @@ static int function_handler(struct trace_seq *s, struct pevent_record *record, > unsigned long long pfunction; > const char *func; > const char *parent; > - int index; > + int index = 0; > > if (pevent_get_field_val(s, event, "ip", record, &function, 1)) > return trace_seq_putc(s, '!'); > -- > 2.10.0.rc2.dirty ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-17 14:17 [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Honggyu Kim 2016-10-17 14:17 ` [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim 2016-10-17 14:17 ` [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables Honggyu Kim @ 2016-10-18 2:01 ` Namhyung Kim 2016-10-18 15:29 ` Steven Rostedt 2 siblings, 1 reply; 14+ messages in thread From: Namhyung Kim @ 2016-10-18 2:01 UTC (permalink / raw) To: Honggyu Kim; +Cc: linux-kernel, Arnaldo Carvalho de Melo, Steven Rostedt 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. Thanks, Namhyung On Mon, Oct 17, 2016 at 11:17:10PM +0900, Honggyu Kim wrote: > Since current traceevent somehow does not have an optimization flag, > this patch just adds -O2 to optimize its code. > > Signed-off-by: Honggyu Kim <hong.gyu.kim@lge.com> > --- > tools/lib/traceevent/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile > index 7851df1..56d223b 100644 > --- a/tools/lib/traceevent/Makefile > +++ b/tools/lib/traceevent/Makefile > @@ -124,7 +124,7 @@ else > endif > > # Append required CFLAGS > -override CFLAGS += -fPIC > +override CFLAGS += -O2 -fPIC > override CFLAGS += $(CONFIG_FLAGS) $(INCLUDES) $(PLUGIN_DIR_SQ) > override CFLAGS += $(udis86-flags) -D_GNU_SOURCE > > -- > 2.10.0.rc2.dirty > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-18 2:01 ` [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Namhyung Kim @ 2016-10-18 15:29 ` Steven Rostedt 2016-10-19 17:48 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 14+ messages in thread From: Steven Rostedt @ 2016-10-18 15:29 UTC (permalink / raw) To: Namhyung Kim; +Cc: Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo 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> -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-18 15:29 ` Steven Rostedt @ 2016-10-19 17:48 ` Arnaldo Carvalho de Melo 2016-10-19 18:05 ` Arnaldo Carvalho de Melo 2016-10-19 18:21 ` Steven Rostedt 0 siblings, 2 replies; 14+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-10-19 17:48 UTC (permalink / raw) To: Steven Rostedt Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo 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... [acme@jouet linux]$ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/6.2.1/lto-wrapper Target: x86_64-redhat-linux Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --disable-libgcj --with-isl --enable-libmpx --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux Thread model: posix gcc version 6.2.1 20160916 (Red Hat 6.2.1-2) (GCC) [acme@jouet linux]$ LD /tmp/build/perf/plugin_mac80211-in.o kbuffer-parse.c: In function ‘__old_next_event’: kbuffer-parse.c:339:27: warning: ‘length’ may be used uninitialized in this function [-Wmaybe-uninitialized] kbuf->next = kbuf->index + length; ~~~~~~~~~~~~^~~~~~~~ kbuffer-parse.c:297:15: note: ‘length’ was declared here unsigned int length; ^~~~~~ CC /tmp/build/perf/plugin_sched_switch.o CC /tmp/build/perf/run-command.o event-parse.c: In function ‘pevent_find_event_by_name’: event-parse.c:3513:21: warning: ‘event’ may be used uninitialized in this function [-Wmaybe-uninitialized] pevent->last_event = event; ~~~~~~~~~~~~~~~~~~~^~~~~~~ CC /tmp/build/perf/sigchain.o LD /tmp/build/perf/plugin_sched_switch-in.o CC /tmp/build/perf/plugin_function.o event-parse.c: In function ‘pevent_data_lat_fmt’: event-parse.c:5156:4: warning: ‘migrate_disable’ may be used uninitialized in this function [-Wmaybe-uninitialized] trace_seq_printf(s, "%d", migrate_disable); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ event-parse.c:5163:4: warning: ‘lock_depth’ may be used uninitialized in this function [-Wmaybe-uninitialized] trace_seq_printf(s, "%d", lock_depth); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ plugin_function.c: In function ‘function_handler’: plugin_function.c:133:6: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized] int index; ^~~~~ CC /tmp/build/perf/subcmd-config.o GEN perf-archive LD /tmp/build/perf/plugin_function-in.o GEN perf-with-kcore CC /tmp/build/perf/plugin_xen.o event-parse.c: In function ‘pevent_event_info’: event-parse.c:5003:7: warning: ‘len_arg’ may be used uninitialized in this function [-Wmaybe-uninitialized] trace_seq_printf(s, format, len_arg, (char)val); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ event-parse.c:4846:6: note: ‘len_arg’ was declared here int len_arg; ^~~~~~~ MKDIR /tmp/build/perf/util/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-19 17:48 ` Arnaldo Carvalho de Melo @ 2016-10-19 18:05 ` Arnaldo Carvalho de Melo 2016-10-19 18:06 ` Arnaldo Carvalho de Melo 2016-10-19 19:21 ` Steven Rostedt 2016-10-19 18:21 ` Steven Rostedt 1 sibling, 2 replies; 14+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-10-19 18:05 UTC (permalink / raw) To: Steven Rostedt Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo 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, Thanks, - Arnaldo > [acme@jouet linux]$ gcc -v > Using built-in specs. > COLLECT_GCC=gcc > COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/6.2.1/lto-wrapper > Target: x86_64-redhat-linux > Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --disable-libgcj --with-isl --enable-libmpx --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux > Thread model: posix > gcc version 6.2.1 20160916 (Red Hat 6.2.1-2) (GCC) > [acme@jouet linux]$ > > LD /tmp/build/perf/plugin_mac80211-in.o > kbuffer-parse.c: In function ‘__old_next_event’: > kbuffer-parse.c:339:27: warning: ‘length’ may be used uninitialized in this function [-Wmaybe-uninitialized] > kbuf->next = kbuf->index + length; > ~~~~~~~~~~~~^~~~~~~~ > kbuffer-parse.c:297:15: note: ‘length’ was declared here > unsigned int length; > ^~~~~~ > CC /tmp/build/perf/plugin_sched_switch.o > CC /tmp/build/perf/run-command.o > event-parse.c: In function ‘pevent_find_event_by_name’: > event-parse.c:3513:21: warning: ‘event’ may be used uninitialized in this function [-Wmaybe-uninitialized] > pevent->last_event = event; > ~~~~~~~~~~~~~~~~~~~^~~~~~~ > CC /tmp/build/perf/sigchain.o > LD /tmp/build/perf/plugin_sched_switch-in.o > CC /tmp/build/perf/plugin_function.o > event-parse.c: In function ‘pevent_data_lat_fmt’: > event-parse.c:5156:4: warning: ‘migrate_disable’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", migrate_disable); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > event-parse.c:5163:4: warning: ‘lock_depth’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", lock_depth); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > plugin_function.c: In function ‘function_handler’: > plugin_function.c:133:6: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized] > int index; > ^~~~~ > CC /tmp/build/perf/subcmd-config.o > GEN perf-archive > LD /tmp/build/perf/plugin_function-in.o > GEN perf-with-kcore > CC /tmp/build/perf/plugin_xen.o > event-parse.c: In function ‘pevent_event_info’: > event-parse.c:5003:7: warning: ‘len_arg’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, format, len_arg, (char)val); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > event-parse.c:4846:6: note: ‘len_arg’ was declared here > int len_arg; > ^~~~~~~ > MKDIR /tmp/build/perf/util/ > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-19 18:05 ` Arnaldo Carvalho de Melo @ 2016-10-19 18:06 ` Arnaldo Carvalho de Melo 2016-10-19 19:26 ` Steven Rostedt 2016-10-19 19:21 ` Steven Rostedt 1 sibling, 1 reply; 14+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-10-19 18:06 UTC (permalink / raw) To: Steven Rostedt Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo 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; 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; 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; ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-19 18:06 ` Arnaldo Carvalho de Melo @ 2016-10-19 19:26 ` Steven Rostedt 0 siblings, 0 replies; 14+ messages in thread From: Steven Rostedt @ 2016-10-19 19:26 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-19 18:05 ` Arnaldo Carvalho de Melo 2016-10-19 18:06 ` Arnaldo Carvalho de Melo @ 2016-10-19 19:21 ` Steven Rostedt 1 sibling, 0 replies; 14+ messages in thread From: Steven Rostedt @ 2016-10-19 19:21 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo On Wed, 19 Oct 2016 15:05:48 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > 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, > > Thanks, Note, that code is for the first version of the ftrace ring buffer that got changed around 2.6.32 I believe. And since trace-cmd was the only tool that directly looked at the code, I was able to "break" abi and update trace-cmd to have a new version. So that code isn't even used anywhere on newer kernels. That said, could you add in the case statement for OLD_RINGBUF_TYPE_TIME_EXTEND: length = 0; I think that should be fine. -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent 2016-10-19 17:48 ` Arnaldo Carvalho de Melo 2016-10-19 18:05 ` Arnaldo Carvalho de Melo @ 2016-10-19 18:21 ` Steven Rostedt 1 sibling, 0 replies; 14+ messages in thread From: Steven Rostedt @ 2016-10-19 18:21 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Honggyu Kim, linux-kernel, Arnaldo Carvalho de Melo On Wed, 19 Oct 2016 14:48:45 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > 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... > > [acme@jouet linux]$ gcc -v > Using built-in specs. > COLLECT_GCC=gcc > COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/6.2.1/lto-wrapper > Target: x86_64-redhat-linux > Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --disable-libgcj --with-isl --enable-libmpx --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux > Thread model: posix > gcc version 6.2.1 20160916 (Red Hat 6.2.1-2) (GCC) > [acme@jouet linux]$ > > LD /tmp/build/perf/plugin_mac80211-in.o > kbuffer-parse.c: In function ‘__old_next_event’: > kbuffer-parse.c:339:27: warning: ‘length’ may be used uninitialized in this function [-Wmaybe-uninitialized] > kbuf->next = kbuf->index + length; > ~~~~~~~~~~~~^~~~~~~~ > kbuffer-parse.c:297:15: note: ‘length’ was declared here > unsigned int length; > ^~~~~~ Actually, that may be a bug. > CC /tmp/build/perf/plugin_sched_switch.o > CC /tmp/build/perf/run-command.o > event-parse.c: In function ‘pevent_find_event_by_name’: > event-parse.c:3513:21: warning: ‘event’ may be used uninitialized in this function [-Wmaybe-uninitialized] > pevent->last_event = event; > ~~~~~~~~~~~~~~~~~~~^~~~~~~ > CC /tmp/build/perf/sigchain.o > LD /tmp/build/perf/plugin_sched_switch-in.o > CC /tmp/build/perf/plugin_function.o > event-parse.c: In function ‘pevent_data_lat_fmt’: > event-parse.c:5156:4: warning: ‘migrate_disable’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", migrate_disable); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > event-parse.c:5163:4: warning: ‘lock_depth’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, "%d", lock_depth); These two don't look like bugs because they need to have both migrate_disable_exists and lock_depth_exists to be set, and when they are those variables are. Funny it only complains about the one in the trace_seq_printf() and not the compare before them. ie. if (migrate_disable < 0) which is checked before calling the trace_seq_printf(). > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > plugin_function.c: In function ‘function_handler’: > plugin_function.c:133:6: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized] > int index; > ^~~~~ Ah, this is a bug too. > CC /tmp/build/perf/subcmd-config.o > GEN perf-archive > LD /tmp/build/perf/plugin_function-in.o > GEN perf-with-kcore > CC /tmp/build/perf/plugin_xen.o > event-parse.c: In function ‘pevent_event_info’: > event-parse.c:5003:7: warning: ‘len_arg’ may be used uninitialized in this function [-Wmaybe-uninitialized] > trace_seq_printf(s, format, len_arg, (char)val); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > event-parse.c:4846:6: note: ‘len_arg’ was declared here > int len_arg; Again, the "len_as_arg" needs to be set for this to be an issue. We could just initialize len_arg to zero to shut gcc up. -- Steve > ^~~~~~~ > MKDIR /tmp/build/perf/util/ ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-19 19:26 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-17 14:17 [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Honggyu Kim 2016-10-17 14:17 ` [PATCH 2/3] tools lib traceevent: Check the return value of asprintf Honggyu Kim 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 2016-10-19 0:25 ` Namhyung Kim 2016-10-17 14:17 ` [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables Honggyu Kim 2016-10-18 17:59 ` Arnaldo Carvalho de Melo 2016-10-18 2:01 ` [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent Namhyung Kim 2016-10-18 15:29 ` Steven Rostedt 2016-10-19 17:48 ` Arnaldo Carvalho de Melo 2016-10-19 18:05 ` Arnaldo Carvalho de Melo 2016-10-19 18:06 ` Arnaldo Carvalho de Melo 2016-10-19 19:26 ` Steven Rostedt 2016-10-19 19:21 ` Steven Rostedt 2016-10-19 18:21 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox