[arm] respect I1 and I2 on gsharedvt out transition (mono/mono#16000)

[arm] respect I1 and I2 on gsharedvt out transition

In order to trigger the problem, we must call into gsharedvt code ("in" transition) and then escape the gsharedvt world ("out" transition). The bug happens in the latter. Also some other stars need to align (I haven't fully understood it), thus the repro is quite long.

The provided repro only triggers with LLVM, which we don't have enabled on CI yet: https://github.com/mono/mono/issues/15999

Fixes https://github.com/mono/mono/issues/15261
Fixes https://github.com/mono/mono/issues/15262
Fixes https://github.com/mono/mono/issues/15263
Fixes https://github.com/mono/mono/issues/15307

Commit migrated from https://github.com/mono/mono/commit/fe0f311a848068ab2d17a9b9dd15326e5712d520

diff --git a/src/mono/mono/mini/gshared.cs b/src/mono/mono/mini/gshared.cs
index 823e596..db44168 100644 (file)
--- a/src/mono/mono/mini/gshared.cs
+++ b/src/mono/mono/mini/gshared.cs
@@ -1893,6 +1893,84 @@ public class Tests
                return obj.foo<IFaceTest, int> (ref t);
        }
 
+
+       class MyBaseTest<TOutput> {
+               public void Verify<TInput> (Func<TInput, TOutput> convert, TInput[] testValues, TOutput[] expectedValues) {
+                       MyAssert.Equal (expectedValues.Length, testValues.Length);
+
+                       for (int i = 0; i < testValues.Length; i++) {
+                               TOutput result = convert (testValues[i]);
+                               MyAssert.Equal (expectedValues[i], result);
+                       }
+               }
+       }
+
+       class MyAssert {
+               public static void Equal<T>(T expected, T actual) {
+                       if (!GetEqualityComparer<T> ().Equals (expected, actual))
+                               throw new MyEqualException (expected, actual);
+               }
+
+               static IEqualityComparer<T> GetEqualityComparer<T>() {
+                       return new AssertEqualityComparer<T>();
+               }
+       }
+
+       class AssertEqualityComparer<T> : IEqualityComparer<T> {
+               public AssertEqualityComparer() { }
+
+               public bool Equals(T x, T y) {
+                       var equatable = x as IEquatable<T>;
+                       if (equatable != null)
+                               return equatable.Equals (y);
+                       throw new NotImplementedException( );
+               }
+
+               public int GetHashCode(T obj) {
+                       throw new NotImplementedException();
+               }
+       }
+
+       class MyEqualException : Exception {
+               public MyEqualException (object expected, object actual) {
+                       Console.WriteLine ("MyEqualException: expected = " + expected + " vs. actual = " + actual);
+               }
+       }
+
+       class SByteTestClass : MyBaseTest<sbyte> {
+               public static int execute () {
+                       Int32[] testValues = { 100, -100, 0 };
+                       SByte[] expectedValues = { 100, -100, 0 };
+                       try {
+                               new SByteTestClass ().Verify (Convert.ToSByte, testValues, expectedValues);
+                               return 0;
+                       } catch (MyEqualException) {
+                               return 1;
+                       }
+                       return 2;
+               }
+       }
+       static int test_0_out_sbyte () {
+               return SByteTestClass.execute ();
+       }
+
+       class Int16TestClass : MyBaseTest<Int16> {
+               public static int execute () {
+                       Int32[] testValues = { 100, -100, 0 };
+                       Int16[] expectedValues = { 100, -100, 0 };
+                       try {
+                               new Int16TestClass ().Verify (Convert.ToInt16, testValues, expectedValues);
+                               return 0;
+                       } catch (MyEqualException) {
+                               return 1;
+                       }
+                       return 2;
+               }
+       }
+       static int test_0_out_int16 () {
+               return Int16TestClass.execute ();
+       }
+
        // Sign extension tests
        // 0x55   == 85    == 01010101
        // 0xAA   == 170   == 10101010

diff --git a/src/mono/mono/mini/mini-amd64-gsharedvt.c b/src/mono/mono/mini/mini-amd64-gsharedvt.c
index b6c2c9e..d929a91 100644 (file)
--- a/src/mono/mono/mini/mini-amd64-gsharedvt.c
+++ b/src/mono/mono/mini/mini-amd64-gsharedvt.c
@@ -379,16 +379,15 @@ mono_arch_get_gsharedvt_call_info (gpointer addr, MonoMethodSignature *normal_si
 
                if (arg_marshal == GSHAREDVT_ARG_BYREF_TO_BYVAL && dst_info->byte_arg_size) {
                        /* Have to load less than 4 bytes */
-                       // FIXME: Signed types
                        switch (dst_info->byte_arg_size) {
                        case 1:
-                               arg_marshal = GSHAREDVT_ARG_BYREF_TO_BYVAL_U1;
+                               arg_marshal = dst_info->is_signed ? GSHAREDVT_ARG_BYREF_TO_BYVAL_I1 : GSHAREDVT_ARG_BYREF_TO_BYVAL_U1;
                                break;
                        case 2:
-                               arg_marshal = GSHAREDVT_ARG_BYREF_TO_BYVAL_U2;
+                               arg_marshal = dst_info->is_signed ? GSHAREDVT_ARG_BYREF_TO_BYVAL_I2 : GSHAREDVT_ARG_BYREF_TO_BYVAL_U2;
                                break;
                        default:
-                               arg_marshal = GSHAREDVT_ARG_BYREF_TO_BYVAL_U4;
+                               arg_marshal = dst_info->is_signed ? GSHAREDVT_ARG_BYREF_TO_BYVAL_I4 : GSHAREDVT_ARG_BYREF_TO_BYVAL_U4;
                                break;
                        }
                }

diff --git a/src/mono/mono/mini/mini-amd64-gsharedvt.h b/src/mono/mono/mini/mini-amd64-gsharedvt.h
index 5cc7a48..539dff9 100644 (file)
--- a/src/mono/mono/mini/mini-amd64-gsharedvt.h
+++ b/src/mono/mono/mini/mini-amd64-gsharedvt.h
@@ -18,8 +18,11 @@ typedef enum {
        GSHAREDVT_ARG_NONE = 0,
        GSHAREDVT_ARG_BYVAL_TO_BYREF,
        GSHAREDVT_ARG_BYREF_TO_BYVAL,
+       GSHAREDVT_ARG_BYREF_TO_BYVAL_I1,
        GSHAREDVT_ARG_BYREF_TO_BYVAL_U1,
+       GSHAREDVT_ARG_BYREF_TO_BYVAL_I2,
        GSHAREDVT_ARG_BYREF_TO_BYVAL_U2,
+       GSHAREDVT_ARG_BYREF_TO_BYVAL_I4,
        GSHAREDVT_ARG_BYREF_TO_BYVAL_U4
 } GSharedVtArgMarshal;
 

diff --git a/src/mono/mono/mini/mini-amd64.c b/src/mono/mono/mini/mini-amd64.c
index ed95c38..c069cc6 100644 (file)
--- a/src/mono/mono/mini/mini-amd64.c
+++ b/src/mono/mono/mini/mini-amd64.c
@@ -1000,16 +1000,19 @@ get_call_info (MonoMemPool *mp, MonoMethodSignature *sig)
                ptype = mini_get_underlying_type (sig->params [i]);
                switch (ptype->type) {
                case MONO_TYPE_I1:
+                       ainfo->is_signed = 1;
                case MONO_TYPE_U1:
                        add_general (&gr, &stack_size, ainfo);
                        ainfo->byte_arg_size = 1;
                        break;
                case MONO_TYPE_I2:
+                       ainfo->is_signed = 1;
                case MONO_TYPE_U2:
                        add_general (&gr, &stack_size, ainfo);
                        ainfo->byte_arg_size = 2;
                        break;
                case MONO_TYPE_I4:
+                       ainfo->is_signed = 1;
                case MONO_TYPE_U4:
                        add_general (&gr, &stack_size, ainfo);
                        ainfo->byte_arg_size = 4;

diff --git a/src/mono/mono/mini/mini-amd64.h b/src/mono/mono/mini/mini-amd64.h
index 8c3db26..2cc821b 100644 (file)
--- a/src/mono/mono/mini/mini-amd64.h
+++ b/src/mono/mono/mini/mini-amd64.h
@@ -323,6 +323,7 @@ typedef struct {
        // Size in bytes for small arguments
        int byte_arg_size;
        guint8 pass_empty_struct : 1; // Set in scenarios when empty structs needs to be represented as argument.
+       guint8 is_signed : 1;
 } ArgInfo;
 
 struct CallInfo {

diff --git a/src/mono/mono/mini/mini-arm-gsharedvt.c b/src/mono/mono/mini/mini-arm-gsharedvt.c
index 92ba802..04bfa88 100644 (file)
--- a/src/mono/mono/mini/mini-arm-gsharedvt.c
+++ b/src/mono/mono/mini/mini-arm-gsharedvt.c
@@ -221,13 +221,12 @@ mono_arch_get_gsharedvt_call_info (gpointer addr, MonoMethodSignature *normal_si
 
                        if (ainfo2->storage == RegTypeGeneral && ainfo2->size != 0 && ainfo2->size != sizeof (target_mgreg_t)) {
                                /* Have to load less than 4 bytes */
-                               // FIXME: Signed types
                                switch (ainfo2->size) {
                                case 1:
-                                       arg_marshal = GSHAREDVT_ARG_BYREF_TO_BYVAL_U1;
+                                       arg_marshal = ainfo2->is_signed ? GSHAREDVT_ARG_BYREF_TO_BYVAL_I1 : GSHAREDVT_ARG_BYREF_TO_BYVAL_U1;
                                        break;
                                case 2:
-                                       arg_marshal = GSHAREDVT_ARG_BYREF_TO_BYVAL_U2;
+                                       arg_marshal = ainfo2->is_signed ? GSHAREDVT_ARG_BYREF_TO_BYVAL_I2 : GSHAREDVT_ARG_BYREF_TO_BYVAL_U2;
                                        break;
                                default:
                                        g_assert_not_reached ();

diff --git a/src/mono/mono/mini/mini-arm.c b/src/mono/mono/mini/mini-arm.c
index f581655..ff8122c 100644 (file)
--- a/src/mono/mono/mini/mini-arm.c
+++ b/src/mono/mono/mini/mini-arm.c
@@ -1431,11 +1431,13 @@ get_call_info (MonoMemPool *mp, MonoMethodSignature *sig)
                t = mini_get_underlying_type (sig->params [i]);
                switch (t->type) {
                case MONO_TYPE_I1:
+                       cinfo->args [n].is_signed = 1;
                case MONO_TYPE_U1:
                        cinfo->args [n].size = 1;
                        add_general (&gr, &stack_size, ainfo, TRUE);
                        break;
                case MONO_TYPE_I2:
+                       cinfo->args [n].is_signed = 1;
                case MONO_TYPE_U2:
                        cinfo->args [n].size = 2;
                        add_general (&gr, &stack_size, ainfo, TRUE);

diff --git a/src/mono/mono/mini/mini-arm.h b/src/mono/mono/mini/mini-arm.h
index 856c642..1f0df87 100644 (file)
--- a/src/mono/mono/mini/mini-arm.h
+++ b/src/mono/mono/mini/mini-arm.h
@@ -211,6 +211,7 @@ typedef struct {
        /* RegTypeStructByVal */
        gint32  struct_size, align;
        guint8  size    : 4; /* 1, 2, 4, 8, or regs used by RegTypeStructByVal */
+       guint8  is_signed : 1;
 } ArgInfo;
 
 struct CallInfo {

diff --git a/src/mono/mono/mini/tramp-amd64-gsharedvt.c b/src/mono/mono/mini/tramp-amd64-gsharedvt.c
index 476b755..9d2d5a5 100644 (file)
--- a/src/mono/mono/mini/tramp-amd64-gsharedvt.c
+++ b/src/mono/mono/mini/tramp-amd64-gsharedvt.c
@@ -88,6 +88,13 @@ mono_amd64_start_gsharedvt_call (GSharedVtCallInfo *info, gpointer *caller, gpoi
                        DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- [%d] (%d words) (%p) <- (%p)\n", dest_reg, source_reg, slot_count, &callee [dest_reg], &caller [source_reg]);
                        break;
                }
+               case GSHAREDVT_ARG_BYREF_TO_BYVAL_I1: {
+                       gint8 *addr = (gint8*)caller [source_reg];
+
+                       callee [dest_reg] = (gpointer)(host_mgreg_t)*addr;
+                       DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- (i1) [%d] (%p) <- (%p)\n", dest_reg, source_reg, &callee [dest_reg], &caller [source_reg]);
+                       break;
+               }
                case GSHAREDVT_ARG_BYREF_TO_BYVAL_U1: {
                        guint8 *addr = (guint8*)caller [source_reg];
 
@@ -95,6 +102,13 @@ mono_amd64_start_gsharedvt_call (GSharedVtCallInfo *info, gpointer *caller, gpoi
                        DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- (u1) [%d] (%p) <- (%p)\n", dest_reg, source_reg, &callee [dest_reg], &caller [source_reg]);
                        break;
                }
+               case GSHAREDVT_ARG_BYREF_TO_BYVAL_I2: {
+                       gint16 *addr = (gint16*)caller [source_reg];
+
+                       callee [dest_reg] = (gpointer)(host_mgreg_t)*addr;
+                       DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- (i2) [%d] (%p) <- (%p)\n", dest_reg, source_reg, &callee [dest_reg], &caller [source_reg]);
+                       break;
+               }
                case GSHAREDVT_ARG_BYREF_TO_BYVAL_U2: {
                        guint16 *addr = (guint16*)caller [source_reg];
 
@@ -102,6 +116,13 @@ mono_amd64_start_gsharedvt_call (GSharedVtCallInfo *info, gpointer *caller, gpoi
                        DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- (u2) [%d] (%p) <- (%p)\n", dest_reg, source_reg, &callee [dest_reg], &caller [source_reg]);
                        break;
                }
+               case GSHAREDVT_ARG_BYREF_TO_BYVAL_I4: {
+                       gint32 *addr = (gint32*)caller [source_reg];
+
+                       callee [dest_reg] = (gpointer)(host_mgreg_t)*addr;
+                       DEBUG_AMD64_GSHAREDVT_PRINT ("[%d] <- (i4) [%d] (%p) <- (%p)\n", dest_reg, source_reg, &callee [dest_reg], &caller [source_reg]);
+                       break;
+               }
                case GSHAREDVT_ARG_BYREF_TO_BYVAL_U4: {
                        guint32 *addr = (guint32*)caller [source_reg];