249

Here is the example with comments:

class Program
{
    // first version of structure
    public struct D1
    {
        public double d;
        public int f;
    }

    // during some changes in code then we got D2 from D1
    // Field f type became double while it was int before
    public struct D2 
    {
        public double d;
        public double f;
    }

    static void Main(string[] args)
    {
        // Scenario with the first version
        D1 a = new D1();
        D1 b = new D1();
        a.f = b.f = 1;
        a.d = 0.0;
        b.d = -0.0;
        bool r1 = a.Equals(b); // gives true, all is ok

        // The same scenario with the new one
        D2 c = new D2();
        D2 d = new D2();
        c.f = d.f = 1;
        c.d = 0.0;
        d.d = -0.0;
        bool r2 = c.Equals(d); // false! this is not the expected result        
    }
}

So, what do you think about this?

9
  • 2
    To make things stranger c.d.Equals(d.d) evaluates to true as does c.f.Equals(d.f) Mar 24, 2010 at 15:39
  • 2
    Don't compare floats with exact comparison like .Equals. It's simply a bad idea.
    – Thorsten79
    Mar 24, 2010 at 15:46
  • 8
    @Thorsten79: How is that relevant here?
    – Ben M
    Mar 24, 2010 at 15:47
  • 2
    This is most strange. Using a long instead a double for f introduces the same behavior. And adding another short field corrects it again...
    – Jens
    Mar 24, 2010 at 15:49
  • 1
    Weird -- it only seems to happen when both are the same type (float or double). Change one to float (or decimal) and D2 works the same as D1.
    – tvanfosson
    Mar 24, 2010 at 15:50

11 Answers 11

394

The bug is in the following two lines of System.ValueType: (I stepped into the reference source)

if (CanCompareBits(this)) 
    return FastEqualsCheck(thisObj, obj);

(Both methods are [MethodImpl(MethodImplOptions.InternalCall)])

When all of the fields are 8 bytes wide, CanCompareBits mistakenly returns true, resulting in a bitwise comparison of two different, but semantically identical, values.

When at least one field is not 8 bytes wide, CanCompareBits returns false, and the code proceeds to use reflection to loop over the fields and call Equals for each value, which correctly treats -0.0 as equal to 0.0.

Here is the source for CanCompareBits from SSCLI:

FCIMPL1(FC_BOOL_RET, ValueTypeHelper::CanCompareBits, Object* obj)
{
    WRAPPER_CONTRACT;
    STATIC_CONTRACT_SO_TOLERANT;

    _ASSERTE(obj != NULL);
    MethodTable* mt = obj->GetMethodTable();
    FC_RETURN_BOOL(!mt->ContainsPointers() && !mt->IsNotTightlyPacked());
}
FCIMPLEND
4
  • 172
    Stepping into System.ValueType? That's pretty hardcore bro.
    – Pierreten
    Jun 4, 2010 at 20:25
  • 2
    You don't explain what the significance of "8 bytes wide" is. Would a struct with all 4-byte fields not have the same result? I'm guessing that having a single 4-byte field and an 8-byte fields just triggers IsNotTightlyPacked.
    – Gabe
    Jan 20, 2013 at 6:50
  • 1
    @Gabe I wrote earlier that The bug also happens with floats, but only happens if the fields in the struct add up to a multiple of 8 bytes.
    – SLaks
    Jan 20, 2013 at 14:01
  • 2
    With .NET being open source software now, here is a link to the Core CLR implementation of ValueTypeHelper::CanCompareBits. Didn't want to update your answer since the implementation is slightly changed from the reference source you posted. Apr 10, 2017 at 12:30
59

I found the answer at http://blogs.msdn.com/xiangfan/archive/2008/09/01/magic-behind-valuetype-equals.aspx.

The core piece is the source comment on CanCompareBits, which ValueType.Equals uses to determine whether to use memcmp-style comparison:

The comment of CanCompareBits says "Return true if the valuetype does not contain pointer and is tightly packed". And FastEqualsCheck use "memcmp" to speed up the comparison.

The author goes on to state exactly the problem described by the OP:

Imagine you have a structure which only contains a float. What will occur if one contains +0.0, and the other contains -0.0? They should be the same, but the underlying binary representation are different. If you nest other structure which override the Equals method, that optimization will also fail.

2
  • I wonder if the behavior of Equals(Object) for double, float, and Decimal changed during the early drafts of .net; I would think that it's more important to have the virtual X.Equals((Object)Y) only return true when X and Y are indistinguishable, than to have that method match the behavior of other overloads (especially given that, because of implicit type coercion, overloaded Equals methods don't even define an equivalence relation!, e.g. 1.0f.Equals(1.0) yields false, but 1.0.Equals(1.0f) yields true!) The real problem IMHO is not with the way structures are compared...
    – supercat
    Dec 13, 2012 at 0:01
  • 1
    ...but with the way that those value types override Equals to mean something other than equivalence. Suppose, for example, one wants to write a method which takes an immutable object and, if it hasn't been cached yet, performs ToString on it and caches the result; if it has been cached, simply return the cached string. Not an unreasonable thing to do, but it would fail badly with Decimal since two values might compare equal but yield different strings.
    – supercat
    Dec 13, 2012 at 0:06
