New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid data passed as a struct "pair of doubles" using reflection on Linux 1.0.x runtime #14391

Closed
jskeet opened this Issue Oct 9, 2017 · 6 comments

Comments

Projects
None yet
6 participants
@jskeet

jskeet commented Oct 9, 2017

Originally reported in this blog post: https://codeblog.jonskeet.uk/2017/10/08/diagnosing-a-linux-only-unit-test-failure/

Basically, I'm calling a static method via reflection, passing in a simple struct that consists of two double values. On netcoreapp1.0 on Linux only, the value the method receives has a bogus second value.

Project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp1.0;netcoreapp1.1;netcoreapp2.0</TargetFrameworks>
  </PropertyGroup>
</Project>

Source code:

using System;
using System.Reflection;

class Program
{
   static void Main(string[] args)
   {
       DoublePair pair = new DoublePair(1, 2);
       var method = typeof(DoublePair).GetMethod("Print");
       method.Invoke(null, new object[] { pair });
   }
}

struct DoublePair
{
    private readonly double x;
    private readonly double y;
    
    public DoublePair(double x, double y)
    {
        this.x = x;
        this.y = y;
    }
    
    public static void Print(DoublePair pair) =>
        Console.WriteLine($"{pair.x} {pair.y}");
}

Expected output: 1 2

Sample output on netcoreapp1.0 on Linux: 1 1.83063495561695E-316

I get the expected output in netcoreapp1.1 and netcoreapp2.0, but not in netcoreapp1.0. Currently using fx 1.0.7 as installed via dotnet-dev-1.1.4. Everything works fine in Windows.

Applying explicit struct layout to DoublePair fixes the issue, suggesting an invalid JIT optimization somewhere.

@jskeet jskeet changed the title from Invalid data passed as a struct "pair of doubles" using reflection on 1.0.x to Invalid data passed as a struct "pair of doubles" using reflection on Linux 1.0.x runtime Oct 9, 2017

@eerhardt

This comment has been minimized.

Show comment
Hide comment
@eerhardt

eerhardt Oct 9, 2017

Member

I discussed this issue with @janvorli this morning and he said the fix was #7716, which was ported to 1.1.x in #7748, but was never ported to 1.0.x.

Here's the original reported issue: #7685.

@Petermarcu - do we want to port this fix to 1.0.x? It seems like it would cause hard to diagnose problems in user's apps.

Member

eerhardt commented Oct 9, 2017

I discussed this issue with @janvorli this morning and he said the fix was #7716, which was ported to 1.1.x in #7748, but was never ported to 1.0.x.

Here's the original reported issue: #7685.

@Petermarcu - do we want to port this fix to 1.0.x? It seems like it would cause hard to diagnose problems in user's apps.

@mihailik

This comment has been minimized.

Show comment
Hide comment
@mihailik

mihailik Oct 11, 2017

@janvorli @eerhardt does this affect KeyValuePair<double,double>?

I guess less people care for little pesky structs. But keeping Dictionary<double,double> out of harm's way is a different business.

mihailik commented Oct 11, 2017

@janvorli @eerhardt does this affect KeyValuePair<double,double>?

I guess less people care for little pesky structs. But keeping Dictionary<double,double> out of harm's way is a different business.

@stephentoub

This comment has been minimized.

Show comment
Hide comment
@stephentoub

stephentoub Oct 11, 2017

Member

But keeping Dictionary<double,double> out of harm's way is a different business.

Dictionary doesn't use reflection invocation.

Member

stephentoub commented Oct 11, 2017

But keeping Dictionary<double,double> out of harm's way is a different business.

Dictionary doesn't use reflection invocation.

@mihailik

This comment has been minimized.

Show comment
Hide comment
@mihailik

mihailik Oct 11, 2017

@stephentoub Dictionary doesn't, but people using D<double,double> possibly do — consider seiralization scenarios.

mihailik commented Oct 11, 2017

@stephentoub Dictionary doesn't, but people using D<double,double> possibly do — consider seiralization scenarios.

@mihailik

This comment has been minimized.

Show comment
Hide comment
@mihailik

mihailik Oct 11, 2017

Here's a live environment with exact reproduction (dotnet 1.0, 1.1 and 2.0 installed), on Cloud9:

https://ide.c9.io/mihailik/dotnet-bug

mihailik commented Oct 11, 2017

Here's a live environment with exact reproduction (dotnet 1.0, 1.1 and 2.0 installed), on Cloud9:

https://ide.c9.io/mihailik/dotnet-bug

@RussKeldorph RussKeldorph added the area-VM label Nov 3, 2017

@jkotas

This comment has been minimized.

Show comment
Hide comment
@jkotas

jkotas Nov 3, 2017

Member

The fix was ported to 1.0.x. It will show up in next 1.0.x servicing release.

Member

jkotas commented Nov 3, 2017

The fix was ported to 1.0.x. It will show up in next 1.0.x servicing release.

@jkotas jkotas closed this Nov 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment