Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

Steven Rostedt
2006-05-10T17:19:42Z

On Wed, 10 May 2006, Adrian Bunk wrote:

>
> It seems you don't understand the problem at all:
>

Actually I do, altough I admit my reply was for more the general case with
pointers. And that I was using TAGS to jump around the function and didn't
notice that copy_semid_from_user was defined just above the function :-/
OK, it's getting late for me.

>
> First of all note that your example does not apply in this case:
>
> copy_semid_from_user() does _not_ initialize sembuf in all cases.

Yes, but the attribute could still remove the warning without adding more
instructions. And the fact that the attribute is there, we can use it to
know that it was looked at and remind us that it's used.

OK, we could just have an attribute on the declaration of the variable
that keeps that warning turned off. It also lets us know that someone
looked at that variable to make sure it is ok. And we can still undefine
the attribute definition with a make parameter if we want to recheck all
those warnings.

>
> And you do not understand where gcc's problem is:
>
> gcc does in fact see that setbuf is always initialized if
> copy_semid_from_user() returns 0.

>
> setbuf is only initialized in the (cmd == IPC_SET) case and later only
> used in the (cmd == IPC_SET) case. And cmd can't change between the two
> occurences.
>
> Therefore, gcc should in theory already have enough information to prove
> that sembuf is always initialized before it's used.
>

OK in theory, with this example it does have enough info. But how much
info do you want gcc to hold while it's compiling?

To know that it's ok you need to know the following:

(what you've mentioned)
setbuf is initialized
cmd == IPC_SET
and
if copy_sem_from_user returns zero.

setbuf isn't used unless cmd == IPC_SET

Now gcc will also need to know (which is there, but extra info for
compiling):

setbuf->uid is updated in copy_sem_from_user and it returns zero.
setbuf->out is updated in copy_sem_from_user and it returns zero.
setbuf->mode is updated in copy_sem_from_user and it returns zero.

So it isn't enough to just know sembuf was updated, but you also need to
know what parts of the structure is initialized and used. This example is
trivial, but what happens when you have structures with 100 elements, and
some with pointers to other elements. Should gcc keep track of all that
too?

-- Steve