Extract cleanup changes from #21793. (#21852)

* Cleanup changes from #21793.

* Emit the data pointer offset directly into the IL stream (and calculate it as needed instead of passing it through)

* Fix broken assumption that OverrideProcArgs::na::m_pMT is the array type instead of the element type (which it was).

6 files changed, 85 insertions, 45 deletions

diff --git a/src/vm/ilmarshalers.cpp b/src/vm/ilmarshalers.cpp
index d3869c28b1..1912e02e05 100644
--- a/src/vm/ilmarshalers.cpp
+++ b/src/vm/ilmarshalers.cpp
@@ -3777,10 +3777,6 @@ void ILNativeArrayMarshaler::EmitMarshalArgumentCLRToNative()
         // the other.  Since there is no enforcement of this, apps blithely depend
         // on it.  
         //
-        
-        // The base offset should only be 0 for System.Array parameters for which
-        // OleVariant::GetMarshalerForVarType(vt) should never return NULL.
-        _ASSERTE(m_pargs->na.m_optionalbaseoffset != 0);
 
         EmitSetupSigAndDefaultHomesCLRToNative();
 
@@ -3794,13 +3790,21 @@ void ILNativeArrayMarshaler::EmitMarshalArgumentCLRToNative()
         EmitStoreNativeValue(m_pcsMarshal);
 
         EmitLoadManagedValue(m_pcsMarshal);
-        m_pcsMarshal->EmitBRFALSE(pNullRefLabel);
-
+        m_pcsMarshal->EmitBRFALSE(pNullRefLabel);        
+
+        // COMPAT: We cannot generate the same code that the C# compiler generates for
+        // a fixed() statement on an array since we need to provide a non-null value
+        // for a 0-length array. For compat reasons, we need to preserve old behavior.
+        // Additionally, we need to ensure that we do not pass non-null for a zero-length
+        // array when interacting with GDI/GDI+ since they fail on null arrays but succeed
+        // on 0-length arrays.
         EmitLoadManagedValue(m_pcsMarshal);
         m_pcsMarshal->EmitSTLOC(dwPinnedLocal);
         m_pcsMarshal->EmitLDLOC(dwPinnedLocal);
         m_pcsMarshal->EmitCONV_I();
-        m_pcsMarshal->EmitLDC(m_pargs->na.m_optionalbaseoffset);
+        // Optimize marshalling by emitting the data ptr offset directly into the IL stream
+        // instead of doing an FCall to recalulate it each time when possible.
+        m_pcsMarshal->EmitLDC(ArrayBase::GetDataPtrOffset(m_pargs->m_pMarshalInfo->GetArrayElementTypeHandle().MakeSZArray().GetMethodTable()));
         m_pcsMarshal->EmitADD();
         EmitStoreNativeValue(m_pcsMarshal);
 
@@ -4620,6 +4624,7 @@ LocalDesc ILHiddenLengthArrayMarshaler::GetNativeType()
 LocalDesc ILHiddenLengthArrayMarshaler::GetManagedType()
 {
     LIMITED_METHOD_CONTRACT;
+
     return LocalDesc(ELEMENT_TYPE_OBJECT);
 }
 
@@ -4676,9 +4681,17 @@ void ILHiddenLengthArrayMarshaler::EmitMarshalArgumentCLRToNative()
         m_pcsMarshal->EmitSTLOC(dwPinnedLocal);
 
         // native = pinnedLocal + dataOffset
+
+        // COMPAT: We cannot generate the same code that the C# compiler generates for
+        // a fixed() statement on an array since we need to provide a non-null value
+        // for a 0-length array. For compat reasons, we need to preserve old behavior.
+        EmitLoadManagedValue(m_pcsMarshal);
+        m_pcsMarshal->EmitSTLOC(dwPinnedLocal);
         m_pcsMarshal->EmitLDLOC(dwPinnedLocal);
         m_pcsMarshal->EmitCONV_I();
