Re: [PATCH 5/5] futex: Silence uninitialized warnings

[Davidlohr Bueso]

On Mon, 2014-01-13 at 11:16 +0100, Ingo Molnar wrote:
> * Davidlohr Bueso <davidlohr@xxxxxx> wrote:
> 
> > From: Davidlohr Bueso <davidlohr@xxxxxx>
> > 
> > Callers of cmpxchg_futex_value_locked() can trigger the following:
> > 
> > kernel/futex.c: In function âfutex_lock_pi_atomicâ:
> > kernel/futex.c:725: warning: âcurvalâ may be used uninitialized in this function
> > 
> > This was initially addressed by commit 7cfdaf38, but others still remain. Silence
> > these messages once and for all as the variables really aren't uninitialized.
> > 
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Acked-by: Darren Hart <dvhart@xxxxxxxxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > Cc: Mike Galbraith <efault@xxxxxx>
> > Cc: Jeff Mahoney <jeffm@xxxxxxxx>
> > Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Scott Norton <scott.norton@xxxxxx>
> > Cc: Tom Vaden <tom.vaden@xxxxxx>
> > Cc: Aswin Chandramouleeswaran <aswin@xxxxxx>
> > Cc: Waiman Long <Waiman.Long@xxxxxx>
> > Cc: Jason Low <jason.low2@xxxxxx>
> > Signed-off-by: Davidlohr Bueso <davidlohr@xxxxxx>
> > ---
> >  kernel/futex.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index be6399a..8d40953 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -838,7 +838,7 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb,
> >  				struct task_struct *task, int set_waiters)
> >  {
> >  	int lock_taken, ret, force_take = 0;
> > -	u32 uval, newval, curval, vpid = task_pid_vnr(task);
> > +	u32 uval, newval, uninitialized_var(curval), vpid = task_pid_vnr(task);
> >  
> >  retry:
> >  	ret = lock_taken = 0;
> > @@ -2227,7 +2227,7 @@ static int futex_unlock_pi(u32 __user *uaddr, unsigned int flags)
> >  	struct futex_hash_bucket *hb;
> >  	struct futex_q *this, *next;
> >  	union futex_key key = FUTEX_KEY_INIT;
> > -	u32 uval, vpid = task_pid_vnr(current);
> > +	u32 uninitialized_var(uval), vpid = task_pid_vnr(current);
> >  	int ret;
> >  
> >  retry:
> > @@ -2843,7 +2843,7 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> >  
> >  static int __init futex_init(void)
> >  {
> > -	u32 curval;
> > +	u32 uninitialized_var(curval);
> >  	unsigned long i;
> >  
> >  #if CONFIG_BASE_SMALL
> 
> I'm skipping this patch though.
> 
> I consider the use of uninitialized_var() dangerous and broken: if for 
> whatever reason a future change to the code makes the warning trigger 
> and makes it _true_, then we won't notice it because it's hidden 
> unconditionally ...

Sure, so we should then get rid of the already existing ones from
7cfdaf38.

> 
> The following alternative measures can be used to make spurious 
> old-compiler warnings go away:
> 
>  - Consider upgrading your compiler.
> 
>  - If for whatever reason you can't upgrade your compiler then
>    restructure the code so that the flow of logic is more apparent 
>    even to older GCC versions. (Chances are that the flow will be more 
>    readable to humans too, so it's a win-win!)
> 
>  - If you think the flow is exactly perfect and (older) GCC which you
>    cannot upgrade is still being silly, then initialize the variable
>    to zero. On a new compiler this won't mean a thing because GCC will 
>    notice the superfluous initialization and will optimize it out -
>    and it's a lot safer than just shutting the warning up forever.

Ok, we could go wit this last path then.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/