shared/json: stop using long double

It seems that the implementation of long double on ppc64el doesn't really work:
long double cast to integer and back compares as unequal to itself. Strangely,
this effect happens without optimization and both with gcc and clang, so it
seems to be an effect of how long double is implemented by the architecture.

Dumping the values shows the following pattern:
00 00 00 00 00 00 24 40  00 00 00 00 00 00 00 00   # long double v = 10;
00 00 00 00 00 00 24 40  00 00 00 00 00 00 80 39   # (long double)(intmax_t) v

Instead of trying to make this work, I think it's most reasonable to switch to
normal doubles. Notably, we had no tests for floating point behaviour. The
first test we added (for values even not in the range outside of double),
showed failures.

Common implementations of JSON (in particular JavaScript) use 64 bit double.
If we stick to this, users are likely to be happy when they exchange data with
those tools. Exporting values that cannot be represented in other tools would
just cause interop problems.

I don't think the extra precision would be much used. Long double seems to make
most sense as a transient format used in calculations to get extra precision in
operations, and not a storage or exchange format. So I expect low-level
numerical routines that have to know about hardware to make use of it, but it
shouldn't be used by our (higher-level) system library. In particular, we would
have to add tests for implementations conforming to IEEE 754, and those that
don't conform, and account for various implementation differences. It just
doesn't seem worth the effort.

https://en.wikipedia.org/wiki/Long_double#Implementations shows that the
situation is "complicated":

> On the x86 architecture, most C compilers implement long double as the 80-bit
> extended precision type supported by x86 hardware. An exception is Microsoft
> Visual C++ for x86, which makes long double a synonym for double. The Intel
> C++ compiler on Microsoft Windows supports extended precision, but requires
> the /Qlong‑double switch for long double to correspond to the hardware's
> extended precision format.

> Compilers may also use long double for the IEEE 754 quadruple-precision
> binary floating-point format (binary128). This is the case on HP-UX,
> Solaris/SPARC, MIPS with the 64-bit or n32 ABI, 64-bit ARM (AArch64) (on
> operating systems using the standard AAPCS calling conventions, such as
> Linux), and z/OS with FLOAT(IEEE). Most implementations are in software, but
> some processors have hardware support.

> On some PowerPC and SPARCv9 machines, long double is implemented as a
> double-double arithmetic, where a long double value is regarded as the exact
> sum of two double-precision values, giving at least a 106-bit precision; with
> such a format, the long double type does not conform to the IEEE
> floating-point standard. Otherwise, long double is simply a synonym for
> double (double precision), e.g. on 32-bit ARM, 64-bit ARM (AArch64) (on
> Windows and macOS) and on 32-bit MIPS (old ABI, a.k.a. o32).

> With the GNU C Compiler, long double is 80-bit extended precision on x86
> processors regardless of the physical storage used for the type (which can be
> either 96 or 128 bits). On some other architectures, long double can be
> double-double (e.g. on PowerPC) or 128-bit quadruple precision (e.g. on
> SPARC). As of gcc 4.3, a quadruple precision is also supported on x86, but as
> the nonstandard type __float128 rather than long double.

> Although the x86 architecture, and specifically the x87 floating-point
> instructions on x86, supports 80-bit extended-precision operations, it is
> possible to configure the processor to automatically round operations to
> double (or even single) precision. Conversely, in extended-precision mode,
> extended precision may be used for intermediate compiler-generated
> calculations even when the final results are stored at a lower precision
> (i.e. FLT_EVAL_METHOD == 2). With gcc on Linux, 80-bit extended precision is
> the default; on several BSD operating systems (FreeBSD and OpenBSD),
> double-precision mode is the default, and long double operations are
> effectively reduced to double precision. (NetBSD 7.0 and later, however,
> defaults to 80-bit extended precision). However, it is possible to override
> this within an individual program via the FLDCW "floating-point load
> control-word" instruction. On x86_64, the BSDs default to 80-bit extended
> precision. Microsoft Windows with Visual C++ also sets the processor in
> double-precision mode by default, but this can again be overridden within an
> individual program (e.g. by the _controlfp_s function in Visual C++). The
> Intel C++ Compiler for x86, on the other hand, enables extended-precision
> mode by default. On IA-32 OS X, long double is 80-bit extended precision.

