[RFC] bloody mess with __attribute__() syntax

[RFC] bloody mess with __attribute__() syntax

[Al Viro]

	We have a fun problem and for a change it's not sparse fault.
It's gcc folks' one.  Basically, __attribute__((...)) behaves in
an idiotic way and it's an intentional (and documented) behaviour.
In declaration of form
	T __attribute__((foo)) **v;
the attribute applies to v, not to **v.  IOW, in that position it
behaves (regardless of the nature of attribute) as storage class,
not as a qualifier.  Even if the same attribute can be used in
	T * __attribute__((foo)) *v;
where it will apply to *v.  Intended way to have it apply to **v is
	T (__attribute__((foo)) **v);

To put it mildly, that blows.  Note that qualifiers can *not* behave
that way - direct declarator can not expand to (<qualifier> <something>).
I.e. if you replace __attribute__((foo)) with qualifier in the
above, you'll get invalid syntax.

Now, that idiocy would be none of our concern, if not for the fact
that noderef and address_space() are definitely supposed to imitate
qualifiers.  If anybody seriously suggests switching to syntax
like
	int (__user *p);
all over the place, well...

Note that gcc rules for __attribute__() (and that's the only source
of rules we _have_ for the damn thing) clearly say that
	int __user *p;
is the same thing as
	int *__user p;

Now, we could declare gcc people responsible for that turd rejects
of Vogon Construction Fleet and handle the damn thing sanely.
The first part is clearly the right thing to do, but the second one...
Can't do without breaking gccisms using __attribute__.  E.g.
	int (__attribute__((mode(__pointer__))) *p);
is a gcc way to say "pointer to integer type equivalent to intptr_t" and
	int __attribute__((mode(__pointer__))) *p;
is exactly the same thing as
	int *p;
since the damn attribute applies to the entire type here (and is obviously
a no-op).

Frankly, I would rather add a new primitive (__qualifier__) mirroring the
__attribute__, but acting like real qualifiers do.  And switched the
noderef et.al. to it.  The only real alternative is to have __attribute__
behaviour dependent on its guts and that's not feasible - remember that
there can be more than one attribute in the list insider the damn thing.
Besides, it's bloody disgusting.

And yes, I realize that it's an incompatible change, i.e. not a step
to be taken lightly.  Better ways out of that mess are more than
welcome; I don't see any ;-/

Re: [RFC] bloody mess with __attribute__() syntax

[Arnd Bergmann]

On Thursday 05 July 2007, Al Viro wrote:
> Frankly, I would rather add a new primitive (__qualifier__) mirroring the
> __attribute__, but acting like real qualifiers do.  And switched the
> noderef et.al. to it.  The only real alternative is to have __attribute__
> behaviour dependent on its guts and that's not feasible - remember that
> there can be more than one attribute in the list insider the damn thing.
> Besides, it's bloody disgusting.
> 
> And yes, I realize that it's an incompatible change, i.e. not a step
> to be taken lightly.  Better ways out of that mess are more than
> welcome; I don't see any ;-/

Uli Weigand has put some work into making the 'Multiple Address
Spaces' feature of Embedded C work with gcc. We need that in order
to address the process address space from a program running on an SPU
on Cell, but it should syntactically be the same thing we need for
__user, __iomem, etc. I.e., iit follows the same rules as 'const'
and 'volatile' qualifiers.
AFAIK, the infrastructure for this is supposed to go into gcc-4.3,
but has not been posted publically yet. If you add something like
this to sparse, it would be good to keep it compatible with the
new gcc extension, so that gcc can do the right thing at some point
in the future for the kernel.

Uli, can you point us to any public code or specfication about the
multiple address spaces feature?

	Arnd <><

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 02:52:22PM +0200, Ulrich Weigand wrote:
> The Named Address Space extension is documented in chapter 5 of
> Technical Report 18037:
>   ISO/IEC JTC1 SC22 WG14 N1169
>   "Programming languages - C - Extensions to support embedded processors"
>   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1169.pdf

Humm...  The only syntax for defining address spaces I see there is in
appendix B and frankly, it doesn't look right.  BTW, they forgot to
ban auto struct {<address_space> int x;} y - my preference would be
to ban address_space on struct/union members, but the tastes differ.
In any case, since they prohibit auto <address_space> int z, they'd
better find some approach prohibiting that one.

> We've currently implemented support for one Cell-specific address
> space qualifier according to that syntax.  Before submitting this
> for inclusion in GCC, we're planning to make the implementation
> more general to support arbitrary address space qualifiers;
> something like the sparse "__user" attribute would appear to be
> a natural use case for that syntax extension.
> 
> (CC'ing Ben Elliston, who is working on the GCC front-end
> implementation.)

OK.  AFAICS,
	a) __attribute__ is not an option since it doesn't act as
a qualifier (i.e. won't satisfy the requirements of section 5 in
there).  In constructs like int __attribute__((foo)) *p, attribite
applies to int *, not to int, according to gcc documentation.
	b) we still need noderef, unless implementation provides
some way to tie that to address space itself.
	c) we need some way to declare address spaces that would *not*
be subsets of generic one; rules for pointer conversions allow conversion
from nested address space on assignment and we definitely don't want
that for __user -> generic or __iomem -> generic.
	d) addressmod() doesn't look right for declaration of address space.
Looks like it's a macro anyway (it accepts macros for accessors, so it
can't live after preprocessing phase), so...  what syntax are you using
for declarations?
	e) [IMPORTANT] typeof() behaviour: at the very least, we want
some way to strip __user on typeof().  Even if it turns into a new
primitive (__unqualify__(type)).  gcc typeof leaves qualifiers in place.
We very definitely want a way to have an auto variable declared with
given type sans the address space; think of obvious uses in macros.
If __user et.al. are based on some sparse-only mechanism, we can special-case
them all we want, but if it becomes something gcc would understand...  We
will need the same semantics.
	So we either need gcc typeof() to remove address space qualifier
(IMO unlikely to happen) *or* an independent primitive that would strip
qualifiers from a type, so that with
	typedef const int C;
	typedef int A[2];
we would have
	__unqualify__(const int) => int
	__unqualify__(const int *) => const int * // qualifier inside
	__unqualify__(__user int) => int
	__unqualify__(int __user *) => int __user * // qualifier inside
	__unqualify__(int * __user) => int *
	__unqualify__(C) => int
	__unqualify__(const A) => int[2]
	__unqualify__(const int [2]) => int[2] // same type as in previous

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

[l-k removed from Cc]

	FWIW, I have several problems with that TR.  All of them have the
same root cause: arrays vs. qualifiers.

1) unless 6.7.3[8] is explicitly changed, qualifiers apply to element type
of array; when you manage to apply qualifier to an array (possible via
creative use of typedef), it still applies to the element (i.e. trickles
down until it hits non-array).

2) consequently, changes to 6.2.5 are broken.  They require _any_ array
type T to be in generic address space; of course, this is a nonsense.

3) in changes to 6.7.2.1 (saying that struct or union member types should
not carry an address space qualifier) we have a variant of the same problem -
arrays escape that requirement, but they clearly should not.

4) in places where some attempt to dance around that problem is made,
results are clumsy and actually still wrong.  A typical example:

"The pointer into enclosed address space is first converted to a pointer
to the same reference type except with any address-space qualifier
removed and any address-space qualifer of the other pointer's reference
type added".

We clearly should not replace such qualifiers deep in type derivation.


Now, AFAICS a clean solution would be to leave 6.7.3[8] alone, but
* define 'type T belongs to address space A' by
	if T1 is a type and _A is an address-space qualifier, then 
the type '_A-qualified T1' belongs to corresponding address space.
	if a type T is an array of T1, T belongs to address space A
if and only if T1 belongs to address space A.
* in 6.7.2.1 change the requirement to 'the type of a member shall
not belong to non-generic address space' or equivalent.
* in places dealing with conversion of pointers between address spaces
for comparisons, etc., speak of address-space qualifiers on the outermost
non-array in derivation of reference type.  Might be worth a definition
('counterpart of type T in enclosing address space').

Note that this is very easy to get wrong; similar questions about
const and volatile lead to a subtle difference between C and C++ type
systems and in that particular case C++ one is actually saner.

In C you can assign a pointer to array of const int to object of type
pointer to void, exactly because the constraints refer to qualifiers
on reference type and do not take into account the case when qualifier
sits on array element.  C++ prohibits that - you can assign to const void *,
but not to void *.  I.e. the 'constness' is inherited by arrays there
as far as this kind of implicit conversions is concerned.

For address spaces we definitely want the same kind of semantics;
array elements obviously belong to the same address space as the
array itself.  I have not checked the C++98 (or drafts of C++0x), but
if the construction they are using to describe that part of C++
semantics of const and volatile is not too atrocious it might make
sense to borrow it...

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 3098 bytes --]

Al Viro wrote:
> 	We have a fun problem and for a change it's not sparse fault.
> It's gcc folks' one.  Basically, __attribute__((...)) behaves in
> an idiotic way and it's an intentional (and documented) behaviour.
> In declaration of form
> 	T __attribute__((foo)) **v;
> the attribute applies to v, not to **v.  IOW, in that position it
> behaves (regardless of the nature of attribute) as storage class,
> not as a qualifier.  Even if the same attribute can be used in
> 	T * __attribute__((foo)) *v;
> where it will apply to *v.  Intended way to have it apply to **v is
> 	T (__attribute__((foo)) **v);
> 
> To put it mildly, that blows.  Note that qualifiers can *not* behave
> that way - direct declarator can not expand to (<qualifier> <something>).
> I.e. if you replace __attribute__((foo)) with qualifier in the
> above, you'll get invalid syntax.

Wow.  Insane.  So these all declare the same type:
__attribute__((foo)) T *v;
T __attribute__((foo)) *v;
T *__attribute__((foo)) v;
?  Specifically, they point to a foo-T, for convenient shooting?

> Now, that idiocy would be none of our concern, if not for the fact
> that noderef and address_space() are definitely supposed to imitate
> qualifiers.

context also represents a qualifier; the position of the qualifier should
determine things like whether you want to enforce the context when you access
a pointer or dereference a pointer.

> If anybody seriously suggests switching to syntax
> like
> 	int (__user *p);
> all over the place, well...

Definitely not an option.

> Note that gcc rules for __attribute__() (and that's the only source
> of rules we _have_ for the damn thing) clearly say that
> 	int __user *p;
> is the same thing as
> 	int *__user p;
>
> Now, we could declare gcc people responsible for that turd rejects
> of Vogon Construction Fleet and handle the damn thing sanely.
> The first part is clearly the right thing to do, but the second one...
> Can't do without breaking gccisms using __attribute__.  E.g.
> 	int (__attribute__((mode(__pointer__))) *p);
> is a gcc way to say "pointer to integer type equivalent to intptr_t" and
> 	int __attribute__((mode(__pointer__))) *p;
> is exactly the same thing as
> 	int *p;
> since the damn attribute applies to the entire type here (and is obviously
> a no-op).
>
> Frankly, I would rather add a new primitive (__qualifier__) mirroring the
> __attribute__, but acting like real qualifiers do.  And switched the
> noderef et.al. to it.

Something like that sounds vaguely reasonable.  It should allow the same set
of attributes, and just change what they apply to.  To use your example,
T __qualifier__((foo)) *v;
and
T (__attribute__((foo)) *v);
would mean the same thing.

> The only real alternative is to have __attribute__
> behaviour dependent on its guts and that's not feasible - remember that
> there can be more than one attribute in the list insider the damn thing.
> Besides, it's bloody disgusting.

Agreed.  Not an option, even if we *could* implement it.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 08:36:35AM -0700, Josh Triplett wrote:
> Wow.  Insane.  So these all declare the same type:
> __attribute__((foo)) T *v;
> T __attribute__((foo)) *v;
> T *__attribute__((foo)) v;
> ?  Specifically, they point to a foo-T, for convenient shooting?

They all give you foo-pointer-to-T.  
	T (__attribute__((foo)) *v);
would give pointer-to-foo-T.

> context also represents a qualifier; the position of the qualifier should
> determine things like whether you want to enforce the context when you access
> a pointer or dereference a pointer.

Since __context__ is (sparse-only) keyword, we are not constrained by
anything anyway.
 
> > Frankly, I would rather add a new primitive (__qualifier__) mirroring the
> > __attribute__, but acting like real qualifiers do.  And switched the
> > noderef et.al. to it.
> 
> Something like that sounds vaguely reasonable.  It should allow the same set
> of attributes, and just change what they apply to.  To use your example,
> T __qualifier__((foo)) *v;
> and
> T (__attribute__((foo)) *v);
> would mean the same thing.

Yup, except that it would not accept storage-class-like attributes (e.g.
always_inline).  And yes, __qualifier__((context(...))) probably might
be a replacement for __context__, to reduce the number of primitives.

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Thu, 2007-07-05 at 17:43 +0100, Al Viro wrote:
> On Thu, Jul 05, 2007 at 08:36:35AM -0700, Josh Triplett wrote:
> > Wow.  Insane.  So these all declare the same type:
> > __attribute__((foo)) T *v;
> > T __attribute__((foo)) *v;
> > T *__attribute__((foo)) v;
> > ?  Specifically, they point to a foo-T, for convenient shooting?
> 
> They all give you foo-pointer-to-T.  
> 	T (__attribute__((foo)) *v);
> would give pointer-to-foo-T.

Doesn't that do exactly what we want, then?  If we say
T __attribute__((noderef)) *v;
, we want a noderef-pointer-to-T, not a pointer-to-noderef-T.  noderef
should modify a pointer.

> > context also represents a qualifier; the position of the qualifier should
> > determine things like whether you want to enforce the context when you access
> > a pointer or dereference a pointer.
> 
> Since __context__ is (sparse-only) keyword, we are not constrained by
> anything anyway.

No, I mean __attribute__((context(...))), which means something
different.  __context__() works as a statement statement changing the
context.  __attribute__((context(...))) works as an attribute modifying
a type to say that it requires a given context, and that
accessing/calling it changes the context.  Somewhat of an odd
distinction, but sparse currently works that way.

> > > Frankly, I would rather add a new primitive (__qualifier__) mirroring the
> > > __attribute__, but acting like real qualifiers do.  And switched the
> > > noderef et.al. to it.
> > 
> > Something like that sounds vaguely reasonable.  It should allow the same set
> > of attributes, and just change what they apply to.  To use your example,
> > T __qualifier__((foo)) *v;
> > and
> > T (__attribute__((foo)) *v);
> > would mean the same thing.
> 
> Yup, except that it would not accept storage-class-like attributes (e.g.
> always_inline).

So, to clarify what you mean by storage-class-like: __qualifier__ would
accept all attributes that modify a *type* rather than a *declaration*.

> And yes, __qualifier__((context(...))) probably might
> be a replacement for __context__, to reduce the number of primitives.

No, see above.  __context__ works as a statement, not as a way to modify
a type.

