cast truncates bits from constant value (8000000000000000 becomes 0)

cast truncates bits from constant value (8000000000000000 becomes 0)

[Geert Uytterhoeven]

On Tue, 21 Nov 2006, Geoff Levand wrote:
> +enum ps3_vendor_id {
> +	PS3_VENDOR_ID_NONE = 0,
> +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
> +};

I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
and noticed that sparse (cloned from
git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
complains about the second value with:

| warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Section 6.7.2.2.4 of C99 says:

| Each enumerated type shall be compatible with char, a signed integer type, or
| an unsigned integer type. The choice of type is implementation-defined, but
| shall be capable of representing the values of all the members of the
| enumeration.

The code snippet

| u64 x = PS3_VENDOR_ID_SONY;
| printk("PS3_VENDOR_ID_SONY = %lu\n", x);

does print the expected (i.e. non-zero) result.

Hence this looks like a bug in sparse.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
Geert.Uytterhoeven@sonycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Al Viro]

On Fri, Dec 01, 2006 at 03:25:08PM +0100, Geert Uytterhoeven wrote:
> On Tue, 21 Nov 2006, Geoff Levand wrote:
> > +enum ps3_vendor_id {
> > +	PS3_VENDOR_ID_NONE = 0,
> > +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
> > +};
> 
> I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
> and noticed that sparse (cloned from
> git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
> complains about the second value with:
> 
> | warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> 
> Section 6.7.2.2.4 of C99 says:
> 
> | Each enumerated type shall be compatible with char, a signed integer type, or
> | an unsigned integer type. The choice of type is implementation-defined, but
> | shall be capable of representing the values of all the members of the
> | enumeration.

FWIW, that code is *not* valid C99; note that all enumeration members must
fit the range of int (see 6.7.2.2.2).  What you quote speaks about the
objects of type enum <something>, which is not the same as type of enum
members.  In C99 it's int, plain and simple.  Value outside of the range
of int => undefined behaviour.  Note that with
	enum foo {A, B} x;
you might very well have sizeof(A) != sizeof(x).

IOW, you are using a gccism in an area where gcc is bloody inconsistent
in the set of bugs it shows in different versions.

Generally safe way is to split the anonymous huge enum members into
single-element enums and pray that gcc will at least stay consistent
in handling of those.

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Geert Uytterhoeven]

On Fri, 1 Dec 2006, Al Viro wrote:
> On Fri, Dec 01, 2006 at 03:25:08PM +0100, Geert Uytterhoeven wrote:
> > On Tue, 21 Nov 2006, Geoff Levand wrote:
> > > +enum ps3_vendor_id {
> > > +	PS3_VENDOR_ID_NONE = 0,
> > > +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
> > > +};
> > 
> > I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
> > and noticed that sparse (cloned from
> > git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
> > complains about the second value with:
> > 
> > | warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > 
> > Section 6.7.2.2.4 of C99 says:
> > 
> > | Each enumerated type shall be compatible with char, a signed integer type, or
> > | an unsigned integer type. The choice of type is implementation-defined, but
> > | shall be capable of representing the values of all the members of the
> > | enumeration.
> 
> FWIW, that code is *not* valid C99; note that all enumeration members must
> fit the range of int (see 6.7.2.2.2).  What you quote speaks about the

You're right. I missed that one.

> IOW, you are using a gccism in an area where gcc is bloody inconsistent
> in the set of bugs it shows in different versions.

Hmmm...

> Generally safe way is to split the anonymous huge enum members into
> single-element enums and pray that gcc will at least stay consistent
> in handling of those.

Or fall back to #defines, which is what we were trying to avoid in the first
place (i.e. group related values together in enums)...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
Geert.Uytterhoeven@sonycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Linus Torvalds]

On Fri, 1 Dec 2006, Geert Uytterhoeven wrote:
>
> On Tue, 21 Nov 2006, Geoff Levand wrote:
> > +enum ps3_vendor_id {
> > +	PS3_VENDOR_ID_NONE = 0,
> > +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
> > +};
> 
> I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
> and noticed that sparse (cloned from
> git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
> complains about the second value with:
> 
> | warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> 
> Section 6.7.2.2.4 of C99 says:
> 
> | Each enumerated type shall be compatible with char, a signed integer type, or
> | an unsigned integer type. The choice of type is implementation-defined, but
> | shall be capable of representing the values of all the members of the
> | enumeration.
> 
> The code snippet
> 
> | u64 x = PS3_VENDOR_ID_SONY;
> | printk("PS3_VENDOR_ID_SONY = %lu\n", x);
> 
> does print the expected (i.e. non-zero) result.
> 
> Hence this looks like a bug in sparse.

It's really a bug in gcc, but it's documented, so it's a "feature".

Gcc allows large enums, but does so in such a strange manner that it's 
totally hopeless to catch problems. Also, putting a value that is larger 
than "unsigned int" into an enum is really setting yourself up for bugs 
and not even guaranteed to work for standard C, so sparse takes a dim view 
of it and just says that enums are limited in size to "int" or "unsigned 
int".

You can either ignore that warning or just use a #define. 

			Linus

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Geoff Levand]

Linus Torvalds wrote:
> 
> On Fri, 1 Dec 2006, Geert Uytterhoeven wrote:
>>
>> On Tue, 21 Nov 2006, Geoff Levand wrote:
>> > +enum ps3_vendor_id {
>> > +	PS3_VENDOR_ID_NONE = 0,
>> > +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
>> > +};
>> 
>> I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
>> and noticed that sparse (cloned from
>> git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
>> complains about the second value with:
>> 
>> | warning: cast truncates bits from constant value (8000000000000000 becomes 0)
>> 
>> Section 6.7.2.2.4 of C99 says:
>> 
>> | Each enumerated type shall be compatible with char, a signed integer type, or
>> | an unsigned integer type. The choice of type is implementation-defined, but
>> | shall be capable of representing the values of all the members of the
>> | enumeration.
>> 
>> The code snippet
>> 
>> | u64 x = PS3_VENDOR_ID_SONY;
>> | printk("PS3_VENDOR_ID_SONY = %lu\n", x);
>> 
>> does print the expected (i.e. non-zero) result.
>> 
>> Hence this looks like a bug in sparse.
> 
> It's really a bug in gcc, but it's documented, so it's a "feature".
> 
> Gcc allows large enums, but does so in such a strange manner that it's 
> totally hopeless to catch problems. Also, putting a value that is larger 
> than "unsigned int" into an enum is really setting yourself up for bugs 
> and not even guaranteed to work for standard C, so sparse takes a dim view 
> of it and just says that enums are limited in size to "int" or "unsigned 
> int".
> 
> You can either ignore that warning or just use a #define. 

One of the gcc maintainers (Andrew Pinski) told me to set it up that
way, so I figured it was safe.

-Geoff

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Linus Torvalds]

On Fri, 1 Dec 2006, Geoff Levand wrote:
> 
> One of the gcc maintainers (Andrew Pinski) told me to set it up that
> way, so I figured it was safe.

It _is_ safe, as long as you stick to gcc, and as long as you don't mix 
signs in your values, for example.

But we actually want to let people compile even the kernel with other 
compilers than gcc. Most of the time, that means that we strongly 
encourage those compiler people to support all the gcc extensions (let's 
face it, standardization is good, and open standards work better than 
closed ones, and gcc is the most widely spread open and portable compiler 
BY FAR, so it would be stupid to _not_ do that).

But at times, some of the gcc extensions aren't necessarily that well 
defined or thought out, or simply not worth it. The extended type system 
for enums in gcc is just basically messy, and it doesn't really offer you 
anything important.

Compared with inline assembly, for example, inline assembly may be even 
messier and more complicated, but inline assembly definitely offers you 
something very important. The strange enum type extensions? What do they 
really offer above and beyond the standard C preprocessor #define's? Apart 
from some (very limited) name scoping advantages, there really isn't 
anything that it really helps with.

And as Al already mentioned, some of the extensions aren't even compatible 
within different versions of gcc itself. Admittedly they are all fairly 
special, but basically the "type" of different individual enum members 
simply isn't well-defined in all cases, and will even depend on thigns 
like the order they were declared in, if I recall correctly.

Messy, messy.

			Linus

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Derek M Jones]

Linus,

>> One of the gcc maintainers (Andrew Pinski) told me to set it up that
>> way, so I figured it was safe.
> 
> It _is_ safe, as long as you stick to gcc, and as long as you don't mix 
> signs in your values, for example.
> 
> But we actually want to let people compile even the kernel with other 
> compilers than gcc. Most of the time, that means that we strongly 
> encourage those compiler people to support all the gcc extensions (let's 
> face it, standardization is good, and open standards work better than 
> closed ones, and gcc is the most widely spread open and portable compiler 
> BY FAR, so it would be stupid to _not_ do that).

The C++ Standard explicitly supports enumeration constants not
having type int.  See sentence 858 of
http://www.coding-guidelines.com/cbook/cbook1_0b.pdf
for a discussion of the issues (plus some other sentences).

C compiler vendors, at least those who extend the language, are more
likely to want to follow the C++ rules (which are documented) than the
gcc rules (which are poorly documented).

The C++ way of doing things is also likely to be followed by vendors
whose C compiler is enabled by a command line switch on their C++
compiler (eg, at least one vendor based in Seattle).

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

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Linus Torvalds]

On Fri, 1 Dec 2006, Linus Torvalds wrote:
> 
> But at times, some of the gcc extensions aren't necessarily that well 
> defined or thought out, or simply not worth it. The extended type system 
> for enums in gcc is just basically messy, and it doesn't really offer you 
> anything important.

Btw, try this stupid program, to see just how _strange_ gcc enums are.. A 
sizeof of the enum is not the same as the size of the individual entries. 

Notice also how the size of the enum entry is _not_ tied to the type of 
the expression it had, but literally to its _value_. The size of "one" 
ends up being 4, even though it was initialized with a "1ll" value.

So with gcc-enums, you CANNOT get a sane type result.

In contrast, if you want sane types, you could easily do

	#define one (1ull)
	#define other (0x10000ull)
	#define strange (0x100000000ull)

and they'd all have the same type (and having the same type means that 
they act the same in expressions - you get the same expression type in 
mixing these values, _unlike_ the insane gcc enum cases)

		Linus

---
enum hello {
	one = 1ll,
	other = 0x10000,
	bigval = 0x1000000000000ll,
};

int main(int argc, char **argv)
{
	printf("%zu %zu %zu %zu\n",
		sizeof(enum hello),
		sizeof(one),
		sizeof(other),
		sizeof(bigval));
	return 0;
}

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Al Viro]

On Fri, Dec 01, 2006 at 08:49:19PM +0000, Derek M Jones wrote:
> The C++ Standard explicitly supports enumeration constants not
> having type int.  See sentence 858 of
> http://www.coding-guidelines.com/cbook/cbook1_0b.pdf
> for a discussion of the issues (plus some other sentences).
> 
> C compiler vendors, at least those who extend the language, are more
> likely to want to follow the C++ rules (which are documented) than the
> gcc rules (which are poorly documented).
> 
> The C++ way of doing things is also likely to be followed by vendors
> whose C compiler is enabled by a command line switch on their C++
> compiler (eg, at least one vendor based in Seattle).

"C++ way of doing things" is hardly an endorsement.  _IF_ we are changing
the way enum works, might as well do it sanely and have the type of
enumeration constant same as that of expression initializing it (with
usual implicit initializers).  But yes, that explicitly changes semantics -
enum { A = 0L, B, C }; will have A, B and C long, not int.  Direct
contradiction with C90/C99...

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Geoff Levand]

Linus Torvalds wrote:
> Btw, try this stupid program, to see just how _strange_ gcc enums are.. A 
> sizeof of the enum is not the same as the size of the individual entries. 

Yes, interesting.  I think your comment regarding compatibility with other
compilers is the convincing one.

Another way is to change the encoding of the value such that it fits into
the range of an enum.  For this particular case, the values are actually
for a field in the high 3 bits, so I can just do the shift when the value
is used.

