Bugs in ieee754.c

[cyberphone]

During testing of the D-CBOR concept I ported a Java encoder to C. Just for fun I adjusted it a bit to test with the QCBOR test suite. However, "DoubleAsSmallestTest" failed. After some debugging it turned out that the master table is incorrect. I have verified my findings with https://cbor.me as well.

static const uint8_t spExpectedSmallest[] = {
 // AR Updated:
      0xF9, 0x00, 0x01,
 // AR Removed:
 //     0xFA, 0x33, 0x80, 0x00, 0x00,

 // AR Updated:
      0xF9, 0x02, 0x00,
 // AR Removed:
 //     0xFA, 0x38, 0x00, 0x00, 0x00,

 // AR updated:
      0xFA, 0x00, 0x40, 0x00, 0x00,
 // AR removed:
 //    0xFB, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,

};

The enclosed file contains the method I removed from ieee754.c in order to test with my code added to the file set. It appears to generate less code than the original IEEE754_DoubleToSmallestInternal. 324 bytes versus 476 using gcc on Linux64.

d-cbor-ieee754.txt

[laurencelundblade]

Yes, this is true. Here's the comment from the ieee754.c // This will not convert to half-precion or single-precision // subnormals. Values that could be converted will be output as // the double they are or occasionally to a normal single. This // could be implemented, but it is more code and would rarely be // used and rarely reduce the output size.

QCBOR is still producing a correct value, just not the smallest possible for a few cases.

One could say that QCBOR is not producing correct deterministic encoding of floats because of this. (Another area of concern for deterministic floats encoding is NaN payloads; not even sure there is enough of a standard for them).

Did your d-cbor code use PLATFORM_SUPPORTS_FLOAT_CAST? If so it relies on the floating point HW to do the work and will of course be less code.

Thanks for reporting. I'm not sure if I'll fix this or when as it will take some time to fix, but I will leave the issue open.

[cyberphone]

Hi @laurencelundblade I have read the docs :)

AFAICT, my take on IEEE754_DoubleToSmallestInternal generates short(er) code also when not relying on floating point HW. BTW, doesn't your code actually depend on that as well? https://github.com/laurencelundblade/QCBOR/blob/master/src/ieee754.c#L491

Regarding NaN, RFC8949 is a bit unclear on that. In the Java platform it is not technically possible setting any kind of payload to NaN so using this feature is asking for problems. In my take on the matter NaN is f97e00, everything else is considered an error.

If you want, I could imagine integrating my code in ieee754.c.

[laurencelundblade]

Hi Anders,

One problem with your code is that it is the Apache license which is not really compatible with QCBOR's MIT license.

Maybe we should see clarification on NaN deterministic encoding in the WG? There's also the quiet NaN and non-quiet NaN thing.

[cyberphone]

Hi Laurence, If I would port the code it would have to follow QCBOR licensing. There is also a minor formatting difference. I have used K&R brace style.

NaN clarifications is probably not going to happen because that is too late. The D-CBOR specification is supposed to address issues that are unclear and some that doesn't fit at all like using floating point numbers as keys. Well, actually the latter works perfect but RFC8949 destroyed it by requiring numeric comparisons which is an unnecessary complication since CBOR objects are just strings of bytes and map key comparisons only have to treat them as such.

[cyberphone]

FWIW, the floating point reduction algorithm has just been updated and doesn't rely on loops anymore: https://github.com/cyberphone/openkeystore/blob/90736d4420aafff9aea5b8cfab19bc863ab8a1d0/library/src/org/webpki/cbor/CBORFloat.java#L117

BTW, given Carsten't recent upgrade of https://cbor.me, deterministic CBOR seems to be a deal done. QCBOR would fit very nicely in this scheme.

[laurencelundblade]

Hi Anders,

Been busy with COSE these days. Hope to get back to QCBOR after IETF 117 in July.

Hopefully some of the deterministic stuff and other can get done then.

[laurencelundblade]

Hi Anders,

Finally getting around to looking at this carefully. I like your approach to numeric reduction for double to float to half. It seems like less object code than what I have. I'd like to try integrating it into QCBOR.

It's all code you wrote, right? You have the full rights to change the license to BSD, right? If so, maybe you can put it in GitHub with the BSD 3 clause license?

For now this is primarily about implementing RFC 8949, not CDE or dCBOR, though I expect to move on to CDE and dCBOR.

Thx

LL

[cyberphone]

Hi Laurence, I wrote the code from scratch (but looked a little bit into other implementations to get "inspiration"), so I can provide it to you using whatever license you want. I have recently upgraded the algorithm so that there are no loops anymore. Shifts does the same job. I will look into this next week,

Java version of the updated algorithm: https://github.com/cyberphone/openkeystore/blob/dcc068256034239536fee5b6c2b2ab5af98b7f07/library/src/org/webpki/cbor/CBORFloat.java#L52

Regards, Anders

[cyberphone]

Updated algorithm in C: https://github.com/cyberphone/D-CBOR/blob/main/lib/d-cbor-ieee754.c https://github.com/cyberphone/D-CBOR/blob/main/ieee754-test/ieee754-test.c

[cyberphone]

This might be the right time for "gearing up" for CDE/dCBOR. Since I have already implemented the CDE versus "Legacy" decoder switch in both Java and JavaScript, I could easily do that for QCBOR. However, I think we need to talk a bit before I take any further steps. Would WhatsApp be OK?

For decoding I have "cheated" (well...) by first converting F16, F32, and F64 to double and then run the double-to-shortest encoder and see if they match on a binary level. I'm in favor of shortest possible (source) code :) https://github.com/cyberphone/CBOR.js/blob/2b9dcde6da61d19f40f3ac892730ca7f5a62147c/src/cbor.js#L880

[laurencelundblade]

Hi Anders, what I'd like is just a simple posting of your C code with the BSD license. https://github.com/cyberphone/D-CBOR/blob/main/lib/d-cbor-ieee754.c

I like the way it works and handles subnormals and want to work it into QCBOR.

Thx

[laurencelundblade]

Fixed by #192. I ended up writing all the code on my own from scratch.