That said, "calling this function requires this context or changes the
context" and "accessing this variable requires this context or changes
the context" may represent different types of attributes.  I don't know.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
> On Thu, 2007-07-05 at 17:43 +0100, Al Viro wrote:
> > On Thu, Jul 05, 2007 at 08:36:35AM -0700, Josh Triplett wrote:
> > > Wow.  Insane.  So these all declare the same type:
> > > __attribute__((foo)) T *v;
> > > T __attribute__((foo)) *v;
> > > T *__attribute__((foo)) v;
> > > ?  Specifically, they point to a foo-T, for convenient shooting?
> > 
> > They all give you foo-pointer-to-T.  
> > 	T (__attribute__((foo)) *v);
> > would give pointer-to-foo-T.
> 
> Doesn't that do exactly what we want, then?  If we say
> T __attribute__((noderef)) *v;
> , we want a noderef-pointer-to-T, not a pointer-to-noderef-T.  noderef
> should modify a pointer.

No.  int __user *v is pointer to noderef,address_space(1) int.  Same
as int const *v is pointer to const int.  Noderef is a property of
object being pointed to, _not_ the pointer itself.

And yes, I know that we store it ->modifiers of SYM_PTR - that saves us
a SYM_NODE we'd have to insert otherwise.  Same as with the rest of
qualifiers.

The same goes for address_space.  The same goes for const and volatile.

If you have struct foo {int x;}; struct foo __user *p; then &p->x will
be &((*p).x), i.e. &(<__user struct foo>.x), i.e. &(<__user int>), i.e.
int __user *.  __user is not a property of pointer; it couldn't work if
it would be.

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Thu, 2007-07-05 at 20:13 +0100, Al Viro wrote:
> On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
> > On Thu, 2007-07-05 at 17:43 +0100, Al Viro wrote:
> > > On Thu, Jul 05, 2007 at 08:36:35AM -0700, Josh Triplett wrote:
> > > > Wow.  Insane.  So these all declare the same type:
> > > > __attribute__((foo)) T *v;
> > > > T __attribute__((foo)) *v;
> > > > T *__attribute__((foo)) v;
> > > > ?  Specifically, they point to a foo-T, for convenient shooting?
> > > 
> > > They all give you foo-pointer-to-T.  
> > > 	T (__attribute__((foo)) *v);
> > > would give pointer-to-foo-T.
> > 
> > Doesn't that do exactly what we want, then?  If we say
> > T __attribute__((noderef)) *v;
> > , we want a noderef-pointer-to-T, not a pointer-to-noderef-T.  noderef
> > should modify a pointer.
> 
> No.  int __user *v is pointer to noderef,address_space(1) int.  Same
> as int const *v is pointer to const int.  Noderef is a property of
> object being pointed to, _not_ the pointer itself.

OK, that seems inconsistent with what you said before.  You said that 
T __attribute__((foo)) *v;
gives you a foo-pointer-to-T.  So shouldn't
int __attribute__((noderef)) *v;
give you a noderef-pointer-to-int?

> And yes, I know that we store it ->modifiers of SYM_PTR - that saves us
> a SYM_NODE we'd have to insert otherwise.  Same as with the rest of
> qualifiers.
> 
> The same goes for address_space.  The same goes for const and volatile.
> 
> If you have struct foo {int x;}; struct foo __user *p; then &p->x will
> be &((*p).x), i.e. &(<__user struct foo>.x), i.e. &(<__user int>), i.e.
> int __user *.  __user is not a property of pointer; it couldn't work if
> it would be.

OK, that makes sense; address_space describes the actual storage of the
thing pointed to, not the pointer.  It *could* describe the pointer, if
you had a pointer that resided in user address space, but that occurs
less often, and would use a different syntax.

However, noderef seems like a property of a pointer, hence why I
proposed the example I did.  A warning should occur when you do
*(<noderef T *>v) to get a T, not when you do *(<* noderef T>v) to get a
noderef T.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 12:35:53PM -0700, Josh Triplett wrote:
> OK, that seems inconsistent with what you said before.  You said that 
> T __attribute__((foo)) *v;

... in gcc.

> gives you a foo-pointer-to-T.  So shouldn't
> int __attribute__((noderef)) *v;
> give you a noderef-pointer-to-int?

... if we followed gcc rules. 

> However, noderef seems like a property of a pointer, hence why I
> proposed the example I did.  A warning should occur when you do
> *(<noderef T *>v) to get a T, not when you do *(<* noderef T>v) to get a
> noderef T.

Nope.  __noderef is a property of object being pointed to.  Again,
consider &p->x.  It should not be int *.  And it should not be
an error.  We want it to be int __noderef *.

Semantics of noderef is simple: you should not access or modify the value
of noderef object.  That's all.  int __noderef * is an absolutely normal
pointer to such object.  Think of __noderef as of a stronger variant of const.

It's just another qualifier, with usual qualifier syntax.  It can occur
in the same places where const can, giving the same kind of effect on type.
See 6.7.5.1 and 6.7.3 for general stuff on qualifiers...

Re: [RFC] bloody mess with __attribute__() syntax

[Linus Torvalds]

On Thu, 5 Jul 2007, Al Viro wrote:
> 
> Nope.  __noderef is a property of object being pointed to.  Again,
> consider &p->x.  It should not be int *.  And it should not be
> an error.  We want it to be int __noderef *.

A more interesting example to some degree is what happens to

	&*p

when p is a pointer to noderef.

The end result should be a pointer to noderef again, and it should be 
legal to do (ie we didn't actually _really_ derefence it).

It's interesting for a couple of reasons, specifically it shows how sparse 
ends up carrying around the "node" information in two _different_ places: 
either in the SYM_NODE (or SYM_ARRAY) that actually "is" the object, _or_ 
in the SYM_PTR thing that describes a pointer to the object.

So this behaviour can be directly seen in the sparse type logic when we 
convert to/from a pointer:

 - it shows how the "pointerness" is secondary, even if we actually 
   do end up putting some object flags in the SYM_PTR node.

   So the "*" needs to move the noderef from the SYM_PTR node into the 
   resulting SYM_NODE node. This is what "MOD_PTRINHERIT" is all about 
   (see its use in "create_pointer": it peels off the SYM_NODE'ness, but 
   inserts the node modifiers into the SYM_PTR).

 - the address-of operator does the reverse, and we merge the information 
   from the pointer into the resulting node (everything but the storage 
   modifiers, to be exact, see "merge_type()").

So I think Josh may be confused by the fact that the SYM_PTR node actually 
contains information about the thing the pointer _points_ to, and then the 
SYM_NODE for the pointer object actually contains information about the 
pointer itself!

So some of the flags about the object are really in the SYM_PTR node, but 
despite that, they are really about the *object*, not about the pointer, 
and that shows most clearly exactly when converting an object to a pointer 
("&" - evaluate_addressof() and crreate_pointer()) and when dereferencing 
a pointer to an object ("*" - evaluate_dereference() and merge_type())

Is it slightly complex? Yes. It's a bit strange that the SYM_PTR doesn't 
contain the information about the *pointer*, and the real information 
about an object is actually "one removed" from the type infromation, but 
it's a rather direct result of how sparse parses and maintains the type 
information.

Maybe it could have been done differently. I dunno. But it does end up 
being how the C types parse most naturally, I think.

			Linus

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 01:56:35PM -0700, Linus Torvalds wrote:

> Is it slightly complex? Yes. It's a bit strange that the SYM_PTR doesn't 
> contain the information about the *pointer*, and the real information 
> about an object is actually "one removed" from the type infromation, but 
> it's a rather direct result of how sparse parses and maintains the type 
> information.

Not only that, but it's a fairly natural if you look at that as
lazy expression in type space...  Fortunately, we do have referential
transparency there, unlike e.g. in expression graphs handling.

BTW, one really ugly thing about __attribute__((mode(...))) is that
int *A;
int B;
typeof (A) __attribute__((mode(__pointer__))) p;
typeof (B) __attribute__((mode(__pointer__))) q;

gives int *p and intptr_t q resp.  IOW, we can't eliminate the damn thing
in parser unless we are willing to deal with typeof() in there, and I'd
rather not.

It really looks like we have to delay at least some of those suckers
until examine_... time.  IOW, new kind of SYM_... nodes.

FWIW, I'm going to kill off direct messing with symbol->type et.al.
in evaluate.c and trim that stuff down to few primitives provided
by symbol.c; classify_type() is one such thing, but it really ought
to be lazy - i.e. do not assume that type is already examined,
do just enough type expression evaluation to get the derivation
type and be done with that; we probably want to get more degrees
of ->examined.  Plus "find all qualifiers", "find all qualifiers of
pointed-to", type-related part of degenerate(), type_difference()
(after lifting !Wtypesign stuff into compatible_assignment_types())
and "calculate compatible type".  All lazy...  We probably want
to go for more grades of ->examined, while we are at it.

After that we'll have much more straightforward logics in evaluate.c
and free hands for fixing the handling of attributes, etc.

Eventually I'd like to kill off MOD_CHAR/MOD_SHORT/MOD_LONG/MOD_LONGLONG
as ->modifiers bits and separate the use of struct ctype for declaration
parser state from that in struct symbol; we *are* tight on bits
there, we have a bunch of MOD_... that make sense only in parser
state (MOD_BITWISE is the same kind of thing, BTW) and parser context
might need to grow, which is obviously not nice for struct symbol.
Very few places really care about MOD_SPECIFIER outside of parser and
they could be dealt with in saner way...

BTW, what the hell is struct symbol ->value and what's SYM_MEMBER is
supposed to be used for?  AFAICS, nothing ever sets them these days
and SYM_MEMBER appears to have never been used in the entire history
of sparse...

I'm documenting the existing type system (i.e. uses of struct symbol,
etc.); I think I've got most of the picture by now, will post when
it's done.

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Thu, 2007-07-05 at 21:08 +0100, Al Viro wrote:
> On Thu, Jul 05, 2007 at 12:35:53PM -0700, Josh Triplett wrote:
> > OK, that seems inconsistent with what you said before.  You said that 
> > T __attribute__((foo)) *v;
> 
> ... in gcc.
> 
> > gives you a foo-pointer-to-T.  So shouldn't
> > int __attribute__((noderef)) *v;
> > give you a noderef-pointer-to-int?
> 
> ... if we followed gcc rules. 

Ah, OK.

> > However, noderef seems like a property of a pointer, hence why I
> > proposed the example I did.  A warning should occur when you do
> > *(<noderef T *>v) to get a T, not when you do *(<* noderef T>v) to get a
> > noderef T.
> 
> Nope.  __noderef is a property of object being pointed to.  Again,
> consider &p->x.  It should not be int *.  And it should not be
> an error.  We want it to be int __noderef *.
> 
> Semantics of noderef is simple: you should not access or modify the value
> of noderef object.  That's all.  int __noderef * is an absolutely normal
> pointer to such object.  Think of __noderef as of a stronger variant of const.

OK.  It hadn't occurred to me that "noderef int x" could have any useful
meaning on its own, but you've given a clear explanation of why it does,
which makes it meaningful to apply noderef to the pointer target rather
than the pointer.  Thanks.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
 
> No, I mean __attribute__((context(...))), which means something
> different.  __context__() works as a statement statement changing the
> context.  __attribute__((context(...))) works as an attribute modifying
> a type to say that it requires a given context, and that
> accessing/calling it changes the context.  Somewhat of an odd
> distinction, but sparse currently works that way.
 
That's actually not a qualifier from the syntax point of view...
It makes sense *only* on function types - we simply ignore it
on anything else.  And C syntax doesn't allow qualifiers on functions -
not without creative use of typedef (which gets you in nasal demon
country - undefined behaviour).

What syntax do we want?  Note that the *only* possible way to get
it kinda-sorta like qualifiers is by
typedef int F(void);
F __attribute__((context(x,0,1))) f;
with no way to actually define the sucker, since for function
definition you can't use such tricks.  IOW, no matter what we do,
it won't look like standard C.

If anything, it ought to be an analog of noreturn/pure/fastcall/etc. - same
kind of property should use the same syntax; no need to add confusion.
Unfortunately, gcc does _not_ give a sane way to declare pointers to
such beasts.  To quote gcc info:

|                        ...  At present, such attribute specifiers apply
|to the declared object or function, but in future they may attach to the
|outermost adjacent declarator.  In simple cases there is no difference,
|but, for example, in
|
|     void (****f)(void) __attribute__((noreturn));
|
|at present the `noreturn' attribute applies to `f', which causes a
|warning since `f' is not a function, but in future it may apply to the
|function `****f'.  The precise semantics of what attributes in such
|cases will apply to are not yet specified.

	Frankly, my preference would be to have that kind of attributes
follow ( parameter-type-list ), ( identifier-list ) or ( ) in productions
for direct-declarator.  That would allow uses like

int foo(void) ATTR;             // function with attribute
int foo(void) ATTR {....}       // function with attribute
int (*foo)(void) ATTR;          // pointer to function with attribute
int (*bar(int) ATTR)(void) {...}// function with attribute returning pointer to
                                // normal function
int (*bar(int))(void) ATTR {...}// normal function returning a pointer to
                                // function with attribute
AFAICS, that doesn't lead to any problems with parser - no deep lookahead,
etc.  Allowing them in declaration-specifiers, OTOH...

	Do we want to allow
__attribute__((context(x,0,1))) int foo(void) {....}
int (__attribute__((context(x,0,1))) *foo)(void);
and similar uses?  I'd rather prohibit that...

	Another fun question: how do we compare these suckers?  Suppose
you have foo() and bar() having some kind of contexts.  What contexts
do we put into n ? foo : bar and when do we allow that?  Similar question
for p = foo; where p is a pointer to function, etc.  I realize that the
first argument of __attribute__((context(...,...,...))) is currently ignored
and pretty much hopeless as defined, but...

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]

Al Viro wrote:
> On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
>  
>> No, I mean __attribute__((context(...))), which means something
>> different.  __context__() works as a statement statement changing the
>> context.  __attribute__((context(...))) works as an attribute modifying
>> a type to say that it requires a given context, and that
>> accessing/calling it changes the context.  Somewhat of an odd
>> distinction, but sparse currently works that way.
>  
> That's actually not a qualifier from the syntax point of view...
> It makes sense *only* on function types - we simply ignore it
> on anything else.

For now, yes.  I intend to make use of the context attribute on arbitrary
pointers or data.  For example, I want to specify that you must hold a given
lock in order to access a structure field, and enforce that context when you
access the field.

> And C syntax doesn't allow qualifiers on functions -
> not without creative use of typedef (which gets you in nasal demon
> country - undefined behaviour).

For functions, yes.  In the case of pointers or data, I do want the context
attribute to work like a qualifier: you might want to apply it to a pointer,
or to the pointer target, or to a structure field, or an entire structure...

[snip several function-specific issues]

