* does gcc gives a false warning in kernel/trace/trace_events_filter.c ?
@ 2012-09-02 9:04 Toralf Förster
2012-09-05 17:08 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Toralf Förster @ 2012-09-02 9:04 UTC (permalink / raw)
To: Steven Rostedt, Frederic Weisbecker, Ingo Molnar; +Cc: Linux Kernel
The current git tree of linux gave with gcc-4.6.3 :
kernel/trace/trace_events_filter.c: In function ‘ftrace_function_set_filter_cb’:
kernel/trace/trace_events_filter.c:2074:8: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
which refers to this piece of code:
2061 static int ftrace_function_set_filter_cb(enum move_type move,
2062 struct filter_pred *pred,
2063 int *err, void *data)
2064 {
2065 /* Checking the node is valid for function trace. */
2066 if ((move != MOVE_DOWN) ||
2067 (pred->left != FILTER_PRED_INVALID)) {
2068 *err = ftrace_function_check_pred(pred, 0);
2069 } else {
2070 *err = ftrace_function_check_pred(pred, 1);
2071 if (*err)
2072 return WALK_PRED_ABORT;
2073
2074 *err = __ftrace_function_set_filter(pred->op == OP_EQ,
2075 pred->regex.pattern,
2076 pred->regex.len,
2077 data);
2078 }
2079
2080 return (*err) ? WALK_PRED_ABORT : WALK_PRED_DEFAULT;
2081 }
2082
>From a Gentoo forum user I got a hint :
"Maybe it's some kind of a weird inlining issue? I think it's referring to the ret in __ftrace_function_set_filter(), which would be uninitialized if the for-loop does not run (re_cnt ≤ 0)"
Now I'm wondering if re_cnt can become zero or if gcc is wrong here ?
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: does gcc gives a false warning in kernel/trace/trace_events_filter.c ?
2012-09-02 9:04 does gcc gives a false warning in kernel/trace/trace_events_filter.c ? Toralf Förster
@ 2012-09-05 17:08 ` Steven Rostedt
2012-09-06 16:35 ` Toralf Förster
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2012-09-05 17:08 UTC (permalink / raw)
To: Toralf Förster; +Cc: Frederic Weisbecker, Ingo Molnar, Linux Kernel
On Sun, 2012-09-02 at 11:04 +0200, Toralf Förster wrote:
> The current git tree of linux gave with gcc-4.6.3 :
>
> kernel/trace/trace_events_filter.c: In function ‘ftrace_function_set_filter_cb’:
> kernel/trace/trace_events_filter.c:2074:8: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
>
>
> which refers to this piece of code:
>
>
> 2061 static int ftrace_function_set_filter_cb(enum move_type move,
> 2062 struct filter_pred *pred,
> 2063 int *err, void *data)
> 2064 {
> 2065 /* Checking the node is valid for function trace. */
> 2066 if ((move != MOVE_DOWN) ||
> 2067 (pred->left != FILTER_PRED_INVALID)) {
> 2068 *err = ftrace_function_check_pred(pred, 0);
> 2069 } else {
> 2070 *err = ftrace_function_check_pred(pred, 1);
> 2071 if (*err)
> 2072 return WALK_PRED_ABORT;
> 2073
> 2074 *err = __ftrace_function_set_filter(pred->op == OP_EQ,
> 2075 pred->regex.pattern,
> 2076 pred->regex.len,
> 2077 data);
> 2078 }
> 2079
> 2080 return (*err) ? WALK_PRED_ABORT : WALK_PRED_DEFAULT;
> 2081 }
> 2082
>
>
> >From a Gentoo forum user I got a hint :
>
> "Maybe it's some kind of a weird inlining issue? I think it's
> referring to the ret in __ftrace_function_set_filter(), which would be
> uninitialized if the for-loop does not run (re_cnt ≤ 0)"
>
> Now I'm wondering if re_cnt can become zero or if gcc is wrong here ?
>
Strange, as ret is initialized to 'ret = -EINVAL;' in
__ftrace_function_set_filter(). I'm thinking that gcc got confused here.
Maybe report it to the gcc maintainers?
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: does gcc gives a false warning in kernel/trace/trace_events_filter.c ?
2012-09-05 17:08 ` Steven Rostedt
@ 2012-09-06 16:35 ` Toralf Förster
2012-09-06 17:31 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Toralf Förster @ 2012-09-06 16:35 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Frederic Weisbecker, Ingo Molnar, Linux Kernel
On 09/05/2012 07:08 PM, Steven Rostedt wrote:
> On Sun, 2012-09-02 at 11:04 +0200, Toralf Förster wrote:
>> The current git tree of linux gave with gcc-4.6.3 :
>>
>> kernel/trace/trace_events_filter.c: In function ‘ftrace_function_set_filter_cb’:
>> kernel/trace/trace_events_filter.c:2074:8: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
>>
>>
>> which refers to this piece of code:
>>
>>
>> 2061 static int ftrace_function_set_filter_cb(enum move_type move,
>> 2062 struct filter_pred *pred,
>> 2063 int *err, void *data)
>> 2064 {
>> 2065 /* Checking the node is valid for function trace. */
>> 2066 if ((move != MOVE_DOWN) ||
>> 2067 (pred->left != FILTER_PRED_INVALID)) {
>> 2068 *err = ftrace_function_check_pred(pred, 0);
>> 2069 } else {
>> 2070 *err = ftrace_function_check_pred(pred, 1);
>> 2071 if (*err)
>> 2072 return WALK_PRED_ABORT;
>> 2073
>> 2074 *err = __ftrace_function_set_filter(pred->op == OP_EQ,
>> 2075 pred->regex.pattern,
>> 2076 pred->regex.len,
>> 2077 data);
>> 2078 }
>> 2079
>> 2080 return (*err) ? WALK_PRED_ABORT : WALK_PRED_DEFAULT;
>> 2081 }
>> 2082
>>
>>
>> >From a Gentoo forum user I got a hint :
>>
>> "Maybe it's some kind of a weird inlining issue? I think it's
>> referring to the ret in __ftrace_function_set_filter(), which would be
>> uninitialized if the for-loop does not run (re_cnt ≤ 0)"
>>
>> Now I'm wondering if re_cnt can become zero or if gcc is wrong here ?
>>
>
> Strange, as ret is initialized to 'ret = -EINVAL;' in
> __ftrace_function_set_filter(). I'm thinking that gcc got confused here.
> Maybe report it to the gcc maintainers?
>
> -- Steve
>
I filed a bug report
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54495
and got this answer :
--- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-09-05 22:14:00 UTC ---
But if the call to ftrace_function_filter_re sets re_cnt to 0, then ret indeed
will be used uninitialized AFAICT. What am I missing?
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: does gcc gives a false warning in kernel/trace/trace_events_filter.c ?
2012-09-06 16:35 ` Toralf Förster
@ 2012-09-06 17:31 ` Steven Rostedt
2012-09-06 22:22 ` Toralf Förster
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2012-09-06 17:31 UTC (permalink / raw)
To: Toralf Förster; +Cc: Frederic Weisbecker, Ingo Molnar, Linux Kernel
On Thu, 2012-09-06 at 18:35 +0200, Toralf Förster wrote:
> I filed a bug report
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54495
> and got this answer :
>
> --- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-09-05 22:14:00 UTC ---
> But if the call to ftrace_function_filter_re sets re_cnt to 0, then ret indeed
> will be used uninitialized AFAICT. What am I missing?
>
That I think we are looking at two different code bases ;-)
I've been looking at what's been queued for 3.7 and not what's in
mainline. If you look at tip/master, or even linux-next, you'll find:
commit 92d8d4a8b0f "tracing/filter: Add missing initialization"
Which does:
static int __ftrace_function_set_filter(int filter, char *buf, int len,
struct function_filter_data *data)
{
- int i, re_cnt, ret;
+ int i, re_cnt, ret = -EINVAL;
int *reset;
char **re;
Thus, you were correct. This could have been marked urgent, but as it
isn't that big of a deal I just queued it for the next merge window.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: does gcc gives a false warning in kernel/trace/trace_events_filter.c ?
2012-09-06 17:31 ` Steven Rostedt
@ 2012-09-06 22:22 ` Toralf Förster
0 siblings, 0 replies; 5+ messages in thread
From: Toralf Förster @ 2012-09-06 22:22 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Frederic Weisbecker, Ingo Molnar, Linux Kernel
On 09/06/2012 07:31 PM, Steven Rostedt wrote:
> On Thu, 2012-09-06 at 18:35 +0200, Toralf Förster wrote:
>
>> I filed a bug report
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54495
>> and got this answer :
>>
>> --- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-09-05 22:14:00 UTC ---
>> But if the call to ftrace_function_filter_re sets re_cnt to 0, then ret indeed
>> will be used uninitialized AFAICT. What am I missing?
>>
>
> That I think we are looking at two different code bases ;-)
>
> I've been looking at what's been queued for 3.7 and not what's in
> mainline. If you look at tip/master, or even linux-next, you'll find:
>
> commit 92d8d4a8b0f "tracing/filter: Add missing initialization"
>
> Which does:
>
> static int __ftrace_function_set_filter(int filter, char *buf, int len,
> struct function_filter_data *data)
> {
> - int i, re_cnt, ret;
> + int i, re_cnt, ret = -EINVAL;
> int *reset;
> char **re;
>
>
> Thus, you were correct. This could have been marked urgent, but as it
> isn't that big of a deal I just queued it for the next merge window.
>
> -- Steve
>
>
>
ah - thx
:-)
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-06 22:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-02 9:04 does gcc gives a false warning in kernel/trace/trace_events_filter.c ? Toralf Förster
2012-09-05 17:08 ` Steven Rostedt
2012-09-06 16:35 ` Toralf Förster
2012-09-06 17:31 ` Steven Rostedt
2012-09-06 22:22 ` Toralf Förster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.