PJK / libcbor

CBOR protocol implementation for C
MIT License
334 stars 95 forks source link

half-precision float NaN loss of information (nitpicky) #203

Open ranvis opened 2 years ago

ranvis commented 2 years ago

The behavior of NaN for half-precision float is not consistent with single/double-float.

https://github.com/PJK/libcbor/blob/7b806bf9662b2e85ee199df88364b5e8c1073a39/src/cbor/internal/loaders.c#L63-L64

// nan.c
#include <stdio.h>
#include <math.h>
#include <inttypes.h>
void main() {
  float fco = NAN;
  float ffn = nanf("");
  printf("%" PRIx32 "  %" PRIx32 "\n", *(uint32_t *)&fco, *(uint32_t *)&ffn);
  printf("%f, %f\n", fco, ffn);
}
gcc.exe nan.c && a.exe  # MinGW'ish
7fc00000  7fc00000
1.#QNAN0, 1.#QNAN0

cl.exe nan.c && nan.exe  # Visual Studio
ffc00000  7fc00000       # NAN constant has sign bit on
-nan(ind), nan

Also _cbor_decode_half() ignores fraction bits (payloads.) In contrast, single-precision float CBOR is loaded as is, and those bits are kept when you cast it to double.

void main()
{
  float f32;
  double f64 = NAN;
  printf("%" PRIx64 "\n", *(uint64_t *)&f64);
  *(uint32_t *)&f32 = 0x7fc110ff;
  f64 = f32;
  printf("%" PRIx64 "\n", *(uint64_t *)&f64);
}
7ff8000000000000
7ff8221fe0000000

Half-float NaN is always encoded into 7e00. https://github.com/PJK/libcbor/blob/7b806bf9662b2e85ee199df88364b5e8c1073a39/src/cbor/encoding.c#L136-L139 The code in cbor_encode_half() refers to RFC 7049 section 3.9 "Canonical CBOR".

If NaN is an allowed value, it must always be represented as 0xf97e00.

But that section looks like mentioning a virtual protocol using CBOR as a data format. It would not directly apply to a generic encoder. This section corresponds to RFC 8949 section 4.2.2 "Additional Deterministic Encoding Considerations".

If NaN is an allowed value, and there is no intent to support NaN payloads or signaling NaNs, the protocol needs to pick a single representation, typically 0xf97e00.

Don't come up with a real problem it can make, but if some more bits are retained for 2-byte NaN, it sounds neat.

ranvis commented 2 years ago

a sign bit (signaling) of half-float NaN

This was wrong. The bit is not for signaling.