So, in short, the only thing that can be said is that nothing can be said. In
common scenarios, we are getting only a bit of extra precision (80 bits instead
of 64), but use space for padding. In other scenarios we are getting no extra
precision. And the variance in implementations is a big issue: we can expect
strange differences in behaviour between architectures, systems, compiler
versions, compilation options, and even the other things that the program is
doing.

Fixes #21390.

diff --git a/src/shared/json-internal.h b/src/shared/json-internal.h
index 72a735171c0e85e40d3c5921634b71c6c3bd7963..652fde6147f3f4fb4102fdb9c26745cb15f11048 100644 (file)
--- a/src/shared/json-internal.h
+++ b/src/shared/json-internal.h
@@ -9,16 +9,16 @@
  * interface with this. */
 
 typedef union JsonValue  {
-        /* Encodes a simple value. On x86-64 this structure is 16 bytes wide (as long double is 128bit). */
+        /* Encodes a simple value. This structure is generally 8 bytes wide (as double is 64bit). */
         bool boolean;
-        long double real;
+        double real;
         intmax_t integer;
         uintmax_t unsig;
 } JsonValue;
 
 /* Let's protect us against accidental structure size changes on our most relevant arch */
 #ifdef __x86_64__
-assert_cc(sizeof(JsonValue) == 16U);
+assert_cc(sizeof(JsonValue) == 8U);
 #endif
 
 #define JSON_VALUE_NULL ((JsonValue) {})

diff --git a/src/shared/json.c b/src/shared/json.c
index 1bb95078cbe17a6b494e86fb2f84eff86b126eed..0ee14e7898312b55692aa9d1ef9c81f2146ad346 100644 (file)
--- a/src/shared/json.c
+++ b/src/shared/json.c
@@ -114,16 +114,16 @@ struct JsonVariant {
         };
 };
 
-/* Inside string arrays we have a series of JasonVariant structures one after the other. In this case, strings longer
+/* Inside string arrays we have a series of JsonVariant structures one after the other. In this case, strings longer
  * than INLINE_STRING_MAX are stored as references, and all shorter ones inline. (This means — on x86-64 — strings up
- * to 15 chars are stored within the array elements, and all others in separate allocations) */
+ * to 7 chars are stored within the array elements, and all others in separate allocations) */
 #define INLINE_STRING_MAX (sizeof(JsonVariant) - offsetof(JsonVariant, string) - 1U)
 
 /* Let's make sure this structure isn't increased in size accidentally. This check is only for our most relevant arch
  * (x86-64). */
 #ifdef __x86_64__
