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

System.Runtime.CompilerServices.UnsafeTests.ReadWriteDoublePointer test failure on devices in Xamarin.iOS #14872

Closed
akoeplinger opened this issue Jun 7, 2019 · 9 comments

Comments

@akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Jun 7, 2019

Steps to Reproduce

  1. Clone https://github.com/xamarin/xamarin-macios
  2. Checkout mono-2019-06 branch at 9209b4eb49bb59906500fd9baafd7d27cb74dd1b
  3. ./configure --enable-xamarin && make world
  4. make -C tests runner
  5. Run any Debug configuration in [xUnit] Mono BCL tests group 3 under iOS Unified 64-bits - device
  6. Observe test failure

Not that this only happens on devices and only in the Debug configuration!

Current Behavior

  Test name: System.Runtime.CompilerServices.UnsafeTests.ReadWriteDoublePointer
   Exception messages: Assert.Equal() Failure
Expected: 20
Actual:   10
   Exception stack traces:   at System.Runtime.CompilerServices.UnsafeTests.ReadWriteDoublePointer () [0x00020] in /Users/alexander/dev/xamarin-macios/xamarin-macios/external/mono/external/corefx/src/System.Runtime.CompilerServices.Unsafe/tests/UnsafeTests.cs:96 
  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in /Users/alexander/dev/xamarin-macios/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:395 

Expected Behavior

No failure

On which platforms did you notice this

[ x ] macOS
[ ] Linux
[ ] Windows

Version Used: 2019-06

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Jun 11, 2019

@SamMonoRT could you assign this issue

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Jun 12, 2019

Desktop testcase:

using System;
using System.Threading;
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;

public class Tests
{
	public unsafe static int Main (String[] args) {
		int value1 = 10;
		int value2 = 20;
		int* valueAddress = (int*)Unsafe.AsPointer(ref value1);
		int** valueAddressPtr = &valueAddress;
		Unsafe.Write(valueAddressPtr, new IntPtr(&value2));

		Console.WriteLine ("X1: " + *(*valueAddressPtr));
		return 0;
	}
}

mono bug.exe
X1: 20

mono -O=-inline bug.exe
X1: 10

mono -O=-alias-analysis,-inline bug.exe
X1: 20

It looks like the problem is in the alias-analysis pass.

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Jun 12, 2019

It happens in debug mode on ios which disables inlining.

@BrzVlad

This comment has been minimized.

Copy link
Member

@BrzVlad BrzVlad commented Jun 12, 2019

@vargaz thanks for the repro. Are you looking into this bug or I can take it ?

@mandel-macaque

This comment has been minimized.

Copy link
Contributor

@mandel-macaque mandel-macaque commented Jun 14, 2019

This also happens on tvOS devices. But I'm not surprise since it happens on iOS too.

@mandel-macaque

This comment has been minimized.

Copy link
Contributor

@mandel-macaque mandel-macaque commented Jun 14, 2019

Interestingly on the apple tvOS the test does pass on SDK framework (release)

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jun 14, 2019

Interestingly on the apple tvOS the test does pass on SDK framework (release)

Release uses LLVM

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Jun 17, 2019

@BrzVlad any update on this issue?

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Jun 17, 2019

Its a problem in the JITs alias analysis pass.

BrzVlad added a commit to BrzVlad/mono that referenced this issue Jun 19, 2019
Alias analysis attempts to remove indirection when using OP_LDADDR (which fetches the address of a variable). For example :

	ldaddr r1 <- r0
	load_mem r2 <- r1

is equivalent to

	mov r2 <- r0

The remembered r1 alias needs to be forgotten in case r1 is changed. We were handling this for direct assignment with MOV, but not for the more subtle case where an alias to r1 would be passed to a call, which could lead to changing r1.

mono#14872
BrzVlad added a commit that referenced this issue Jun 20, 2019
Alias analysis attempts to remove indirection when using OP_LDADDR (which fetches the address of a variable). For example :

	ldaddr r1 <- r0
	load_mem r2 <- r1

is equivalent to

	mov r2 <- r0

The remembered r1 alias needs to be forgotten in case r1 is changed. We were handling this for direct assignment with MOV, but not for the more subtle case where an alias to r1 would be passed to a call, which could lead to changing r1.

#14872
@BrzVlad BrzVlad closed this Jun 20, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Jun 20, 2019
Alias analysis attempts to remove indirection when using OP_LDADDR (which fetches the address of a variable). For example :

	ldaddr r1 <- r0
	load_mem r2 <- r1

is equivalent to

	mov r2 <- r0

The remembered r1 alias needs to be forgotten in case r1 is changed. We were handling this for direct assignment with MOV, but not for the more subtle case where an alias to r1 would be passed to a call, which could lead to changing r1.

mono#14872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.