Can anyone explain this strange behavior with signed floats in C#?

Question

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?

To make things stranger `c.d.Equals(d.d)` evaluates to `true` as does `c.f.Equals(d.f)` — Justin Niessner, Mar 24 '10 at 15:39
I tried adding another field to D2 (a `bool`), and the second `Equals` now returns `true`. — Ben M, Mar 24 '10 at 15:45
Don't compare floats with exact comparison like .Equals. It's simply a bad idea. — Thorsten79, Mar 24 '10 at 15:46
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 '10 at 15:49
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 '10 at 15:50
This was just fixed in Core: https://github.com/dotnet/coreclr/pull/13164 — Dono, Aug 24 '17 at 00:00

score 394 · Accepted Answer · edited Jul 05 '17 at 16:15

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

edited Jul 05 '17 at 16:15

Dayan

7,634
11
49
76

answered Mar 24 '10 at 15:55

SLaks

868,454
176
1,908
1,964

172

Stepping into System.ValueType? That's pretty hardcore bro. – Pierreten Jun 04 '10 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 '13 at 06: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 '13 at 14:01
2

With .NET being open source software now, here is a link to the Core CLR implementation of [ValueTypeHelper::CanCompareBits](https://github.com/dotnet/coreclr/blob/7e4afb4fbf900b789f53ccb816c6ddba7807de68/src/vm/comutilnative.cpp#L2628). Didn't want to update your answer since the implementation is slightly changed from the reference source you posted. – IInspectable Apr 10 '17 at 12:30

score 59 · Answer 2 · answered Mar 24 '10 at 15:56

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.

answered Mar 24 '10 at 15:56

Ben M

22,262
3
67
71

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 '12 at 00: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 '12 at 00:06

score 51 · Answer 3 · answered Mar 24 '10 at 16:03

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.

answered Mar 24 '10 at 16:03

Eric Lippert

647,829
179
1,238
2,067

3

Then why it isn't a bug? Even though MS recommends to override Equals on value types always. – Alexander Efimov Mar 24 '10 at 16:06
For obvious reasons, it also repro's with an `IntPtr` field when running 64-bit. However, it doesn't repro with an 8-byte-wide struct field. Why aren't nested structs tightly-packed? – SLaks Mar 24 '10 at 16:08
14

Beats the heck out of me. I'm not an expert on the internals of the CLR. – Eric Lippert Mar 24 '10 at 16:53
4

... You aren't? Surely your knowledge of the C# internals would lead to considerable knowledge on how the CLR works. – CaptainCasey Mar 24 '10 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. – Eric Lippert Mar 24 '10 at 22:27
1

My mistake, I thought the CLR and the VB/C# compilers were more tightly coupled... so C#/VB -> CIL -> CLR – CaptainCasey Mar 25 '10 at 03:16

score 22 · Answer 4 · answered Mar 24 '10 at 15:53

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?)

score 18 · Answer 5 · answered Mar 24 '10 at 15:42

18

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

answered Mar 24 '10 at 15:42

Matthew Flaschen

278,309
50
514
539

5

Yes, I've also tried it in Mono, and it gives me true too. Looks like MS does some magic inside :) – Alexander Efimov Mar 24 '10 at 15:43
3

interesting, we all ship to Mono? – WeNeedAnswers Mar 24 '10 at 16:50

SLaks · Answer 6 · 2010-03-24T19:28:11.937

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.

edited Mar 24 '10 at 19:28

answered Mar 24 '10 at 15:48

SLaks

868,454
176
1,908
1,964

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 '10 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. – Driss Zouak Mar 24 '10 at 15:53
6

@Driss: The default value for `double` _is_ `0`. You're wrong. – SLaks Mar 24 '10 at 15:57

score 11 · Answer 7 · answered Mar 24 '10 at 15:52

11

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

answered Mar 24 '10 at 15:52

João Angelo

56,552
12
145
147

score 6 · Answer 8 · answered Nov 28 '11 at 06:13

6

…what do you think about this?

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

answered Nov 28 '11 at 06:13

Viacheslav Ivanov

1,535
13
22

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](https://msdn.microsoft.com/en-us/library/ms182276.aspx) exists. – Joe Amenta Apr 30 '15 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. – Viacheslav Ivanov May 13 '15 at 08: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 '15 at 10:38

Jim Ma · Answer 9 · 2017-08-24T07:17:57.460

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.

edited Aug 24 '17 at 07:17

answered Aug 24 '17 at 04:33

Jim Ma

709
5
15

score 2 · Answer 10 · edited Apr 22 '16 at 10:34

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.

edited Apr 22 '16 at 10:34

Sнаđошƒаӽ

16,753
12
73
90

answered Mar 24 '10 at 15:54

Morten Anderson

2,301
15
20

score 1 · Answer 11 · answered Mar 24 '10 at 15:50

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...

answered Mar 24 '10 at 15:50

user243357

181
3

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

Can anyone explain this strange behavior with signed floats in C#?

11 Answers11

Linked