public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC] bloody mess with __attribute__() syntax
@ 2007-07-05  9:35 Al Viro
  2007-07-05 12:03 ` Arnd Bergmann
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Al Viro @ 2007-07-05  9:35 UTC (permalink / raw)
  To: linux-sparse; +Cc: Linus Torvalds, linux-kernel

	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 ;-/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05  9:35 [RFC] bloody mess with __attribute__() syntax Al Viro
@ 2007-07-05 12:03 ` Arnd Bergmann
       [not found]   ` <OFC2AA6078.1DF7BE7E-ON4225730F.0044BE34-4225730F.0046B6F1@de.ibm.com>
  2007-07-05 15:36 ` Josh Triplett
  2007-07-05 16:41 ` Linus Torvalds
  2 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2007-07-05 12:03 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-sparse, Linus Torvalds, linux-kernel, Ulrich Weigand

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 <><

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05  9:35 [RFC] bloody mess with __attribute__() syntax Al Viro
  2007-07-05 12:03 ` Arnd Bergmann
@ 2007-07-05 15:36 ` Josh Triplett
  2007-07-05 16:43   ` Al Viro
  2007-07-05 16:41 ` Linus Torvalds
  2 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2007-07-05 15:36 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-sparse, Linus Torvalds, linux-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
       [not found]   ` <OFC2AA6078.1DF7BE7E-ON4225730F.0044BE34-4225730F.0046B6F1@de.ibm.com>
@ 2007-07-05 16:27     ` Al Viro
  0 siblings, 0 replies; 19+ messages in thread
From: Al Viro @ 2007-07-05 16:27 UTC (permalink / raw)
  To: Ulrich Weigand
  Cc: Arnd Bergmann, linux-kernel, linux-sparse, Linus Torvalds,
	Ben Elliston

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05  9:35 [RFC] bloody mess with __attribute__() syntax Al Viro
  2007-07-05 12:03 ` Arnd Bergmann
  2007-07-05 15:36 ` Josh Triplett
@ 2007-07-05 16:41 ` Linus Torvalds
  2007-07-05 16:53   ` Al Viro
  2007-07-05 17:09   ` Al Viro
  2 siblings, 2 replies; 19+ messages in thread
From: Linus Torvalds @ 2007-07-05 16:41 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-sparse, linux-kernel



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 15:36 ` Josh Triplett
@ 2007-07-05 16:43   ` Al Viro
  2007-07-05 18:50     ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: Al Viro @ 2007-07-05 16:43 UTC (permalink / raw)
  To: Josh Triplett; +Cc: linux-sparse, Linus Torvalds, linux-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 16:41 ` Linus Torvalds
@ 2007-07-05 16:53   ` Al Viro
  2007-07-05 17:02     ` Chris Lattner
  2007-07-05 17:09   ` Al Viro
  1 sibling, 1 reply; 19+ messages in thread
From: Al Viro @ 2007-07-05 16:53 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-sparse, linux-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 16:53   ` Al Viro
@ 2007-07-05 17:02     ` Chris Lattner
  0 siblings, 0 replies; 19+ messages in thread
From: Chris Lattner @ 2007-07-05 17:02 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, linux-sparse, linux-kernel

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/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 16:41 ` Linus Torvalds
  2007-07-05 16:53   ` Al Viro
@ 2007-07-05 17:09   ` Al Viro
  2007-07-05 17:26     ` Linus Torvalds
  1 sibling, 1 reply; 19+ messages in thread
From: Al Viro @ 2007-07-05 17:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-sparse, linux-kernel

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...

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 17:09   ` Al Viro
@ 2007-07-05 17:26     ` Linus Torvalds
  2007-07-05 18:07       ` Al Viro
  0 siblings, 1 reply; 19+ messages in thread
From: Linus Torvalds @ 2007-07-05 17:26 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-sparse, linux-kernel



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 17:26     ` Linus Torvalds
@ 2007-07-05 18:07       ` Al Viro
  2007-07-05 18:56         ` Linus Torvalds
  0 siblings, 1 reply; 19+ messages in thread
