dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 527 forks source link

Runtime unable to handle certain IL #1194

Closed borrrden closed 6 years ago

borrrden commented 6 years ago

Steps to Reproduce

Consider the following struct:

unsafe partial struct C4BlobKey
{
    public fixed byte bytes[20];
}

and a corresponding C struct

typedef struct C4BlobKey {
    uint8_t bytes[20];
} C4BlobKey;

I want to determine equality so I write the following Equals() function:

public override bool Equals(object obj)
{
    if(!(obj is C4BlobKey)) {
        return false;
    }

    var other = (C4BlobKey)obj;
    fixed(byte* b = bytes) {
        for(int i = 0; i < _Size; i++) {
            if(b[i] != other.bytes[i]) {
                return false;
            }
        }
    }

    return true;
 }

Debug and release mode both have the same following symptoms, but obviously with different lines since the release mode is optimized.

Invalid IL code in LiteCore.Interop.C4BlobKey.Equals (object); IL_001c: stloc.2

In fact I have to instances of this type of class that do similar things and they both fail in the same way. Here is the IL code (up to around the line in question):

.method public hidebysig virtual instance bool 
Equals(
  object obj
) cil managed 
{
.maxstack 3
.locals init (
  [0] valuetype LiteCore.Interop.C4BlobKey other,
  [1] unsigned int8* b,
  [2] unsigned int8*& pinned V_2,
  [3] int32 i
)

// [51 13 - 51 36]
IL_0000: ldarg.1      // obj
IL_0001: isinst       LiteCore.Interop.C4BlobKey
IL_0006: brtrue.s     IL_000a

// [52 17 - 52 30]
IL_0008: ldc.i4.0     
IL_0009: ret          

// [55 13 - 55 40]
IL_000a: ldarg.1      // obj
IL_000b: unbox.any    LiteCore.Interop.C4BlobKey
IL_0010: stloc.0      // other
IL_0011: ldarg.0      // this
IL_0012: ldflda       valuetype LiteCore.Interop.C4BlobKey/'<bytes>e__FixedBuffer' LiteCore.Interop.C4BlobKey::bytes
IL_0017: ldflda       unsigned int8 LiteCore.Interop.C4BlobKey/'<bytes>e__FixedBuffer'::FixedElementField
IL_001c: stloc.2      // V_2

// [56 19 - 56 34]
IL_001d: ldloc.2      // V_2
IL_001e: conv.u       
IL_001f: stloc.1      // b

// [57 21 - 57 30]
IL_0020: ldc.i4.0     
IL_0021: stloc.3      // i

IL_0022: br.s         IL_0040
// start of loop, entry point: IL_0040

This same IL works fine on UWP, and .NET Core. This problem also appears to exist in Xamarin iOS.

Expected Behavior

The IL gets handled as on other platforms

Actual Behavior

The above exception

Version Information

Microsoft Visual Studio Community 2017 Version 15.5.3 VisualStudio.15.Release/15.5.3+27130.2020 Microsoft .NET Framework Version 4.7.02556

Installed Version: Community

Visual Basic 2017 00369-60000-00001-AA631 Microsoft Visual Basic 2017

Visual C# 2017 00369-60000-00001-AA631 Microsoft Visual C# 2017

Visual C++ 2017 00369-60000-00001-AA631 Microsoft Visual C++ 2017

Visual F# 4.1 00369-60000-00001-AA631 Microsoft Visual F# 4.1

Application Insights Tools for Visual Studio Package 8.10.01106.1 Application Insights Tools for Visual Studio

ASP.NET and Web Tools 2017 15.0.31127.0 ASP.NET and Web Tools 2017

ASP.NET Core Razor Language Services 1.0 Provides languages services for ASP.NET Core Razor.

ASP.NET Web Frameworks and Tools 2012 4.0.20601.0 For additional information, visit https://www.asp.net/

ASP.NET Web Frameworks and Tools 2017 5.2.51007.0 For additional information, visit https://www.asp.net/

Azure App Service Tools v3.0.0 15.0.31106.0 Azure App Service Tools v3.0.0

Common Azure Tools 1.10 Provides common services for use by Azure Mobile Services and Microsoft Azure Tools.

JavaScript Language Service 2.0 JavaScript Language Service

JavaScript Project System 2.0 JavaScript Project System

JavaScript UWP Project System 2.0 JavaScript UWP Project System

JetBrains ReSharper Ultimate 2017.3.1 Build 111.0.20171221.145902 JetBrains ReSharper Ultimate package for Microsoft Visual Studio. For more information about ReSharper Ultimate, visit http://www.jetbrains.com/resharper. Copyright © 2018 JetBrains, Inc.

Merq 1.1.17-rc (cba4571) Command Bus, Event Stream and Async Manager for Visual Studio extensions.

Microsoft Continuous Delivery Tools for Visual Studio 0.3 Simplifying the configuration of continuous build integration and continuous build delivery from within the Visual Studio IDE.

Microsoft JVM Debugger 1.0 Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines

Microsoft MI-Based Debugger 1.0 Provides support for connecting Visual Studio to MI compatible debuggers

Microsoft Visual C++ Wizards 1.0 Microsoft Visual C++ Wizards

Microsoft Visual Studio VC Package 1.0 Microsoft Visual Studio VC Package

Mono Debugging for Visual Studio 4.8.4-pre (3fe64e3) Support for debugging Mono processes with Visual Studio.

NuGet Package Manager 4.5.0 NuGet Package Manager in Visual Studio. For more information about NuGet, visit http://docs.nuget.org/.

Project System Tools 1.0 Tools for working with C#, VisualBasic, and F# projects.

SQL Server Data Tools 15.1.61710.120 Microsoft SQL Server Data Tools

TypeScript Tools 15.5.11025.1 TypeScript Tools for Microsoft Visual Studio

Visual Studio Code Debug Adapter Host Package 1.0 Interop layer for hosting Visual Studio Code debug adapters in Visual Studio

Visual Studio Tools for CMake 1.0 Visual Studio Tools for CMake

Visual Studio Tools for Universal Windows Apps 15.0.27130.2020 The Visual Studio Tools for Universal Windows apps allow you to build a single universal app experience that can reach every device running Windows 10: phone, tablet, PC, and more. It includes the Microsoft Windows 10 Software Development Kit.

VisualStudio.Mac 1.0 Mac Extension for Visual Studio

Xamarin 4.8.0.757 (7f9ec2a) Visual Studio extension to enable development for Xamarin.iOS and Xamarin.Android.

Xamarin Designer 4.8.188 (c5813fa34) Visual Studio extension to enable Xamarin Designer tools in Visual Studio.

Xamarin.Android SDK 8.1.3.0 (HEAD/ef47226b7) Xamarin.Android Reference Assemblies and MSBuild support.

Xamarin.iOS and Xamarin.Mac SDK 11.6.1.2 (6857dfc) Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support.

Log File

Nothing of interest. This exception was caught and reported by our unit test runner.

kumpera commented 6 years ago

Looks like the report is enough for a repro. I'll be working on it.

kumpera commented 6 years ago

Hey Marek,

This looks like a bug in the version of Roslyn on Windows. Neither mcs or csc from mono 5.4 and 5.8 generate the broken pattern.

Could you contact them and sort this out?

kumpera commented 6 years ago

I can't repro the given binary with any Roslyn version available to me. I can't repro the issue by hand editing the IL to follow the pattern in the bug report with neither mono 5.4 or mono 5.8.

Please provide a standard alone executable that shows the behavior so we can work on. Please try upgrading your dotnet SDK/C# compiler version to the latest.

kumpera commented 6 years ago

@borrrden could go to the developer command prompt on your machine and run csc /version. I failed to reproduce with csc 2.3.2.61928 (ec1cde8b).

grendello commented 6 years ago

@borrrden could you attach (or provide otherwise, if you can't share publically) the binaries produced by the compiler? Also, did you run peverify on the failing assembly? If no, could you please do so and let us know whether it passes verification?

borrrden commented 6 years ago

My version of csc is 2.6.0.62329 (5429b35d)

borrrden commented 6 years ago

peverify returns quite a few errors regarding unexpected numeric types and non-compatible types on the stack. Just FYI this is a .NET Standard 1.4 assembly.

borrrden commented 6 years ago

Couchbase.Lite.dll.zip is the resulting assembly.

grendello commented 6 years ago

@borrrden can you attach the full output of peverify? Thanks!

borrrden commented 6 years ago

Full output https://gist.github.com/borrrden/22f1f064fbcad7ebda486b8b89fc428c

grendello commented 6 years ago

@borrrden thanks! It looks like the issue is with the compiler you're using, would you be able to try building the solution on a different machine with a different version of .NET installed?

kumpera commented 6 years ago

Peverify will rightfully say that an assembly with unsafe code is unverfiable as it violates the sandboxing rules. A CLI repro would be appreciated.

borrrden commented 6 years ago

would you be able to try building the solution on a different machine with a different version of .NET installed?

I would be willing to if Microsoft didn't make it a gigantic pain in the ass. The only way to downgrade, I am told, is to uninstall Visual Studio completely and then reinstall it and even then only the previous minor version is allowed.

This is the standard 15.5.3 install on Windows. I will try to make a CLI repro, but as I mentioned the issue is observed on Xamarin Android (I don't make desktop Mono builds, and .NET Core does not show these symptoms given the same IL) so "CLI" is a bit dubious in this sense.

borrrden commented 6 years ago

Ok I reproduced the problem quite simply with CLI. This only happens when the library is built via Visual Studio on Windows. Using msbuild on Mac does not produce the same problem.

For completeness, I will attach four items:

InvalidIL.zip this is the project for generating the DLL with the Invalid IL inside. InvalidIL_win.dll.zip The resulting DLL when built on Windows InvalidIL_mac.dll.zip The resulting DLL when built on Mac via msbuild command line

Spike.txt (rename to .cs). This is the test program.

Compiled using

mcs Spike.cs /reference:InvalidIL.dll /reference:$HOME/.nuget/packages/system.runtime/4.3.0/lib/net462/System.Runtime.dll

When I run with the Windows DLL this is the output:

$ mono Spike.exe [12:24:25]

Unhandled Exception: System.InvalidProgramException: Invalid IL code in InvalidIL.C4UUID:Equals (object): IL_0028: stloc.s 4

at Test.Program.Main (System.String[] args) [0x00010] in <6cb257d0d2154dc8a3984b2a5b4ee37f>:0 [ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidProgramException: Invalid IL code in InvalidIL.C4UUID:Equals (object): IL_0028: stloc.s 4

at Test.Program.Main (System.String[] args) [0x00010] in <6cb257d0d2154dc8a3984b2a5b4ee37f>:0

No output as expected with the DLL compiled on Mac.

borrrden commented 6 years ago

FYI:

$ mono --version [12:25:17] Mono JIT compiler version 5.4.1.7 (2017-06/e66d9abbb27 Wed Oct 25 12:10:41 EDT 2017) Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com TLS: normal SIGSEGV: altstack Notification: kqueue Architecture: amd64 Disabled: none Misc: softdebug LLVM: yes(3.6.0svn-mono-master/8b1520c8aae) GC: sgen (concurrent by default)

grendello commented 6 years ago

@borrrden Thanks for providing the files and the test project! I can confirm that the code produced by Mono on Mac (and Linux) is valid IL and no crash happens on the run time. It indeed appears this is an issue with the C# compiler bundled with VS on Windows (or with .NET on Windows) as well as that the issue is not specific to Xamarin.Android. /cc @kumpera /cc @marek-safar

marek-safar commented 6 years ago

Please provide full repro of how to build you windows version of InvalidIL_win.dll.zip. I tried to compile netstandard project with following code to replicate it but no luck

using System;

unsafe partial struct C4BlobKey
{
    public fixed byte bytes[20];
    public static int _Size = 20;

    public override bool Equals(object obj)
    {
        if(!(obj is C4BlobKey)) {
            return false;
        }

        var other = (C4BlobKey)obj;
        fixed(byte* b = bytes) {
            for(int i = 0; i < _Size; i++) {
                if(b[i] != other.bytes[i]) {
                    return false;
                }
            }
        }

        return true;
     }    
}

public class Program
{

    public static void Main(string[] args)
    {
        var publicID = new C4BlobKey();
        var privateID = new C4BlobKey();
        publicID.Equals(privateID);
    }
}

There is very suspicious temporary local [0] introduced in your version which looks like csc bug but as I said I cannot reproduce it with my version of Visual Studio.

[0]: uint8*& pinned V_4,

borrrden commented 6 years ago

I literally just right clicked on the solution in Debug mode and chose "Build" inside of Visual Studio 15.5.3 (edit: Or, as it turns out, 15.5.4).

(EDIT: Using the above provided project)

marek-safar commented 6 years ago

I don't see any provided project only few code snippets and dlls. Could you attach the full project so I can also "just right click on the solution and Build" ?

borrrden commented 6 years ago

It’s there above. Here is the link again

https://github.com/xamarin/xamarin-android/files/1637608/InvalidIL.zip

bruno-david commented 6 years ago

I have the same issue. Were you able to find a solution for the IL error?

borrrden commented 6 years ago

It’s still there with the latest VS? I ended up commenting out the offending source code because I didn’t need it to be in that particular location but no I never found the underlying reason.

marek-safar commented 6 years ago

I believe this is duplicate of https://github.com/mono/mono/issues/8403

borrrden commented 6 years ago

I'm willing to try out the test program again if you can let me know how to get a build with the fix in it (or the version of VS that includes the fix).

marek-safar commented 6 years ago

Visual Studio 15.7 (and included products) should have the fix included.

borrrden commented 6 years ago

Alright I've confirmed that both building with Mono JIT compiler version 5.10.1.57 (2017-12/ea8a24b1bbf Tue Apr 24 14:53:01 EDT 2018) and running the previous "Invalid" build both work now.