-Geoff

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Linus Torvalds]

On Fri, 1 Dec 2006, Al Viro wrote:
> 
> "C++ way of doing things" is hardly an endorsement.  _IF_ we are changing
> the way enum works, might as well do it sanely and have the type of
> enumeration constant same as that of expression initializing it (with
> usual implicit initializers).  But yes, that explicitly changes semantics -
> enum { A = 0L, B, C }; will have A, B and C long, not int.  Direct
> contradiction with C90/C99...

Well, the C++ definition at least means that enumerated names have the 
same types _after_ the enumaration is closed (ie the example I sent out 
will at least give the same value for the sizeof() of everything).

So the C++ definition is a lot saner than what gcc does now.

On the other hand, I do agree that "keeping the enumerated type the same 
as the initializer expression type" is _also_ a sane situation. And it's 
better than the C++ situation in the sense that at least the sizes are 
_consistent_ (which C++ is not - the size AT DEFINITION time is 
potentially different from the size AFTER the definition).

So in the C++ world, you have the odd situation that

	enum strange {
		one = (char) 1,
		other = sizeof(one),
	};

we will actually end up with

	other != sizeof(one)

AFTER the declaration (because when "other" got its value, "one" had a 
type of "char", but _after_ the declaration, "one" will have a type of 
"size_t" or "int" or whatever, because the final type of "one" depends on 
the type of the enumerator.

So the C++ definition is really odd, but it's better than what gcc does, 
because at least all the values end up having a common type at the end.

The version where all the values _keep_ their types all the time, and you 
can force them to be whatever you want (by just making sure the 
initializer expression has the right type) is the most flexible one and at 
least doesn't have inconsistencies like the above example, but it's 
neither what gcc nor the C++ standard (or any older C standard, for that 
matter) actually says.

Anyway, because of all this, enum types are a mess. sparse warns if you 
can't fit it in an "int", which is the traditional and fairly dependable 
old C behaviour, and basically says "extended integer types for enums are 
flaky as hell - warn about them!"

		Linus

Re: cast truncates bits from constant value (8000000000000000 becomes 0)

[Michael Ellerman]

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

On Fri, 2006-12-01 at 15:55 +0100, Geert Uytterhoeven wrote:
> On Fri, 1 Dec 2006, Al Viro wrote:
> > On Fri, Dec 01, 2006 at 03:25:08PM +0100, Geert Uytterhoeven wrote:
> > > On Tue, 21 Nov 2006, Geoff Levand wrote:
> > > > +enum ps3_vendor_id {
> > > > +	PS3_VENDOR_ID_NONE = 0,
> > > > +	PS3_VENDOR_ID_SONY = 0x8000000000000000UL,
> > > > +};
> > > 
> > > I've just ran `make C=1' (PPC in 64-bit mode, and sparse is called with -m64),
> > > and noticed that sparse (cloned from
> > > git://git.kernel.org/pub/scm/devel/sparse/sparse.git a few minutes ago)
> > > complains about the second value with:
> > > 
> > > | warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > > 
> > > Section 6.7.2.2.4 of C99 says:
> > > 
> > > | Each enumerated type shall be compatible with char, a signed integer type, or
> > > | an unsigned integer type. The choice of type is implementation-defined, but
> > > | shall be capable of representing the values of all the members of the
> > > | enumeration.
> > 
> > FWIW, that code is *not* valid C99; note that all enumeration members must
> > fit the range of int (see 6.7.2.2.2).  What you quote speaks about the
> 
> You're right. I missed that one.
> 
> > IOW, you are using a gccism in an area where gcc is bloody inconsistent
> > in the set of bugs it shows in different versions.
> 
> Hmmm...
> 
> > Generally safe way is to split the anonymous huge enum members into
> > single-element enums and pray that gcc will at least stay consistent
> > in handling of those.
> 
> Or fall back to #defines, which is what we were trying to avoid in the first
> place (i.e. group related values together in enums)...

The enum achieves very little in this case IMHO, it's just a more
verbose way of #defining - and it doesn't even have reliable semantics.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]