> 	Another fun question: how do we compare these suckers?  Suppose
> you have foo() and bar() having some kind of contexts.  What contexts
> do we put into n ? foo : bar and when do we allow that?

If foo requires context x, and bar requires context y, then (n ? foo : bar)();
*might* require context x and *might* require context y.  Similar problems
arise for issues like conditional locking.  So far, the solution involves
saying "if it doesn't look obvious to the compiler, it doesn't look obvious to
the coder, so rewrite it such that it does"; that works fairly well for Linux,
though long-term I'd like to handle increasingly complex cases.  In some
cases, specifying more attributes helps; for instance, you can remove
ambiguity by specifying context attributes on function pointers.

> Similar question for p = foo; where p is a pointer to function, etc.

See above.  Eventually we might have advanced dataflow analysis deriving
attributes for us; for now, function pointers will need explicit contexts.

> I realize that the first argument of __attribute__((context(...,...,...)))
> is currently ignored and pretty much hopeless as defined, but...

Currently ignored, yes.  I certainly hope that providing a context expression
proves sufficient to specify a context.  Yes, problems arise if you need to do
complex unification of context expressions, but I *think* that we can handle
the simpler cases first and the complicated cases as needed.  If you have any
suggestions that might improve context checking, I'd love to discuss them with
you.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Fri, Jul 06, 2007 at 01:33:30AM -0700, Josh Triplett wrote:
> Al Viro wrote:
> > On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
> >  
> >> No, I mean __attribute__((context(...))), which means something
> >> different.  __context__() works as a statement statement changing the
> >> context.  __attribute__((context(...))) works as an attribute modifying
> >> a type to say that it requires a given context, and that
> >> accessing/calling it changes the context.  Somewhat of an odd
> >> distinction, but sparse currently works that way.
> >  
> > That's actually not a qualifier from the syntax point of view...
> > It makes sense *only* on function types - we simply ignore it
> > on anything else.
> 
> For now, yes.  I intend to make use of the context attribute on arbitrary
> pointers or data.  For example, I want to specify that you must hold a given
> lock in order to access a structure field, and enforce that context when you
> access the field.

What kind of annotations on functions do you expect to need for that
enforcing?
 
> For functions, yes.  In the case of pointers or data, I do want the context
> attribute to work like a qualifier: you might want to apply it to a pointer,
> or to the pointer target, or to a structure field, or an entire structure...

I'm not sure I like the idea of having the same qualifier mean very
different things on functions and data objects ["gets locks" vs. "needs
locks"]...
 
> If foo requires context x, and bar requires context y, then (n ? foo : bar)();
> *might* require context x and *might* require context y.

... the hell?  On functions it's not about "requires", it's about "changes".

> See above.  Eventually we might have advanced dataflow analysis deriving
> attributes for us; for now, function pointers will need explicit contexts.

How do you compare two contexts for equality?

> Currently ignored, yes.  I certainly hope that providing a context expression
> proves sufficient to specify a context.  Yes, problems arise if you need to do
> complex unification of context expressions, but I *think* that we can handle
> the simpler cases first and the complicated cases as needed.  If you have any
> suggestions that might improve context checking, I'd love to discuss them with
> you.

What do you call simpler cases?  A constant being a context expression?

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Fri, 2007-07-06 at 16:52 +0100, Al Viro wrote:
> On Fri, Jul 06, 2007 at 01:33:30AM -0700, Josh Triplett wrote:
> > Al Viro wrote:
> > > On Thu, Jul 05, 2007 at 11:50:56AM -0700, Josh Triplett wrote:
> > >> No, I mean __attribute__((context(...))), which means something
> > >> different.  __context__() works as a statement statement changing the
> > >> context.  __attribute__((context(...))) works as an attribute modifying
> > >> a type to say that it requires a given context, and that
> > >> accessing/calling it changes the context.  Somewhat of an odd
> > >> distinction, but sparse currently works that way.
> > >  
> > > That's actually not a qualifier from the syntax point of view...
> > > It makes sense *only* on function types - we simply ignore it
> > > on anything else.
> > 
> > For now, yes.  I intend to make use of the context attribute on arbitrary
> > pointers or data.  For example, I want to specify that you must hold a given
> > lock in order to access a structure field, and enforce that context when you
> > access the field.
> 
> What kind of annotations on functions do you expect to need for that
> enforcing?

The existing context annotations should suffice for many cases.
compiler.h will need new wrappers for cases like requiring a lock.  For
a very simple example:

compiler.h:
#define require_context(x) __attribute__((context(x,1,1))

magic_device.c:
DEFINE_SPINLOCK(device_lock);
struct the_device the_device require_context(device_lock);

void f(void)
{
	spin_lock(device_lock);
	frobulate(the_device.guts);
	spin_unlock(device_lock);
}

When Sparse sees an access to the_device.guts, it will check for a
device_lock context >= 1.

The same thing will work for functions which require the context:

void g(void) require_context(device_lock)
{
	glork(the_device.fnord);
}

In this case, Sparse will enforce the context on calls to g(), and then
assume the context inside g().

For special cases like preemption and RCU, which lack a lock expression
but still represent a distinct lock, how about:

compiler.h:
#ifdef __CHECKER__
#define fake_context(x) extern const int x
#else
#define fake_context(x)
#endif

preempt.h:
fake_context(preemption);
#define preempt_disable() ... __acquire(preemption)
#define preempt_enable() ... __release(preemption)
#define might_sleep_attr() ... /* something */

In this case, the "at least this value" semantic for the precondition
makes it hard to write the assertion we want.  I *almost* wonder if we
should just change this into something more like real code:

__compile_time_assert__(__context(preemption) == 0)
__something_else__(__context(preemption)++)
__something_else__(__context(preemption)--)

That would make the hard cases much easier, and the easy cases equally
hard. :)

Issues will also arise for other cases, such as referring to another
lock in the same structure, or a lock elsewhere in the parameter list of
a function.  I think this could work fairly easily by introducing some
additional scopes for looking up symbols in a context expression.

> > For functions, yes.  In the case of pointers or data, I do want the context
> > attribute to work like a qualifier: you might want to apply it to a pointer,
> > or to the pointer target, or to a structure field, or an entire structure...
> 
> I'm not sure I like the idea of having the same qualifier mean very
> different things on functions and data objects ["gets locks" vs. "needs
> locks"]...
> 
> > If foo requires context x, and bar requires context y, then (n ? foo : bar)();
> > *might* require context x and *might* require context y.
> 
> ... the hell?  On functions it's not about "requires", it's about "changes".

Both:
__attribute__((context(x,0,1))) means "you need not hold x before, but
you will hold one more of x after".
__attribute__((context(x,1,0))) means "you must already hold x, and you
will no longer hold x after".
__attribute__((context(x,1,1))) means "you must already hold x, and you
will continue to hold x".

Sparse does not yet enforce all of these conditions.  Also, the "at
least this value" semantic for the precondition makes it hard to use
contexts for things like "this blocks" and "may not block in this
context".  As I said, it needs work.  However, I intend for it to mean
*exactly* the same thing on functions or variables, except that in the
former case it means "when called", and in the latter case it means
"when accessed".  In both cases, you can require a context and change
the context.

> > See above.  Eventually we might have advanced dataflow analysis deriving
> > attributes for us; for now, function pointers will need explicit contexts.
> 
> How do you compare two contexts for equality?

A fine question.  In the simple cases, "same symbol" will work fairly
well; certainly better than the current "always equal" comparison. :)
Ideally, however, you want "refers to the same value".  Alias analysis
could do a fairly good job of that, I think.

