Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

[Josh Triplett]

Al Viro wrote:
> On Wed, Apr 02, 2008 at 06:50:20PM -0700, Linus Torvalds wrote:
>>> BTW, what happened with sparse.git?
>> Yeah, we do seem to have lost a maintainer.
> 
> Have we?  I've seen Josh on sparse list quite recently, IIRC...
> Yes - http://marc.info/?l=linux-sparse&m=120716477203477 just
> yesterday...
> 
> Josh, do you have any pending patches?  I can just start a branch in
> my tree, of course, but I'd rather deal with merge headache earlier
> than later...

Starting around the beginning of this year, I got buried under
research temporarily in the Winter term, along with a couple of other
things.  I got caught up, and then proceeded to get sick for a week,
and that and recovery time put me far enough behind on both email and
research that I didn't catch up again until recently.

I mostly kept up with Sparse email, hence the recent mails, but built
up a backlog of patches to deal with.  (Sparse has a fairly low patch
rate, so that backlog constitutes less than 10 patches, counting the
ones that still need further work or discussion before they can
merge.)  Also, in switching my email to an IMAP server, I managed to
lose the read/unread status on some of my list mail, including
linux-sparse, which slowed me down a fair bit when trying to process
those mails.

I have a small handful of pending patches to work through on
linux-sparse, with yours at the top of that list; I also have some
local patches that I need to test and push.  Apologies for the
processing delay.

I should have thought to send a "still alive" note with an explanation
to linux-sparse.  I'll make sure to do so in the future.  (Some time
in the next couple of years I have a thesis to write. :) )

I'll catch the backlog up and then send out an "all patches merged"
mail, so that anyone with additional patches or anyone who thinks
their patch got lost can resend.

- Josh Triplett

Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

[Al Viro]

On Thu, Apr 03, 2008 at 01:31:00PM -0700, Josh Triplett wrote:
> I mostly kept up with Sparse email, hence the recent mails, but built
> up a backlog of patches to deal with.  (Sparse has a fairly low patch
> rate, so that backlog constitutes less than 10 patches, counting the
> ones that still need further work or discussion before they can
> merge.)  Also, in switching my email to an IMAP server, I managed to
> lose the read/unread status on some of my list mail, including
> linux-sparse, which slowed me down a fair bit when trying to process
> those mails.
> 
> I have a small handful of pending patches to work through on
> linux-sparse, with yours at the top of that list; I also have some
> local patches that I need to test and push.  Apologies for the
> processing delay.
> 
> I should have thought to send a "still alive" note with an explanation
> to linux-sparse.  I'll make sure to do so in the future.  (Some time
> in the next couple of years I have a thesis to write. :) )
> 
> I'll catch the backlog up and then send out an "all patches merged"
> mail, so that anyone with additional patches or anyone who thinks
> their patch got lost can resend.

OK...  FWIW, I'll probably do relaxed integer constant expressions on
top of whatever shows up in your tree + patches I've sent to the list
at some point in the next couple of weeks.  For now I'd say that we
can keep ignoring the errors from _IO...() uses in context that expect
an i-c-e (mostly - indices in array initializers); it's not widespread
and we'd lived with that for quite a while.

As far as annoyances go, lack of VLA support is responsible for far more
lost warnings.  I don't have anything in that direction beyond rather
vague plans (*and* a monstrous backlog of my own, both kernel-side and
sparse-side, so it's unlikely to drift up the list in the next couple
of months).  Does anybody have any pending work in that direction?  It's
not _that_ hard to do, but for pity sake, go with C99 standard when doing
it; gcc is choke-full of broken and barely documented extensions in that
area, so reverse-engineering them is going to result in utter mess.

C99 VLAs are fairly dumb and straightforward - the underlying idea is
"for each variable of type that involves a VLA, compiler should be able
to slap a shadow variable containing the array size in scope nearby".
Thus
	* no variably-modified members in structs or unions (not only
no VLA, but no pointers to VLA, etc.); we would need shadow variables
for each struct instance and that obviously wouldn't work in that model).
	* no functions returning variably-modified type.
	* no variably-modified objects in global scope.
	* no VLA static in function.
	* typedef *can* be variably-modified, but only in block scope.
Warning: this can get sticky for us - all sizes are evaluated when
typedef is reached.  IOW,
	typedef int a[n];
	a x;
	if (n++ == 5) {
		a y;
		int z[n];
	}
will have size of y equal to that of x, but *not* equal to that of z.
	* do *NOT* jump inside the scope of anything variably modified;
not with goto, not with switch (again, you'll miss initialization of
shadows).

Passing a VLA (let alone pointer to such, etc.) to a function is done
exactly as we would for normal arrays; $DEITY help you if the function's
idea of sizes doesn't match that of caller - shadow stuff is _NOT_ passed
at all.  No match => undefined behaviour.

Mixing VLA with normal arrays: compatible if the elements are compatible,
but if you have actual sizes that do not match, you are in nasal daemon
country.  So composite type of two VLA ones is *either*, and if actual
sizes differ, well, too bad for you.

Basically, everything works as if you had SYM_VLA(element, symbol) that
behaved almost as SYM_ARRAY.  With initializers for symbols created at
the point where we declare the object in question (or type, in case of
typedef).  Passing such sucker to function converts to SYM_VLA with *new*
symbol - one in function scope and initialized there.  Note that this
is where the things get extremely ugly for gcc - there you can pass
an object out of its scope and _that_ is what leads to lovely internal
compiler errors for things like
	sizeof *({int n = f(); int a[n]; &a;})
since we get shadow symbol dead and gone with the scope while the type
(SYM_PTR(SYM_VLA(int, n)) outlives the scope, leaving a landmine for
sizeof.

Another thing to keep in mind is that sizeof(VLA) is not a constant
expression *and* that sizeof argument is evaluated.  IOW, not only
	sizeof(int [n = f()])
has side effects, but in
	int (*p)[n];
	...
	sizeof(*(g(), p))
will have g() evaluated.  Note that if p had been declared as
	int (*p)[4];
the same sizeof() would *NOT* have called anything.  This, BTW, is where
the rules for what an integer constant expression is are getting bloody
important: 

int n = 1;
int f(void)
{
	int (*p)[1 + (0 && n)];
	return sizeof(*(g(), p));
}

_must_ call g() according to C99, while the same with
enum {n = 1;} must not, even though n won't be even looked at in either
case *and* any sane compiler will find return value of f() at compile
time, turning it to
inf f(void)
{
	g();
	return sizeof(int);
}
in the first case and
int f(void)
{
	return sizeof(int);
}
in the second.

One more thing: use of sizeof(variably-modified type) may or may not
evaluate size expressions in there if they do not affect the result.
IOW, it's unspecified whether
	sizeof(int (*)[n++])
increments n; different compilers are broken in different ways and it
might be worth generating a warning on such.  Note that
	sizeof(int [n++])
*is* guaranteed to increment n - the unspecified part is for size expressions
in such sizeof(type) buried behind a pointer.

Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

[Al Viro]

On Fri, Apr 04, 2008 at 02:19:51AM +0100, Al Viro wrote:
> 	* no functions returning variably-modified type.

Note: *pointer* to function returning a variably-modified type is possible,
is variably-modified itself and as such can appear only in function and
the same "compiler will consider VLA compatible with any array that has
as compatible element, but if the size doesn't match it's on your head"
applies.  IOW,

int a[2][2] = {{1, 2}, {3, 4}};

int (*f(void))[2] /* return a pointer to two-element array of int */
{
        return &a[0];
}

int h(int n)
{
	/* pointer to function that returns a pointer to n-element VLA of int */
        int (*(*p)(void))[n];
	/* OK if n is 2, undefined otherwise */
        p = f;
        return p()[1][1];
}

is fine and h(2) will give you 4, but if you ever do e.g. h(1), you are in
nasal daemon country.  In reality h(1) will _probably_ give a[1][0], but
compiler has every right to silently produce a binary that'll wipe your disk
or do the same itself...

Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

[Derek M Jones]

Al,

> 	* typedef *can* be variably-modified, but only in block scope.
> Warning: this can get sticky for us - all sizes are evaluated when
> typedef is reached.  IOW,
> 	typedef int a[n];
> 	a x;
> 	if (n++ == 5) {
> 		a y;
> 		int z[n];
> 	}
> will have size of y equal to that of x, but *not* equal to that of z.

An opportunity for Sparse to issue a warning :-)

> Another thing to keep in mind is that sizeof(VLA) is not a constant
> expression *and* that sizeof argument is evaluated.  IOW, not only
> 	sizeof(int [n = f()])
> has side effects, but in
> 	int (*p)[n];
> 	...
> 	sizeof(*(g(), p))
> will have g() evaluated.  Note that if p had been declared as
> 	int (*p)[4];
> the same sizeof() would *NOT* have called anything.  This, BTW, is where
> the rules for what an integer constant expression is are getting bloody
> important:

Any side effect appearing in a sizeof operand ought to be flagged.
There are people out there who think that the side effects occur (even
in C90).

Sentence 1122: http://c0x.coding-guidelines.com/6.5.3.4.html
"If the type of the operand is a variable length array type, the operand 
is evaluated;"

But watch out for sentence 1584: 
http://c0x.coding-guidelines.com/6.7.5.2.html
"Where a size expression is part of the operand of a sizeof operator and
changing the value of the size expression would not affect the result of
the operator, it is unspecified whether or not the size expression is
evaluated."

> int n = 1;
> int f(void)
> {
> 	int (*p)[1 + (0 && n)];
> 	return sizeof(*(g(), p));
> }
> 
> _must_ call g() according to C99, while the same with
> enum {n = 1;} must not, even though n won't be even looked at in either
> case *and* any sane compiler will find return value of f() at compile
> time, turning it to

Any sane compiler that performs the analysis needed to deduce that
the expression inside the parenthesis always evaluated to zero
will turn it into....

> One more thing: use of sizeof(variably-modified type) may or may not
> evaluate size expressions in there if they do not affect the result.
> IOW, it's unspecified whether
> 	sizeof(int (*)[n++])
> increments n; different compilers are broken in different ways and it

This language feature came about because at least one vendor on the
WG14 committee had a compiler that optimized away subexpressions
within a sizeof that did not contribute to the result of the
evaluation.  My attempt to stop the behavior being unspecified
did not succeed :-(

-- 
Derek M. Jones                              tel: +44 (0) 1252 520 667
Knowledge Software Ltd                      mailto:derek@knosof.co.uk
Applications Standards Conformance Testing    http://www.knosof.co.uk

Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

[Al Viro]

On Fri, Apr 04, 2008 at 03:08:01PM +0100, Derek M Jones wrote:
> >	* typedef *can* be variably-modified, but only in block scope.
> >Warning: this can get sticky for us - all sizes are evaluated when
> >typedef is reached.  IOW,
> >	typedef int a[n];
> >	a x;
> >	if (n++ == 5) {
> >		a y;
> >		int z[n];
> >	}
> >will have size of y equal to that of x, but *not* equal to that of z.
> 
> An opportunity for Sparse to issue a warning :-)

Not without data flow analysis, and sparse really doesn't do that class
of checks...

> Any side effect appearing in a sizeof operand ought to be flagged.
> There are people out there who think that the side effects occur (even
> in C90).

Not by default it shouldn't; it's about the only way to do polymorphic
typechecking a-la "this argument of macro is a function pointer and
that argument could be legitimately passed to it", etc.
 
> Sentence 1122: http://c0x.coding-guidelines.com/6.5.3.4.html
> "If the type of the operand is a variable length array type, the operand 
> is evaluated;"
> 
> But watch out for sentence 1584: 
> http://c0x.coding-guidelines.com/6.7.5.2.html
> "Where a size expression is part of the operand of a sizeof operator and
> changing the value of the size expression would not affect the result of
> the operator, it is unspecified whether or not the size expression is
> evaluated."

Dealt with below, but note that the wording in 6.7.5.2 is lousy: as stated
it covers not only intended sizeof(VM) with side effects in size expressions,
but sizeof(sizeof(int [n++])) as well, which certainly should *not* be
unspecified - the n++ is a part of operand of outer sizeof and it does not
affect its value (it's sizeof(size_t)), but it certainly should _not_ be
evaluated at all since the entire argument of the outer sizeof should not
be evaluated.

AFAICS, intended rules are:
	sizeof(expression of non-VLA type): constant, argument not evaluated
	sizeof(expression of VLA type): constant, argument evaluated
	sizeof(non-VM type): constant
	sizeof(VM type): unspecified whether all size expressions are evaluated

> This language feature came about because at least one vendor on the
> WG14 committee had a compiler that optimized away subexpressions
> within a sizeof that did not contribute to the result of the
> evaluation.  My attempt to stop the behavior being unspecified
> did not succeed :-(

The really interesting question is what the hell did gcc folks intend for
their ({ ... }) and typeof extensions...  Well, aside of "some cases
when ({ ... }) would result in VM type are clearly undefined behaviour" ;-/

BTW, I wish somebody would have come up with a sane set of type-surgery
primitives...  Part of that can be emulated with typeof, but you don't get
"turn qualified-type into type" and "give the type of Nth argument of
function type" that way.