Describe the bug
float32/float64 NaNs have a quiet vs. signaling state. This state is encoded using a couple bits in the significand.
Unpacking a float64 signaling NaN using msgpack-c 6.0.0 preserves the quiet vs. signaling state. However, unpacking a float32 signaling NaN always results in a quiet NaN even if the packed value is a signaling NaN. The behavior is inconsistent between float32 and float64.
To Reproduce
static bool is_quiet_nan(double nan_val) {
uint64_t bit_pattern = reinterpret_cast<uint64_t&>(nan_val);
int is_quiet_bit_index = DBL_MANT_DIG - 2;
return (bit_pattern >> is_quiet_bit_index) & 1;
}
TEST(MSGPACKC, simple_buffer_float_signaling_nan)
{
if (!numeric_limits<float>::has_signaling_NaN) {
return;
}
float val = numeric_limits<float>::signaling_NaN();
msgpack_sbuffer sbuf;
msgpack_sbuffer_init(&sbuf);
msgpack_packer pk;
msgpack_packer_init(&pk, &sbuf, msgpack_sbuffer_write);
msgpack_pack_float(&pk, val);
msgpack_zone z;
msgpack_zone_init(&z, 2048);
msgpack_object obj;
msgpack_unpack_return ret =
msgpack_unpack(sbuf.data, sbuf.size, NULL, &z, &obj);
ASSERT_EQ(MSGPACK_UNPACK_SUCCESS, ret);
ASSERT_EQ(MSGPACK_OBJECT_FLOAT32, obj.type);
EXPECT_TRUE(isnan(obj.via.f64));
EXPECT_FALSE(is_quiet_nan(obj.via.f64));
#if defined(MSGPACK_USE_LEGACY_NAME_AS_FLOAT)
EXPECT_TRUE(isnan(obj.via.dec));
EXPECT_FALSE(is_quiet_nan(obj.via.dec));
#endif // MSGPACK_USE_LEGACY_NAME_AS_FLOAT
msgpack_zone_destroy(&z);
msgpack_sbuffer_destroy(&sbuf);
}
TEST(MSGPACKC, simple_buffer_double_signaling_nan)
{
if (!numeric_limits<double>::has_signaling_NaN) {
return;
}
double val = numeric_limits<double>::signaling_NaN();
msgpack_sbuffer sbuf;
msgpack_sbuffer_init(&sbuf);
msgpack_packer pk;
msgpack_packer_init(&pk, &sbuf, msgpack_sbuffer_write);
msgpack_pack_double(&pk, val);
msgpack_zone z;
msgpack_zone_init(&z, 2048);
msgpack_object obj;
msgpack_unpack_return ret =
msgpack_unpack(sbuf.data, sbuf.size, NULL, &z, &obj);
ASSERT_EQ(MSGPACK_UNPACK_SUCCESS, ret);
ASSERT_EQ(MSGPACK_OBJECT_FLOAT64, obj.type);
ASSERT_EQ(MSGPACK_OBJECT_FLOAT, obj.type);
#if defined(MSGPACK_USE_LEGACY_NAME_AS_FLOAT)
ASSERT_EQ(MSGPACK_OBJECT_DOUBLE, obj.type);
#endif // MSGPACK_USE_LEGACY_NAME_AS_FLOAT
EXPECT_TRUE(isnan(obj.via.f64));
EXPECT_FALSE(is_quiet_nan(obj.via.f64));
#if defined(MSGPACK_USE_LEGACY_NAME_AS_FLOAT)
EXPECT_TRUE(isnan(obj.via.dec));
EXPECT_FALSE(is_quiet_nan(obj.via.dec));
#endif // MSGPACK_USE_LEGACY_NAME_AS_FLOAT
msgpack_zone_destroy(&z);
msgpack_sbuffer_destroy(&sbuf);
}
Expected behavior
Either both tests should pass or both tests should fail. Currently, simple_buffer_double_signaling_nan passes but simple_buffer_float_signaling_nan fails.
Root cause
The unpacker returns float32 values as double. This conversion implicitly causes the float32 signaling NaN to become a float64 quiet NaN.
Possible solution
The unpacker could return float32 values as float instead of converting them to double. This might be a breaking change because callers may need to update their usage to check a different msgpack_object_union field. However, in addition to resolving this issue, it could also be more convenient for callers because they wouldn’t need to cast the double field back to float.
Describe the bug
float32/float64 NaNs have a quiet vs. signaling state. This state is encoded using a couple bits in the significand.
Unpacking a float64 signaling NaN using msgpack-c 6.0.0 preserves the quiet vs. signaling state. However, unpacking a float32 signaling NaN always results in a quiet NaN even if the packed value is a signaling NaN. The behavior is inconsistent between float32 and float64.
To Reproduce
Expected behavior
Either both tests should pass or both tests should fail. Currently,
simple_buffer_double_signaling_nanpasses butsimple_buffer_float_signaling_nanfails.Root cause
The unpacker returns float32 values as
double. This conversion implicitly causes the float32 signaling NaN to become a float64 quiet NaN.Possible solution
The unpacker could return float32 values as
floatinstead of converting them todouble. This might be a breaking change because callers may need to update their usage to check a differentmsgpack_object_unionfield. However, in addition to resolving this issue, it could also be more convenient for callers because they wouldn’t need to cast thedoublefield back tofloat.