[PATCH] futex: fix uninitialized warning

[PATCH] futex: fix uninitialized warning

[yad.naveen]

From: Naveen Yadav <yad.naveen@gmail.com>


Signed-off-by: Naveen Yadav <yad.naveen@gmail.com>
---
 kernel/futex.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 6487e4c..fdb93b0 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1588,7 +1588,7 @@ static int fixup_pi_state_owner(u32 __user *uaddr, struct futex_q *q,
 	u32 newtid = task_pid_vnr(newowner) | FUTEX_WAITERS;
 	struct futex_pi_state *pi_state = q->pi_state;
 	struct task_struct *oldowner = pi_state->owner;
-	u32 uval, curval, newval;
+	u32 uval, uninitialized_var(curval), newval;
 	int ret;
 
 	/* Owner died? */
@@ -2493,7 +2493,7 @@ err_unlock:
  */
 int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
 {
-	u32 uval, nval, mval;
+	u32 uval, uninitialized_var(nval), mval;
 
 retry:
 	if (get_user(uval, uaddr))
-- 
1.7.8.4

Re: [PATCH] futex: fix uninitialized warning

[Steven Rostedt]

On Thu, 2012-02-16 at 15:15 +0530, yad.naveen@gmail.com wrote:
> From: Naveen Yadav <yad.naveen@gmail.com>
> 
> 
> Signed-off-by: Naveen Yadav <yad.naveen@gmail.com>
> ---
>  kernel/futex.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/futex.c b/kernel/futex.c
> index 6487e4c..fdb93b0 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -1588,7 +1588,7 @@ static int fixup_pi_state_owner(u32 __user *uaddr, struct futex_q *q,
>  	u32 newtid = task_pid_vnr(newowner) | FUTEX_WAITERS;
>  	struct futex_pi_state *pi_state = q->pi_state;
>  	struct task_struct *oldowner = pi_state->owner;
> -	u32 uval, curval, newval;
> +	u32 uval, uninitialized_var(curval), newval;

NAK, an uninitialized_var() macro should not be added unless there's a
really good reason that gcc is not detecting it. Some logic does require
this.

If it ends up being that this macro should be added, then it should have
a comment to why it is added, and why gcc is failing to figure out that
it is initialized.

What gcc version is complaining and what arch is this for? There could
definitely be a case here that it may really be uninitialized, and this
patch would hide that bug.

The cmpxchg_futex_* is arch specific, and if an arch screws it up, we
will never know that this value is uninitialized because this patch
would have hidden that bug. Hiding a bug is much worse than dealing with
some "might be uninitialized" warnings.

-- Steve

>  	int ret;
>  
>  	/* Owner died? */
> @@ -2493,7 +2493,7 @@ err_unlock:
>   */
>  int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
>  {
> -	u32 uval, nval, mval;
> +	u32 uval, uninitialized_var(nval), mval;
>  
>  retry:
>  	if (get_user(uval, uaddr))

Re: [PATCH] futex: fix uninitialized warning

[naveen yadav]

Thanks a lot Steven for your comments:

I am Building for ARM versatile Board. and
I check with gcc version gcc version 4.4.1 and gcc version 4.6.3.

If we check the code flow, we can confirm that this is fake warning ,
since value will be updated cmpxchg_futex_value_locked() function.
May be this is limitation of GCC.
So we can ignore the warning ..

Thanks.

naveen



On Thu, Feb 16, 2012 at 7:02 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 2012-02-16 at 15:15 +0530, yad.naveen@gmail.com wrote:
>> From: Naveen Yadav <yad.naveen@gmail.com>
>>
>>
>> Signed-off-by: Naveen Yadav <yad.naveen@gmail.com>
>> ---
>>  kernel/futex.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/futex.c b/kernel/futex.c
>> index 6487e4c..fdb93b0 100644
>> --- a/kernel/futex.c
>> +++ b/kernel/futex.c
>> @@ -1588,7 +1588,7 @@ static int fixup_pi_state_owner(u32 __user *uaddr, struct futex_q *q,
>>       u32 newtid = task_pid_vnr(newowner) | FUTEX_WAITERS;
>>       struct futex_pi_state *pi_state = q->pi_state;
>>       struct task_struct *oldowner = pi_state->owner;
>> -     u32 uval, curval, newval;
>> +     u32 uval, uninitialized_var(curval), newval;
>
> NAK, an uninitialized_var() macro should not be added unless there's a
> really good reason that gcc is not detecting it. Some logic does require
> this.
>
> If it ends up being that this macro should be added, then it should have
> a comment to why it is added, and why gcc is failing to figure out that
> it is initialized.
>
> What gcc version is complaining and what arch is this for? There could
> definitely be a case here that it may really be uninitialized, and this
> patch would hide that bug.
>
> The cmpxchg_futex_* is arch specific, and if an arch screws it up, we
> will never know that this value is uninitialized because this patch
> would have hidden that bug. Hiding a bug is much worse than dealing with
> some "might be uninitialized" warnings.
>
> -- Steve
>
>>       int ret;
>>
>>       /* Owner died? */
>> @@ -2493,7 +2493,7 @@ err_unlock:
>>   */
>>  int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
>>  {
>> -     u32 uval, nval, mval;
>> +     u32 uval, uninitialized_var(nval), mval;
>>
>>  retry:
>>       if (get_user(uval, uaddr))
>
>