-        m_pcsMarshal->EmitLDC(m_pargs->na.m_optionalbaseoffset);
+        // Optimize marshalling by emitting the data ptr offset directly into the IL stream
+        // instead of doing an FCall to recalulate it each time.
+        m_pcsMarshal->EmitLDC(ArrayBase::GetDataPtrOffset(m_pargs->m_pMarshalInfo->GetArrayElementTypeHandle().MakeSZArray().GetMethodTable()));
         m_pcsMarshal->EmitADD();
         EmitStoreNativeValue(m_pcsMarshal);
 
@@ -4974,11 +4987,6 @@ bool ILHiddenLengthArrayMarshaler::CanUsePinnedArray()
         return false;
     }
 
-    if (m_pargs->na.m_optionalbaseoffset == 0)
-    {
-        return false;
-    }
-
     return true;
 }
 
@@ -5123,7 +5131,7 @@ MethodDesc *ILHiddenLengthArrayMarshaler::GetExactMarshalerMethod(MethodDesc *pG
         pGenericMD,
         pGenericMD->GetMethodTable(),
         FALSE,                                 // forceBoxedEntryPoint
-        m_pargs->na.m_pMT->GetInstantiation(), // methodInst
+        m_pargs->m_pMarshalInfo->GetArrayElementTypeHandle().GetInstantiation(), // methodInst
         FALSE,                                 // allowInstParam
         TRUE);                                 // forceRemotableMethod
 }
diff --git a/src/vm/ilmarshalers.h b/src/vm/ilmarshalers.h
index 6a67bf686e..056690346e 100644
--- a/src/vm/ilmarshalers.h
+++ b/src/vm/ilmarshalers.h
@@ -142,7 +142,6 @@ public:
         }
         CONTRACTL_END;
         
-        CONSISTENCY_CHECK(pManagedType->cbType == 1);
         if (pManagedType->IsValueClass())
         {
             EmitLoadHomeAddr(pslILEmit);    // dest
diff --git a/src/vm/mlinfo.cpp b/src/vm/mlinfo.cpp
index 24af47c708..d220e5dd6d 100644
--- a/src/vm/mlinfo.cpp
+++ b/src/vm/mlinfo.cpp
@@ -2434,7 +2434,7 @@ MarshalInfo::MarshalInfo(Module* pModule,
                                 ParamInfo.m_SafeArrayElementVT = VT_VARIANT;
                             }
                             
-                            IfFailGoto(HandleArrayElemType(&ParamInfo, 0, thElement, -1, FALSE, isParam, pAssembly), lFail);
+                            IfFailGoto(HandleArrayElemType(&ParamInfo, thElement, -1, FALSE, isParam, pAssembly), lFail);
                             break;
                         }
 
@@ -2745,18 +2745,8 @@ MarshalInfo::MarshalInfo(Module* pModule,
                 }
             }
 
-            unsigned ofs = 0;
-            if (arrayTypeHnd.GetMethodTable())
-            {
-                ofs = ArrayBase::GetDataPtrOffset(arrayTypeHnd.GetMethodTable());
-                if (ofs > 0xffff)
-                {
-                    ofs = 0;   // can't represent it, so pass on magic value (which causes fallback to regular ML code)
-                }
-            }
-
             // Handle retrieving the information for the array type.
-            IfFailGoto(HandleArrayElemType(&ParamInfo, (UINT16)ofs, thElement, asArray->GetRank(), mtype == ELEMENT_TYPE_SZARRAY, isParam, pAssembly), lFail);
+            IfFailGoto(HandleArrayElemType(&ParamInfo, thElement, asArray->GetRank(), mtype == ELEMENT_TYPE_SZARRAY, isParam, pAssembly), lFail);
             break;
         }
         
@@ -2958,7 +2948,7 @@ VOID MarshalInfo::EmitOrThrowInteropParamException(NDirectStubLinker* psl, BOOL
 }
 
 
