Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 2 Dec 2014 12:49:40 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/4] the CMPLX macros must be usable in
 initializations of static variables

On Wed, Nov 26, 2014 at 02:07:55PM +0100, Jens Gustedt wrote:
> Because of some boundary cases for infinities and negative zeros, doing
> this properly is only possible with either support for _Imaginary or some
> compiler magic.
> 
> For the moment, we only know such magic for clang and gcc. There it is
> only available for newer compilers. Therefore we make the CMPLX macros
> only available when in C11 mode (or actually even in C1X mode).
> 
> Internally for the compilation of the complex functions of the math
> library we use such a macro, but that doesn't have the constraint of
> being usable for static initializations. So if we are not in C11, we
> provide such a macro as __CMPLX_NC in complex.h and map the CMPLX macros
> internally to that.
> 
> As an effect this reverts commit faea4c9937d36b17e53fdc7d5a254d7e936e1755.
> ---
>  include/complex.h   | 30 ++++++++++++++++++++++++++++--
>  src/internal/libm.h |  8 ++++++++
>  2 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/include/complex.h b/include/complex.h
> index 13a45c5..e88cf13 100644
> --- a/include/complex.h
> +++ b/include/complex.h
> @@ -112,12 +112,38 @@ long double creall(long double complex);
>  #define cimagf(x) __CIMAG(x, float)
>  #define cimagl(x) __CIMAG(x, long double)
>  
> -#define __CMPLX(x, y, t) \
> -	((union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> +#ifdef _Imaginary_I
> +# define __CMPLX(x, y, t) ((t)(x) + _Imaginary_I*(t)(y))
> +#else
> +# define __CMPLX_I(x, y, t) ((t)(x) + _Complex_I*(t)(y))
> +#endif

I was wondering about the purpose of the casts here, and I got to
thinking: what is the behavior supposed to be with regards to excess
precision? Before it was truncated by type punning and now it's
truncated by cast. But without the cast it would not be truncated.
Which behavior is right?

> +#ifndef __CMPLX
> +# if defined(__clang__)
> +  /* Clang allows initializer lists for complex numbers and compound
> +     literals for the initialization of static variables. */
> +#  define __CMPLX(x, y, t) (+(_Complex t){ (x), (y) })
> +# elif 100*__GNUC__+__GNUC_MINOR__ >= 407
> +#  define __CMPLX(x, y, t) __builtin_complex((t)(x), (t)(y))
> +# endif
> +#endif

Does clang not support the GCC builtin? I would highly prefer not to
use the clang form since it's a compiler bug that should be fixed in
future clang.

> +#ifndef __CMPLX
> +# if __STDC_VERSION__ >= 201000L
> +#  warning for this compiler, macros CMPLX, CMPLXF and CMPLXL are not standard
> +#  warning conforming for infinities and signed zeros
> +#  define __CMPLX(x, y, t) __CMPLX_I(x, y, t)
> +# endif
> +# define __CMPLX_NC(x, y, t) (+(union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> +#else
> +# define __CMPLX_NC(x, y, t) __CMPLX(x, y, t)
> +#endif
> +
> +#if __STDC_VERSION__ >= 201000L
>  #define CMPLX(x, y) __CMPLX(x, y, double)
>  #define CMPLXF(x, y) __CMPLX(x, y, float)
>  #define CMPLXL(x, y) __CMPLX(x, y, long double)
> +#endif

__CMPLX_NC is for internal use only and has no reason to be in the
public header. Also I'd like to keep the behavior in the public header
a lot more explicit rather than having lots of nested conditionals and
#ifndef __CMPLX where it's necessary to track back through all the
conditions to reason about which case gets used.

My preference would be if we could just define CMPLX either in terms
of __builtin_complex, and only exposing it in C11 mode. _Imaginary_I
is not supported yet at all and would require further changes to this
header to add anyway (this header is responsible for defining it), so
conditioning on its definition is not meaningful. And pre-4.7 versions
of GCC don't support C11 anyway (and even 4.7 and 4.8 are highly
incomplete) so I don't think much is lost if a C11 feature fails to
work on older compilers. The big question is how this fares for clang
compatibility though. I expect pcc and cparser/firm will provide a
compatible __builtin_complex (if they don't already) as they improve
C11 support.

> diff --git a/src/internal/libm.h b/src/internal/libm.h
> index ebcd784..f916e2e 100644
> --- a/src/internal/libm.h
> +++ b/src/internal/libm.h
> @@ -155,4 +155,12 @@ long double __tanl(long double, long double, int);
>  long double __polevll(long double, const long double *, int);
>  long double __p1evll(long double, const long double *, int);
>  
> +/* complex */
> +
> +#ifndef CMPLX
> +#define CMPLX(x, y) __CMPLX_NC(x, y, double)
> +#define CMPLXF(x, y) __CMPLX_NC(x, y, float)
> +#define CMPLXL(x, y) __CMPLX_NC(x, y, long double)
> +#endif
> +
>  #endif

This should probably use the unions unconditionally, after including
complex.h and #undef'ing CMPLX[FL], since musl does not require a C11
compiler to build it. Depending on whether a fallback is provided for
old compilers or not, I think your approach could lead to musl being
miscompiled. It probably doesn't happen since CMPLX is no longer
exposed except in C11 mode, and musl is compiled with -std=c99, but
this looks fragile and could cause breakage to go unnoticed if we
switch to preferring -std=c11 some time later. (There's been talk of
preferring -std=c11 if it works.)

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.