51

Vilx's conjecture is correct. What "CanCompareBits" does is checks to see whether the value type in question is "tightly packed" in memory. A tightly packed struct is compared by simply comparing the binary bits that make up the structure; a loosely packed structure is compared by calling Equals on all the members.

This explains SLaks' observation that it repros with structs that are all doubles; such structs are always tightly packed.

Unfortunately as we've seen here, that introduces a semantic difference because bitwise comparison of doubles and Equals comparison of doubles gives different results.

6
  • 3
    Then why it isn't a bug? Even though MS recommends to override Equals on value types always. Mar 24, 2010 at 16:06
  • 14
    Beats the heck out of me. I'm not an expert on the internals of the CLR. Mar 24, 2010 at 16:53
  • 4
    ... You aren't? Surely your knowledge of the C# internals would lead to considerable knowledge on how the CLR works. Mar 24, 2010 at 22:00
  • 38
    @CaptainCasey: I've spent five years studying the internals of the C# compiler and probably in total a couple of hours studying the internals of the CLR. Remember, I am a consumer of the CLR; I understand its public surface area reasonably well, but its internals are a black box to me. Mar 24, 2010 at 22:27
  • 1
    My mistake, I thought the CLR and the VB/C# compilers were more tightly coupled... so C#/VB -> CIL -> CLR Mar 25, 2010 at 3:16
22

Half an answer:

Reflector tells us that ValueType.Equals() does something like this:

if (CanCompareBits(this))
    return FastEqualsCheck(this, obj);
else
    // Use reflection to step through each member and call .Equals() on each one.

Unfortunately both CanCompareBits() and FastEquals() (both static methods) are extern ([MethodImpl(MethodImplOptions.InternalCall)]) and have no source available.

Back to guessing why one case can be compared by bits, and the other cannot (alignment issues maybe?)

18

It does give true for me, with Mono's gmcs 2.4.2.3.

2
  • 5
    Yes, I've also tried it in Mono, and it gives me true too. Looks like MS does some magic inside :) Mar 24, 2010 at 15:43
  • 3
    interesting, we all ship to Mono? Mar 24, 2010 at 16:50
14

Simpler test case:

Console.WriteLine("Good: " + new Good().Equals(new Good { d = -.0 }));
Console.WriteLine("Bad: " + new Bad().Equals(new Bad { d = -.0 }));

public struct Good {
    public double d;
    public int f;
}

public struct Bad {
    public double d;
}

EDIT: The bug also happens with floats, but only happens if the fields in the struct add up to a multiple of 8 bytes.

3
  • Looks like an optimizer rule that goes: if its all doubles than do a bit-compare, else do separate double.Equal calls
    – H H
    Mar 24, 2010 at 15:52
  • I don't think this is the same test case as what the issue presented here seems to be is that the default value for Bad.f is not 0, whereas the other case seems to be an Int vs. Double issue. Mar 24, 2010 at 15:53
  • 6
    @Driss: The default value for double is 0. You're wrong.
    – SLaks
    Mar 24, 2010 at 15:57
11

It must be related to a bit by bit comparison, since 0.0 should differ from -0.0 only by the signal bit.

6

…what do you think about this?

Always override Equals and GetHashCode on value types. It will be fast and correct.

3
  • Other than a caveat that this is only necessary when equality is relevant, this is exactly what I was thinking. As fun as it is to look at quirks of the default value type equality behavior like the highest voted answers do, there's a reason why CA1815 exists.
    – Joe Amenta
    Apr 30, 2015 at 16:13
  • @JoeAmenta Sorry for a late answer. In my view (just in my view, of course), the equality is always () relevant for value types. Default equality implementation is not acceptable in common cases. () Except very special cases. Very. Very special. When you exactly known what are you doing and why. May 13, 2015 at 8:28
  • I think we agree that overriding the equality checks for value types is virtually always possible and meaningful with very few exceptions, and will usually make it strictly more correct. The point I was trying to convey with the word "relevant" was that there are some value types whose instances will never be compared with other instances for equality, so overriding would result in dead code that needs to be maintained. Those (and the weird special cases you allude to) would be the only places I would skip it.
    – Joe Amenta
    May 13, 2015 at 10:38
4

Just an update for this 10 years old bug: it has been fixed (Disclaimer: I'm the author of this PR) in .NET Core which would be probably released in .NET Core 2.1.0.

The blog post explained the bug and how I fixed it.

2

If you make D2 like this

public struct D2
{
    public double d;
    public double f;
    public string s;
}

it's true.

if you make it like this

public struct D2
{
    public double d;
    public double f;
    public double u;
}

It's still false.

it seems like it's false if the struct only holds doubles.

0
1

It must be zero related, since changing the line

d.d = -0.0

to:

d.d = 0.0

results in the comparison being true...

1
  • Conversely NaN's could compare equal to each other for a change, when they actually use the same bit pattern.
    – harold
    Nov 28, 2011 at 1:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.