From: Al Viro @ 2007-07-05 18:07 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-sparse, linux-kernel

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...

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 16:43   ` Al Viro
@ 2007-07-05 18:50     ` Josh Triplett
  2007-07-05 19:13       ` Al Viro
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2007-07-05 18:50 UTC (permalink / raw)
  To: Al Viro; +Cc: Josh Triplett, linux-sparse, Linus Torvalds, linux-kernel

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 18:07       ` Al Viro
@ 2007-07-05 18:56         ` Linus Torvalds
  0 siblings, 0 replies; 19+ messages in thread
From: Linus Torvalds @ 2007-07-05 18:56 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-sparse, linux-kernel



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 18:50     ` Josh Triplett
@ 2007-07-05 19:13       ` Al Viro
  2007-07-05 19:35         ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: Al Viro @ 2007-07-05 19:13 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Josh Triplett, linux-sparse, Linus Torvalds, linux-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 19:13       ` Al Viro
@ 2007-07-05 19:35         ` Josh Triplett
  2007-07-05 20:08           ` Al Viro
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2007-07-05 19:35 UTC (permalink / raw)
  To: Al Viro; +Cc: Josh Triplett, linux-sparse, Linus Torvalds, linux-kernel

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 19:35         ` Josh Triplett
@ 2007-07-05 20:08           ` Al Viro
  2007-07-05 20:56             ` Linus Torvalds
  2007-07-05 21:09             ` Josh Triplett
  0 siblings, 2 replies; 19+ messages in thread
From: Al Viro @ 2007-07-05 20:08 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Josh Triplett, linux-sparse, Linus Torvalds, linux-kernel

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...

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 20:08           ` Al Viro
@ 2007-07-05 20:56             ` Linus Torvalds
  2007-07-06  3:26               ` Al Viro
  2007-07-05 21:09             ` Josh Triplett
  1 sibling, 1 reply; 19+ messages in thread
From: Linus Torvalds @ 2007-07-05 20:56 UTC (permalink / raw)
  To: Al Viro; +Cc: Josh Triplett, Josh Triplett, linux-sparse, linux-kernel



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 20:08           ` Al Viro
  2007-07-05 20:56             ` Linus Torvalds
@ 2007-07-05 21:09             ` Josh Triplett
  1 sibling, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2007-07-05 21:09 UTC (permalink / raw)
  To: Al Viro; +Cc: Josh Triplett, linux-sparse, Linus Torvalds, linux-kernel

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] bloody mess with __attribute__() syntax
  2007-07-05 20:56             ` Linus Torvalds
@ 2007-07-06  3:26               ` Al Viro
  0 siblings, 0 replies; 19+ messages in thread
From: Al Viro @ 2007-07-06  3:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Josh Triplett, Josh Triplett, linux-sparse, linux-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2007-07-06  3:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-05  9:35 [RFC] bloody mess with __attribute__() syntax Al Viro
2007-07-05 12:03 ` Arnd Bergmann
     [not found]   ` <OFC2AA6078.1DF7BE7E-ON4225730F.0044BE34-4225730F.0046B6F1@de.ibm.com>
2007-07-05 16:27     ` Al Viro
2007-07-05 15:36 ` Josh Triplett
2007-07-05 16:43   ` Al Viro
2007-07-05 18:50     ` Josh Triplett
2007-07-05 19:13       ` Al Viro
2007-07-05 19:35         ` Josh Triplett
2007-07-05 20:08           ` Al Viro
2007-07-05 20:56             ` Linus Torvalds
2007-07-06  3:26               ` Al Viro
2007-07-05 21:09             ` Josh Triplett
2007-07-05 16:41 ` Linus Torvalds
2007-07-05 16:53   ` Al Viro
2007-07-05 17:02     ` Chris Lattner
2007-07-05 17:09   ` Al Viro
2007-07-05 17:26     ` Linus Torvalds
2007-07-05 18:07       ` Al Viro
2007-07-05 18:56         ` Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox