Merge pull request #3085 from lateralusX/jlorenss/win-x64-pinvoke-empty-struct
authorJohan Lorensson <lateralusx.github@gmail.com>
Thu, 9 Jun 2016 13:49:33 +0000 (15:49 +0200)
committerJohan Lorensson <lateralusx.github@gmail.com>
Thu, 9 Jun 2016 13:49:33 +0000 (15:49 +0200)
Fix for non GCC win x64 calling convention when passing empty structs over pinvoke.

1  2 
mono/mini/mini-amd64.c
mono/mini/mini-amd64.h

diff --combined mono/mini/mini-amd64.c
index 3e59c9da16d42fd0f6aae2165428f3ff2ba3d523,71db6b5b283c888811dc2a0ff9b45c7c14d8ebfd..4771ab861844e3f850c96be5d0d2470403076d39
@@@ -392,6 -392,17 +392,17 @@@ add_valuetype_win64 (MonoMethodSignatur
  
        klass = mono_class_from_mono_type (type);
        size = mini_type_stack_size_full (&klass->byval_arg, NULL, sig->pinvoke);
+       /*
+       * Standard C and C++ doesn't allow empty structs, empty structs will always have a size of 1 byte.
+       * GCC have an extension to allow empty structs, https://gcc.gnu.org/onlinedocs/gcc/Empty-Structures.html.
+       * This cause a little dilemma since runtime build using none GCC compiler will not be compatible with
+       * GCC build C libraries and the other way around. On platforms where empty structs has size of 1 byte
+       * it must be represented in call and cannot be dropped.
+       */
+       if (0 == size && MONO_TYPE_ISSTRUCT (type) && sig->pinvoke)
+               ainfo->pass_empty_struct = TRUE;
+       
        if (!sig->pinvoke)
                pass_on_stack = TRUE;
  
        } else {
                g_assert (info);
  
-               if (!fields) {
+               /*Only drop value type if its not an empty struct as input that must be represented in call*/
+               if ((!fields && !ainfo->pass_empty_struct) || (!fields && ainfo->pass_empty_struct && is_return)) {
                        ainfo->storage = ArgValuetypeInReg;
                        ainfo->pair_storage [0] = ainfo->pair_storage [1] = ArgNone;
                        return;
                }
  
                switch (info->native_size) {
+               case 0:
+                       g_assert (!fields && MONO_TYPE_ISSTRUCT (type) && !is_return);
+                       break;
                case 1: case 2: case 4: case 8:
                        break;
                default:
                guint32 align;
                ArgumentClass class1;
  
-               if (nfields == 0)
+               if (nfields == 0 && ainfo->pass_empty_struct) {
+                       g_assert (!fields && !is_return);
+                       class1 = ARG_CLASS_INTEGER;
+               }
+               else if (nfields == 0)
                        class1 = ARG_CLASS_MEMORY;
                else
                        class1 = ARG_CLASS_NO_CLASS;
@@@ -586,6 -605,7 +605,7 @@@ add_valuetype (MonoMethodSignature *sig
  
        klass = mono_class_from_mono_type (type);
        size = mini_type_stack_size_full (&klass->byval_arg, NULL, sig->pinvoke);
        if (!sig->pinvoke && ((is_return && (size == 8)) || (!is_return && (size <= 16)))) {
                /* We pass and return vtypes of size 8 in a register */
        } else if (!sig->pinvoke || (size == 0) || (size > 16)) {
  /*
   * get_call_info:
   *
 - *  Obtain information about a call according to the calling convention.
 - * For AMD64, see the "System V ABI, x86-64 Architecture Processor Supplement 
 + * Obtain information about a call according to the calling convention.
 + * For AMD64 System V, see the "System V ABI, x86-64 Architecture Processor Supplement
   * Draft Version 0.23" document for more information.
 + * For AMD64 Windows, see "Overview of x64 Calling Conventions",
 + * https://msdn.microsoft.com/en-us/library/ms235286.aspx
   */
  static CallInfo*
  get_call_info (MonoMemPool *mp, MonoMethodSignature *sig)
@@@ -2112,7 -2130,7 +2132,7 @@@ mono_arch_emit_call (MonoCompile *cfg, 
                                /* Continue normally */
                        }
  
-                       if (size > 0) {
+                       if (size > 0 || ainfo->pass_empty_struct) {
                                MONO_INST_NEW (cfg, arg, OP_OUTARG_VT);
                                arg->sreg1 = in->dreg;
                                arg->klass = mono_class_from_mono_type (t);
@@@ -2210,21 -2228,28 +2230,28 @@@ mono_arch_emit_outarg_vt (MonoCompile *
                        if (ainfo->pair_storage [part] == ArgNone)
                                continue;
  
-                       MONO_INST_NEW (cfg, load, arg_storage_to_load_membase (ainfo->pair_storage [part]));
-                       load->inst_basereg = src->dreg;
-                       load->inst_offset = part * sizeof(mgreg_t);
-                       switch (ainfo->pair_storage [part]) {
-                       case ArgInIReg:
-                               load->dreg = mono_alloc_ireg (cfg);
-                               break;
-                       case ArgInDoubleSSEReg:
-                       case ArgInFloatSSEReg:
-                               load->dreg = mono_alloc_freg (cfg);
-                               break;
-                       default:
-                               g_assert_not_reached ();
+                       if (ainfo->pass_empty_struct) {
+                               //Pass empty struct value as 0 on platforms representing empty structs as 1 byte.
+                               NEW_ICONST (cfg, load, 0);
                        }
+                       else {
+                               MONO_INST_NEW (cfg, load, arg_storage_to_load_membase (ainfo->pair_storage [part]));
+                               load->inst_basereg = src->dreg;
+                               load->inst_offset = part * sizeof(mgreg_t);
+                               switch (ainfo->pair_storage [part]) {
+                               case ArgInIReg:
+                                       load->dreg = mono_alloc_ireg (cfg);
+                                       break;
+                               case ArgInDoubleSSEReg:
+                               case ArgInFloatSSEReg:
+                                       load->dreg = mono_alloc_freg (cfg);
+                                       break;
+                               default:
+                                       g_assert_not_reached ();
+                               }
+                       }
                        MONO_ADD_INS (cfg->cbb, load);
  
                        add_outarg_reg (cfg, call, ainfo->pair_storage [part], ainfo->pair_regs [part], load);
@@@ -2327,12 -2352,15 +2354,12 @@@ dyn_call_supported (MonoMethodSignatur
  {
        int i;
  
 -#ifdef HOST_WIN32
 -      return FALSE;
 -#endif
 -
        switch (cinfo->ret.storage) {
        case ArgNone:
        case ArgInIReg:
        case ArgInFloatSSEReg:
        case ArgInDoubleSSEReg:
 +      case ArgValuetypeAddrInIReg:
                break;
        case ArgValuetypeInReg: {
                ArgInfo *ainfo = &cinfo->ret;
                        if (ainfo->pair_storage [1] != ArgNone && ainfo->pair_storage [1] != ArgInIReg)
                                return FALSE;
                        break;
 +              case ArgOnStack:
 +                      if (!(ainfo->offset + (ainfo->arg_size / 8) <= DYN_CALL_STACK_ARGS))
 +                              return FALSE;
 +                      break;
                default:
                        return FALSE;
                }
@@@ -2440,15 -2464,6 +2467,15 @@@ mono_arch_start_dyn_call (MonoDynCallIn
        int arg_index, greg, freg, i, pindex;
        MonoMethodSignature *sig = dinfo->sig;
        int buffer_offset = 0;
 +      static int param_reg_to_index [16];
 +      static gboolean param_reg_to_index_inited;
 +
 +      if (!param_reg_to_index_inited) {
 +              for (i = 0; i < PARAM_REGS; ++i)
 +                      param_reg_to_index [param_regs [i]] = i;
 +              mono_memory_barrier ();
 +              param_reg_to_index_inited = 1;
 +      }
  
        g_assert (buf_len >= sizeof (DynCallArgs));
  
        if (dinfo->cinfo->ret.storage == ArgValuetypeAddrInIReg || dinfo->cinfo->ret.storage == ArgGsharedvtVariableInReg)
                p->regs [greg ++] = PTR_TO_GREG(ret);
  
 -      for (i = pindex; i < sig->param_count; i++) {
 -              MonoType *t = mini_get_underlying_type (sig->params [i]);
 +      for (; pindex < sig->param_count; pindex++) {
 +              MonoType *t = mini_get_underlying_type (sig->params [pindex]);
                gpointer *arg = args [arg_index ++];
 +              ArgInfo *ainfo = &dinfo->cinfo->args [pindex + sig->hasthis];
 +              int slot;
 +
 +              if (ainfo->storage == ArgOnStack) {
 +                      slot = PARAM_REGS + (ainfo->offset / sizeof (mgreg_t));
 +              } else {
 +                      slot = param_reg_to_index [ainfo->reg];
 +              }
  
                if (t->byref) {
 -                      p->regs [greg ++] = PTR_TO_GREG(*(arg));
 +                      p->regs [slot] = PTR_TO_GREG(*(arg));
 +                      greg ++;
                        continue;
                }
  
                case MONO_TYPE_I8:
                case MONO_TYPE_U8:
  #endif
 -                      g_assert (dinfo->cinfo->args [i + sig->hasthis].reg == param_regs [greg]);
 -                      p->regs [greg ++] = PTR_TO_GREG(*(arg));
 +                      p->regs [slot] = PTR_TO_GREG(*(arg));
                        break;
  #if defined(__mono_ilp32__)
                case MONO_TYPE_I8:
                case MONO_TYPE_U8:
 -                      g_assert (dinfo->cinfo->args [i + sig->hasthis].reg == param_regs [greg]);
 -                      p->regs [greg ++] = *(guint64*)(arg);
 +                      p->regs [slot] = *(guint64*)(arg);
                        break;
  #endif
                case MONO_TYPE_U1:
 -                      p->regs [greg ++] = *(guint8*)(arg);
 +                      p->regs [slot] = *(guint8*)(arg);
                        break;
                case MONO_TYPE_I1:
 -                      p->regs [greg ++] = *(gint8*)(arg);
 +                      p->regs [slot] = *(gint8*)(arg);
                        break;
                case MONO_TYPE_I2:
 -                      p->regs [greg ++] = *(gint16*)(arg);
 +                      p->regs [slot] = *(gint16*)(arg);
                        break;
                case MONO_TYPE_U2:
 -                      p->regs [greg ++] = *(guint16*)(arg);
 +                      p->regs [slot] = *(guint16*)(arg);
                        break;
                case MONO_TYPE_I4:
 -                      p->regs [greg ++] = *(gint32*)(arg);
 +                      p->regs [slot] = *(gint32*)(arg);
                        break;
                case MONO_TYPE_U4:
 -                      p->regs [greg ++] = *(guint32*)(arg);
 +                      p->regs [slot] = *(guint32*)(arg);
                        break;
                case MONO_TYPE_R4: {
                        double d;
                        break;
                case MONO_TYPE_GENERICINST:
                    if (MONO_TYPE_IS_REFERENCE (t)) {
 -                              p->regs [greg ++] = PTR_TO_GREG(*(arg));
 +                              p->regs [slot] = PTR_TO_GREG(*(arg));
                                break;
                        } else if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type (t))) {
                                        MonoClass *klass = mono_class_from_mono_type (t);
                                /* Fall through */
                        }
                case MONO_TYPE_VALUETYPE: {
 -                      ArgInfo *ainfo = &dinfo->cinfo->args [i + sig->hasthis];
 -
 -                      g_assert (ainfo->storage == ArgValuetypeInReg);
 -                      if (ainfo->pair_storage [0] != ArgNone) {
 -                              g_assert (ainfo->pair_storage [0] == ArgInIReg);
 -                              p->regs [greg ++] = ((mgreg_t*)(arg))[0];
 -                      }
 -                      if (ainfo->pair_storage [1] != ArgNone) {
 -                              g_assert (ainfo->pair_storage [1] == ArgInIReg);
 -                              p->regs [greg ++] = ((mgreg_t*)(arg))[1];
 +                      switch (ainfo->storage) {
 +                      case ArgValuetypeInReg:
 +                              if (ainfo->pair_storage [0] != ArgNone) {
 +                                      slot = param_reg_to_index [ainfo->pair_regs [0]];
 +                                      g_assert (ainfo->pair_storage [0] == ArgInIReg);
 +                                      p->regs [slot] = ((mgreg_t*)(arg))[0];
 +                              }
 +                              if (ainfo->pair_storage [1] != ArgNone) {
 +                                      slot = param_reg_to_index [ainfo->pair_regs [1]];
 +                                      g_assert (ainfo->pair_storage [1] == ArgInIReg);
 +                                      p->regs [slot] = ((mgreg_t*)(arg))[1];
 +                              }
 +                              break;
 +                      case ArgOnStack:
 +                              for (i = 0; i < ainfo->arg_size / 8; ++i)
 +                                      p->regs [slot + i] = ((mgreg_t*)(arg))[i];
 +                              break;
 +                      default:
 +                              g_assert_not_reached ();
 +                              break;
                        }
                        break;
                }
                        g_assert_not_reached ();
                }
        }
 -
 -      g_assert (greg <= PARAM_REGS);
  }
  
  /*
@@@ -4716,12 -4716,6 +4743,12 @@@ mono_arch_output_basic_block (MonoCompi
                                amd64_sse_movsd_reg_membase (code, i, AMD64_R11, MONO_STRUCT_OFFSET (DynCallArgs, fregs) + (i * sizeof (double)));
                        amd64_patch (label, code);
  
 +                      /* Set stack args */
 +                      for (i = 0; i < DYN_CALL_STACK_ARGS; ++i) {
 +                              amd64_mov_reg_membase (code, AMD64_RAX, AMD64_R11, MONO_STRUCT_OFFSET (DynCallArgs, regs) + ((PARAM_REGS + i) * sizeof(mgreg_t)), sizeof(mgreg_t));
 +                              amd64_mov_membase_reg (code, AMD64_RSP, i * sizeof (mgreg_t), AMD64_RAX, sizeof (mgreg_t));
 +                      }
 +
                        /* Set argument registers */
                        for (i = 0; i < PARAM_REGS; ++i)
                                amd64_mov_reg_membase (code, param_regs [i], AMD64_R11, i * sizeof(mgreg_t), sizeof(mgreg_t));
diff --combined mono/mini/mini-amd64.h
index 68aaf3925464c1d3b7d4b27adef32eafa586d2df,43b825c9ca606c98937d6187cb0bf0e2941d7c77..11fc30a435f28b08366bd6cae009fe21602147d4
@@@ -230,10 -230,8 +230,10 @@@ typedef struct 
        gpointer bp_addrs [MONO_ZERO_LEN_ARRAY];
  } SeqPointInfo;
  
 +#define DYN_CALL_STACK_ARGS 6
 +
  typedef struct {
 -      mgreg_t regs [PARAM_REGS];
 +      mgreg_t regs [PARAM_REGS + DYN_CALL_STACK_ARGS];
        mgreg_t res;
        guint8 *ret;
        double fregs [8];
@@@ -269,6 -267,7 +269,7 @@@ typedef struct 
        int nregs;
        /* Only if storage == ArgOnStack */
        int arg_size; // Bytes, will always be rounded up/aligned to 8 byte boundary
+       gboolean pass_empty_struct; // Set in scenarios when empty structs needs to be represented as argument.
  } ArgInfo;
  
  typedef struct {
  
  #define MONO_ARCH_GSHARED_SUPPORTED 1
  #define MONO_ARCH_DYN_CALL_SUPPORTED 1
 -#define MONO_ARCH_DYN_CALL_PARAM_AREA 0
 +#define MONO_ARCH_DYN_CALL_PARAM_AREA (DYN_CALL_STACK_ARGS * 8)
  
  #define MONO_ARCH_LLVM_SUPPORTED 1
  #define MONO_ARCH_HAVE_HANDLER_BLOCK_GUARD 1
  #define MONO_ARCH_HAVE_TLS_GET_REG 1
  #endif
  
 -#if !defined (TARGET_WIN32)
  #define MONO_ARCH_GSHAREDVT_SUPPORTED 1
 -#endif
  
  
  #if defined(TARGET_APPLETVOS)