From owner-svn-src-all@FreeBSD.ORG Thu Dec 15 14:12:45 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 74A651065676; Thu, 15 Dec 2011 14:12:45 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail06.syd.optusnet.com.au (mail06.syd.optusnet.com.au [211.29.132.187]) by mx1.freebsd.org (Postfix) with ESMTP id 0F9638FC0C; Thu, 15 Dec 2011 14:12:44 +0000 (UTC) Received: from c211-28-227-231.carlnfd1.nsw.optusnet.com.au (c211-28-227-231.carlnfd1.nsw.optusnet.com.au [211.28.227.231]) by mail06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id pBFECGva032176 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 16 Dec 2011 01:12:38 +1100 Date: Fri, 16 Dec 2011 01:12:16 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Schouten In-Reply-To: <20111215122047.GN1771@hoeg.nl> Message-ID: <20111216000117.R2632@besplex.bde.org> References: <201112140909.pBE99bS3090646@svn.freebsd.org> <20111214234615.B3839@besplex.bde.org> <20111215122047.GN1771@hoeg.nl> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans Subject: Re: svn commit: r228495 - head/sys/sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Dec 2011 14:12:45 -0000 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 , etc. works because size_t is necessarily declared there. BTW, the ifdef for __offsetof in is bogus since 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 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 % % 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