Dotnet: .Net 4.7 Incorrectly adds the lengths of the arrays

Created on 14 Jul 2017 · 12Comments · Source: microsoft/dotnet

using System;
using System.Text;

namespace CountingMystery
{
///

/// This is simplified scrap of production code. Problem is on line 38. Sum of arrays lenghts 7 + 7 + 6 + 1 = 36 instead of 21. You must meet these conditions:
/// - build in Release mode (Debug is OK)
/// - target Framework set to .NETFramework 4 or higher (3.5 is OK)
/// - run on .Net Framework 4.7 (4.6.2. is OK)
///

class Computing
{
public Guid TargetId { get; set; } = new Guid("3fdc5edb-bda6-4773-b782-d0ad34ba979b");
public Guid SourceId { get; set; } = new Guid("b9334825-0efd-4f94-b0b1-b94b1411b784");
public string Registration { get; set; } = "3AT1134";
public DateTime PassageTime { get; set; } = DateTime.Parse("2017-07-13T07:01:17.6080000");
public string GpsStatus { get; set; } = "AOOOOO";
public Byte VehicleClass { get; set; } = 0;

    public byte[] ToByteArray()
    {
        byte[] reg = new byte[0];
        if (!string.IsNullOrEmpty(Registration))
            reg = Encoding.ASCII.GetBytes(Registration);
        byte[] dt = getBytesFromDate(PassageTime);//year,month,day,milliseconds
        byte[] gps = Encoding.ASCII.GetBytes(GpsStatus);
        byte[] sourceId = SourceId.ToByteArray();
        byte[] targetId = TargetId.ToByteArray();
        byte[] vehicleClass = new byte[] { this.VehicleClass };

        int arrayLen = reg.Length + dt.Length + gps.Length + vehicleClass.Length + sourceId.Length + targetId.Length;


        byte[] arrayData = new byte[arrayLen];

        //problem here in Release mode: reg.Length + dt.Length + gps.Length + vehicleClass.Length = 7 + 7 + 6 + 1 = 36 instead of 21
        Console.WriteLine(reg.Length + dt.Length + gps.Length + vehicleClass.Length);//36 !why???

        //just changing order give right result: 7 + 6 + 7 + 1 = 21
        Console.WriteLine(dt.Length + gps.Length + reg.Length + vehicleClass.Length);//21 OK
        Console.ReadLine();

        //without following line of code, all work correct. Why??!!!
        Array.Copy(targetId, 0, arrayData, reg.Length + dt.Length + gps.Length + vehicleClass.Length + sourceId.Length, targetId.Length);

        return arrayData;
    }

    internal byte[] getBytesFromDate(DateTime dt)
    {
        byte[] result = new byte[7];
        result[0] = Convert.ToByte(dt.Year - 2000);
        result[1] = Convert.ToByte(dt.Month);
        result[2] = Convert.ToByte(dt.Day);
        int i = (dt.Hour * 60 * 60 * 1000 + dt.Minute * 60 * 1000 + dt.Second * 1000 + dt.Millisecond);
        Array.Copy(BitConverter.GetBytes(i), 0, result, 3, 4);
        return result;
    }
}

}
CountingMystery.zip

Source

fojtujakub

👍4 😕1

All 12 comments

@fojtujakub Wow, kewl. Repros on my machine! That's crazy! Sounds like a JIT bug since you mention it doesn't repro running on 4.6.2. Waiting with interest for a smart person to explain what's going on. =)

Fyi you can surround your code like this to make it easier to read:

```c# // Code ```

jnm2 on 14 Jul 2017

I have managed to simplify the code to:

```c#
using System;
using System.Runtime.CompilerServices;

class Computing
{
static Byte VehicleClass => 0;

static void ToByteArray(int a = 6, int c = 16, int d = 16)
{
    var vehicleClass = new byte[] { VehicleClass };

    Ignore(a + vehicleClass.Length + c + d);

    // problem here in Release mode: a + vehicleClass.Length = 6 + 1 = 22 instead of 7
    Console.WriteLine(a + vehicleClass.Length);

    Ignore(a + vehicleClass.Length + c);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Ignore(int i) {}

static void Main() => ToByteArray();

}
```

Based on this code, it looks to me like some kind of issue with common subexpression elimination.

With this code, I can reproduce the issue on .Net Framework 4.7 and .Net Core 1.1, but not on .Net Core 1.0 or .Net Core 2.0 Preview 3. So, this might already be fixed in source and the fix might be on its way for a future version of .Net Framework.

svick on 14 Jul 2017

👍1

This looks very similar to https://github.com/dotnet/coreclr/issues/11574, which was fixed in https://github.com/dotnet/coreclr/pull/11579 for .Net Core 2.0. I'm going to check if that's indeed the case.

svick on 14 Jul 2017

Yeah, the commit that merged https://github.com/dotnet/coreclr/pull/11579 does not have the issue, but the one just before does have it. So that PR fixed it and now it's just a question of when will that fix get into .Net Framework.

svick on 14 Jul 2017

I cannot repro the code from @svick (thanks!) on current .NET Framework 4.7 or .NET Core 2.0.

/cc @cmckinsey @russellhadley

richlander on 15 Jul 2017

Regarding the fix #11579: It is not in the 4.7 release, but is in the 4.7.1 release later this year.

cmckinsey on 15 Jul 2017

@richlander

I cannot repro the code from @svick (thanks!) on current .NET Framework 4.7 or .NET Core 2.0.

Like I said, it does not repro on .Net Core 2.0 Preview 3. It does repro for me on .Net Framework 4.7, though only in 64-bit mode (which nobody mentioned until now). So unless "current" means some private preview version of .Net 4.7, I think the most likely reason why you couldn't repro it is because you used the default 32-bit mode.

svick on 15 Jul 2017

Should we be warning people to stay off 4.7 until 4.7.1 is out? If there's no backport, will people be automatically upgraded?

jnm2 on 15 Jul 2017

This is a duplicate report of an issue that we have already fixed:

Link to GH Issue:
https://github.com/dotnet/coreclr/issues/11574

Link to GH Fix:
https://github.com/dotnet/coreclr/pull/11579