Date: Fri, 16 Dec 2011 01:12:16 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Schouten <ed@80386.nl> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r228495 - head/sys/sys Message-ID: <20111216000117.R2632@besplex.bde.org> In-Reply-To: <20111215122047.GN1771@hoeg.nl> References: <201112140909.pBE99bS3090646@svn.freebsd.org> <20111214234615.B3839@besplex.bde.org> <20111215122047.GN1771@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 15 Dec 2011, Ed Schouten wrote:
> Hello Bruce,
>
> After reading through your email, I think it's best to fix things using
> the following patch. Essentially it does the following:
>
> - It makes sure __alignof() is always present, by defining it as a macro
> for GCC < 2.95.
I tested this with a K&R compiler. It worked (since it is similar to
the fallback for offsetof() which always worked) after fixing 2 bugs
that prevented the fallback working with any compiler.
>From your patch (quoted below):
> | +#if !__GNUC_PREREQ__(2, 95)
> | +#define __alignof(x) __offsetof(struct { char __a; e __b; }, __b)
> | +#endif
The 2 bugs are:
1. Even __offsetof() is broken in the fallback case (that's all cases
where it uses a struct like the above and doesn't use a gccish builtin).
This is because __offsetof() uses size_t, but size_t is not necessarily
in declared. offsetof() in <stddef.h>, etc. works because size_t is
necessarily declared there. BTW, the ifdef for __offsetof in
<stddef.h> is bogus since <sys/cdefs.h> is always included so
__offsetof is always defined.
__size_t could be used, but that would give lots of pollution for
cdefs.h, and the file that declares __size_t, namely
<machine/types.h> is broken in fallback cases:
- recently broken for K&R since it now uses signed instead of __signed
- broken for longer for K&R and C90 since it uses long long. In working
versions, the __int64_t declarations were hacked for 32-bit machines
on to make them compile (but not work). Non-hacked versions should
simply not declare the 64-bit types if the compiler doesn't support
them.
2. The macro parameter x is now mispelled, so it is never used, and the
non-misspelled macro parameter e gives a syntax error when it is
used in the macro body.
Test program:
% #include <sys/cdefs.h>
%
% typedef unsigned size_t; /* XXX */
%
% #define __alignof(e) __offsetof(struct { char __a; e __b; }, __b)
%
% int c = __alignof(char);
% int s = __alignof(short);
% int i = __alignof(int);
% int l = __alignof(long);
% int f = __alignof(float);
% int d = __alignof(double);
Fixes:
1. Just a hack to make __offsetof() work. I don't know a good way to
declare __size_t. Of course, size_t should not be used.
2. Change the spelling back to e. I prefer x, but all the other
definitions use e.
> - All the C1X macros can now be implemented using the ones we already
> had (__dead2, etc). This means we don't need to repeat all the
> compiler version specific checks.
> - While there, add struct __hack to the _Static_assert, as it always
> requires a semicolon.
Good.
> | Index: sys/sys/cdefs.h
> | ===================================================================
> | --- sys/sys/cdefs.h (revision 228504)
> | +++ sys/sys/cdefs.h (working copy)
> | @@ -230,27 +230,24 @@
> | #elif defined(__STDC_VERSION__) && __STDC_VERSION__ > 201000L
> | /* Do nothing. They are language keywords. */
> | #else
> | -/* Not supported. Implement them manually. */
> | -#ifdef __GNUC__
> | -#define _Alignas(e) __attribute__((__aligned__(e)))
> | -#define _Alignof(e) __alignof__(e)
> | -#define _Noreturn __attribute__((__noreturn__))
> | +/* Not supported. Implement them using our versions. */
It may be worth emphasizing that in some cases, "our version" doesn't
exist, and that this is intentional (we omit the definition when none
is possible, and this results in __aligned() sometimes not being defined
and __thread never being defined).
> | +#define _Alignas(e) __aligned(e)
> | +#define _Alignof(e) __alignof(e)
I prefer to always spell the parameter as x. Most older macros in this
file, and CTASSERT() in another file, spell their parameters (x, y, ...).
> | +#define _Noreturn __dead2
> | #define _Thread_local __thread
> | -#else
> | -#define _Alignas(e)
> | -#define _Alignof(e) __offsetof(struct { char __a; e __b; }, __b)
> | -#define _Noreturn
> | -#define _Thread_local
> | -#endif
> | #ifdef __COUNTER__
> | #define _Static_assert(e, s) __Static_assert(e, __COUNTER__)
> | #define __Static_assert(e, c) ___Static_assert(e, c)
> | #define ___Static_assert(e, c) typedef char __assert ## c[(e) ? 1 : -1]
> | #else
> | -#define _Static_assert(e, s)
> | +#define _Static_assert(e, s) struct __hack
CTASSERT() has regressed for compilers that don't support __COUNTER__,
since it uses this. Previously:
- CTASSERT() never worked for K&R compilers, since it uses C90 token pasting
- CTASSERT() worked for all C90 and later compilers.
> | #endif
> | #endif
> |
> | +#if !__GNUC_PREREQ__(2, 95)
> | +#define __alignof(x) __offsetof(struct { char __a; e __b; }, __b)
> | +#endif
Needs the spelling fix.
> | +
> | #if __GNUC_PREREQ__(2, 96)
> | #define __malloc_like __attribute__((__malloc__))
> | #define __pure __attribute__((__pure__))
Check the sorting of the new macros, especially __alignof(). I think
the ordering should be mostly on the gcc and standards version, not on
the order in which the macros were added to cdefs.h. You sorted the 2.95
ifdef before a 2.96 ifdef, but this 2.96 ifdef is especially disordered
according to my rule. Even if the order is "logical" instead of historical,
it makes more sense to define basic macros like __alignof() before using
them.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111216000117.R2632>