Bug 176628 - [headers] [patch] use safer way of definint __WORDSIZE
Summary: [headers] [patch] use safer way of definint __WORDSIZE
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 9.0-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2013-03-04 01:50 UTC by Dmitry Marakasov
Modified: 2022-10-17 12:36 UTC (History)
0 users

See Also:


Attachments
wordsize.patch (572 bytes, patch)
2013-03-04 01:50 UTC, Dmitry Marakasov
no flags Details | Diff
file.diff (442 bytes, patch)
2013-03-04 01:50 UTC, Dmitry Marakasov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Marakasov freebsd_committer freebsd_triage 2013-03-04 01:50:00 UTC
r228529 introduced __WORDSIZE macro:

--- sys/sys/stdint.h
+#if defined(UINTPTR_MAX) && defined(UINT64_MAX) && (UINTPTR_MAX == UINT64_MAX)
+#define	__WORDSIZE		64
+#else
+#define	__WORDSIZE		32
+#endif
---

However the way it's defined is utterly unsafe: when UINTPTR_MAX or UINT64_MAX are not defined (which is the case for C++, as their definitions in e.g. x86/_stint.h are wrapped in

#if !defined(__cplusplus) || defined(__STDC_LIMIT_MACROS)

__WORDSIZE is always defined as 32, which is wrong on 64bit systems.

I have two solutions for the problem.
First one uses the same way of testing for 64bit pointers, but doesn't define __WORDSIZE if it can't be detected reliably.
Second one uses different way of testing for 64bit pointers with checking for __LP64__.

The second one looks much more useful, but I'm not sure if __LP64__ has the right semantics and will work in all platforms.

Also, can't it just be unconditionally defined to (sizeof(int*)*8)?
Comment 1 Bruce Evans freebsd_committer freebsd_triage 2013-03-04 08:04:12 UTC
On Mon, 4 Mar 2013, Dmitry Marakasov wrote:

>> Description:
> r228529 introduced __WORDSIZE macro:
>
> --- sys/sys/stdint.h
> +#if defined(UINTPTR_MAX) && defined(UINT64_MAX) && (UINTPTR_MAX == UINT64_MAX)
> +#define	__WORDSIZE		64
> +#else
> +#define	__WORDSIZE		32
> +#endif
> ---
>
> However the way it's defined is utterly unsafe: when UINTPTR_MAX or UINT64_MAX are not defined (which is the case for C++, as their definitions in e.g. x86/_stint.h are wrapped in

Please use the unix newline character.

UINTPTR_MAX and UINT64_MAX are always defined here.  Except for C++ :-(.
So the `if defined()' parts of the ifdef are basically...

>
> #if !defined(__cplusplus) || defined(__STDC_LIMIT_MACROS)

... an obfuscated version of this ifdef.

The ifdef is also utterly unportable. It assumes various things about
pointers and integer sizes.

It also has many style bugs (excessive parentheses, and I don't like
supporting non-C compilers (cc -Wundef).  It's interesting that the
ifdefs to support non-C compilers just break their warning about
UINTPTR_MAX and UINT64_MAX being 0 in the cpp expression when they are
not defined.  They are still used when they are not defined, but now
via tests for them being defined and doing a wrong thing when they are
not defined).

> __WORDSIZE is always defined as 32, which is wrong on 64bit systems.
>
> I have two solutions for the problem.
> First one uses the same way of testing for 64bit pointers, but doesn't define __WORDSIZE if it can't be detected reliably.

C++ code should be happier with __WORDSIZE being undefined that with it
being defined to garbage.

> Second one uses different way of testing for 64bit pointers with checking for __LP64__.
>
> The second one looks much more useful, but I'm not sure if __LP64__ has the right semantics and will work in all platforms.

Not right, but better than the UINTPTR_MAX test.  Both are broken and default
to __WORDSIZE == 32 if pointers are not precisely 64 bits.  Actually, the
__LP64__ test is more fragile, since it fails if __L32_P64__ and many systems
use that.  Just not any FreeBSD systems, so it is valid to use a hackish
ifdef that only works on current FreeBSD systems.

> Also, can't it just be unconditionally defined to (sizeof(int*)*8)?

No one knows, since __WORDSIZE is nonstandard.  But other macros for
integer sizes are specified by standards to work in cpp expressions,
and sizeof() would break that.  stdint.h would be much simpler if
sizeof() and casts worked right (cpp should be part of the compiler).

> diff --git sys/sys/stdint.h sys/sys/stdint.h
> index 762e879..de10869 100644
> --- sys/sys/stdint.h
> +++ sys/sys/stdint.h
> @@ -65,10 +65,12 @@ typedef	__uintmax_t		uintmax_t;
> #endif
>
> /* GNU and Darwin define this and people seem to think it's portable */

The comment has another style bug.

> -#if defined(UINTPTR_MAX) && defined(UINT64_MAX) && (UINTPTR_MAX == UINT64_MAX)
> -#define	__WORDSIZE		64
> -#else
> -#define	__WORDSIZE		32
> +#if defined(UINTPTR_MAX) && defined(UINT64_MAX)
> +# if UINTPTR_MAX == UINT64_MAX
> +#  define	__WORDSIZE		64
> +# else
> +#  define	__WORDSIZE		32
> +# endif
> #endif

This adds many style bugs in the ifdefs.

>
> /* Limits of wchar_t. */
> --- wordsize.patch ends here ---
>
> --- wordsize.1.patch begins here ---
> diff --git sys/sys/stdint.h sys/sys/stdint.h
> index 762e879..b921b99 100644
> --- sys/sys/stdint.h
> +++ sys/sys/stdint.h
> @@ -65,7 +65,7 @@ typedef	__uintmax_t		uintmax_t;
> #endif
>
> /* GNU and Darwin define this and people seem to think it's portable */
> -#if defined(UINTPTR_MAX) && defined(UINT64_MAX) && (UINTPTR_MAX == UINT64_MAX)
> +#if defined(__LP64__)
> #define	__WORDSIZE		64
> #else
> #define	__WORDSIZE		32
> --- wordsize.1.patch ends here ---

Better.  The only new style bug is `if defined()' instead of ifdef.

clang pre-declares zillions of variable sizes: on amd64

% #define __SIG_ATOMIC_WIDTH__ 32
% #define __SIZEOF_DOUBLE__ 8
% #define __SIZEOF_FLOAT__ 4
% #define __SIZEOF_INT__ 4
% #define __SIZEOF_LONG_DOUBLE__ 16
% #define __SIZEOF_LONG_LONG__ 8
% #define __SIZEOF_LONG__ 8
% #define __SIZEOF_POINTER__ 8
% #define __SIZEOF_PTRDIFF_T__ 8
% #define __SIZEOF_SHORT__ 2
% #define __SIZEOF_SIZE_T__ 8
% #define __SIZEOF_WCHAR_T__ 4
% #define __SIZEOF_WINT_T__ 4
% #define __SIZE_TYPE__ long unsigned int
% #define __SIZE_WIDTH__ 64
% #define __WCHAR_TYPE__ int
% #define __WCHAR_WIDTH__ 32
% #define __WINT_WIDTH__ 32

but these are almost useless since they are unportable (most use of __FOO
give undefined behaviour).  Most or all of these are not pre-defined by
gcc, so even the implementation cannot use them.

__SIG_ATOMIC_WIDTH__ is also broken.  sig_atomic_t has always been long
for amd64 in FreeBSD.  SIG_ATOMIC_MAX/MIN used to be inconsistent with
sig_atomic_t for amd64 (they were INT32_MAX/MIN), but someone fixed that
without changing clang, so clang is now wrong for the MAX/MIN too.

Anyway, none of the above obviously has the same semantics as __WORDSIZE.
__SIZEOF_POINTER__ is closest.

The wchar_t and wint_t are more variable than all of the other types
supported by the above, and the definitions for them could be used to
replace various ifdefs, but again the above support for them isn't
pre-defined by gcc.  gcc pre-defines __WCHAR_TYPE__, __WINT_TYPE__
and __WCHAR_MAX__.  __WCHAR_WIDTH__ can be decoded from the latter,
but there is nothing similar for __WINT_MAX__.

Bruce
Comment 2 Dmitry Marakasov 2013-03-06 20:38:44 UTC
* Bruce Evans (brde@optusnet.com.au) wrote:

> > __WORDSIZE is always defined as 32, which is wrong on 64bit systems.
> >
> > I have two solutions for the problem.
> > First one uses the same way of testing for 64bit pointers, but doesn't define __WORDSIZE if it can't be detected reliably.
> 
> C++ code should be happier with __WORDSIZE being undefined that with it
> being defined to garbage.
> 
> > Second one uses different way of testing for 64bit pointers with checking for __LP64__.
> >
> > The second one looks much more useful, but I'm not sure if __LP64__ has the right semantics and will work in all platforms.
> 
> Not right, but better than the UINTPTR_MAX test.  Both are broken and default
> to __WORDSIZE == 32 if pointers are not precisely 64 bits.  Actually, the
> __LP64__ test is more fragile, since it fails if __L32_P64__ and many systems
> use that.  Just not any FreeBSD systems, so it is valid to use a hackish
> ifdef that only works on current FreeBSD systems.

So, what solution do you think is the best in current situation?

-- 
Dmitry Marakasov   .   55B5 0596 FF1E 8D84 5F56  9510 D35A 80DD F9D2 F77D
amdmi3@amdmi3.ru  ..:  jabber: amdmi3@jabber.ru    http://www.amdmi3.ru
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:59:32 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 4 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:36:52 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>