> > Currently ignored, yes.  I certainly hope that providing a context expression
> > proves sufficient to specify a context.  Yes, problems arise if you need to do
> > complex unification of context expressions, but I *think* that we can handle
> > the simpler cases first and the complicated cases as needed.  If you have any
> > suggestions that might improve context checking, I'd love to discuss them with
> > you.
> 
> What do you call simpler cases?  A constant being a context expression?

That definitely qualifies, but I imagined "simple" as up to and
including bare symbols with no operators.

See above regarding more complicated cases.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Fri, Jul 06, 2007 at 12:29:27PM -0700, Josh Triplett wrote:
> The existing context annotations should suffice for many cases.
> compiler.h will need new wrappers for cases like requiring a lock.  For
> a very simple example:
> 
> compiler.h:
> #define require_context(x) __attribute__((context(x,1,1))
> 
> magic_device.c:
> DEFINE_SPINLOCK(device_lock);
> struct the_device the_device require_context(device_lock);

*ugh*

More ugly syntax...

> compiler.h:
> #ifdef __CHECKER__
> #define fake_context(x) extern const int x
> #else
> #define fake_context(x)
> #endif
> 
> preempt.h:
> fake_context(preemption);
> #define preempt_disable() ... __acquire(preemption)
> #define preempt_enable() ... __release(preemption)
> #define might_sleep_attr() ... /* something */
 
Still not expressive enough...  Consider e.g.

struct foo *lookup_foo(char *s); // lookup by name, return NULL if failed
				 // or pointer to struct foo with ->mutex
				 // held.  Caller should unlock.

It's legitimate, not particulary rare and AFAICS can't be expressed.

> Sparse does not yet enforce all of these conditions.  Also, the "at
> least this value" semantic for the precondition makes it hard to use
> contexts for things like "this blocks" and "may not block in this
> context".  As I said, it needs work.  However, I intend for it to mean
> *exactly* the same thing on functions or variables, except that in the
> former case it means "when called", and in the latter case it means
> "when accessed".  In both cases, you can require a context and change
> the context.

_What_ change in case of objects?

> A fine question.  In the simple cases, "same symbol" will work fairly
> well; certainly better than the current "always equal" comparison. :)
> Ideally, however, you want "refers to the same value".  Alias analysis
> could do a fairly good job of that, I think.

Not particulary useful, since for
	int foo(struct bar *p) __acquires(&p->lock);
	int bar(struct bar *p) __acquires(&p->lock);
you will get false negative in (n ? foo : bar) *and* it will persist even if
you go for
	int foo(struct bar *p) __acquires(p);
	int bar(struct bar *p) __acquires(p);
since you will have different symbols (same name, different scopes) here.

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Sat, 2007-07-07 at 03:11 +0100, Al Viro wrote:
> On Fri, Jul 06, 2007 at 12:29:27PM -0700, Josh Triplett wrote:
> > The existing context annotations should suffice for many cases.
> > compiler.h will need new wrappers for cases like requiring a lock.  For
> > a very simple example:
> > 
> > compiler.h:
> > #define require_context(x) __attribute__((context(x,1,1))
> > 
> > magic_device.c:
> > DEFINE_SPINLOCK(device_lock);
> > struct the_device the_device require_context(device_lock);
> 
> *ugh*
> 
> More ugly syntax...

Open to suggestions; just basing it on the existing syntax defined by
compiler.h.

> > compiler.h:
> > #ifdef __CHECKER__
> > #define fake_context(x) extern const int x
> > #else
> > #define fake_context(x)
> > #endif
> > 
> > preempt.h:
> > fake_context(preemption);
> > #define preempt_disable() ... __acquire(preemption)
> > #define preempt_enable() ... __release(preemption)
> > #define might_sleep_attr() ... /* something */
> 
> Still not expressive enough...  Consider e.g.
> 
> struct foo *lookup_foo(char *s); // lookup by name, return NULL if failed
> 				 // or pointer to struct foo with ->mutex
> 				 // held.  Caller should unlock.
> 
> It's legitimate, not particulary rare and AFAICS can't be expressed.

Very true; conditional locking doesn't work so well with Sparse.
Currently, you *can* express it with a bit of a hack; see the cond_lock
macro, used for spin_trylock.  Sparse will then correctly handle this:
if(spin_trylock(...)) {
	...
	spin_unlock(...);
}

That got me thinking about the idea I suggested of just using
compile-time asserts and increments/decrements.  I dunno whether that
has any hope of working or not, but it would certainly give you
expressiveness.  You might need some magic syntax to say "the return
value".

> > Sparse does not yet enforce all of these conditions.  Also, the "at
> > least this value" semantic for the precondition makes it hard to use
> > contexts for things like "this blocks" and "may not block in this
> > context".  As I said, it needs work.  However, I intend for it to mean
> > *exactly* the same thing on functions or variables, except that in the
> > former case it means "when called", and in the latter case it means
> > "when accessed".  In both cases, you can require a context and change
> > the context.
> 
> _What_ change in case of objects?

I don't immediately see the use case for changing the context when you
access an object.  I can vaguely imagine some case like "if you change
this field, you must later call this function", or vice versa.

> > A fine question.  In the simple cases, "same symbol" will work fairly
> > well; certainly better than the current "always equal" comparison. :)
> > Ideally, however, you want "refers to the same value".  Alias analysis
> > could do a fairly good job of that, I think.
> 
> Not particulary useful, since for
> 	int foo(struct bar *p) __acquires(&p->lock);
> 	int bar(struct bar *p) __acquires(&p->lock);
> you will get false negative in (n ? foo : bar) *and* it will persist even if
> you go for
> 	int foo(struct bar *p) __acquires(p);
> 	int bar(struct bar *p) __acquires(p);
> since you will have different symbols (same name, different scopes) here.

Not necessarily.  You need to substitute the actual arguments for the
formal arguments when you look at the context expression.  Given
sufficiently good analysis, you could figure this case out; in
(n ? foo : bar)(p), both cases want to acquire &p->lock, or p in the
latter case.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Fri, Jul 06, 2007 at 07:28:03PM -0700, Josh Triplett wrote:
> > 	int foo(struct bar *p) __acquires(p);
> > 	int bar(struct bar *p) __acquires(p);
> > since you will have different symbols (same name, different scopes) here.
> 
> Not necessarily.  You need to substitute the actual arguments for the
> formal arguments when you look at the context expression.  Given
> sufficiently good analysis, you could figure this case out; in
> (n ? foo : bar)(p), both cases want to acquire &p->lock, or p in the
> latter case.

baz = n ? foo : bar;

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Sat, Jul 07, 2007 at 03:11:52AM +0100, Al Viro wrote:
> Still not expressive enough...  Consider e.g.
> 
> struct foo *lookup_foo(char *s); // lookup by name, return NULL if failed
> 				 // or pointer to struct foo with ->mutex
> 				 // held.  Caller should unlock.
> 
> It's legitimate, not particulary rare and AFAICS can't be expressed.

Another fun problem: consider a structure with pair of methods - ->start()
and ->stop().  The only locking requirement is that calls are inverse wrt
locking (i.e. foo->start();foo->stop(); leaves the locking state unchanged).
Different instances may deal with different locks, different _kinds_ of
locks or no locks at all.  How do you annotate ->start() and ->stop()?

We have just such a beast in the kernel - seq_operations (see fs/seq_file.c
for code that calls these methods and grep for seq_operations to see users).

Re: [RFC] bloody mess with __attribute__() syntax

[Josh Triplett]

