Fix UB in gdtoa hexadecimal float scanf and strtod (#1288)

When reading hexadecimal floats, cosmopolitan would previously sometimes
print a number of warnings relating to undefined behavior on left shift:

third_party/gdtoa/gethex.c:172: ubsan warning: signed left shift changed
sign bit or overflowed 12 'int' 28 'int' is undefined behavior

This is because gdtoa assumes left shifts are safe when overflow happens
even on signed integers - this is false: the C standard considers it UB.
This is easy to fix, by simply casting the shifted value to unsigned, as
doing so does not change the value or the semantics of the left shifting
(except for avoiding the undefined behavior, as the C standard specifies
that unsigned overflow yields wraparound, avoiding undefined behaviour).

This commit does this, and adds a testcase that previously triggered UB.
(this also adds test macros to test for exact float equality, instead of
the existing {EXPECT,ASSERT}_FLOAT_EQ macros which only tests inputs for
being "almost equal" (with a significant epsilon) whereas exact equality
makes more sense for certain things such as reading floats from strings,
and modifies other testcases for sscanf/fscanf of floats to utilize it).

libc/testlib/BUILD.mk

@@ -50,6 +50,7 @@ LIBC_TESTLIB_A_SRCS_C =						\
 	libc/testlib/clearxmmregisters.c			\
 	libc/testlib/contains.c					\
 	libc/testlib/endswith.c					\
+	libc/testlib/exactlyequallongdouble.c		 	\
 	libc/testlib/extract.c					\
 	libc/testlib/ezbenchcontrol.c				\
 	libc/testlib/ezbenchreport.c				\

libc/testlib/exactlyequallongdouble.c

@@ -0,0 +1,35 @@
+/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│
+│ vi: set et ft=c ts=2 sts=2 sw=2 fenc=utf-8                               :vi │
+╞══════════════════════════════════════════════════════════════════════════════╡
+│ Copyright 2024 Justine Alexandra Roberts Tunney                              │
+│                                                                              │
+│ Permission to use, copy, modify, and/or distribute this software for         │
+│ any purpose with or without fee is hereby granted, provided that the         │
+│ above copyright notice and this permission notice appear in all copies.      │
+│                                                                              │
+│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL                │
+│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED                │
+│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE             │
+│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL         │
+│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR        │
+│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER               │
+│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR             │
+│ PERFORMANCE OF THIS SOFTWARE.                                                │
+╚─────────────────────────────────────────────────────────────────────────────*/
+#include "libc/math.h"
+#include "libc/testlib/testlib.h"
+
+bool testlib_exactlyequallongdouble(long double x, long double y) {
+  if (isnan(x) && isnan(y))
+    return true;
+  // Check that we don't have e.g. one input denormal and the other not
+  // (a denormal and a non-denormal can sometimes compare equal)
+  if (fpclassify(x) != fpclassify(y))
+    return false;
+  // Check that we don't have -0 and 0
+  if (signbit(x) != signbit(y))
+    return false;
+  if (x != y)
+    return false;
+  return true;
+}

libc/testlib/testlib.h

@@ -195,6 +195,13 @@ void TearDownOnce(void);
 #define ASSERT_LDBL_LT(VAL, GOT) \
   assertLongDoubleLessThan(VAL, GOT, #VAL " < " #GOT, true)
 
+#define ASSERT_FLOAT_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, true)
+#define ASSERT_DOUBLE_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, true)
+#define ASSERT_LDBL_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, true)
+
 /*───────────────────────────────────────────────────────────────────────────│─╗
 │ cosmopolitan § testing library » assert or log                           ─╬─│┼
 ╚────────────────────────────────────────────────────────────────────────────│*/
@@ -271,6 +278,13 @@ void TearDownOnce(void);
 #define EXPECT_LGBL_LT(VAL, GOT) \
   expectLongDoubleLessThan(VAL, GOT, #VAL " < " #GOT, false)
 
+#define EXPECT_FLOAT_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, false)
+#define EXPECT_DOUBLE_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, false)
+#define EXPECT_LDBL_EXACTLY_EQ(WANT, GOT) \
+  assertLongDoubleExactlyEquals(FILIFU WANT, GOT, #GOT, false)
+
 /*───────────────────────────────────────────────────────────────────────────│─╗
 │ cosmopolitan § testing library » implementation details                  ─╬─│┼
 ╚────────────────────────────────────────────────────────────────────────────│*/
@@ -404,6 +418,7 @@ void testlib_formatbinaryashex(const char *, const void *, size_t, char **,
 void testlib_formatbinaryasglyphs(const char16_t *, const void *, size_t,
                                   char **, char **);
 bool testlib_almostequallongdouble(long double, long double);
+bool testlib_exactlyequallongdouble(long double, long double);
 void testlib_incrementfailed(void);
 void testlib_clearxmmregisters(void);
 
@@ -696,5 +711,20 @@ forceinline void assertLongDoubleEquals(FILIFU_ARGS long double want,
   testlib_onfail2(isfatal);
 }
 
+forceinline void assertLongDoubleExactlyEquals(FILIFU_ARGS long double want,
+                                               long double got,
+                                               const char *gotcode,
+                                               bool isfatal) {
+  ++g_testlib_ran;
+  if (testlib_exactlyequallongdouble(want, got))
+    return;
+  if (g_testlib_shoulddebugbreak)
+    DebugBreak();
+  testlib_showerror(file, line, func, "assertLongDoubleExactlyEquals", "≠",
+                    gotcode, testlib_formatfloat(want),
+                    testlib_formatfloat(got));
+  testlib_onfail2(isfatal);
+}
+
 COSMOPOLITAN_C_END_
 #endif /* COSMOPOLITAN_LIBC_TESTLIB_H_ */

test/libc/stdio/fscanf_test.c

@@ -27,7 +27,7 @@ TEST(fscanf, test_readAfterFloat) {
   EXPECT_EQ(4, fscanf(f, "%f%x%f%x", &f1, &i1, &f2, &i2));
   EXPECT_TRUE(isinf(f1));
   EXPECT_EQ(0xDEAD, i1);
-  EXPECT_EQ(-0.125e-2f, f2);
+  EXPECT_FLOAT_EXACTLY_EQ(-0.125e-2f, f2);
   EXPECT_EQ(0xBEEF, i2);
   fclose(f);
 }

test/libc/stdio/sscanf_test.c

@@ -338,17 +338,17 @@ TEST(sscanf, flexdecimal_hex) {
 TEST(sscanf, floating_point_simple) {
   float x = 666.666f, y = x, z = y;
   EXPECT_EQ(3, sscanf("0.3715 .3715 3715", "%f %f %f", &x, &y, &z));
-  EXPECT_EQ(0.3715f, x);
-  EXPECT_EQ(0.3715f, y);
-  EXPECT_EQ(3715.0f, z);
+  EXPECT_FLOAT_EXACTLY_EQ(0.3715f, x);
+  EXPECT_FLOAT_EXACTLY_EQ(0.3715f, y);
+  EXPECT_FLOAT_EXACTLY_EQ(3715.0f, z);
 }
 
 TEST(sscanf, floating_point_simple_double_precision) {
   double x = 666.666, y = x, z = y;
   EXPECT_EQ(3, sscanf("0.3715 .3715 3715", "%lf %lf %lf", &x, &y, &z));
-  EXPECT_EQ(0.3715, x);
-  EXPECT_EQ(0.3715, y);
-  EXPECT_EQ(3715.0, z);
+  EXPECT_DOUBLE_EXACTLY_EQ(0.3715, x);
+  EXPECT_DOUBLE_EXACTLY_EQ(0.3715, y);
+  EXPECT_DOUBLE_EXACTLY_EQ(3715.0, z);
 }
 
 TEST(sscanf, floating_point_nan) {
@@ -426,12 +426,12 @@ TEST(sscanf, floating_point_documentation_examples) {
       2, sscanf("0X1.BC70A3D70A3D7P+6 1.18973e+4932zzz -0.0000000123junk junk",
                 "%f %f %f %f %f", &f, &g, &h, &i, &j));
 
-  EXPECT_EQ(111.11f, a);
-  EXPECT_EQ(-2.22f, b);
+  EXPECT_FLOAT_EXACTLY_EQ(111.11f, a);
+  EXPECT_FLOAT_EXACTLY_EQ(-2.22f, b);
   EXPECT_TRUE(isnan(c));
   EXPECT_TRUE(isnan(d));
   EXPECT_TRUE(isinf(e));
-  EXPECT_EQ(0X1.BC70A3D70A3D7P+6f, f);
+  EXPECT_FLOAT_EXACTLY_EQ(0X1.BC70A3D70A3D7P+6f, f);
   EXPECT_TRUE(isinf(g));
 }
 
@@ -445,12 +445,12 @@ TEST(sscanf, floating_point_documentation_examples_double_precision) {
       2, sscanf("0X1.BC70A3D70A3D7P+6 1.18973e+4932zzz -0.0000000123junk junk",
                 "%lf %lf %lf %lf %lf", &f, &g, &h, &i, &j));
 
-  EXPECT_EQ(111.11, a);
-  EXPECT_EQ(-2.22, b);
+  EXPECT_DOUBLE_EXACTLY_EQ(111.11, a);
+  EXPECT_DOUBLE_EXACTLY_EQ(-2.22, b);
   EXPECT_TRUE(isnan(c));
   EXPECT_TRUE(isnan(d));
   EXPECT_TRUE(isinf(e));
-  EXPECT_EQ(0X1.BC70A3D70A3D7P+6, f);
+  EXPECT_DOUBLE_EXACTLY_EQ(0X1.BC70A3D70A3D7P+6, f);
   EXPECT_TRUE(isinf(g));
 }
 
@@ -506,3 +506,9 @@ TEST(scanf, n) {
   ASSERT_EQ(1848, port);
   ASSERT_EQ(12, len);
 }
+
+TEST(sscanf, floating_point_hexadecimal) {
+  double a = 0;
+  ASSERT_EQ(1, sscanf("0x1.5014c3472bc2c0000000p-123", "%lf", &a));
+  ASSERT_DOUBLE_EXACTLY_EQ(0x1.5014c3472bc2c0000000p-123, a);
+}

third_party/gdtoa/gethex.c

@@ -169,7 +169,9 @@ pcheck:
 			L = 0;
 			n = 0;
 		}
-		L |= (__gdtoa_hexdig[*s1] & 0x0f) << n;
+		// We can shift in a way that changes the sign bit or overflows,
+		// so we need to cast to unsigned to avoid undefined behavior
+		L |= (unsigned)(__gdtoa_hexdig[*s1] & 0x0f) << n;
 		n += 4;
 	}
 	*x++ = L;