Merge pull request #335 from PJK/mips-ci

Fix NaN handling on mips/mipsel and Windows; disable value propagation for signaling NaNs; add mips CI

.circleci/config.yml

@@ -3,30 +3,38 @@ version: 2.1
 commands:
   linux-setup:
     steps:
-    - run: sudo apt-get update
-    # NEEDRESTART_MODE prevents automatic restarts which seem to hang.
-    - run: sudo NEEDRESTART_MODE=l apt-get install -y cmake ${TOOLCHAIN_PACKAGES}
-    - run: sudo NEEDRESTART_MODE=l apt-get install -y libcmocka-dev libcjson-dev
+      - run: sudo apt-get update
+      # NEEDRESTART_MODE prevents automatic restarts which seem to hang.
+      - run: sudo NEEDRESTART_MODE=l apt-get install -y cmake ${TOOLCHAIN_PACKAGES}
+      - run: sudo NEEDRESTART_MODE=l apt-get install -y libcmocka-dev libcjson-dev
+  install-cmocka-from-source:
+    steps:
+      - run: git clone https://git.cryptomilk.org/projects/cmocka.git ~/cmocka
+      - run: >
+          cd $(mktemp -d /tmp/build.XXXX) &&
+          cmake ~/cmocka &&
+          make &&
+          sudo make install
   build:
     steps:
-    - run: >
-        cmake -DWITH_TESTS=ON \
-          -DWITH_EXAMPLES=ON \
-          -DCMAKE_BUILD_TYPE=Debug \
-          -DSANITIZE=OFF \
-          -DCOVERAGE="${CMAKE_COVERAGE:='OFF'}" \
-          .
-    - run: make -j 16 VERBOSE=1
+      - run: >
+          cmake -DWITH_TESTS=ON \
+            -DWITH_EXAMPLES=ON \
+            -DCMAKE_BUILD_TYPE=Debug \
+            -DSANITIZE=OFF \
+            -DCOVERAGE="${CMAKE_COVERAGE:='OFF'}" \
+            .
+      - run: make -j 16 VERBOSE=1
   build-release:
     steps:
-    - run: >
-        cmake -DWITH_TESTS=ON \
-          -DCMAKE_BUILD_TYPE=Release \
-          .
-    - run: make -j 16 VERBOSE=1
+      - run: >
+          cmake -DWITH_TESTS=ON \
+            -DCMAKE_BUILD_TYPE=Release \
+            .
+      - run: make -j 16 VERBOSE=1
   test:
     steps:
-    - run: ctest -VV
+      - run: ctest -VV
 
 orbs:
   codecov: codecov/[email protected]
@@ -203,7 +211,23 @@ jobs:
       - run: /c/Program\ Files/Cmake/bin/cmake --build libcbor_build
       - run: >
           export PATH="$(pwd)/cmocka_build/src/Debug/:$PATH" &&
-          /c/Program\ Files/Cmake/bin/ctest.exe --test-dir libcbor_build --output-on-failure
+          /c/Program\ Files/Cmake/bin/ctest.exe --test-dir libcbor_build -C Debug --output-on-failure
+
+  build-and-test-mips: &dockcross-job
+    docker:
+      - image: dockcross/linux-mips-lts
+    steps: 
+      - checkout
+      - attach_workspace:
+          at: /home/circleci/project
+      - install-cmocka-from-source
+      - build
+      - test
+
+  build-and-test-mipsel:
+    <<: *dockcross-job
+    docker:
+      - image: dockcross/linux-mipsel-lts
 
 workflows:
   build-and-test:
@@ -215,6 +239,8 @@ workflows:
       - build-and-test-release-clang
       - build-and-test-arm
       - build-and-test-win
+      - build-and-test-mips
+      - build-and-test-mipsel
       - build-bazel
       - llvm-coverage
       # OSX builds are expensive, run only on master

CHANGELOG.md

@@ -4,6 +4,12 @@ Template:
 Next
 ---------------------
 - BUILD BREAKING: [Respect `INTERPROCEDURAL_OPTIMIZATION` and use the default value](https://github.com/PJK/libcbor/issues/315) 
+- BREAKING: Changes to NaN encoding
+  - [Fix NaN encoding on Windows](https://github.com/PJK/libcbor/issues/271)
+  - [Fix NaN encoding on mips/mipsel](https://github.com/PJK/libcbor/issues/329)
+  - [Signaling NaNs will from now on be encoded as canonical quiet NaNs](https://github.com/PJK/libcbor/pull/335). This was already the existing behavior for half-precision floats
+  - Decoding is unchanged
+  - Please note that this is an intermediate state and likely to be revisited (https://github.com/PJK/libcbor/issues/336)
 
 0.11.0 (2024-02-04)
 ---------------------

doc/source/api/type_7_floats_ctrls.rst

@@ -60,9 +60,16 @@ Manipulating existing items
 .. doxygenfunction:: cbor_set_float8
 
 
-.. _api_type_7_floats_ctrls_hard_floats:
+.. _api_type_7_floats_ctrls_half_floats:
 
 Half floats
 ~~~~~~~~~~~~
 CBOR supports two `bytes wide ("half-precision") <https://en.wikipedia.org/wiki/Half-precision_floating-point_format>`_
 floats which are not supported by the C language. *libcbor* represents them using `float <https://en.cppreference.com/w/c/language/type>` values throughout the API. Encoding will be performed by :func:`cbor_encode_half`, which will handle any values that cannot be represented as a half-float.
+
+Signaling NaNs
+~~~~~~~~~~~~~~~~
+
+`Signaling NaNs <https://en.wikipedia.org/wiki/NaN#Signaling_NaN)>`_ are always encoded as a standard, "quiet" NaN.
+
+The reason for this simplification is that standard C does not offer a way to handle the signaling payload without assumptions about the host architecture. See https://github.com/PJK/libcbor/issues/336 for more context.

doc/source/standard_conformance.rst

@@ -13,5 +13,5 @@ There is no explicit limitation of indefinite length byte strings. [#]_ *libcbor
 ---------------------------------
 As of C99 and even C11, there is no standard implementation for 2 bytes floats. *libcbor* packs them as a `float <https://en.cppreference.com/w/c/language/type>`. When encoding, *libcbor* selects the appropriate wire representation based on metadata and the actual value. This applies both to canonical and normal mode.
 
-For more information on half-float serialization, please refer to the section on :ref:`api_type_7_floats_ctrls_hard_floats`.
+For more information on half-float serialization, please refer to the section on :ref:`api_type_7_floats_ctrls_half_floats`.
 

src/cbor/encoding.c

@@ -6,6 +6,9 @@
  */
 
 #include "encoding.h"
+
+#include <math.h>
+
 #include "internal/encoders.h"
 
 size_t cbor_encode_uint8(uint8_t value, unsigned char *buffer,
@@ -126,6 +129,7 @@ size_t cbor_encode_undef(unsigned char *buffer, size_t buffer_size) {
 
 size_t cbor_encode_half(float value, unsigned char *buffer,
                         size_t buffer_size) {
+  // TODO: Broken on systems that do not use IEEE 754
   /* Assuming value is normalized */
   uint32_t val = ((union _cbor_float_helper){.as_float = value}).as_uint;
   uint16_t res;
@@ -134,11 +138,8 @@ size_t cbor_encode_half(float value, unsigned char *buffer,
   uint32_t mant =
       val & 0x7FFFFFu; /* 0b0000_0000_0111_1111_1111_1111_1111_1111 */
   if (exp == 0xFF) {   /* Infinity or NaNs */
-    if (value != value) {
-      // We discard information bits in half-float NaNs. This is
-      // not required for the core CBOR protocol (it is only a suggestion in
-      // Section 3.9).
-      // See https://github.com/PJK/libcbor/issues/215
+    if (isnan(value)) {
+      // Note: Values of signaling NaNs are discarded. See `cbor_encode_single`.
       res = (uint16_t)0x007e00;
     } else {
       // If the mantissa is non-zero, we have a NaN, but those are handled
@@ -178,13 +179,26 @@ size_t cbor_encode_half(float value, unsigned char *buffer,
 
 size_t cbor_encode_single(float value, unsigned char *buffer,
                           size_t buffer_size) {
+  // Note: Values of signaling NaNs are discarded. There is no standard
+  // way to extract it without assumptions about the internal float
+  // representation.
+  if (isnan(value)) {
+    return _cbor_encode_uint32(0x7FC0 << 16, buffer, buffer_size, 0xE0);
+  }
+  // TODO: Broken on systems that do not use IEEE 754
   return _cbor_encode_uint32(
       ((union _cbor_float_helper){.as_float = value}).as_uint, buffer,
       buffer_size, 0xE0);
 }
 
 size_t cbor_encode_double(double value, unsigned char *buffer,
                           size_t buffer_size) {
+  // Note: Values of signaling NaNs are discarded. See `cbor_encode_single`.
+  if (isnan(value)) {
+    return _cbor_encode_uint64((uint64_t)0x7FF8 << 48, buffer, buffer_size,
+                               0xE0);
+  }
+  // TODO: Broken on systems that do not use IEEE 754
   return _cbor_encode_uint64(
       ((union _cbor_double_helper){.as_double = value}).as_uint, buffer,
       buffer_size, 0xE0);

src/cbor/encoding.h

@@ -118,13 +118,22 @@ _CBOR_NODISCARD CBOR_EXPORT size_t cbor_encode_undef(unsigned char *, size_t);
  * lost.
  *   - In all other cases, the sign bit, the exponent, and 10 most significant
  * bits of the significand are kept
+ *
+ * Note: Signaling NaNs are encoded as a standard, "quiet" NaN.
  */
 _CBOR_NODISCARD CBOR_EXPORT size_t cbor_encode_half(float, unsigned char *,
                                                     size_t);
-
+/** Encodes a single precision float
+ *
+ * Note: Signaling NaNs are encoded as a standard, "quiet" NaN.
+ */
 _CBOR_NODISCARD CBOR_EXPORT size_t cbor_encode_single(float, unsigned char *,
                                                       size_t);
 
+/** Encodes a double precision float
+ *
+ * Note: Signaling NaNs are encoded as a standard, "quiet" NaN.
+ */
 _CBOR_NODISCARD CBOR_EXPORT size_t cbor_encode_double(double, unsigned char *,
                                                       size_t);
 

src/cbor/internal/encoders.c

@@ -6,6 +6,7 @@
  */
 
 #include "encoders.h"
+
 #include <string.h>
 
 size_t _cbor_encode_uint8(uint8_t value, unsigned char *buffer,
@@ -27,38 +28,38 @@ size_t _cbor_encode_uint8(uint8_t value, unsigned char *buffer,
 
 size_t _cbor_encode_uint16(uint16_t value, unsigned char *buffer,
                            size_t buffer_size, uint8_t offset) {
-  if (buffer_size >= 3) {
-    buffer[0] = 0x19 + offset;
+  if (buffer_size < 3) {
+    return 0;
+  }
+  buffer[0] = 0x19 + offset;
 
 #ifdef IS_BIG_ENDIAN
-    memcpy(buffer + 1, &value, 2);
+  memcpy(buffer + 1, &value, 2);
 #else
-    buffer[1] = (unsigned char)(value >> 8);
-    buffer[2] = (unsigned char)value;
+  buffer[1] = (unsigned char)(value >> 8);
+  buffer[2] = (unsigned char)value;
 #endif
 
-    return 3;
-  } else
-    return 0;
+  return 3;
 }
 
 size_t _cbor_encode_uint32(uint32_t value, unsigned char *buffer,
                            size_t buffer_size, uint8_t offset) {
-  if (buffer_size >= 5) {
-    buffer[0] = 0x1A + offset;
+  if (buffer_size < 5) {
+    return 0;
+  }
+  buffer[0] = 0x1A + offset;
 
 #ifdef IS_BIG_ENDIAN
-    memcpy(buffer + 1, &value, 4);
+  memcpy(buffer + 1, &value, 4);
 #else
-    buffer[1] = (unsigned char)(value >> 24);
-    buffer[2] = (unsigned char)(value >> 16);
-    buffer[3] = (unsigned char)(value >> 8);
-    buffer[4] = (unsigned char)value;
+  buffer[1] = (unsigned char)(value >> 24);
+  buffer[2] = (unsigned char)(value >> 16);
+  buffer[3] = (unsigned char)(value >> 8);
+  buffer[4] = (unsigned char)value;
 #endif
 
-    return 5;
-  } else
-    return 0;
+  return 5;
 }
 
 size_t _cbor_encode_uint64(uint64_t value, unsigned char *buffer,

src/cbor/internal/loaders.c

@@ -51,6 +51,8 @@ uint64_t _cbor_load_uint64(const unsigned char *source) {
 
 /* As per https://www.rfc-editor.org/rfc/rfc8949.html#name-half-precision */
 float _cbor_decode_half(unsigned char *halfp) {
+  // TODO: Broken if we are not on IEEE 754
+  // (https://github.com/PJK/libcbor/issues/336)
   int half = (halfp[0] << 8) + halfp[1];
   int exp = (half >> 10) & 0x1f;
   int mant = half & 0x3ff;
@@ -70,11 +72,15 @@ float _cbor_load_half(cbor_data source) {
 }
 
 float _cbor_load_float(cbor_data source) {
+  // TODO: Broken if we are not on IEEE 754
+  // (https://github.com/PJK/libcbor/issues/336)
   union _cbor_float_helper helper = {.as_uint = _cbor_load_uint32(source)};
   return helper.as_float;
 }
 
 double _cbor_load_double(cbor_data source) {
+  // TODO: Broken if we are not on IEEE 754
+  // (https://github.com/PJK/libcbor/issues/336)
   union _cbor_double_helper helper = {.as_uint = _cbor_load_uint64(source)};
   return helper.as_double;
 }

test/CMakeLists.txt

@@ -19,12 +19,7 @@ foreach(test_file ${TESTS})
     target_link_libraries(${NAME} ${MATH_LIBRARY})
   endif()
   target_include_directories(${NAME} PUBLIC ${CMOCKA_INCLUDE_DIR})
-  # See https://stackoverflow.com/a/10824578/499521
-  add_test(ctest_build_test_${NAME} "${CMAKE_COMMAND}" --build
-           ${CMAKE_BINARY_DIR} --target ${NAME})
-  add_test(ctest_run_${NAME} ${NAME})
-  set_tests_properties(ctest_run_${NAME} PROPERTIES DEPENDS
-                                                    ctest_build_test_${NAME})
+  add_test(NAME ${NAME} COMMAND ${NAME})
   add_dependencies(coverage ${NAME})
 endforeach()
 

test/float_ctrl_encoders_test.c

@@ -122,10 +122,6 @@ static void test_half_special(void **_CBOR_UNUSED(_state)) {
   assert_memory_equal(buffer, ((unsigned char[]){0xF9, 0x7E, 0x00}), 3);
   assert_half_float_codec_identity();
 
-  // We discard all information bits in half-float NaNs. This is
-  // not required for the core CBOR protocol (it is only a suggestion in
-  // Section 3.9).
-  // See https://github.com/PJK/libcbor/issues/215
   assert_size_equal(3, cbor_encode_half(nanf("2"), buffer, 512));
   assert_memory_equal(buffer, ((unsigned char[]){0xF9, 0x7E, 0x00}), 3);
   assert_half_float_codec_identity();
@@ -150,12 +146,9 @@ static void test_float(void **_CBOR_UNUSED(_state)) {
   assert_memory_equal(buffer, ((unsigned char[]){0xFA, 0x7F, 0xC0, 0x00, 0x00}),
                       5);
 
-#ifndef _WIN32
-  // TODO: https://github.com/PJK/libcbor/issues/271
   assert_size_equal(5, cbor_encode_single(nanf("3"), buffer, 512));
-  assert_memory_equal(buffer, ((unsigned char[]){0xFA, 0x7F, 0xC0, 0x00, 0x03}),
+  assert_memory_equal(buffer, ((unsigned char[]){0xFA, 0x7F, 0xC0, 0x00, 0x00}),
                       5);
-#endif
 
   assert_size_equal(5, cbor_encode_single(strtof("Inf", NULL), buffer, 512));
   assert_memory_equal(buffer, ((unsigned char[]){0xFA, 0x7F, 0x80, 0x00, 0x00}),
@@ -179,14 +172,11 @@ static void test_double(void **_CBOR_UNUSED(_state)) {
       ((unsigned char[]){0xFB, 0x7F, 0xF8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
       9);
 
-#ifndef _WIN32
-  // TODO: https://github.com/PJK/libcbor/issues/271
   assert_size_equal(9, cbor_encode_double(nan("3"), buffer, 512));
   assert_memory_equal(
       buffer,
-      ((unsigned char[]){0xFB, 0x7F, 0xF8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03}),
+      ((unsigned char[]){0xFB, 0x7F, 0xF8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
       9);
-#endif
 
   assert_size_equal(9, cbor_encode_double(strtod("Inf", NULL), buffer, 512));
   assert_memory_equal(