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

Groups > linux.kernel > #1502010 > unrolled thread

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

Started byHonggyu Kim <hong.gyu.kim@lge.com>
First post2016-10-17 16:20 +0200
Last post2016-10-19 20:30 +0200
Articles 14 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [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

#1502010 — [PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent

FromHonggyu Kim <hong.gyu.kim@lge.com>
Date2016-10-17 16:20 +0200
Subject[PATCH 1/3] tools lib traceevent: Add -O2 option to traceevent
Message-ID<stgVH-4zx-5@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1502014 — [PATCH 2/3] tools lib traceevent: Check the return value of asprintf

FromHonggyu Kim <hong.gyu.kim@lge.com>
Date2016-10-17 16:20 +0200
Subject[PATCH 2/3] tools lib traceevent: Check the return value of asprintf
Message-ID<stgVH-4zx-23@gated-at.bofh.it>
In reply to#1502010
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

[toc] | [prev] | [next] | [standalone]

#1503265 — Re: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-10-18 20:00 +0200
SubjectRe: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf
Message-ID<stGQa-55X-17@gated-at.bofh.it>
In reply to#1502014
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

[toc] | [prev] | [next] | [standalone]

#1503467 — Re: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf

FromNamhyung Kim <namhyung@kernel.org>
Date2016-10-19 02:30 +0200
SubjectRe: [PATCH 2/3] tools lib traceevent: Check the return value of asprintf
Message-ID<stMVz-1oo-7@gated-at.bofh.it>
In reply to#1503265
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

[toc] | [prev] | [next] | [standalone]

#1502017 — [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables

FromHonggyu Kim <hong.gyu.kim@lge.com>
Date2016-10-17 16:20 +0200
Subject[PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables
Message-ID<stgVI-4zx-39@gated-at.bofh.it>
In reply to#1502010
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

[toc] | [prev] | [next] | [standalone]

#1503268 — Re: [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-10-18 20:10 +0200
SubjectRe: [PATCH 3/3] tools lib traceevent: Fix to set uninitialized variables
Message-ID<stGZP-5pH-7@gated-at.bofh.it>
In reply to#1502017
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

[toc] | [prev] | [next] | [standalone]

#1502656

FromNamhyung Kim <namhyung@kernel.org>
Date2016-10-18 04:10 +0200
Message-ID<sts0O-3Pu-29@gated-at.bofh.it>
In reply to#1502010
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
>

[toc] | [prev] | [next] | [standalone]

#1503121

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-10-18 17:40 +0200
Message-ID<stEEF-3BE-25@gated-at.bofh.it>
In reply to#1502656
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

[toc] | [prev] | [next] | [standalone]

#1504186

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-10-19 19:50 +0200
Message-ID<su3a2-4te-37@gated-at.bofh.it>
In reply to#1503121
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/

[toc] | [prev] | [next] | [standalone]

#1504208

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-10-19 20:10 +0200
Message-ID<su3tn-4Ru-5@gated-at.bofh.it>
In reply to#1504186
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/
>

[toc] | [prev] | [next] | [standalone]

#1504213

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-10-19 20:10 +0200
Message-ID<su3tn-4Ru-13@gated-at.bofh.it>
In reply to#1504208
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;

[toc] | [prev] | [next] | [standalone]

#1504273

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-10-19 21:30 +0200
Message-ID<su4IO-5Ai-35@gated-at.bofh.it>
In reply to#1504213
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

[toc] | [prev] | [next] | [standalone]

#1504272

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-10-19 21:30 +0200
Message-ID<su4IN-5Ai-7@gated-at.bofh.it>
In reply to#1504208
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

[toc] | [prev] | [next] | [standalone]

#1504223

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-10-19 20:30 +0200
Message-ID<su3MJ-4ZU-7@gated-at.bofh.it>
In reply to#1504186
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web