.NET中值类型比较的问题以及修复过程

.NET中值类型(ValueType)比较时隐藏着一个底层的bug, 这个bug已经有接近十年未被修复。从 .NET Framework 到 .NET Core一直存在。StackOverflow上早在2010年就有人提出了这个问题: 10 years old bug.

随着我的PR被合并到 .NET Core主干,这个bug终于被修复了。预计在 .NET Core 2.1.0发布时会包含这个bug fix.

存在了10年的神奇BUG

从表象上看,这个bug可以分为2个问题(虽然本质上是同一个问题):

  • struct中有浮点数类型(double/float)且字节对齐时,.NET Framework在某些特殊情况下无法正确比较两个该类型的实例。

  • 当用户重写(override)了ValueTypeEquals方法时,.NET Framework无法正确比较两个该类型的实例。

关于第一个问题,可以看
Stackoverflow的例子

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

很明显我们期望0.0-0.0在逻辑上是相等的(虽然它们的二进制内容是不同的,可以看IEEE double precision floating point format). 但实际上在上述例子的D2实例比较时, .NET Framework 认为0.0-0.0不等,所以得出了错误的结果。

为什么?

当比较两个值类型实例时,.NET Framework会尽量避免使用反射机制(为了提升性能)。当满足以下条件时,它会直接比较两个实例在内存中的二进制内容:

  • 值类型不包含任何指针。也就是说struct中不能有class类型的字段。

  • 值类型tightly packed. 可以理解为struct字节对齐。比如在上面的例子中,D1结构体不是tightly packed, 因为它的字段含有2种类型,doubleint两种数据类型的大小是不同的; 但D2的2个字段都是double, 所以D2tightly packed.

以上逻辑相关的代码如下:

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

    _ASSERTE(obj != NULL);
    MethodTable* mt = obj->GetMethodTable();
    FC_RETURN_BOOL(!mt->ContainsPointers() && !mt->IsNotTightlyPacked());
}
FCIMPLEND

可以看到,D1不满足使用二进制比较(bit comparison)的条件。所以 .NET Framework会使用反射一个一个地比较它的字段,得到正确的结果。D2满足了二进制比较的条件,速度是提升了不少,但却得到了错误的结果。

看来问题出在使用二进制比较上。为什么它会出错?其实从原理上看不难发现,0.0-0.0虽然逻辑上应该相等,但实际上用于表示它们的二进制内容是不同的。当 .NET Framework 使用bit comparison快速比较2个struct实例时,会认为0.0-0.0不同,所以得出了错误的结果。

而使用反射进行比较的话,实际上是调用了Double==Equals方法,显然 .NET Framework 会保证==Equals能正确处理0.0-0.0 (你可以试着直接比较2个double值,一个为0.0, 一个为-0.0),所以能得出正确的结果。

总体上来说,我们不应该在有浮点数存在的时候,使用bit comparison. 但是现在.NET的实现并没有这个判断,所以导致了我们选择了错误的比较方式。

重写Equals引起的bug

类似地,如果我们重写了ValueTypeEquals方法, 同时 .NET Framework 使用bit comparison进行判断,则可能得出错误答案。因为Equals并不会被调用。可以看以下例子:

using System;

struct NeverEquals
{
    byte _a;

    public override bool Equals(object obj)
    {
        return false;
    }

    public override int GetHashCode()
    {
        return 0;
    }
}

struct MyStruct
{
    NeverEquals _x;
    NeverEquals _y;
}

class Program
{
    static void Main(string[] args)
    {
        MyStruct i = new MyStruct();
        MyStruct j = new MyStruct();

        Console.WriteLine(i.Equals(j));
    }
}

我们期待结果为false, 因为我们重写了NeverEqualsEquals方法,使得它永远返回fasle. 但实际上运行结果却为true. 因为 .NET Framework 使用了bit comparison, 并不会调用Equals.

Fix it

可以看出,整个问题都在于我们没有在bit comparison和普通的比较方式之间做出正确的选择。很明显,只依赖于现在的判断条件(ContainsPointersIsNotTightlyPacked)是不够的。如果要使用bit comparison, 我们需要增加两个额外的条件:

  • 在整个类型树中没有浮点数类型的字段。就是说如果类型继承了很多层,任意一层有浮点数类型的字段我们都不能使用bit comparison.

  • 在整个类型树中没有重写(override)的Equals.就是说如果类型继承了很多层,任意一层重写了Equals我们都不能使用bit comparison.

显然这是个深度优先搜索的算法。但是为了提高效率,需要做适当的缓存,使得只有第一次使用该类型进行比较时才需要做搜索。另外,还可以利用一些边界条件进行剪枝,核心算法实现如下:

static BOOL CanCompareBitsOrUseFastGetHashCode(MethodTable* mt)
{
    CONTRACTL
    {
        THROWS;
        GC_TRIGGERS;
        MODE_COOPERATIVE;
    } CONTRACTL_END;

    _ASSERTE(mt != NULL);

    if (mt->HasCheckedCanCompareBitsOrUseFastGetHashCode())
    {
        return mt->CanCompareBitsOrUseFastGetHashCode();
    }

    if (mt->ContainsPointers()
        || mt->IsNotTightlyPacked())
    {
        mt->SetHasCheckedCanCompareBitsOrUseFastGetHashCode();
        return FALSE;
    }

    MethodTable* valueTypeMT = MscorlibBinder::GetClass(CLASS__VALUE_TYPE);
    WORD slotEquals = MscorlibBinder::GetMethod(METHOD__VALUE_TYPE__EQUALS)->GetSlot();
    WORD slotGetHashCode = MscorlibBinder::GetMethod(METHOD__VALUE_TYPE__GET_HASH_CODE)->GetSlot();

    // Check the input type.
    if (HasOverriddenMethod(mt, valueTypeMT, slotEquals)
        || HasOverriddenMethod(mt, valueTypeMT, slotGetHashCode))
    {
        mt->SetHasCheckedCanCompareBitsOrUseFastGetHashCode();

        // If overridden Equals or GetHashCode found, stop searching further.
        return FALSE;
    }

    BOOL canCompareBitsOrUseFastGetHashCode = TRUE;

    // The type itself did not override Equals or GetHashCode, go for its fields.
    ApproxFieldDescIterator iter = ApproxFieldDescIterator(mt, ApproxFieldDescIterator::INSTANCE_FIELDS);
    for (FieldDesc* pField = iter.Next(); pField != NULL; pField = iter.Next())
    {
        if (pField->GetFieldType() == ELEMENT_TYPE_VALUETYPE)
        {
            // Check current field type.
            MethodTable* fieldMethodTable = pField->GetApproxFieldTypeHandleThrowing().GetMethodTable();
            if (!CanCompareBitsOrUseFastGetHashCode(fieldMethodTable))
            {
                canCompareBitsOrUseFastGetHashCode = FALSE;
                break;
            }
        }
        else if (pField->GetFieldType() == ELEMENT_TYPE_R8
                || pField->GetFieldType() == ELEMENT_TYPE_R4)
        {
            // We have double/single field, cannot compare in fast path.
            canCompareBitsOrUseFastGetHashCode = FALSE;
            break;
        }
    }

    // We've gone through all instance fields. It's time to cache the result.
    // Note SetCanCompareBitsOrUseFastGetHashCode(BOOL) ensures the checked flag
    // and canCompare flag being set atomically to avoid race.
    mt->SetCanCompareBitsOrUseFastGetHashCode(canCompareBitsOrUseFastGetHashCode);

    return canCompareBitsOrUseFastGetHashCode;
}

What's Next?

The most performant way to write these FCalls is to have the code that executes in steady state in the main method, and have all one time initialization in the helper method.

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 202,723评论 5 476
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 85,080评论 2 379
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 149,604评论 0 335
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 54,440评论 1 273
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 63,431评论 5 364
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 48,499评论 1 281
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 37,893评论 3 395
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 36,541评论 0 256
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 40,751评论 1 296
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 35,547评论 2 319
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 37,619评论 1 329
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 33,320评论 4 318
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 38,890评论 3 307
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 29,896评论 0 19
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 31,137评论 1 259
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 42,796评论 2 349
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 42,335评论 2 342

推荐阅读更多精彩内容