Runtime: double.GetHashCode and double.Equals do not agree (When NaN and -NaN are compared for equality)

Created on 12 Jul 2016  路  10Comments  路  Source: dotnet/runtime

It is an invariant that two objects for which o1.Equals(o2), their hash codes should agree, i.e. o1.GetHashCode() == o2.GetHashCode(). See the specification, which says

Two objects that are equal return hash codes that are equal.

This invariant is assumed by many APIs that use these methods, and any types that fail to satisfy them are "broken" and cannot be expected to work with the core APIs. However, the implementation for double does not satisfy this invariant. Specifically, the following snippet of code prints "True False".

        object o1 = double.NaN;
        object o2 = -double.NaN;
        Console.WriteLine(o1.Equals(o2));
        Console.WriteLine(o1.GetHashCode() == o2.GetHashCode());
area-System.Runtime bug up-for-grabs

Most helpful comment

@paulomorgado There are many legal representations of NaN, and double.Equals will return true if both the receiver and argument are NaN, while double.GetHashCode returns a hash of the bit pattern of the double, so two different NaNs won't hash to the same value - violating the invariant. Zero and negative zero are already special-cased in double.GetHashCode to avoid this problem.

All 10 comments

This code:

object o1 = double.NaN;
object o2 = -double.NaN;
Console.WriteLine(o1.Equals(o2));
Console.WriteLine(o1.GetHashCode() == o2.GetHashCode());
Console.WriteLine(typeof(double).Assembly.ImageRuntimeVersion);

prints:

| LInqPad4 - C# 5.0 | LINQPad5 - C# 6.0 |
| --- | --- |
| True | True |
| True | False |
| v4.0.30319 | v4.0.30319 |

There's a slight IL difference. C# 5.0 usees two local variables (o1 and o2) but C# 6.0 uses just one (o2).

@paulomorgado That might be a bug in C# 5. I recommend trying

        Console.WriteLine(BitConverter.DoubleToInt64Bits((double)(double.NaN)));
        Console.WriteLine(BitConverter.DoubleToInt64Bits((double)(-double.NaN)));

If those don't print different results, that is a C# compiler bug.

Looks like the C# 5.0 compiler is ignoring the - in -double.NaN.

@paulomorgado You can probably get it to _not_ ignore it by making it a non-constant expression.

Bottomline. doesn't seem to be a runtime (CLR) issue.

@paulomorgado There are many legal representations of NaN, and double.Equals will return true if both the receiver and argument are NaN, while double.GetHashCode returns a hash of the bit pattern of the double, so two different NaNs won't hash to the same value - violating the invariant. Zero and negative zero are already special-cased in double.GetHashCode to avoid this problem.

Here is a program illustrating the issue, which should repro on C# 5 as well.

using System;
using System.Collections.Generic;

class Program
{
    public static void Main()
    {
        var nan0 = MakeNan(0);
        var nan1 = MakeNan(1);
        Assert(nan0.Equals(nan1), "All NaNs are equal to each other");
        HashSet<double> s = new HashSet<double>();
        s.Add(nan0);
        s.Add(nan1);
        Assert(s.Count == 1, "oops - set with one element has Count=" + s.Count);
    }

    public static void Assert(bool condition, string message)
    {
        if (!condition) throw new Exception(message);
    }

    public static double MakeNan(int x)
    {
        var result = BitConverter.Int64BitsToDouble(BitConverter.DoubleToInt64Bits(double.NaN) ^ x);
        Assert(result != result, "NaN should be unequal to itself"); // prove it is a NaN
        return result;
    }
}

Given, that GetHashCode is implemented like the following it can't possibly correct:

public override unsafe int GetHashCode()
{
    double num = this;
    if (num == 0.0)
    {
        return 0;
    }
    long num2 = *((long*) &num);
    return (((int) num2) ^ ((int) (num2 >> 0x20)));
}

(Decompiled from Desktop CLR.)

Note, that Equals treats all NaN values equal to each other which differs from the usual Nan semantics. This is required to satisfy the reflexivity property.

The negative zero case was already fixed in a (fairly) recent release. It was broken for a long time. NaN must get the same treatment.

Single must be fixed as well.

Maybe tests should be added for all of this.

It should probably be

public override unsafe int GetHashCode()
{
    double num = this;
    if (num == 0.0)
    {
        return 0;
    }
    if (num != num)
    {
        return 1;
    }
    long num2 = *((long*) &num);
    return (((int) num2) ^ ((int) (num2 >> 0x20)));
}

I believe this bug should have already been fixed along with dotnet/coreclr#11452 . I'll double check it tomorrow morning.

This bug is still there. I'll work on this issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bencz picture bencz  路  3Comments

aggieben picture aggieben  路  3Comments

iCodeWebApps picture iCodeWebApps  路  3Comments

EgorBo picture EgorBo  路  3Comments

jzabroski picture jzabroski  路  3Comments