On Mon, May 31, 2010 at 11:29:54AM +0100, Ralf Baechle wrote:
> On Mon, May 31, 2010 at 12:19:54AM +0100, Ralf Baechle wrote:
>
> > > > Note: I tried to test a little with bigsur_defconfig
> > > > but get_user() is buggy. Or at least my gcc thinks that
> > > > first argument may be used uninitialized.
> > > > I think mips needs to fix the 64 bit variant of get_user().
> > > > I took a quick look but ran away.
> > >
> > > My gcc:
> > > mips-linux-gcc (GCC) 4.1.2
> > > Copyright (C) 2006 Free Software Foundation, Inc.
>
> I played with it for a bit. The warning is present in all gcc 4.1.0 to
> 4.1.2 and it is bogus. When I first looked into this years ago I just
> gave up on gcc 4.1 as a newer version was already available.
>
> The variable returned by get_user is undefined in case of an error, so
> what get_user() is doing is entirely legitimate. This is different from
> copy_from_user() which in case of an error will clear the remainder of
> the destination area which couldn't not be copied from userspace.
What I looked at:
1) u32 word;
2) if (unlikely(get_user(word, header)))
3) word = 0;
4) if (word == magic.cmp)
5) return PAGE_SIZE;
If gcc does not see an assignment to word in line 2) then
it complains about the use line 4).
If we look at the implementation of get_user it is more or less the
following:
({
int err = -EFAULT;
if (access_ok(VERIFY_READ, header))
switch (sizeof(word)) {
case 4:
word = *(u32 *)header;
err = 0;
break;
default:
__get_user_unknown();
break;
}
err;
})
Simplified a lot - I know. But it shows my point.
(And simplifying the code also helped me understand the macros).
gcc needs to be smart enough to deduce that we always return != 0
in the cases where word is not assigned - in which case line 3)
will do the assignment.
So gcc is indeed wrong when is sas "uninitialized" but considering
the complexity of these macros I think it is excused.
The x86 version has the following assignment
(val) = (__typeof__(*(addr))) __gu_tmp;
unconditionally - so they avoid the " = 0" thing.
sparc has explicit "= 0" assignments.
So refactoring the macros may do the trick too.
But I do not think it is worth the effort.
Sam
