Unpacking float32 does not preserve signaling NaN, unlike float64 #1087
Description
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
.