Skip to content

Unpacking float32 does not preserve signaling NaN, unlike float64 #1087

Open
@fumoboy007

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.

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions