32-bit support: fix _umul call; address warnings (#5)

* 32-bit support: fix _umul call; address warnings

And fix test harness to always preserve the sign bit of NaN even when
strtod does not

* Remove arch flags from makefile since we'll just have 2 jobs

* any old ubuntu will do

* Try the clanker's cross arch script

* Skip amalgamate in crossarch containers

* install clang

* No need for the dirty check in cross containers

Author: kolemannix

.github/workflows/c.yml

@@ -2,17 +2,19 @@ name: Simple Test
 
 on:
   push:
-    branches: [ "main" ]
   pull_request:
-    branches: [ "main" ]
 
 jobs:
   build:
     runs-on: ubuntu-latest
 
     steps:
     - uses: actions/checkout@v4
-    - name: Install 32bit headers
-      run: sudo apt-get update && sudo apt-get install -y gcc-multilib
+    - name: Install dependencies
+      run: sudo apt-get update && sudo apt-get install -y clang gcc make python3
     - name: make test
       run: make test
+    - name: make example
+      run: make example
+    - name: Check dirty single-header
+      run: git diff --exit-code ffc.h

.github/workflows/clanker_cross.yml

@@ -0,0 +1,51 @@
+name: Cross-Arch CI
+
+on:
+  push:
+    branches: [main, master]
+  pull_request:
+
+jobs:
+  native:
+    name: Linux amd64 (native)
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install dependencies
+        run: sudo apt-get update && sudo apt-get install -y build-essential
+      - name: make test
+        run: make test
+      - name: make example
+        run: make example
+      - name: Check dirty single-header
+        run: git diff --exit-code ffc.h
+
+  cross:
+    name: Linux ${{ matrix.arch }}
+    runs-on: ubuntu-22.04
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - arch: i386
+            distro: debian:bookworm
+            install: apt-get update && apt-get install -y build-essential clang
+          - arch: aarch64
+            distro: ubuntu:22.04
+            install: apt-get update && apt-get install -y build-essential clang
+    steps:
+      - uses: actions/checkout@v4
+      - uses: docker/setup-qemu-action@v3
+      - name: Build and test (${{ matrix.arch }})
+        run: |
+          docker run --rm \
+            --platform linux/${{ matrix.arch == 'i386' && '386' || matrix.arch }} \
+            -v "${{ github.workspace }}:/work" \
+            -w /work \
+            ${{ matrix.distro }} \
+            bash -c "
+              ${{ matrix.install }} &&
+              touch ffc.h &&
+              make test &&
+              make example 
+            "

Makefile

@@ -9,16 +9,15 @@ endif
 CLANG_FLAGS := -xc -Wall -Wextra -Wpedantic -O3 -g -std=c99 $(EXTRA_CFLAGS)
 
 out/test_runner: ffc.h test_src/test.c | out
-	gcc -xc -m64 -Wall -Wextra -Wpedantic ffc.h -fsyntax-only
-	gcc -xc -m32 -Wall -Wextra -Wpedantic ffc.h -fsyntax-only
-	clang $(CLANG_FLAGS) -I. -Itest_src -DFFC_IMPL test_src/test.c -o out/test_runner -lm
+	gcc -xc -Wall -Wextra -Wpedantic ffc.h -fsyntax-only
+	clang $(CLANG_FLAGS) -I. -Itest_src test_src/test.c -o out/test_runner -lm
 
 test: out/test_runner out/test_int_runner
 	./out/test_runner
 	./out/test_int_runner
 
 out/test_int_runner: ffc.h test_src/test_int.c | out
-	clang $(CLANG_FLAGS) -I. -Itest_src -DFFC_IMPL test_src/test_int.c -o out/test_int_runner -lm
+	clang $(CLANG_FLAGS) -I. -Itest_src test_src/test_int.c -o out/test_int_runner -lm
 
 ffc.h: src/ffc.h src/common.h src/parse.h src/digit_comparison.h src/api.h src/bigint.h amalgamate.py
 	python3 amalgamate.py > ffc.h

ffc.h

@@ -550,7 +550,7 @@ ffc_u128 ffc_full_multiplication(uint64_t a, uint64_t b) {
   // But MinGW on ARM64 doesn't have native support for 64-bit multiplications
   answer.high = __umulh(a, b);
   answer.low = a * b;
-#elif defined(FFC_32BIT) || (defined(_WIN64) && !defined(__clang__) &&   \
+#elif (defined(_WIN64) && !defined(__clang__) &&   \
                                    !defined(_M_ARM64) && !defined(__GNUC__))
   answer.low = _umul128(a, b, &answer.high); // _umul128 not available on ARM64
 #elif defined(FFC_64BIT) && defined(__SIZEOF_INT128__)
@@ -851,7 +851,7 @@ static const float FFC_FLOAT_POWERS_OF_TEN[] = {
 
 // Largest integer value v so that (5**index * v) <= 1<<53.
 // 0x20000000000000 == 1 << 53
-#define FFC_55555 (5L * 5L * 5L * 5L * 5L)
+#define FFC_55555 (5LL * 5LL * 5LL * 5LL * 5LL)
 static const uint64_t FFC_DOUBLE_MAX_MANTISSA[] = {
     (uint64_t)0x20000000000000L,
     (uint64_t)0x20000000000000L / 5,
@@ -2417,7 +2417,7 @@ bool ffc_char_eq_zero(char const *p, size_t char_width) {
 ffc_internal ffc_inline
 void ffc_skip_zeros(char **first, char *last, size_t char_width) {
   size_t cmp_len;
-  size_t cmp_mask;
+  uint64_t cmp_mask;
   switch (char_width) {
     case 1:
       cmp_len = FFC_INT_CMP_LEN_1;
@@ -2457,7 +2457,7 @@ void ffc_skip_zeros(char **first, char *last, size_t char_width) {
 ffc_internal ffc_inline
 bool ffc_is_truncated(char const *first, char const *last, size_t char_width) {
   size_t cmp_len;
-  size_t cmp_mask;
+  uint64_t cmp_mask;
   switch (char_width) {
     case 1:
       cmp_len = FFC_INT_CMP_LEN_1;

src/common.h

@@ -308,7 +308,7 @@ ffc_u128 ffc_full_multiplication(uint64_t a, uint64_t b) {
   // But MinGW on ARM64 doesn't have native support for 64-bit multiplications
   answer.high = __umulh(a, b);
   answer.low = a * b;
-#elif defined(FFC_32BIT) || (defined(_WIN64) && !defined(__clang__) &&   \
+#elif (defined(_WIN64) && !defined(__clang__) &&   \
                                    !defined(_M_ARM64) && !defined(__GNUC__))
   answer.low = _umul128(a, b, &answer.high); // _umul128 not available on ARM64
 #elif defined(FFC_64BIT) && defined(__SIZEOF_INT128__)
@@ -609,7 +609,7 @@ static const float FFC_FLOAT_POWERS_OF_TEN[] = {
 
 // Largest integer value v so that (5**index * v) <= 1<<53.
 // 0x20000000000000 == 1 << 53
-#define FFC_55555 (5L * 5L * 5L * 5L * 5L)
+#define FFC_55555 (5LL * 5LL * 5LL * 5LL * 5LL)
 static const uint64_t FFC_DOUBLE_MAX_MANTISSA[] = {
     (uint64_t)0x20000000000000L,
     (uint64_t)0x20000000000000L / 5,

src/digit_comparison.h

@@ -235,7 +235,7 @@ bool ffc_char_eq_zero(char const *p, size_t char_width) {
 ffc_internal ffc_inline
 void ffc_skip_zeros(char **first, char *last, size_t char_width) {
   size_t cmp_len;
-  size_t cmp_mask;
+  uint64_t cmp_mask;
   switch (char_width) {
     case 1:
       cmp_len = FFC_INT_CMP_LEN_1;
@@ -275,7 +275,7 @@ void ffc_skip_zeros(char **first, char *last, size_t char_width) {
 ffc_internal ffc_inline
 bool ffc_is_truncated(char const *first, char const *last, size_t char_width) {
   size_t cmp_len;
-  size_t cmp_mask;
+  uint64_t cmp_mask;
   switch (char_width) {
     case 1:
       cmp_len = FFC_INT_CMP_LEN_1;

test_src/double_cases_infnan.csv

@@ -12,19 +12,19 @@ inf,inf,ok,,
 1.8e308,inf,out_of_range,
 1.832312213213213232132132143451234453123412321321312e308,inf,out_of_range,
 2e30000000000000000, inf,out_of_range,
-2e3000, inf,out_of_range,
+2e3000,inf,out_of_range,
 1.9e308,inf,out_of_range,
 1.79769313486231581e308,inf,out_of_range,DBL_MAX + 0.00000000000000001e308
 1.7976931348623159e308,inf,out_of_range,DBL_MAX + 0.0000000000000001e308
 179769313486231580793728971405303415079934132710037826936173778980444968292764750946649017977587207096330286416692887910946555547851940402630657488671505820681908902000708383676273854845817711531764475730270069855571366959622842914819860834936475292719074168444365510704342711559699508093042880177904174497792,inf,out_of_range,( (2 - 0.5*2^(−52)) * 2^1023 ) smallest number that overflows to infinity
 infinity,inf,ok,,
--NaN,-nan,ok,,
+-NaN,-nan,ok,be sure to write '-nan' as its intercepted by the test harness,
 -inf,-inf,ok,,
 nan,nan,ok,,
 Inf,inf,ok,,
 -infinity,-inf,ok,,
 inf,inf,ok,,
--nan,-nan,ok,,
+-nan,-nan,ok,be sure to write '-nan' as its intercepted by the test harness,
 NaN,nan,ok,,
 -Inf,-inf,ok,,
 -+inf,0.0,invalid,

test_src/test.c

@@ -2,6 +2,7 @@
 #include "sonicsv.h"
 
 #define FFC_DEBUG 0
+#define FFC_IMPL
 #include "ffc.h"
 #include <stdlib.h>
 #include <stdio.h>
@@ -311,6 +312,7 @@ void cb_test(const csv_row_t *row, void *ctx) {
   bool is_neg_max = strncmp(expected_field->data, "-MAX", 4) == 0;
   bool is_min = strncmp(expected_field->data, "MIN", 3) == 0;
   bool is_neg_min = strncmp(expected_field->data, "-MIN", 4) == 0;
+  bool is_neg_nan = strncmp(expected_field->data, "-nan", 4) == 0;
   switch (vk) {
   case FFC_VALUE_KIND_DOUBLE:
     if (is_max) {
@@ -321,6 +323,8 @@ void cb_test(const csv_row_t *row, void *ctx) {
       expected_value.d = DBL_MIN;
     } else if (is_neg_min) {
       expected_value.d = -DBL_MIN;
+    } else if (is_neg_nan) {
+      expected_value.d = -NAN;
     } else {
       expected_value.d = strtod(my_strndup(expected_field->size, expected_field->data), NULL);
     };
@@ -334,6 +338,8 @@ void cb_test(const csv_row_t *row, void *ctx) {
       expected_value.f = FLT_MIN;
     } else if (is_neg_min) {
       expected_value.f = -FLT_MIN;
+    } else if (is_neg_nan) {
+      expected_value.f = -NAN;
     } else {
       expected_value.f = strtof(my_strndup(expected_field->size, expected_field->data), NULL);
     };
@@ -616,6 +622,7 @@ int main(void) {
 
   float_special();
 
+  #define FFC_TEST_EXHAUSTIVE 0
   #if FFC_TEST_EXHAUSTIVE
   exhaustive_32_run();
   #endif

test_src/test_int.c

@@ -7,6 +7,7 @@
 #include <assert.h>
 
 #define FFC_DEBUG 0
+#define FFC_IMPL
 #include "ffc.h"
 
 static int FAILS = 0;

tools/ubuntu_32_devel/Dockerfile

@@ -0,0 +1,2 @@
+apt update
+apt install make gcc clang