-assert_cc(sizeof(JsonVariant) == 48U);
-assert_cc(INLINE_STRING_MAX == 15U);
+assert_cc(sizeof(JsonVariant) == 40U);
+assert_cc(INLINE_STRING_MAX == 7U);
 #endif
 
 static JsonSource* json_source_new(const char *name) {
@@ -346,7 +346,7 @@ int json_variant_new_unsigned(JsonVariant **ret, uintmax_t u) {
         return 0;
 }
 
-int json_variant_new_real(JsonVariant **ret, long double d) {
+int json_variant_new_real(JsonVariant **ret, double d) {
         JsonVariant *v;
         int r;
 
@@ -748,7 +748,7 @@ static size_t json_variant_size(JsonVariant* v) {
                 return offsetof(JsonVariant, string) + strlen(v->string) + 1;
 
         case JSON_VARIANT_REAL:
-                return offsetof(JsonVariant, value) + sizeof(long double);
+                return offsetof(JsonVariant, value) + sizeof(double);
 
         case JSON_VARIANT_UNSIGNED:
                 return offsetof(JsonVariant, value) + sizeof(uintmax_t);
@@ -925,11 +925,11 @@ intmax_t json_variant_integer(JsonVariant *v) {
                 converted = (intmax_t) v->value.real;
 
                 DISABLE_WARNING_FLOAT_EQUAL;
-                if ((long double) converted == v->value.real)
+                if ((double) converted == v->value.real)
                         return converted;
                 REENABLE_WARNING;
 
-                log_debug("Real %Lg requested as integer, and cannot be converted losslessly, returning 0.", v->value.real);
+                log_debug("Real %g requested as integer, and cannot be converted losslessly, returning 0.", v->value.real);
                 return 0;
         }
 
@@ -972,11 +972,11 @@ uintmax_t json_variant_unsigned(JsonVariant *v) {
                 converted = (uintmax_t) v->value.real;
 
                 DISABLE_WARNING_FLOAT_EQUAL;
-                if ((long double) converted == v->value.real)
+                if ((double) converted == v->value.real)
                         return converted;
                 REENABLE_WARNING;
 
-                log_debug("Real %Lg requested as unsigned integer, and cannot be converted losslessly, returning 0.", v->value.real);
+                log_debug("Real %g requested as unsigned integer, and cannot be converted losslessly, returning 0.", v->value.real);
                 return 0;
         }
 
@@ -989,7 +989,7 @@ mismatch:
         return 0;
 }
 
-long double json_variant_real(JsonVariant *v) {
+double json_variant_real(JsonVariant *v) {
         if (!v)
                 return 0.0;
         if (v == JSON_VARIANT_MAGIC_ZERO_INTEGER ||
@@ -1007,9 +1007,7 @@ long double json_variant_real(JsonVariant *v) {
                 return v->value.real;
 
         case JSON_VARIANT_INTEGER: {
-                long double converted;
-
-                converted = (long double) v->value.integer;
+                double converted = (double) v->value.integer;
 
                 if ((intmax_t) converted == v->value.integer)
                         return converted;
@@ -1019,9 +1017,7 @@ long double json_variant_real(JsonVariant *v) {
         }
 
         case JSON_VARIANT_UNSIGNED: {
-                long double converted;
-
-                converted = (long double) v->value.unsig;
+                double converted = (double) v->value.unsig;
 
                 if ((uintmax_t) converted == v->value.unsig)
                         return converted;
@@ -1123,10 +1119,11 @@ JsonVariantType json_variant_type(JsonVariant *v) {
         return v->type;
 }
 
-_function_no_sanitize_float_cast_overflow_ bool json_variant_has_type(JsonVariant *v, JsonVariantType type) {
+_function_no_sanitize_float_cast_overflow_
+bool json_variant_has_type(JsonVariant *v, JsonVariantType type) {
         JsonVariantType rt;
 
-        /* Note: we turn off ubsan float cast overflo detection for this function, since it would complain
+        /* Note: we turn off ubsan float cast overflow detection for this function, since it would complain
          * about our float casts but we do them explicitly to detect conversion errors. */
 
         v = json_variant_dereference(v);
@@ -1162,17 +1159,17 @@ _function_no_sanitize_float_cast_overflow_ bool json_variant_has_type(JsonVarian
 
         /* Any integer that can be converted lossley to a real and back may also be considered a real */
         if (rt == JSON_VARIANT_INTEGER && type == JSON_VARIANT_REAL)
-                return (intmax_t) (long double) v->value.integer == v->value.integer;
+                return (intmax_t) (double) v->value.integer == v->value.integer;
         if (rt == JSON_VARIANT_UNSIGNED && type == JSON_VARIANT_REAL)
-                return (uintmax_t) (long double) v->value.unsig == v->value.unsig;
+                return (uintmax_t) (double) v->value.unsig == v->value.unsig;
 
         DISABLE_WARNING_FLOAT_EQUAL;
 
         /* Any real that can be converted losslessly to an integer and back may also be considered an integer */
         if (rt == JSON_VARIANT_REAL && type == JSON_VARIANT_INTEGER)
-                return (long double) (intmax_t) v->value.real == v->value.real;
+                return (double) (intmax_t) v->value.real == v->value.real;
         if (rt == JSON_VARIANT_REAL && type == JSON_VARIANT_UNSIGNED)
-                return (long double) (uintmax_t) v->value.real == v->value.real;
+                return (double) (uintmax_t) v->value.real == v->value.real;
 
         REENABLE_WARNING;
 
@@ -1576,7 +1573,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha
                 if (flags & JSON_FORMAT_COLOR)
                         fputs(ansi_highlight_blue(), f);
 
-                fprintf(f, "%.*Le", DECIMAL_DIG, json_variant_real(v));
+                fprintf(f, "%.*e", DECIMAL_DIG, json_variant_real(v));
 
                 if (flags & JSON_FORMAT_COLOR)
                         fputs(ANSI_NORMAL, f);
@@ -2204,7 +2201,7 @@ static int json_variant_copy(JsonVariant **nv, JsonVariant *v) {
                 break;
 
         case JSON_VARIANT_REAL:
-                k = sizeof(long double);
+                k = sizeof(double);
                 value.real = json_variant_real(v);
                 source = &value;
                 break;
@@ -2527,7 +2524,7 @@ static int json_parse_string(const char **p, char **ret) {
 
 static int json_parse_number(const char **p, JsonValue *ret) {
         bool negative = false, exponent_negative = false, is_real = false;
-        long double x = 0.0, y = 0.0, exponent = 0.0, shift = 1.0;
+        double x = 0.0, y = 0.0, exponent = 0.0, shift = 1.0;
         intmax_t i = 0;
         uintmax_t u = 0;
         const char *c;
@@ -2619,7 +2616,7 @@ static int json_parse_number(const char **p, JsonValue *ret) {
         *p = c;
 
         if (is_real) {
-                ret->real = ((negative ? -1.0 : 1.0) * (x + (y / shift))) * exp10l((exponent_negative ? -1.0 : 1.0) * exponent);
+                ret->real = ((negative ? -1.0 : 1.0) * (x + (y / shift))) * exp10((exponent_negative ? -1.0 : 1.0) * exponent);
                 return JSON_TOKEN_REAL;
         } else if (negative) {
                 ret->integer = i;
@@ -3347,14 +3344,14 @@ int json_buildv(JsonVariant **ret, va_list ap) {
                 }
 
                 case _JSON_BUILD_REAL: {
-                        long double d;
+                        double d;
 
                         if (!IN_SET(current->expect, EXPECT_TOPLEVEL, EXPECT_OBJECT_VALUE, EXPECT_ARRAY_ELEMENT)) {
                                 r = -EINVAL;
                                 goto finish;
                         }
 
-                        d = va_arg(ap, long double);
+                        d = va_arg(ap, double);
 
                         if (current->n_suppress == 0) {
                                 r = json_variant_new_real(&add, d);

diff --git a/src/shared/json.h b/src/shared/json.h
index 3912fbd9cca9c7c079d8c0a272cf7197a8ec7b44..6e649a3c339b69012c123325235752f4fde1e313 100644 (file)
--- a/src/shared/json.h
+++ b/src/shared/json.h
@@ -29,10 +29,10 @@
 
   Limitations:
   - Doesn't allow embedded NUL in strings
-  - Can't store integers outside of the -9223372036854775808…18446744073709551615 range (it will use 'long double' for
+  - Can't store integers outside of the -9223372036854775808…18446744073709551615 range (it will use 'double' for
     values outside this range, which is lossy)
   - Can't store negative zero (will be treated identical to positive zero, and not retained across serialization)
-  - Can't store non-integer numbers that can't be stored in "long double" losslessly
+  - Can't store non-integer numbers that can't be stored in "double" losslessly
   - Allows creation and parsing of objects with duplicate keys. The "dispatcher" will refuse them however. This means
     we can parse and pass around such objects, but will carefully refuse them when we convert them into our own data.
 
@@ -61,7 +61,7 @@ int json_variant_new_base64(JsonVariant **ret, const void *p, size_t n);
 int json_variant_new_hex(JsonVariant **ret, const void *p, size_t n);
 int json_variant_new_integer(JsonVariant **ret, intmax_t i);
 int json_variant_new_unsigned(JsonVariant **ret, uintmax_t u);
-int json_variant_new_real(JsonVariant **ret, long double d);
+int json_variant_new_real(JsonVariant **ret, double d);
 int json_variant_new_boolean(JsonVariant **ret, bool b);
 int json_variant_new_array(JsonVariant **ret, JsonVariant **array, size_t n);
 int json_variant_new_array_bytes(JsonVariant **ret, const void *p, size_t n);
@@ -83,7 +83,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(JsonVariant *, json_variant_unref);
 const char *json_variant_string(JsonVariant *v);
 intmax_t json_variant_integer(JsonVariant *v);
 uintmax_t json_variant_unsigned(JsonVariant *v);
-long double json_variant_real(JsonVariant *v);
+double json_variant_real(JsonVariant *v);
 bool json_variant_boolean(JsonVariant *v);
 
 JsonVariantType json_variant_type(JsonVariant *v);
@@ -242,7 +242,7 @@ enum {
 #define JSON_BUILD_STRING(s) _JSON_BUILD_STRING, (const char*) { s }
 #define JSON_BUILD_INTEGER(i) _JSON_BUILD_INTEGER, (intmax_t) { i }
 #define JSON_BUILD_UNSIGNED(u) _JSON_BUILD_UNSIGNED, (uintmax_t) { u }
-#define JSON_BUILD_REAL(d) _JSON_BUILD_REAL, (long double) { d }
+#define JSON_BUILD_REAL(d) _JSON_BUILD_REAL, (double) { d }
 #define JSON_BUILD_BOOLEAN(b) _JSON_BUILD_BOOLEAN, (bool) { b }
 #define JSON_BUILD_ARRAY(...) _JSON_BUILD_ARRAY_BEGIN, __VA_ARGS__, _JSON_BUILD_ARRAY_END
 #define JSON_BUILD_EMPTY_ARRAY _JSON_BUILD_ARRAY_BEGIN, _JSON_BUILD_ARRAY_END

diff --git a/src/test/test-json.c b/src/test/test-json.c
index 34ea7448a93d393258a75626e9e33d7d3e3f6c00..83f3d566a6e76a9f421f950cd304e7f5a8560cba 100644 (file)
--- a/src/test/test-json.c
+++ b/src/test/test-json.c
@@ -46,14 +46,9 @@ static void test_tokenizer(const char *data, ...) {
                         assert_se(streq_ptr(nn, str));
 
                 } else if (t == JSON_TOKEN_REAL) {
-                        long double d;
+                        double d;
 
-                        d = va_arg(ap, long double);
-
-                        /* Valgrind doesn't support long double calculations and automatically downgrades to 80bit:
-                         * http://www.valgrind.org/docs/manual/manual-core.html#manual-core.limits.
-                         * Some architectures might not support long double either.
-                         */
+                        d = va_arg(ap, double);
 
                         assert_se(fabsl(d - v.real) < 1e-10 ||
                                   fabsl((d - v.real) / v.real) < 1e-10);
@@ -523,17 +518,17 @@ static void test_bisect(void) {
 }
 
 static void test_float_match(JsonVariant *v) {
-        const long double delta = 0.0001;
+        const double delta = 0.0001;
 
         assert_se(json_variant_is_array(v));
         assert_se(json_variant_elements(v) == 9);
-        assert_se(fabsl((long double) 1.0 - ((long double) DBL_MIN / json_variant_real(json_variant_by_index(v, 0)))) <= delta);
-        assert_se(fabsl((long double) 1.0 - ((long double) DBL_MAX / json_variant_real(json_variant_by_index(v, 1)))) <= delta);
+        assert_se(fabsl((double) 1.0 - ((double) DBL_MIN / json_variant_real(json_variant_by_index(v, 0)))) <= delta);
+        assert_se(fabsl((double) 1.0 - ((double) DBL_MAX / json_variant_real(json_variant_by_index(v, 1)))) <= delta);
         assert_se(json_variant_is_null(json_variant_by_index(v, 2))); /* nan is not supported by json → null */
         assert_se(json_variant_is_null(json_variant_by_index(v, 3))); /* +inf is not supported by json → null */
         assert_se(json_variant_is_null(json_variant_by_index(v, 4))); /* -inf is not supported by json → null */
         assert_se(json_variant_is_null(json_variant_by_index(v, 5)) ||
-                  fabsl((long double) 1.0 - ((long double) HUGE_VAL / json_variant_real(json_variant_by_index(v, 5)))) <= delta); /* HUGE_VAL might be +inf, but might also be something else */
+                  fabsl((double) 1.0 - ((double) HUGE_VAL / json_variant_real(json_variant_by_index(v, 5)))) <= delta); /* HUGE_VAL might be +inf, but might also be something else */
         assert_se(json_variant_is_real(json_variant_by_index(v, 6)) &&
                   json_variant_is_integer(json_variant_by_index(v, 6)) &&
                   json_variant_integer(json_variant_by_index(v, 6)) == 0);
@@ -586,13 +581,13 @@ int main(int argc, char *argv[]) {
         test_tokenizer("-1234", JSON_TOKEN_INTEGER, (intmax_t) -1234, JSON_TOKEN_END);
         test_tokenizer("18446744073709551615", JSON_TOKEN_UNSIGNED, (uintmax_t) UINT64_MAX, JSON_TOKEN_END);
         test_tokenizer("-9223372036854775808", JSON_TOKEN_INTEGER, (intmax_t) INT64_MIN, JSON_TOKEN_END);
-        test_tokenizer("18446744073709551616", JSON_TOKEN_REAL, (long double) 18446744073709551616.0L, JSON_TOKEN_END);
-        test_tokenizer("-9223372036854775809", JSON_TOKEN_REAL, (long double) -9223372036854775809.0L, JSON_TOKEN_END);
+        test_tokenizer("18446744073709551616", JSON_TOKEN_REAL, (double) 18446744073709551616.0L, JSON_TOKEN_END);
+        test_tokenizer("-9223372036854775809", JSON_TOKEN_REAL, (double) -9223372036854775809.0L, JSON_TOKEN_END);
         test_tokenizer("-1234", JSON_TOKEN_INTEGER, (intmax_t) -1234, JSON_TOKEN_END);
-        test_tokenizer("3.141", JSON_TOKEN_REAL, (long double) 3.141, JSON_TOKEN_END);
-        test_tokenizer("0.0", JSON_TOKEN_REAL, (long double) 0.0, JSON_TOKEN_END);
-        test_tokenizer("7e3", JSON_TOKEN_REAL, (long double) 7e3, JSON_TOKEN_END);
-        test_tokenizer("-7e-3", JSON_TOKEN_REAL, (long double) -7e-3, JSON_TOKEN_END);
+        test_tokenizer("3.141", JSON_TOKEN_REAL, (double) 3.141, JSON_TOKEN_END);
+        test_tokenizer("0.0", JSON_TOKEN_REAL, (double) 0.0, JSON_TOKEN_END);
+        test_tokenizer("7e3", JSON_TOKEN_REAL, (double) 7e3, JSON_TOKEN_END);
+        test_tokenizer("-7e-3", JSON_TOKEN_REAL, (double) -7e-3, JSON_TOKEN_END);
         test_tokenizer("true", JSON_TOKEN_BOOLEAN, true, JSON_TOKEN_END);
         test_tokenizer("false", JSON_TOKEN_BOOLEAN, false, JSON_TOKEN_END);
         test_tokenizer("null", JSON_TOKEN_NULL, JSON_TOKEN_END);