Skip to content
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

Returning tuple broken when enabling fullAOT+LLVM #13610

Closed
spouliot opened this issue Mar 22, 2019 · 12 comments · Fixed by #13704 or xamarin/xamarin-android#2897

Comments

@spouliot
Copy link
Member

@spouliot spouliot commented Mar 22, 2019

Steps to Reproduce

  1. Call the following code from an iOS app
public static async Task<(bool isAnswerCorrect, bool isInternetConnectionAvailable)> Test ()
{
	return (true, true);
}

note: full test case in xamarin/xamarin-macios#5798

  1. Run on device without LLVM -> success
  2. Run on device with LLVM -> failure (false is returned)

Current Behavior

Broken (regression, it works in XI stable) when enabling LLVM

Expected Behavior

Should work on both mono and LLVM fullAOT backend

On which platforms did you notice this

[x] iOS
[ ] macOS
[ ] Linux
[ ] Windows

Version Used:

2018-08 (d16-0), 2018-10 (xamarin-macios/master) and 2019-02 are all broken

Current XI stable is fine (2018-04)

Stacktrace

none

@spouliot

This comment has been minimized.

Copy link
Member Author

@spouliot spouliot commented Mar 22, 2019

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Mar 22, 2019

@alexanderkyte @vargaz please investigate the regression

@lewurm lewurm changed the title Returning tuple broken when enabling AOT+LLVM Returning tuple broken when enabling fullAOT+LLVM Mar 25, 2019
@alexanderkyte

This comment has been minimized.

Copy link
Member

@alexanderkyte alexanderkyte commented Mar 25, 2019

Here, let me add a sane-sized executable reproduction: https://gist.github.com/alexanderkyte/1974de18f51cb2a984908524809ec87c

@alexanderkyte

This comment has been minimized.

Copy link
Member

@alexanderkyte alexanderkyte commented Mar 25, 2019

Looks like we don't have any unit tests for this on the mono side, only acceptance/integration tests. This PR caused this breakage: #13312 .

I'll have a fix for this shortly.

We need to add this bug reproduction to the test harness at some point.

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Mar 25, 2019

This PR caused this breakage: #13312 .

Mentioned PR was merged 6 days ago, but the bug started to happen with 2018-08. Regression was likely introduced by something else.

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 25, 2019

I can reproduce. When was the last time this worked ?

@spouliot

This comment has been minimized.

Copy link
Member Author

@spouliot spouliot commented Mar 25, 2019

mentioned above
2018-08 (d16-0), 2018-10 (xamarin-macios/master) and 2019-02 are all broken
Current XI stable is fine (2018-04)

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 25, 2019

So 2018-04 was still using the old llvm 3.6.

@spouliot

This comment has been minimized.

Copy link
Member Author

@spouliot spouliot commented Mar 25, 2019

ouch :(

@lambdageek

This comment has been minimized.

Copy link
Member

@lambdageek lambdageek commented Mar 26, 2019

Doesn't have to be a tuple. This also fails (prints 0. expected to print 1)

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

public struct OneThing<T1> {
	public T1 Item1;
}

public class P {

	[MethodImpl (MethodImplOptions.NoInlining)]
	public static int TestCaller () {
		var t = Task.FromResult<OneThing<int>>(new OneThing<int> {Item1 = 42});
		t.Wait ();
		int a = t.Result.Item1;
		return (a == 42) ? 1 : 0;
	}

	public static int Main () {
		var t = TestCaller ();
		var i = t;
		Console.WriteLine ($"{i}");
		return 0;
	}
}

compile with mono --runtime=mobile --aot=llvmonly example.exe run with mono --runtime=mobile --llvmonly example.exe

example.opt.ll

@lambdageek

This comment has been minimized.

Copy link
Member

@lambdageek lambdageek commented Mar 26, 2019

Don't need tasks either. Just a generic method will do.

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

public struct OneThing<T1> {
	public T1 Item1;
}

public class S<T> {
	public T Result;

	public S(T val) {
		Result = val;
	}

}

public class Maker {
	[MethodImpl (MethodImplOptions.NoInlining)]
	public static S<T> FromResult<T> (T result) {
		return new S<T>(result);
	}
}

public class P {

	[MethodImpl (MethodImplOptions.NoInlining)]
	public static int TestCaller () {
		var t = Maker.FromResult<OneThing<int>>(new OneThing<int> {Item1 = 42});
		int a = t.Result.Item1;
		return (a == 42) ? 1 : 0;
	}

	public static int Main () {
		var t = TestCaller ();
		var i = t;
		Console.WriteLine ($"{i}");
		return 0;
	}
}
@vargaz vargaz self-assigned this Mar 26, 2019
vargaz added a commit to vargaz/mono that referenced this issue Mar 26, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 26, 2019
marek-safar added a commit that referenced this issue Mar 26, 2019
akoeplinger added a commit that referenced this issue Mar 26, 2019
lambdageek added a commit to lambdageek/mono that referenced this issue Mar 26, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 26, 2019
lambdageek added a commit that referenced this issue Mar 26, 2019
…to_vtype (). (#13675)

Fixes #13610.
marek-safar added a commit that referenced this issue Mar 26, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 28, 2019
marek-safar added a commit that referenced this issue Mar 28, 2019
marek-safar added a commit that referenced this issue Mar 28, 2019
vargaz added a commit to vargaz/mono that referenced this issue Mar 28, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 28, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 28, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Mar 28, 2019
marek-safar added a commit that referenced this issue Mar 29, 2019
marek-safar added a commit that referenced this issue Mar 29, 2019
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Mar 29, 2019
Changes: mono/mono@204d95c...2264e1b

Fixes: mono/mono#12307
Fixes: mono/mono#12843
Fixes: mono/mono#13610

Should fix AOT failures on Windows, when the mono archive is used.
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Mar 29, 2019
Fixes: mono/mono#12307
Fixes: mono/mono#12843
Fixes: mono/mono#13610

May fix AOT failures on Windows, when the mono archive is used.
rolfbjarne added a commit to monojenkins/mono that referenced this issue Apr 2, 2019
marek-safar added a commit that referenced this issue Apr 3, 2019
…n emit_args_to_vtype (). (#13723)

Fixes #13610.

Backport of #13659 to `2018-08`

/cc @vargaz 

Backport of #13675.

/cc @marek-safar @lambdageek
rolfbjarne added a commit to xamarin/xamarin-macios that referenced this issue Apr 3, 2019
Commit list for mono/mono:

* mono/mono@7445137 [2018-08-rc][llvm] Fix the computation of size of gshared instances in emit_args_to_vtype (). (#13723)
* mono/mono@5ad371d Make `System.dll` internals visible to `Mono.Android`.

Diff: mono/mono@163f45d...7445137
rolfbjarne added a commit to xamarin/xamarin-macios that referenced this issue Apr 3, 2019
Commit list for mono/mono:

* mono/mono@7445137 [2018-08-rc][llvm] Fix the computation of size of gshared instances in emit_args_to_vtype (). (#13723)
* mono/mono@5ad371d Make `System.dll` internals visible to `Mono.Android`.

Diff: mono/mono@89d7665...7445137
rolfbjarne added a commit to xamarin/xamarin-macios that referenced this issue Apr 3, 2019
New commits in mono/mono:

* mono/mono@4b020ca Fix transition offset logic (#13553)
* mono/mono@e475d80 [llvm] Fix the computation of size of gshared instances in another place.
* mono/mono@c87a95e [llvm] Fix the computation of size of gshared instances in emit_args_to_vtype (). (#13675)
* mono/mono@75282c4 [amd64] use 32bit variant of lea_membase for 32bit operations
* mono/mono@5756d04 [debugger] Adding a comment to make it clear for the next debuggers.
* mono/mono@484ce49 [debugger] Fixing two crashes while debugging an Android app.
* mono/mono@6a81d1d [System] Fix a race condition in HttpRequestChannel.
* mono/mono@fdb26b0 [merp] Fix anon allocation, use MAP_PRIVATE
* mono/mono@82547c4 [crash] Fallback to MAP_ANONYMOUS for allocator
* mono/mono@693806c Bump bockbuild for GTK fix
* mono/mono@9cb3348 Append .1 to version number
* mono/mono@948b450 Include libgdiplus 5.6.1
* mono/mono@60e6090 [MacSDK] Bump Nuget to 4.8.2

Diff: https://github.com/mono/mono/compare/89d7665e281a7f539b6f91501f4dcf1864961bf5..4b020ca06d46b2af6d9fc0d0b2cf454301f5014e
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Apr 3, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Apr 3, 2019
rolfbjarne added a commit to xamarin/xamarin-macios that referenced this issue Apr 8, 2019
New commits in mono/mono:

* mono/mono@4b020ca Fix transition offset logic (#13553)
* mono/mono@e475d80 [llvm] Fix the computation of size of gshared instances in another place.
* mono/mono@c87a95e [llvm] Fix the computation of size of gshared instances in emit_args_to_vtype (). (#13675)
* mono/mono@75282c4 [amd64] use 32bit variant of lea_membase for 32bit operations
* mono/mono@5756d04 [debugger] Adding a comment to make it clear for the next debuggers.
* mono/mono@484ce49 [debugger] Fixing two crashes while debugging an Android app.
* mono/mono@6a81d1d [System] Fix a race condition in HttpRequestChannel.
* mono/mono@fdb26b0 [merp] Fix anon allocation, use MAP_PRIVATE
* mono/mono@82547c4 [crash] Fallback to MAP_ANONYMOUS for allocator
* mono/mono@693806c Bump bockbuild for GTK fix
* mono/mono@9cb3348 Append .1 to version number
* mono/mono@948b450 Include libgdiplus 5.6.1
* mono/mono@60e6090 [MacSDK] Bump Nuget to 4.8.2

Diff: https://github.com/mono/mono/compare/89d7665e281a7f539b6f91501f4dcf1864961bf5..4b020ca06d46b2af6d9fc0d0b2cf454301f5014e
chamons added a commit to xamarin/xamarin-macios that referenced this issue Apr 10, 2019
Commit list for mono/mono:

* mono/mono@7445137 [2018-08-rc][llvm] Fix the computation of size of gshared instances in emit_args_to_vtype (). (#13723)
* mono/mono@5ad371d Make `System.dll` internals visible to `Mono.Android`.

Diff: mono/mono@163f45d...7445137
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Apr 24, 2019
Bumps to mono/api-snapshot@ae01378
Bumps to mono/reference-assemblies@e5173a5
Bumps to mono/bockbuild@d30329d
Bumps to mono/boringssl@3d87996
Bumps to mono/corefx@72f7d76
Bumps to mono/corert@1b7d4a1
Bumps to mono/helix-binaries@7e893ea
Bumps to mono/illinker-test-assets@f21ff68
Bumps to mono/linker@13d864e
Bumps to mono/llvm@1aaaaa5 [mono]
Bumps to mono/llvm@2c2cffe [xamarin-android]
Bumps to mono/NUnitLite@0029561
Bumps to mono/roslyn-binaries@0bbc9b4
Bumps to mono/xunit-binaries@8f6e62e

	$ git diff --shortstat 886c4901..e66c7667      # mono
        3597 files changed, 350850 insertions(+), 91128 deletions(-)
	$ git diff --shortstat 349752c464c5fc93b32e7d45825f2890c85c8b7d..2c2cffedf01e0fe266b9aaad2c2563e05b750ff4
	 240 files changed, 18562 insertions(+), 6581 deletions(-)

Context: dotnet/coreclr#22046

Fixes: CVE 2018-8292 on macOS
Fixes: http://work.devdiv.io/737323
Fixes: dotnet/corefx#33965
Fixes: dotnet/standard#642
Fixes: mono/mono#6997
Fixes: mono/mono#7326
Fixes: mono/mono#7517
Fixes: mono/mono#7750
Fixes: mono/mono#7859
Fixes: mono/mono#8360
Fixes: mono/mono#8460
Fixes: mono/mono#8766
Fixes: mono/mono#8922
Fixes: mono/mono#9418
Fixes: mono/mono#9507
Fixes: mono/mono#9951
Fixes: mono/mono#10024
Fixes: mono/mono#10030
Fixes: mono/mono#10038
Fixes: mono/mono#10448
Fixes: mono/mono#10735
Fixes: mono/mono#10735
Fixes: mono/mono#10737
Fixes: mono/mono#10743
Fixes: mono/mono#10834
Fixes: mono/mono#10837
Fixes: mono/mono#10838
Fixes: mono/mono#10863
Fixes: mono/mono#10945
Fixes: mono/mono#11020
Fixes: mono/mono#11021
Fixes: mono/mono#11021
Fixes: mono/mono#11049
Fixes: mono/mono#11091
Fixes: mono/mono#11095
Fixes: mono/mono#11123
Fixes: mono/mono#11138
Fixes: mono/mono#11146
Fixes: mono/mono#11202
Fixes: mono/mono#11214
Fixes: mono/mono#11317
Fixes: mono/mono#11326
Fixes: mono/mono#11378
Fixes: mono/mono#11385
Fixes: mono/mono#11478
Fixes: mono/mono#11479
Fixes: mono/mono#11488
Fixes: mono/mono#11489
Fixes: mono/mono#11527
Fixes: mono/mono#11529
Fixes: mono/mono#11596
Fixes: mono/mono#11603
Fixes: mono/mono#11613
Fixes: mono/mono#11623
Fixes: mono/mono#11663
Fixes: mono/mono#11681
Fixes: mono/mono#11684
Fixes: mono/mono#11693
Fixes: mono/mono#11697
Fixes: mono/mono#11779
Fixes: mono/mono#11809
Fixes: mono/mono#11858
Fixes: mono/mono#11895
Fixes: mono/mono#11898
Fixes: mono/mono#11898
Fixes: mono/mono#11965
Fixes: mono/mono#12182
Fixes: mono/mono#12193
Fixes: mono/mono#12218
Fixes: mono/mono#12235
Fixes: mono/mono#12263
Fixes: mono/mono#12307
Fixes: mono/mono#12331
Fixes: mono/mono#12362
Fixes: mono/mono#12374
Fixes: mono/mono#12402
Fixes: mono/mono#12421
Fixes: mono/mono#12461
Fixes: mono/mono#12479
Fixes: mono/mono#12479
Fixes: mono/mono#12552
Fixes: mono/mono#12603
Fixes: mono/mono#12747
Fixes: mono/mono#12831
Fixes: mono/mono#12843
Fixes: mono/mono#12881
Fixes: mono/mono#13030
Fixes: mono/mono#13284
Fixes: mono/mono#13297
Fixes: mono/mono#13455
Fixes: mono/mono#13460
Fixes: mono/mono#13478
Fixes: mono/mono#13479
Fixes: mono/mono#13522
Fixes: mono/mono#13607
Fixes: mono/mono#13610
Fixes: mono/mono#13610
Fixes: mono/mono#13639
Fixes: mono/mono#13672
Fixes: mono/mono#13834
Fixes: mono/mono#13878
Fixes: mono/mono#6352
Fixes: mono/monodevelop#6898
Fixes: xamarin/maccore#1069
Fixes: xamarin/maccore#1407
Fixes: xamarin/maccore#604
Fixes: xamarin/xamarin-macios#4984
Fixes: xamarin/xamarin-macios#5289
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381
Fixes: https://issuetracker.unity3d.com/issues/editor-crashes-with-g-logv-when-entering-play-mode-with-active-flowcanvas-script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.