All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Derek M Jones <derek@knosof.co.uk>
Cc: Josh Triplett <josh@freedesktop.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Harvey Harrison <harvey.harrison@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-sparse@vger.kernel.org
Subject: Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h
Date: Fri, 4 Apr 2008 19:42:21 +0100	[thread overview]
Message-ID: <20080404184221.GF9785@ZenIV.linux.org.uk> (raw)
In-Reply-To: <47F63641.4090103@knosof.co.uk>

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.

      reply	other threads:[~2008-04-04 18:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-03  0:33 [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h Harvey Harrison
2008-04-03  0:57 ` Linus Torvalds
2008-04-03  1:34   ` Al Viro
2008-04-03  1:50     ` Linus Torvalds
2008-04-03  1:59       ` Al Viro
2008-04-03  2:51         ` Linus Torvalds
2008-04-03  2:55           ` Al Viro
2008-04-04  9:19             ` Al Viro
2008-04-03  2:41       ` Al Viro
2008-04-03 20:31         ` Josh Triplett
2008-04-04  1:19           ` Al Viro
2008-04-04  2:00             ` Al Viro
2008-04-04 14:08             ` Derek M Jones
2008-04-04 18:42               ` Al Viro [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080404184221.GF9785@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=derek@knosof.co.uk \
    --cc=harvey.harrison@gmail.com \
    --cc=josh@freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.