-HRESULT MarshalInfo::HandleArrayElemType(NativeTypeParamInfo *pParamInfo, UINT16 optbaseoffset,  TypeHandle thElement, int iRank, BOOL fNoLowerBounds, BOOL isParam, Assembly *pAssembly)
+HRESULT MarshalInfo::HandleArrayElemType(NativeTypeParamInfo *pParamInfo, TypeHandle thElement, int iRank, BOOL fNoLowerBounds, BOOL isParam, Assembly *pAssembly)
 {
     CONTRACTL
     {
@@ -3047,8 +3037,6 @@ HRESULT MarshalInfo::HandleArrayElemType(NativeTypeParamInfo *pParamInfo, UINT16
     {
         // Retrieve the extra information associated with the native array marshaling.
         m_args.na.m_vt  = m_arrayElementType;
-        m_args.na.m_pMT = !m_hndArrayElemType.IsTypeDesc() ? m_hndArrayElemType.AsMethodTable() : NULL;   
-        m_args.na.m_optionalbaseoffset = optbaseoffset;
         m_countParamIdx = pParamInfo->m_CountParamIdx;
         m_multiplier    = pParamInfo->m_Multiplier;
         m_additive      = pParamInfo->m_Additive;
@@ -3056,10 +3044,7 @@ HRESULT MarshalInfo::HandleArrayElemType(NativeTypeParamInfo *pParamInfo, UINT16
 #ifdef FEATURE_COMINTEROP
     else if (m_type == MARSHAL_TYPE_HIDDENLENGTHARRAY)
     {
-        m_args.na.m_optionalbaseoffset = optbaseoffset;
-
         m_args.na.m_vt  = m_arrayElementType;
-        m_args.na.m_pMT = m_hndArrayElemType.AsMethodTable();
         m_args.na.m_cbElementSize = arrayMarshalInfo.GetElementSize();
         m_args.na.m_redirectedTypeIndex = arrayMarshalInfo.GetRedirectedTypeIndex();
     }
diff --git a/src/vm/mlinfo.h b/src/vm/mlinfo.h
index b31e34f138..0ac146c3b7 100644
--- a/src/vm/mlinfo.h
+++ b/src/vm/mlinfo.h
@@ -83,8 +83,6 @@ struct OverrideProcArgs
         struct
         {
             VARTYPE         m_vt;
-            UINT16          m_optionalbaseoffset; //for fast marshaling, offset of dataptr if known and less than 64k (0 otherwise)
-            MethodTable*    m_pMT;
 #ifdef FEATURE_COMINTEROP
             SIZE_T          m_cbElementSize;
             WinMDAdapter::RedirectedTypeIndex m_redirectedTypeIndex;
@@ -470,8 +468,7 @@ public:
     VOID EmitOrThrowInteropParamException(NDirectStubLinker* psl, BOOL fMngToNative, UINT resID, UINT paramIdx);
 
     // These methods retrieve the information for different element types.
-    HRESULT HandleArrayElemType(NativeTypeParamInfo *pParamInfo, 
-                                UINT16 optbaseoffset, 
+    HRESULT HandleArrayElemType(NativeTypeParamInfo *pParamInfo,
                                 TypeHandle elemTypeHnd, 
                                 int iRank, 
                                 BOOL fNoLowerBounds, 
diff --git a/src/vm/stubgen.cpp b/src/vm/stubgen.cpp
index f1690db63d..67a6ca03e9 100644
--- a/src/vm/stubgen.cpp
+++ b/src/vm/stubgen.cpp
@@ -1314,6 +1314,11 @@ void ILCodeStream::EmitLDC_R8(UINT64 uConst)
 #endif // _WIN64
     Emit(CEE_LDC_R8, 1, (UINT_PTR)uConst);
 }
+void ILCodeStream::EmitLDELEMA(int token)
+{
+    WRAPPER_NO_CONTRACT;
+    Emit(CEE_LDELEMA, -1, token);
+}
 void ILCodeStream::EmitLDELEM_REF()
 {
     WRAPPER_NO_CONTRACT;
@@ -1378,11 +1383,21 @@ void ILCodeStream::EmitLDIND_T(LocalDesc* pType)
 {
     CONTRACTL
     {
-        PRECONDITION(pType->cbType == 1);
+        PRECONDITION(pType->cbType >= 1);
     }
     CONTRACTL_END;
+
+    CorElementType elementType = ELEMENT_TYPE_END;
+
+    bool onlyFoundModifiers = true;
+    for(size_t i = 0; i < pType->cbType && onlyFoundModifiers; i++)
+    {
+        elementType = (CorElementType)pType->ElementType[i];
+        onlyFoundModifiers = (elementType == ELEMENT_TYPE_PINNED);
+    }
     
-    switch (pType->ElementType[0])
+
+    switch (elementType)
     {
         case ELEMENT_TYPE_I1:       EmitLDIND_I1(); break;
         case ELEMENT_TYPE_BOOLEAN:  // fall through
@@ -1577,10 +1592,18 @@ void ILCodeStream::EmitSTIND_T(LocalDesc* pType)
 {
     CONTRACTL
     {
-        PRECONDITION(pType->cbType == 1);
+        PRECONDITION(pType->cbType >= 1);
     }
     CONTRACTL_END;
-    
+
+    CorElementType elementType = ELEMENT_TYPE_END;
+
+    bool onlyFoundModifiers = true;
+    for(size_t i = 0; i < pType->cbType && onlyFoundModifiers; i++)
+    {
+        elementType = (CorElementType)pType->ElementType[i];
+        onlyFoundModifiers = (elementType == ELEMENT_TYPE_PINNED);
+    }
     switch (pType->ElementType[0])
     {
         case ELEMENT_TYPE_I1:       EmitSTIND_I1(); break;
diff --git a/src/vm/stubgen.h b/src/vm/stubgen.h
index 7bebfa7610..044029b0af 100644
--- a/src/vm/stubgen.h
+++ b/src/vm/stubgen.h
@@ -60,17 +60,26 @@ struct LocalDesc
 
     void MakeByRef()
     {
+        LIMITED_METHOD_CONTRACT;
         ChangeType(ELEMENT_TYPE_BYREF);
     }
 
     void MakePinned()
     {
+        LIMITED_METHOD_CONTRACT;
         ChangeType(ELEMENT_TYPE_PINNED);
     }
 
+    void MakeArray()
+    {
+        LIMITED_METHOD_CONTRACT;
+        ChangeType(ELEMENT_TYPE_SZARRAY);
+    }
+
     // makes the LocalDesc semantically equivalent to ET_TYPE_CMOD_REQD<IsCopyConstructed>/ET_TYPE_CMOD_REQD<NeedsCopyConstructorModifier>
     void MakeCopyConstructedPointer()
     {
+        LIMITED_METHOD_CONTRACT;
         ChangeType(ELEMENT_TYPE_PTR);
         bIsCopyConstructed = TRUE;
     }
@@ -96,22 +105,40 @@ struct LocalDesc
             THROWS;
             GC_TRIGGERS;
             MODE_ANY;
-            PRECONDITION(cbType == 1);    // this only works on 1-element types for now
         }
         CONTRACTL_END;
+
+        bool lastElementTypeIsValueType = false;
         
         if (ElementType[0] == ELEMENT_TYPE_VALUETYPE)
         {
-            return true;
+            lastElementTypeIsValueType = true;
         }
         else if ((ElementType[0] == ELEMENT_TYPE_INTERNAL) &&
                     (InternalToken.IsNativeValueType() ||
                      InternalToken.GetMethodTable()->IsValueType()))
         {
-            return true;
+            lastElementTypeIsValueType = true;
+        }
+
+        if (!lastElementTypeIsValueType)
+        {
+             return false;
+        }
+
+        // verify that the prefix element types don't make the type a non-value type
+        // this only works on LocalDescs with the prefixes exposed in the Add* methods above.
+        for (size_t i = 0; i < cbType - 1; i++)
+        {
+            if (ElementType[i] == ELEMENT_TYPE_BYREF
+                || ElementType[i] == ELEMENT_TYPE_SZARRAY
+                || ElementType[i] == ELEMENT_TYPE_PTR)
+            {
+                return false;
+            }
         }
 
-        return false;
+        return true;
     }
 };
 
@@ -595,6 +622,7 @@ public:
     void EmitLDC        (DWORD_PTR uConst);
     void EmitLDC_R4     (UINT32 uConst);
     void EmitLDC_R8     (UINT64 uConst);
+    void EmitLDELEMA    (int token);
     void EmitLDELEM_REF ();
     void EmitLDFLD      (int token);
     void EmitLDFLDA     (int token);