Hi all,
On Tue, 14 Apr 2020 11:07:22 +0200 Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> Stephen reported the following build warning on a ARM multi_v7_defconfig
> build with GCC 9.2.1:
>
> kernel/futex.c: In function 'do_futex':
> kernel/futex.c:1676:17: warning: 'oldval' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 1676 | return oldval == cmparg;
> | ~~~~~~~^~~~~~~~~
> kernel/futex.c:1652:6: note: 'oldval' was declared here
> 1652 | int oldval, ret;
> | ^~~~~~
>
> introduced by commit a08971e9488d ("futex: arch_futex_atomic_op_inuser()
> calling conventions change").
>
> While that change should not make any difference it confuses GCC which
> fails to work out that oldval is not referenced when the return value is
> not zero.
>
> GCC fails to properly analyze arch_futex_atomic_op_inuser(). It's not the
> early return, the issue is with the assembly macros. GCC fails to detect
> that those either set 'ret' to 0 and set oldval or set 'ret' to -EFAULT
> which makes oldval uninteresting. The store to the callsite supplied oldval
> pointer is conditional on ret == 0.
>
> The straight forward way to solve this is to make the store unconditional.
>
> Aside of addressing the build warning this makes sense anyway because it
> removes the conditional from the fastpath. In the error case the stored
> value is uninteresting and the extra store does not matter at all.
>
> Reported-by: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/874ku2q18k.fsf@xxxxxxxxxxxxxxxxxxxxxxx
> ---
> arch/arm/include/asm/futex.h | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> --- a/arch/arm/include/asm/futex.h
> +++ b/arch/arm/include/asm/futex.h
> @@ -165,8 +165,13 @@ arch_futex_atomic_op_inuser(int op, int
> preempt_enable();
> #endif
>
> - if (!ret)
> - *oval = oldval;
> + /*
> + * Store unconditionally. If ret != 0 the extra store is the least
> + * of the worries but GCC cannot figure out that __futex_atomic_op()
> + * is either setting ret to -EFAULT or storing the old value in
> + * oldval which results in a uninitialized warning at the call site.
> + */
> + *oval = oldval;
>
> return ret;
> }
Any response to this? I am still getting the warning ...
The warning was introduced by commit
a08971e9488d ("futex: arch_futex_atomic_op_inuser() calling conventions change")
Which has been in Linus' tree since before v5.7-rc1. Should this go in
via the tip tree, the arm tree, or will I just send ti to Linus myself?
--
Cheers,
Stephen Rothwell
Attachment:
pgpgq3I38SBw1.pgp
Description: OpenPGP digital signature