On Sat, 2007-07-07 at 03:30 +0100, Al Viro wrote:
> On Sat, Jul 07, 2007 at 03:11:52AM +0100, Al Viro wrote:
> > Still not expressive enough...  Consider e.g.
> > 
> > struct foo *lookup_foo(char *s); // lookup by name, return NULL if failed
> > 				 // or pointer to struct foo with ->mutex
> > 				 // held.  Caller should unlock.
> > 
> > It's legitimate, not particulary rare and AFAICS can't be expressed.
> 
> Another fun problem: consider a structure with pair of methods - ->start()
> and ->stop().  The only locking requirement is that calls are inverse wrt
> locking (i.e. foo->start();foo->stop(); leaves the locking state unchanged).
> Different instances may deal with different locks, different _kinds_ of
> locks or no locks at all.  How do you annotate ->start() and ->stop()?
>
> We have just such a beast in the kernel - seq_operations (see fs/seq_file.c
> for code that calls these methods and grep for seq_operations to see users).

Yeah, I've used seq_file.  I'd suggest treating this as two separate
problems:
     1. How do you express the semantic of function pointers that
        represent a paired context in a seq_file?  For this case, I
        suggest just saying that start and stop acquire and release the
        seq_operations structure itself, or some similar dummy context
        specific to the instance of the structure.  Any caller that
        calls the function pointers in a seq_operations struct would
        then need to properly pair them.  If you wrote a function that
        took a seq_operations and didn't properly manage the context
        with paired calls, Sparse would warn about it, just as if the
        contexts applied to local function pointers or pointers from any
        other struct.
     2. Does Sparse need to enforce any requirement that the functions
        you assign to the function pointers actually acquire and release
        some context?  If so, you'd have to do some kind of unification.
        However, I don't think Sparse needs to enforce that requirement.
        Either your particular seq_operations need pairing for some
        context reason, in which case you can specify a particular
        context on them, or they don't need pairing for some reason, in
        which case you don't need to specify a context on them.
        Meanwhile, any actual callers of those functions using the
        function pointers will get warnings as appropriate based on the
        solution to problem 1.

- Josh Triplett

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Fri, Jul 06, 2007 at 07:55:03PM -0700, Josh Triplett wrote:
>      2. Does Sparse need to enforce any requirement that the functions
>         you assign to the function pointers actually acquire and release
>         some context?  If so, you'd have to do some kind of unification.
>         However, I don't think Sparse needs to enforce that requirement.
>         Either your particular seq_operations need pairing for some
>         context reason, in which case you can specify a particular
>         context on them, or they don't need pairing for some reason, in
>         which case you don't need to specify a context on them.
>         Meanwhile, any actual callers of those functions using the
>         function pointers will get warnings as appropriate based on the
>         solution to problem 1.

That won't work.  You'll have ->show() calling the stuff that assumes
that locks are already grabbed by ->start() => warnings galore.

Re: [RFC] bloody mess with __attribute__() syntax

[Linus Torvalds]

On Thu, 5 Jul 2007, Al Viro wrote:
> 
> Now, that idiocy would be none of our concern, if not for the fact
> that noderef and address_space() are definitely supposed to imitate
> qualifiers.

Absolutely.

> Note that gcc rules for __attribute__() (and that's the only source
> of rules we _have_ for the damn thing) clearly say that
> 	int __user *p;
> is the same thing as
> 	int *__user p;

Quick question: is there some reason why we have to honor the crazy gcc 
rules, and cannot try to convince gcc people that they are insane?

That said:

> Frankly, I would rather add a new primitive (__qualifier__) mirroring the
> __attribute__, but acting like real qualifiers do.

I'd be ok with that, but I would in many ways just prefer to keep the 
existing syntax, and just make the qualifiers be a separate _namespace_ 
within the attribute setup.

We already really should be able to handle that perfectly well, since we 
look up the attribute names using the normal name lookup functions, which 
already has the notion of different namespaces. 

IOW, as far as I can tell, there's really no reason to add a 
"__qualifier__" handler, because it's not going to help us. Anything we 
can do with __qualifier__, we can already do with our __attribute__ 
parsing: what you suggest would involve having a new place for parsing 
__attributes__, and making the *current* qualifier-like attribute parsing 
trigger on "__qualifier__" instead.

So what I'd suggest is to just have *both* cases trigger on __attribute__, 
but in the qualifier case we'd use NS_QUALIFIER to look up the attribute 
function, and in the non-qualifier case we'd use NS_ATTRIBUTE (right now 
we always use NS_KEYWORD, and that's probably bogus: we should put the 
attribute names in another namespace _anyway_).

That would effectively mean that "__attribute__()" is just a way to escape 
into another set of namespaces. The advantage of that is that we could 
possibly support attributes in other places more naturally (ie we could 
have another namespace for attributes on statements or inline assembly, 
and we wouldn't ever introduce yet another keyword - the *name* in the 
attribute is the real keyword)

		Linus

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 09:41:55AM -0700, Linus Torvalds wrote:
> > Note that gcc rules for __attribute__() (and that's the only source
> > of rules we _have_ for the damn thing) clearly say that
> > 	int __user *p;
> > is the same thing as
> > 	int *__user p;
> 
> Quick question: is there some reason why we have to honor the crazy gcc 
> rules, and cannot try to convince gcc people that they are insane?

AFAICS, they started with storage-class-like attributes.  Consider e.g.
always_inline or section; these are not qualifiers at all and you want
to have
static __attribute__((always_inline)) int foo(int *p);
interpreted with attribute applied to foo, not to its return type.

So they have fsckloads of existing code relying on that parsing.  BTW,
they want things like
int *p __attribute__((section(...)))
and that's a position where qualifiers just do not appear.  Again, existing
codebase (and quite a bit of that is present in the kernel, BTW).

I rather doubt that they'll be able to kill that off and making parsing
dependent on the nature of attribute is not a viable option either -
think of __attribute__((this,that)) where "this" is storage-class-like
and "that" - qualifier-like.

Re: [RFC] bloody mess with __attribute__() syntax

[Chris Lattner]

On Thu, 5 Jul 2007, Al Viro wrote:
> On Thu, Jul 05, 2007 at 09:41:55AM -0700, Linus Torvalds wrote:
>>> Note that gcc rules for __attribute__() (and that's the only source
>>> of rules we _have_ for the damn thing) clearly say that
>>> 	int __user *p;
>>> is the same thing as
>>> 	int *__user p;
>>
>> Quick question: is there some reason why we have to honor the crazy gcc
>> rules, and cannot try to convince gcc people that they are insane?
>
> AFAICS, they started with storage-class-like attributes.  Consider e.g.
> always_inline or section; these are not qualifiers at all and you want
> to have
> static __attribute__((always_inline)) int foo(int *p);
> interpreted with attribute applied to foo, not to its return type.

This is true, but I don't think this is related.  attributes in GCC can 
apply either to types or two decls.  In this case, the always_inline 
attribute is being applied to the decl, but other attributes could be 
applied to the return type.

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 09:41:55AM -0700, Linus Torvalds wrote:
> IOW, as far as I can tell, there's really no reason to add a 
> "__qualifier__" handler, because it's not going to help us. Anything we 
> can do with __qualifier__, we can already do with our __attribute__ 
> parsing: what you suggest would involve having a new place for parsing 
> __attributes__, and making the *current* qualifier-like attribute parsing 
> trigger on "__qualifier__" instead.
> 
> So what I'd suggest is to just have *both* cases trigger on __attribute__, 
> but in the qualifier case we'd use NS_QUALIFIER to look up the attribute 
> function, and in the non-qualifier case we'd use NS_ATTRIBUTE (right now 
> we always use NS_KEYWORD, and that's probably bogus: we should put the 
> attribute names in another namespace _anyway_).

But that's the problem - we have places where *both* qualifiers and
attributes are allowed and they apply to different parts of declaration.

Note that we mishandle __attribute__((mode())) right now in a way that
makes no sense at all.  E.g. if it happens in the end of declaration,
we still apply it to the root type.  gcc applies it to the entire
declaration in that case, and that certainly makes much more sense.

So I'm afraid that we need to change __attribute__ parsing anyway...

Re: [RFC] bloody mess with __attribute__() syntax

[Linus Torvalds]

On Thu, 5 Jul 2007, Al Viro wrote:
> > 
> > So what I'd suggest is to just have *both* cases trigger on __attribute__, 
> > but in the qualifier case we'd use NS_QUALIFIER to look up the attribute 
> > function, and in the non-qualifier case we'd use NS_ATTRIBUTE (right now 
> > we always use NS_KEYWORD, and that's probably bogus: we should put the 
> > attribute names in another namespace _anyway_).
> 
> But that's the problem - we have places where *both* qualifiers and
> attributes are allowed and they apply to different parts of declaration.

So?

If they are allowed int he same place, they have to be parsed in the same 
place.

And if that place allows both attributes and qualifiers, then that place 
*has* to have the parsing for both "__attibute__" and "__qualifier__".

And I'm just saying that:
 (a) having two different magic keywords is silly and stupid, since:
 (b) We already have *one* magic keyword that can (and has to) look at its 
     subwords, and those sub-words have the capability to tell which of 
     the two cases we have (by just using name-space lookups)

> So I'm afraid that we need to change __attribute__ parsing anyway...

YES. That's what I said in my original email. I said:

  "... what you suggest would involve having a new place for parsing
   __attributes__, and making the *current* qualifier-like attribute 
   parsing trigger on "__qualifier__" instead.

   So what I'd suggest is to just have *both* cases trigger on 
   __attribute__, but in the qualifier case we'd use NS_QUALIFIER to look 
   up the attribute function, and in the non-qualifier case we'd use 
   NS_ATTRIBUTE (right now we always use NS_KEYWORD, and that's probably 
   bogus: we should put the attribute names in another namespace 
   _anyway_)"

IOW, nobody disputes that to get the new semantics, we have to have new 
code.  That's obvious.

But what I dispute is that you need to make a whole new keyword. We 
already *have* the keywords. They are the sub-keywords inside the 
"__attribute__()" list.

In other words, the way we really should parse __attribute__ stuff (and 
this is largely how we *do* parse them) is that we end up doing

	__attribute__((x(n),y(m)))

and we turn that into

	__attribute_x__(n) __attribute_y__(m)

where that "__attrubute_x__" really comes from the lookup of "x" in the 
"attribute" namespace (well, right now it's actually NS_KEYWORD, but 
that's a small detail). 

That's literally how we do it now.

And yes, we can do a new top-level name, and have

	__qualifier__((x(n))

turn into

	__qualifier_x__(n)

instead, but I just don't see any advantage. You can already do lookups 
from multiple address spaces at the same time, so I would instead suggest 
that we just *continue* to use

	__attribute__((x(n)))

and in a place where we could accept both qualifiers and gcc attributes, 
we'd look it up with

	struct symbol *sym = lookup_symbol(x, NS_ATTR | NS_QUAL);

and in places where we can just parse one or the other, we'd use just one 
or the other.

See? THAT is what I'm saying. There's no reason to change existing syntax, 
and in fact it is just _bad_ to change existing syntax, because it doesn't 
actually buy us anything, because we already have the capability to parse 
things in different contexts, and in fact allowing things to be parsed at 
the same _time_ in the different contexts.

			Linus

Re: [RFC] bloody mess with __attribute__() syntax

[Al Viro]

On Thu, Jul 05, 2007 at 10:26:40AM -0700, Linus Torvalds wrote:
> And I'm just saying that:
>  (a) having two different magic keywords is silly and stupid, since:
>  (b) We already have *one* magic keyword that can (and has to) look at its 
>      subwords, and those sub-words have the capability to tell which of 
>      the two cases we have (by just using name-space lookups)

I.e. you get per-attribute rules (fsckloads of them) and no way to tell
what do the attributes apply to unless you know the rules for given attribute.

> See? THAT is what I'm saying. There's no reason to change existing syntax, 
> and in fact it is just _bad_ to change existing syntax, because it doesn't 
> actually buy us anything, because we already have the capability to parse 
> things in different contexts, and in fact allowing things to be parsed at 
> the same _time_ in the different contexts.

Oh, I understand what you are proposing (hell, I've mentioned that variant
in the first posting).  The trouble is not with being able to implement it.
I just don't like having no relationship between the syntax and operations
on types, that's all.  gcc variant, nasty as it is for our needs, at least
has that consistent.  IOW, you can get from declaration to type structure
without knowing what each attribute does.  Seeing that gcc rules are
weird enough to be confusing, at least having that weirdness clearly
indicated by __attribute__() instead of recalling which attribute is gcc-like
and which is not...

Re: [RFC] bloody mess with __attribute__() syntax

[Linus Torvalds]

On Thu, 5 Jul 2007, Al Viro wrote:
> 
> I.e. you get per-attribute rules (fsckloads of them) and no way to tell
> what do the attributes apply to unless you know the rules for given attribute.

It basically boils down to "__attribute__((x))" is basically a _single_ 
keyword.

There is no "__attribute__" on its own, or "x" on its own. The two do not 
make sense separately. 

And that's the _accurate_ way of seeing attribute. Seeing it as a "type 
attribute" is actively wrong, and that kind of thinking just leads you 
down the wrong path (not just with qualifiers, but with __attribute__ as 
done by gcc itself, where it's used for more than type attributes).

> I just don't like having no relationship between the syntax and operations
> on types, that's all.

You seem to think that "__attribute__" has some meaning of its own.

Not true. 

Gcc already has things like

	__attribute__((extended))

which acts not on types or variables, but on expressions and statements.

And it does not *make*sense* to think of that as just another "attribute". 
It is *not* about the same kind of attributes as the type attributes at 
all! If you think it is, you're thinking about it wrong. It's a totally 
different kind of thing.

So instead of thinking about "__attribute__" as having some stand-alone 
semantic meaning (it does *not*), think of it the way I do: the 
"__attribute__" part is just an escape sequence into an extended parsing 
thing, and "__attribute__((extended))" is really just another way to give 
a way to say the _extended_ thing, without having to make it a keyword, 
with all that implies.

In other words, I think your "__qualifier__" idea is fundamentally wrong, 
because it misses the real _point_ of "attribute()". 

The real point of "__attribute__" is that it's mis-named. But that does 
not mean that it should be named "__qualifier__", because that just 
re-inforces the initial mistake. Instead, it probably _should_ have been 
named "__extension__", or something like that (or maybe "__escape__" or 
"__magic_keyword__").

That ends up explaining all the gcc semantics. Because "__attribute__" as 
gcc uses it really isn't about attributes per se, it's about extensions to 
C semantics, in a way that doesn't violate the C rules that much, and in a 
way that doesn't introduce infinite numbers of special identifiers in the 
same global name-space.

So yes, I'd be ok with renaming it, and I don't think "__attribute__" is a 
wonderful name, but we named it for gcc compatibility, so we're kind of 
stuck with the name. But that doesn't mean that we have to be stuck with 
thinking about it as a "symbol node attribute", because it really isn't, 
and has never been that, even in gcc.

		Linus