From df804273f7bebbe45cb51f22b748c31b5fbe60cf Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 4 Apr 2019 18:37:57 -0700 Subject: [PATCH] Fix SystemV AMD64 Explicit structure classification (#22041) * Don't bail out on enregistering explicit structs if there are no overlapping fields. * Don't enregister if any unaligned fields. * Enable passing explicit structs by-value by enregistering on systemv. Some edge cases are likely still broken, but just removing our blanket opt-out makes the current tests pass. * Enable MarshalstructAsLayoutExp off-Windows. * Start adding additional tests for explicit layout to try to catch edge cases in SystemV classification. * Added a test that spans across multiple eightbytes and has an overlap in the second eightbyte. * Change repro to use an array of floats and an int field in managed and use a float array for padding in native to force an SSE classification on the first byte. * New algorithm to calculate eightbyte classification by going throw the structure byte-by-byte instead of field-by-field. * Fix updating eightbyte classifications in the loop to actually used the iterated-upon variable. * Consider each element of a fixed array as a separate field (to match native implementations). * Implement correct SystemV classification for fixed buffers in non-blittable structures. Fixed buffers in blittable structures have the managed layout assign classifications, which still is buggy. * Add tests. * Correctly classify blittable fixed buffers. Move "is this field a fixed buffer" tracking into one of the unused bits in FieldDesc as code that isn't in marshalers needs to know about it. * Handle the case where we have a struct that has no fields in an eightbyte that contains (i.e. no fields in the first eight bytes of the structure). * PR feedback. * Only look up FixedBufferAttribute when the type is a value class and the type of the field is a value type. * Use heuristic to determine if a type is a fixed buffer for SystemV classification. * Revert tracking if a field is a fixed buffer in the FieldDesc. * Update comments. * Classify aligned, nonoverlapping, float/double only structures as HFAs even if explicitly laid out * Enable overlapping fields in HFAs. Update NativeType HFA to check for alignment. I checked Godbolt to verify that HFAs for overlapping fields are allowed. * Add HFA tests. * Fix compile errors from HFA alignment check. * Non-valuetypes will never have their managed layout used to classify SystemV eightbytes. * Don't classify a struct with no zero-offset field as an HFA. * Remove duplicate semicolon. * PR feedback. * Add test with 2-field double HFA. * Clean up and add static asserts for struct size. * Add define for static_assert_no_msg to the native test headers * Fix build breaks. * Remove unneeded "size = X bytes" comments. They were holdovers from the .NET Framework test tree. * Use GetNumInstanceFieldBytes instead of GetLayoutInfo()->GetManagedSize() * Fix build break. * Centralize FieldMarshaler offsettting in ClassifyEightBytesWithNativeLayout. * Fix signed/unsigned mismatch * Fix condition to also detect arm64. * Change ifdef to if defined. * Remove duplicate declaration (broken in rebase) * Add some logging in one of the unreproable OSX test failures. * Mark System.Numerics.Vector as intrinsic and don't use the eightbyte classifier to enregister it. * Also explicitly opt-out of HFAs for System.Numerics.Vector`1 for consistency. * Update R2R required version to 3.0. * Remove debugging prints. --- src/inc/readytorun.h | 5 +- src/vm/class.cpp | 38 ++- src/vm/fieldmarshaler.h | 7 + src/vm/methodtable.cpp | 346 +++++++++++++-------- src/vm/readytoruninfo.cpp | 4 +- tests/src/Common/Platform/platformdefines.h | 1 + .../PInvoke/MarshalStructAsLayoutExp.cs | 105 ++++++- .../PInvoke/MarshalStructAsLayoutExp.csproj | 6 +- .../PInvoke/MarshalStructAsLayoutSeq.cs | 110 ++++++- .../PInvoke/MarshalStructAsParamDLL.cpp | 26 ++ .../PInvoke/MarshalStructAsParamDLL.h | 161 +++++++--- .../Interop/StructMarshalling/PInvoke/Struct.cs | 116 +++++++ 12 files changed, 723 insertions(+), 202 deletions(-) diff --git a/src/inc/readytorun.h b/src/inc/readytorun.h index d80a80b..6307bc9 100644 --- a/src/inc/readytorun.h +++ b/src/inc/readytorun.h @@ -16,9 +16,12 @@ #define READYTORUN_SIGNATURE 0x00525452 // 'RTR' #define READYTORUN_MAJOR_VERSION 0x0003 -#define READYTORUN_MINOR_VERSION 0x0002 +#define READYTORUN_MINOR_VERSION 0x0000 +#define MINIMUM_READYTORUN_MAJOR_VERSION 0x003 // R2R Version 2.1 adds the READYTORUN_SECTION_INLINING_INFO section // R2R Version 2.2 adds the READYTORUN_SECTION_PROFILEDATA_INFO section +// R2R Version 3.0 changes calling conventions to correctly handle explicit structures to spec. +// R2R 3.0 is not backward compatible with 2.x. struct READYTORUN_HEADER { diff --git a/src/vm/class.cpp b/src/vm/class.cpp index 1e551a9..af1073f 100644 --- a/src/vm/class.cpp +++ b/src/vm/class.cpp @@ -1243,12 +1243,6 @@ EEClass::CheckForHFA() // This method should be called for valuetypes only _ASSERTE(GetMethodTable()->IsValueType()); - // No HFAs with explicit layout. There may be cases where explicit layout may be still - // eligible for HFA, but it is hard to tell the real intent. Make it simple and just - // unconditionally disable HFAs for explicit layout. - if (HasExplicitFieldOffsetLayout()) - return false; - // The SIMD Intrinsic types are meant to be handled specially and should not be treated as HFA if (GetMethodTable()->IsIntrinsicType()) { @@ -1261,14 +1255,24 @@ EEClass::CheckForHFA() assert(strcmp(namespaceName, "System.Runtime.Intrinsics") == 0); return false; } + + if ((strcmp(className, "Vector`1") == 0) && (strcmp(namespaceName, "System.Numerics") == 0)) + { + return false; + } } CorElementType hfaType = ELEMENT_TYPE_END; FieldDesc *pFieldDescList = GetFieldDescList(); + + bool hasZeroOffsetField = false; + for (UINT i = 0; i < GetNumInstanceFields(); i++) { FieldDesc *pFD = &pFieldDescList[i]; + hasZeroOffsetField |= (pFD->GetOffset() == 0); + CorElementType fieldType = pFD->GetFieldType(); switch (fieldType) @@ -1282,9 +1286,23 @@ EEClass::CheckForHFA() break; case ELEMENT_TYPE_R4: + { + static const int REQUIRED_FLOAT_ALIGNMENT = 4; + if (pFD->GetOffset() % REQUIRED_FLOAT_ALIGNMENT != 0) // HFAs don't have unaligned fields. + { + return false; + } + } + break; case ELEMENT_TYPE_R8: + { + static const int REQUIRED_DOUBLE_ALIGNMENT = 8; + if (pFD->GetOffset() % REQUIRED_DOUBLE_ALIGNMENT != 0) // HFAs don't have unaligned fields. + { + return false; + } + } break; - default: // Not HFA return false; @@ -1310,6 +1328,9 @@ EEClass::CheckForHFA() if (hfaType == ELEMENT_TYPE_END) return false; + + if (!hasZeroOffsetField) // If the struct doesn't have a zero-offset field, it's not an HFA. + return false; int elemSize = (hfaType == ELEMENT_TYPE_R8) ? sizeof(double) : sizeof(float); @@ -1351,7 +1372,8 @@ CorElementType EEClassLayoutInfo::GetNativeHFATypeRaw() case NFT_COPY4: case NFT_COPY8: fieldType = pFieldMarshaler->GetFieldDesc()->GetFieldType(); - if (fieldType != ELEMENT_TYPE_R4 && fieldType != ELEMENT_TYPE_R8) + // An HFA can only have aligned float and double fields + if ((fieldType != ELEMENT_TYPE_R4 && fieldType != ELEMENT_TYPE_R8) || (pFieldMarshaler->GetExternalOffset() % pFieldMarshaler->AlignmentRequirement() != 0)) return ELEMENT_TYPE_END; break; diff --git a/src/vm/fieldmarshaler.h b/src/vm/fieldmarshaler.h index e3eba6c..67ac994 100644 --- a/src/vm/fieldmarshaler.h +++ b/src/vm/fieldmarshaler.h @@ -1038,6 +1038,13 @@ public: return OleVariant::GetElementSizeForVarType(m_vt, GetElementMethodTable()) * m_numElems; } + UINT32 GetNumElements() const + { + LIMITED_METHOD_CONTRACT; + + return m_numElems; + } + MethodTable* GetElementMethodTable() const { return GetElementTypeHandle().GetMethodTable(); diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp index 886bfcb..3e228ee 100644 --- a/src/vm/methodtable.cpp +++ b/src/vm/methodtable.cpp @@ -2275,7 +2275,7 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi } CONTRACTL_END; - WORD numIntroducedFields = GetNumIntroducedInstanceFields(); + DWORD numIntroducedFields = GetNumIntroducedInstanceFields(); // It appears the VM gives a struct with no fields of size 1. // Don't pass in register such structure. @@ -2284,16 +2284,6 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi return false; } - // No struct register passing with explicit layout. There may be cases where explicit layout may be still - // eligible for register struct passing, but it is hard to tell the real intent. Make it simple and just - // unconditionally disable register struct passing for explicit layout. - if (GetClass()->HasExplicitFieldOffsetLayout()) - { - LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithManagedLayout: struct %s has explicit layout; will not be enregistered\n", - nestingLevel * 5, "", this->GetDebugClassName())); - return false; - } - // The SIMD Intrinsic types are meant to be handled specially and should not be passed as struct registers if (IsIntrinsicType()) { @@ -2310,16 +2300,43 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi return false; } + + if ((strcmp(className, "Vector`1") == 0) && (strcmp(namespaceName, "System.Numerics") == 0)) + { + LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithManagedLayout: struct %s is a SIMD intrinsic type; will not be enregistered\n", + nestingLevel * 5, "", this->GetDebugClassName())); + + return false; + } } #ifdef _DEBUG LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify %s (%p), startOffset %d, total struct size %d\n", nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); - int fieldNum = -1; #endif // _DEBUG - FieldDesc *pField = GetApproxFieldDescListRaw(); - FieldDesc *pFieldEnd = pField + numIntroducedFields; + FieldDesc *pFieldStart = GetApproxFieldDescListRaw(); + + CorElementType firstFieldElementType = pFieldStart->GetFieldType(); + + // A fixed buffer type is always a value type that has exactly one value type field at offset 0 + // and who's size is an exact multiple of the size of the field. + // It is possible that we catch a false positive with this check, but that chance is extremely slim + // and the user can always change their structure to something more descriptive of what they want + // instead of adding additional padding at the end of a one-field structure. + // We do this check here to save looking up the FixedBufferAttribute when loading the field + // from metadata. + bool isFixedBuffer = numIntroducedFields == 1 + && ( CorTypeInfo::IsPrimitiveType_NoThrow(firstFieldElementType) + || firstFieldElementType == ELEMENT_TYPE_VALUETYPE) + && (pFieldStart->GetOffset() == 0) + && HasLayout() + && (GetNumInstanceFieldBytes() % pFieldStart->GetSize() == 0); + + if (isFixedBuffer) + { + numIntroducedFields = GetNumInstanceFieldBytes() / pFieldStart->GetSize(); + } // System types are loaded before others, so ByReference would be loaded before Span or any other type that has a // ByReference field. ByReference is the first by-ref-like system type to be loaded (see @@ -2327,14 +2344,23 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi // null, it must be the initial load of ByReference. bool isThisByReferenceOfT = IsByRefLike() && (g_pByReferenceClass == nullptr || HasSameTypeDefAs(g_pByReferenceClass)); - for (; pField < pFieldEnd; pField++) + for (unsigned int fieldIndex = 0; fieldIndex < numIntroducedFields; fieldIndex++) { -#ifdef _DEBUG - ++fieldNum; -#endif // _DEBUG + FieldDesc* pField; + DWORD fieldOffset; - DWORD fieldOffset = pField->GetOffset(); - unsigned normalizedFieldOffset = fieldOffset + startOffsetOfStruct; + if (isFixedBuffer) + { + pField = pFieldStart; + fieldOffset = fieldIndex * pField->GetSize(); + } + else + { + pField = &pFieldStart[fieldIndex]; + fieldOffset = pField->GetOffset(); + } + + unsigned int normalizedFieldOffset = fieldOffset + startOffsetOfStruct; unsigned int fieldSize = pField->GetSize(); _ASSERTE(fieldSize != (unsigned int)-1); @@ -2380,6 +2406,7 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi // use the native classification. If not, continue using the managed layout. if (useNativeLayout && pFieldMT->HasLayout()) { + structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout); } else @@ -2419,7 +2446,7 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi // those fields to be at their natural alignment. LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Field %d %s: offset %d (normalized %d), size %d not at natural alignment; not enregistering struct\n", - nestingLevel * 5, "", fieldNum, fieldNum, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize)); + nestingLevel * 5, "", fieldIndex, fieldIndex, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize)); return false; } @@ -2441,21 +2468,19 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi helperPtr->fieldOffsets[helperPtr->currentUniqueOffsetField] = normalizedFieldOffset; LOG((LF_JIT, LL_EVERYTHING, " %*s**** Field %d %s: offset %d (normalized %d), size %d, currentUniqueOffsetField %d, field type classification %s, chosen field classification %s\n", - nestingLevel * 5, "", fieldNum, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize, helperPtr->currentUniqueOffsetField, + nestingLevel * 5, "", fieldIndex, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize, helperPtr->currentUniqueOffsetField, GetSystemVClassificationTypeName(fieldClassificationType), GetSystemVClassificationTypeName(helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField]))); helperPtr->currentUniqueOffsetField++; #ifdef _DEBUG - ++fieldNum; + ++fieldIndex; #endif // _DEBUG } // Both fields of the special TypedReference struct are handled. - pField = pFieldEnd; - // Done classifying the System.TypedReference struct fields. - continue; + break; } if ((normalizedFieldOffset % fieldSize) != 0) @@ -2464,21 +2489,14 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi // those fields to be at their natural alignment. LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Field %d %s: offset %d (normalized %d), size %d not at natural alignment; not enregistering struct\n", - nestingLevel * 5, "", fieldNum, fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldSize)); + nestingLevel * 5, "", fieldIndex, fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, fieldSize)); return false; } if ((int)normalizedFieldOffset <= helperPtr->largestFieldOffset) { // Find the field corresponding to this offset and update the size if needed. - // We assume that either it matches the offset of a previously seen field, or - // it is an out-of-order offset (the VM does give us structs in non-increasing - // offset order sometimes) that doesn't overlap any other field. - - // REVIEW: will the offset ever match a previously seen field offset for cases that are NOT ExplicitLayout? - // If not, we can get rid of this loop, and just assume the offset is from an out-of-order field. We wouldn't - // need to maintain largestFieldOffset, either, since we would then assume all fields are unique. We could - // also get rid of ReClassifyField(). + // If the offset matches a previously encountered offset, update the classification and field size. int i; for (i = helperPtr->currentUniqueOffsetField - 1; i >= 0; i--) { @@ -2492,15 +2510,12 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi helperPtr->fieldClassifications[i] = ReClassifyField(helperPtr->fieldClassifications[i], fieldClassificationType); LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Field %d %s: offset %d (normalized %d), size %d, union with uniqueOffsetField %d, field type classification %s, reclassified field to %s\n", - nestingLevel * 5, "", fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldSize, i, + nestingLevel * 5, "", fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, fieldSize, i, GetSystemVClassificationTypeName(fieldClassificationType), GetSystemVClassificationTypeName(helperPtr->fieldClassifications[i]))); break; } - // Make sure the field doesn't start in the middle of another field. - _ASSERTE((normalizedFieldOffset < helperPtr->fieldOffsets[i]) || - (normalizedFieldOffset >= helperPtr->fieldOffsets[i] + helperPtr->fieldSizes[i])); } if (i >= 0) @@ -2530,7 +2545,7 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi helperPtr->fieldOffsets[helperPtr->currentUniqueOffsetField] = normalizedFieldOffset; LOG((LF_JIT, LL_EVERYTHING, " %*s**** Field %d %s: offset %d (normalized %d), size %d, currentUniqueOffsetField %d, field type classification %s, chosen field classification %s\n", - nestingLevel * 5, "", fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldSize, helperPtr->currentUniqueOffsetField, + nestingLevel * 5, "", fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, fieldSize, helperPtr->currentUniqueOffsetField, GetSystemVClassificationTypeName(fieldClassificationType), GetSystemVClassificationTypeName(helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField]))); @@ -2571,7 +2586,7 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin return ClassifyEightBytesWithManagedLayout(helperPtr, nestingLevel, startOffsetOfStruct, useNativeLayout); } - const FieldMarshaler *pFieldMarshaler = GetLayoutInfo()->GetFieldMarshalers(); + const FieldMarshaler *pFieldMarshalers = GetLayoutInfo()->GetFieldMarshalers(); UINT numIntroducedFields = GetLayoutInfo()->GetNumCTMFields(); // No fields. @@ -2580,14 +2595,24 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin return false; } - // No struct register passing with explicit layout. There may be cases where explicit layout may be still - // eligible for register struct passing, but it is hard to tell the real intent. Make it simple and just - // unconditionally disable register struct passing for explicit layout. - if (GetClass()->HasExplicitFieldOffsetLayout()) + // A fixed buffer type is always a value type that has exactly one value type field at offset 0 + // and who's size is an exact multiple of the size of the field. + // It is possible that we catch a false positive with this check, but that chance is extremely slim + // and the user can always change their structure to something more descriptive of what they want + // instead of adding additional padding at the end of a one-field structure. + // We do this check here to save looking up the FixedBufferAttribute when loading the field + // from metadata. + CorElementType firstFieldElementType = pFieldMarshalers->GetFieldDesc()->GetFieldType(); + bool isFixedBuffer = numIntroducedFields == 1 + && ( CorTypeInfo::IsPrimitiveType_NoThrow(firstFieldElementType) + || firstFieldElementType == ELEMENT_TYPE_VALUETYPE) + && (pFieldMarshalers->GetExternalOffset() == 0) + && IsValueType() + && (GetLayoutInfo()->GetNativeSize() % pFieldMarshalers->NativeSize() == 0); + + if (isFixedBuffer) { - LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithNativeLayout: struct %s has explicit layout; will not be enregistered\n", - nestingLevel * 5, "", this->GetDebugClassName())); - return false; + numIntroducedFields = GetNativeSize() / pFieldMarshalers->NativeSize(); } // The SIMD Intrinsic types are meant to be handled specially and should not be passed as struct registers @@ -2606,19 +2631,33 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin return false; } + + if ((strcmp(className, "Vector`1") == 0) && (strcmp(namespaceName, "System.Numerics") == 0)) + { + LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithManagedLayout: struct %s is a SIMD intrinsic type; will not be enregistered\n", + nestingLevel * 5, "", this->GetDebugClassName())); + + return false; + } } #ifdef _DEBUG LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify for native struct %s (%p), startOffset %d, total struct size %d\n", nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); - int fieldNum = -1; #endif // _DEBUG - while (numIntroducedFields--) + for (unsigned int fieldIndex = 0; fieldIndex < numIntroducedFields; fieldIndex++) { -#ifdef _DEBUG - ++fieldNum; -#endif // _DEBUG + const FieldMarshaler* pFieldMarshaler; + if (isFixedBuffer) + { + // Reuse the first field marshaler for all fields if a fixed buffer. + pFieldMarshaler = pFieldMarshalers; + } + else + { + pFieldMarshaler = (FieldMarshaler*)(((BYTE*)pFieldMarshalers) + MAXFIELDMARSHALERSIZE * fieldIndex); + } FieldDesc *pField = pFieldMarshaler->GetFieldDesc(); CorElementType fieldType = pField->GetFieldType(); @@ -2629,10 +2668,17 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin return false; } + unsigned int fieldNativeSize = pFieldMarshaler->NativeSize(); DWORD fieldOffset = pFieldMarshaler->GetExternalOffset(); + + if (isFixedBuffer) + { + // Since we reuse the FieldMarshaler for fixed buffers, we need to adjust the offset. + fieldOffset += fieldIndex * fieldNativeSize; + } + unsigned normalizedFieldOffset = fieldOffset + startOffsetOfStruct; - unsigned int fieldNativeSize = pFieldMarshaler->NativeSize(); _ASSERTE(fieldNativeSize != (unsigned int)-1); @@ -2687,17 +2733,27 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin { MethodTable* pFieldMT = ((FieldMarshaler_FixedArray*)pFieldMarshaler)->GetElementMethodTable(); - bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct; - helperPtr->inEmbeddedStruct = true; - bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout); - helperPtr->inEmbeddedStruct = inEmbeddedStructPrev; + unsigned int numElements = ((FieldMarshaler_FixedArray*)pFieldMarshaler)->GetNumElements(); - if (!structRet) + bool structRet = true; + + for (unsigned int i = 0; i < numElements; i++, normalizedFieldOffset += pFieldMT->GetNativeSize()) { - // If the nested struct says not to enregister, there's no need to continue analyzing at this level. Just return do not enregister. - return false; + bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct; + helperPtr->inEmbeddedStruct = true; + structRet = pFieldMT->ClassifyEightBytesWithNativeLayout( + helperPtr, + nestingLevel + 1, + normalizedFieldOffset, + useNativeLayout); + helperPtr->inEmbeddedStruct = inEmbeddedStructPrev; + + if (!structRet) + { + // If the nested struct says not to enregister, there's no need to continue analyzing at this level. Just return do not enregister. + return false; + } } - continue; } case VT_DECIMAL: @@ -2746,7 +2802,11 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct; helperPtr->inEmbeddedStruct = true; - bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout); + bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout( + helperPtr, + nestingLevel + 1, + normalizedFieldOffset, + useNativeLayout); helperPtr->inEmbeddedStruct = inEmbeddedStructPrev; if (!structRet) @@ -2754,7 +2814,6 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin // If the nested struct says not to enregister, there's no need to continue analyzing at this level. Just return do not enregister. return false; } - continue; } else if (cls == NFT_NESTEDVALUECLASS) @@ -2763,7 +2822,11 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct; helperPtr->inEmbeddedStruct = true; - bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout); + bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout( + helperPtr, + nestingLevel + 1, + normalizedFieldOffset, + useNativeLayout); helperPtr->inEmbeddedStruct = inEmbeddedStructPrev; if (!structRet) @@ -2771,7 +2834,6 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin // If the nested struct says not to enregister, there's no need to continue analyzing at this level. Just return do not enregister. return false; } - continue; } else if (cls == NFT_COPY1) @@ -2899,23 +2961,23 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin } } - if ((normalizedFieldOffset % fieldNativeSize) != 0) + if ((normalizedFieldOffset % pFieldMarshaler->AlignmentRequirement()) != 0) { // The spec requires that struct values on the stack from register passed fields expects // those fields to be at their natural alignment. - LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Native Field %d %s: offset %d (normalized %d), native size %d not at natural alignment; not enregistering struct\n", - nestingLevel * 5, "", fieldNum, fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldNativeSize)); + LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Native Field %d %s: offset %d (normalized %d), required alignment %d not at natural alignment; not enregistering struct\n", + nestingLevel * 5, "", fieldIndex, fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, pFieldMarshaler->AlignmentRequirement())); return false; } if ((int)normalizedFieldOffset <= helperPtr->largestFieldOffset) { // Find the field corresponding to this offset and update the size if needed. - // We assume that either it matches the offset of a previously seen field, or - // it is an out-of-order offset (the VM does give us structs in non-increasing - // offset order sometimes) that doesn't overlap any other field. - + // If the offset matches a previously encountered offset, update the classification and field size. + // We do not need to worry about this change incorrectly updating an eightbyte incorrectly + // by updating a field that spans multiple eightbytes since the only field that does so is a fixed array + // and a fixed array is represented by an array object in managed, which nothing can share an offset with. int i; for (i = helperPtr->currentUniqueOffsetField - 1; i >= 0; i--) { @@ -2929,15 +2991,12 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin helperPtr->fieldClassifications[i] = ReClassifyField(helperPtr->fieldClassifications[i], fieldClassificationType); LOG((LF_JIT, LL_EVERYTHING, " %*sxxxx Native Field %d %s: offset %d (normalized %d), native size %d, union with uniqueOffsetField %d, field type classification %s, reclassified field to %s\n", - nestingLevel * 5, "", fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldNativeSize, i, + nestingLevel * 5, "", fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, fieldNativeSize, i, GetSystemVClassificationTypeName(fieldClassificationType), GetSystemVClassificationTypeName(helperPtr->fieldClassifications[i]))); break; } - // Make sure the field doesn't start in the middle of another field. - _ASSERTE((normalizedFieldOffset < helperPtr->fieldOffsets[i]) || - (normalizedFieldOffset >= helperPtr->fieldOffsets[i] + helperPtr->fieldSizes[i])); } if (i >= 0) @@ -2967,13 +3026,12 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin helperPtr->fieldOffsets[helperPtr->currentUniqueOffsetField] = normalizedFieldOffset; LOG((LF_JIT, LL_EVERYTHING, " %*s**** Native Field %d %s: offset %d (normalized %d), size %d, currentUniqueOffsetField %d, field type classification %s, chosen field classification %s\n", - nestingLevel * 5, "", fieldNum, fieldName, fieldOffset, normalizedFieldOffset, fieldNativeSize, helperPtr->currentUniqueOffsetField, + nestingLevel * 5, "", fieldIndex, fieldName, fieldOffset, normalizedFieldOffset, fieldNativeSize, helperPtr->currentUniqueOffsetField, GetSystemVClassificationTypeName(fieldClassificationType), GetSystemVClassificationTypeName(helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField]))); _ASSERTE(helperPtr->currentUniqueOffsetField < SYSTEMV_MAX_NUM_FIELDS_IN_REGISTER_PASSED_STRUCT); helperPtr->currentUniqueOffsetField++; - ((BYTE*&)pFieldMarshaler) += MAXFIELDMARSHALERSIZE; } // end per-field for loop AssignClassifiedEightByteTypes(helperPtr, nestingLevel); @@ -3013,23 +3071,31 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe } // Calculate the eightbytes and their types. - unsigned int accumulatedSizeForEightByte = 0; - unsigned int currentEightByteOffset = 0; - unsigned int currentEightByte = 0; int lastFieldOrdinal = sortedFieldOrder[largestFieldOffset]; unsigned int offsetAfterLastFieldByte = largestFieldOffset + helperPtr->fieldSizes[lastFieldOrdinal]; SystemVClassificationType lastFieldClassification = helperPtr->fieldClassifications[lastFieldOrdinal]; - unsigned offset = 0; - for (unsigned fieldSize = 0; offset < helperPtr->structSize; offset += fieldSize) + unsigned int usedEightBytes = 0; + unsigned int accumulatedSizeForEightBytes = 0; + bool foundFieldInEightByte = false; + for (unsigned int offset = 0; offset < helperPtr->structSize; offset++) { SystemVClassificationType fieldClassificationType; + unsigned int fieldSize = 0; int ordinal = sortedFieldOrder[offset]; if (ordinal == -1) { - // If there is no field that starts as this offset, treat its contents as padding. + if (offset < accumulatedSizeForEightBytes) + { + // We're within a field and there is not an overlapping field that starts here. + // There's no work we need to do, so go to the next loop iteration. + continue; + } + + // If there is no field that starts as this offset and we are not within another field, + // treat its contents as padding. // Any padding that follows the last field receives the same classification as the // last field; padding between fields receives the NO_CLASS classification as per // the SysV ABI spec. @@ -3038,6 +3104,7 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe } else { + foundFieldInEightByte = true; fieldSize = helperPtr->fieldSizes[ordinal]; _ASSERTE(fieldSize > 0); @@ -3045,72 +3112,78 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe _ASSERTE(fieldClassificationType != SystemVClassificationTypeMemory && fieldClassificationType != SystemVClassificationTypeUnknown); } - if (helperPtr->eightByteClassifications[currentEightByte] == fieldClassificationType) - { - // Do nothing. The eight-byte already has this classification. - } - else if (helperPtr->eightByteClassifications[currentEightByte] == SystemVClassificationTypeNoClass) - { - helperPtr->eightByteClassifications[currentEightByte] = fieldClassificationType; - } - else if ((helperPtr->eightByteClassifications[currentEightByte] == SystemVClassificationTypeInteger) || - (fieldClassificationType == SystemVClassificationTypeInteger)) - { - _ASSERTE((fieldClassificationType != SystemVClassificationTypeIntegerReference) && - (fieldClassificationType != SystemVClassificationTypeIntegerByRef)); + unsigned int fieldStartEightByte = offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; + unsigned int fieldEndEightByte = (offset + fieldSize - 1) / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; - helperPtr->eightByteClassifications[currentEightByte] = SystemVClassificationTypeInteger; - } - else if ((helperPtr->eightByteClassifications[currentEightByte] == SystemVClassificationTypeIntegerReference) || - (fieldClassificationType == SystemVClassificationTypeIntegerReference)) - { - helperPtr->eightByteClassifications[currentEightByte] = SystemVClassificationTypeIntegerReference; - } - else if ((helperPtr->eightByteClassifications[currentEightByte] == SystemVClassificationTypeIntegerByRef) || - (fieldClassificationType == SystemVClassificationTypeIntegerByRef)) - { - helperPtr->eightByteClassifications[currentEightByte] = SystemVClassificationTypeIntegerByRef; - } - else - { - helperPtr->eightByteClassifications[currentEightByte] = SystemVClassificationTypeSSE; - } + _ASSERTE(fieldEndEightByte < CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS); - accumulatedSizeForEightByte += fieldSize; - while (accumulatedSizeForEightByte >= SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES) - { - // Save data for this eightbyte. - helperPtr->eightByteSizes[currentEightByte] = SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; - helperPtr->eightByteOffsets[currentEightByte] = currentEightByteOffset; + usedEightBytes = Max(usedEightBytes, fieldEndEightByte + 1); - // Set up for next eightbyte. - currentEightByte++; - _ASSERTE(currentEightByte <= CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS); + for (unsigned int currentFieldEightByte = fieldStartEightByte; currentFieldEightByte <= fieldEndEightByte; currentFieldEightByte++) + { + if (helperPtr->eightByteClassifications[currentFieldEightByte] == fieldClassificationType) + { + // Do nothing. The eight-byte already has this classification. + } + else if (helperPtr->eightByteClassifications[currentFieldEightByte] == SystemVClassificationTypeNoClass) + { + helperPtr->eightByteClassifications[currentFieldEightByte] = fieldClassificationType; + } + else if ((helperPtr->eightByteClassifications[currentFieldEightByte] == SystemVClassificationTypeInteger) || + (fieldClassificationType == SystemVClassificationTypeInteger)) + { + _ASSERTE((fieldClassificationType != SystemVClassificationTypeIntegerReference) && + (fieldClassificationType != SystemVClassificationTypeIntegerByRef)); - currentEightByteOffset += SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; - accumulatedSizeForEightByte -= SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; + helperPtr->eightByteClassifications[currentFieldEightByte] = SystemVClassificationTypeInteger; + } + else if ((helperPtr->eightByteClassifications[currentFieldEightByte] == SystemVClassificationTypeIntegerReference) || + (fieldClassificationType == SystemVClassificationTypeIntegerReference)) + { + helperPtr->eightByteClassifications[currentFieldEightByte] = SystemVClassificationTypeIntegerReference; + } + else if ((helperPtr->eightByteClassifications[currentFieldEightByte] == SystemVClassificationTypeIntegerByRef) || + (fieldClassificationType == SystemVClassificationTypeIntegerByRef)) + { + helperPtr->eightByteClassifications[currentFieldEightByte] = SystemVClassificationTypeIntegerByRef; + } + else + { + helperPtr->eightByteClassifications[currentFieldEightByte] = SystemVClassificationTypeSSE; + } + } - // If a field is large enough to span multiple eightbytes, then set the eightbyte classification to the field's classification. - if (accumulatedSizeForEightByte > 0) + if ((offset + 1) % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0) // If we just finished checking the last byte of an eightbyte + { + if (!foundFieldInEightByte) { - helperPtr->eightByteClassifications[currentEightByte] = fieldClassificationType; + // If we didn't find a field in an eight-byte (i.e. there are no explicit offsets that start a field in this eightbyte) + // then the classification of this eightbyte might be NoClass. We can't hand a classification of NoClass to the JIT + // so set the class to Integer (as though the struct has a char[8] padding) if the class is NoClass. + if (helperPtr->eightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] == SystemVClassificationTypeNoClass) + { + helperPtr->eightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] = SystemVClassificationTypeInteger; + } } + + foundFieldInEightByte = false; } - _ASSERTE(accumulatedSizeForEightByte < SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES); + accumulatedSizeForEightBytes = Max(accumulatedSizeForEightBytes, offset + fieldSize); } - // Handle structs that end in the middle of an eightbyte. - if (accumulatedSizeForEightByte > 0 && accumulatedSizeForEightByte < SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES) + for (unsigned int currentEightByte = 0; currentEightByte < usedEightBytes; currentEightByte++) { - _ASSERTE((helperPtr->structSize % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES) != 0); + unsigned int eightByteSize = accumulatedSizeForEightBytes < (SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES * (currentEightByte + 1)) + ? accumulatedSizeForEightBytes % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES + : SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; - helperPtr->eightByteSizes[currentEightByte] = accumulatedSizeForEightByte; - helperPtr->eightByteOffsets[currentEightByte] = currentEightByteOffset; - currentEightByte++; + // Save data for this eightbyte. + helperPtr->eightByteSizes[currentEightByte] = eightByteSize; + helperPtr->eightByteOffsets[currentEightByte] = currentEightByte * SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES; } - helperPtr->eightByteCount = currentEightByte; + helperPtr->eightByteCount = usedEightBytes; _ASSERTE(helperPtr->eightByteCount <= CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS); @@ -3119,6 +3192,7 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe LOG((LF_JIT, LL_EVERYTHING, " **** Number EightBytes: %d\n", helperPtr->eightByteCount)); for (unsigned i = 0; i < helperPtr->eightByteCount; i++) { + _ASSERTE(helperPtr->eightByteClassifications[i] != SystemVClassificationTypeNoClass); LOG((LF_JIT, LL_EVERYTHING, " **** eightByte %d -- classType: %s, eightByteOffset: %d, eightByteSize: %d\n", i, GetSystemVClassificationTypeName(helperPtr->eightByteClassifications[i]), helperPtr->eightByteOffsets[i], helperPtr->eightByteSizes[i])); } diff --git a/src/vm/readytoruninfo.cpp b/src/vm/readytoruninfo.cpp index 607fcbf..b924637 100644 --- a/src/vm/readytoruninfo.cpp +++ b/src/vm/readytoruninfo.cpp @@ -513,8 +513,8 @@ PTR_ReadyToRunInfo ReadyToRunInfo::Initialize(Module * pModule, AllocMemTracker READYTORUN_HEADER * pHeader = pLayout->GetReadyToRunHeader(); - // Ignore the content if the image major version is higher than the major version currently supported by the runtime - if (pHeader->MajorVersion > READYTORUN_MAJOR_VERSION) + // Ignore the content if the image major version is higher or lower than the major version currently supported by the runtime + if (pHeader->MajorVersion < MINIMUM_READYTORUN_MAJOR_VERSION || pHeader->MajorVersion > READYTORUN_MAJOR_VERSION) { DoLog("Ready to Run disabled - unsupported header version"); return NULL; diff --git a/tests/src/Common/Platform/platformdefines.h b/tests/src/Common/Platform/platformdefines.h index 0c2008b..c76983b 100644 --- a/tests/src/Common/Platform/platformdefines.h +++ b/tests/src/Common/Platform/platformdefines.h @@ -27,6 +27,7 @@ #endif #include +#define static_assert_no_msg(x) static_assert((x), #x) // // types and constants diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.cs b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.cs index 262ec0b..9cffdfa 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.cs +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.cs @@ -15,7 +15,10 @@ public class Managed ByteStructPack2ExplicitId, ShortStructPack4ExplicitId, IntStructPack8ExplicitId, - LongStructPack16ExplicitId + LongStructPack16ExplicitId, + OverlappingLongFloatId, + OverlappingMultipleEightbyteId, + HFAId } [SecuritySafeCritical] @@ -34,7 +37,7 @@ public class Managed if (failures > 0) { Console.WriteLine("\nTEST FAILED!"); - return 101; + return 100 + failures; } else { @@ -209,6 +212,21 @@ public class Managed [DllImport("MarshalStructAsParam")] static extern LongStructPack16Explicit GetLongStruct(long l1, long l2); + [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsExpByValOverlappingLongFloat(OverlappingLongFloat str, long expected); + [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsExpByValOverlappingLongFloat(OverlappingLongFloat2 str, long expected); + + [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsExpByValOverlappingMultipleEightByte(OverlappingMultipleEightbyte str, float i1, float i2, float i3); + + [DllImport("MarshalStructAsParam")] + static extern float ProductHFA(ExplicitHFA hfa); + [DllImport("MarshalStructAsParam")] + static extern float ProductHFA(ExplicitFixedHFA hfa); + [DllImport("MarshalStructAsParam")] + static extern float ProductHFA(OverlappingHFA hfa); + #region Marshal Explicit struct method [SecuritySafeCritical] private static void MarshalStructAsParam_AsExpByVal(StructID id) @@ -351,7 +369,85 @@ public class Managed { failures++; } - break; + break; + case StructID.OverlappingLongFloatId: + OverlappingLongFloat overlappingLongFloat = new OverlappingLongFloat + { + l = 12345, + f = 12.45f + }; + Console.WriteLine("\tCalling MarshalStructAsParam_AsExpByValOverlappingLongFloat..."); + if (!MarshalStructAsParam_AsExpByValOverlappingLongFloat(overlappingLongFloat, overlappingLongFloat.l)) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsExpByValOverlappingLongFloat. Expected:True;Actual:False"); + failures++; + } + OverlappingLongFloat2 overlappingLongFloat2 = new OverlappingLongFloat2 + { + l = 12345, + f = 12.45f + }; + Console.WriteLine("\tCalling MarshalStructAsParam_AsExpByValOverlappingLongFloat (Reversed field order)..."); + if (!MarshalStructAsParam_AsExpByValOverlappingLongFloat(overlappingLongFloat2, overlappingLongFloat.l)) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsExpByValOverlappingLongFloat. Expected:True;Actual:False"); + failures++; + } + break; + case StructID.OverlappingMultipleEightbyteId: + Console.WriteLine("\tCalling MarshalStructAsParam_AsExpByValOverlappingMultipleEightByte..."); + OverlappingMultipleEightbyte overlappingMultipleEightbyte = new OverlappingMultipleEightbyte + { + arr = new float[3] { 1f, 400f, 623289f}, + i = 1234 + }; + if (!MarshalStructAsParam_AsExpByValOverlappingMultipleEightByte( + overlappingMultipleEightbyte, + overlappingMultipleEightbyte.arr[0], + overlappingMultipleEightbyte.arr[1], + overlappingMultipleEightbyte.arr[2])) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsExpByValOverlappingMultipleEightByte. Expected True;Actual:False"); + failures++; + } + break; + case StructID.HFAId: + OverlappingHFA hfa = new OverlappingHFA + { + hfa = new HFA + { + f1 = 2.0f, + f2 = 10.5f, + f3 = 15.2f, + f4 = 0.12f + } + }; + + float expected = hfa.hfa.f1 * hfa.hfa.f2 * hfa.hfa.f3 * hfa.hfa.f4; + float actual; + + Console.WriteLine("\tCalling ProductHFA with Explicit HFA."); + actual = ProductHFA(hfa.explicitHfa); + if (expected != actual) + { + Console.WriteLine($"\tFAILED! Expected {expected}. Actual {actual}"); + failures++; + } + Console.WriteLine("\tCalling ProductHFA with Explicit Fixed HFA."); + actual = ProductHFA(hfa.explicitFixedHfa); + if (expected != actual) + { + Console.WriteLine($"\tFAILED! Expected {expected}. Actual {actual}"); + failures++; + } + Console.WriteLine("\tCalling ProductHFA with Overlapping HFA."); + actual = ProductHFA(hfa); + if (expected != actual) + { + Console.WriteLine($"\tFAILED! Expected {expected}. Actual {actual}"); + failures++; + } + break; default: Console.WriteLine("\tThere is not the struct id"); failures++; @@ -1464,6 +1560,9 @@ public class Managed MarshalStructAsParam_AsExpByVal(StructID.ShortStructPack4ExplicitId); MarshalStructAsParam_AsExpByVal(StructID.IntStructPack8ExplicitId); MarshalStructAsParam_AsExpByVal(StructID.LongStructPack16ExplicitId); + MarshalStructAsParam_AsExpByVal(StructID.OverlappingLongFloatId); + MarshalStructAsParam_AsExpByVal(StructID.OverlappingMultipleEightbyteId); + MarshalStructAsParam_AsExpByVal(StructID.HFAId); } [SecuritySafeCritical] diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.csproj b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.csproj index 2a38e61..4f69d9c 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.csproj +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutExp.csproj @@ -13,10 +13,6 @@ true $(DefineConstants);STATIC 1 - - - true - true @@ -42,4 +38,4 @@ - \ No newline at end of file + diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs index 6eb77a4..20aa1df 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsLayoutSeq.cs @@ -27,7 +27,10 @@ public class Managed IntWithInnerSequentialId, SequentialWrapperId, SequentialDoubleWrapperId, - AggregateSequentialWrapperId + AggregateSequentialWrapperId, + FixedBufferClassificationTestId, + HFAId, + DoubleHFAId } private static void InitialArray(int[] iarr, int[] icarr) @@ -318,10 +321,22 @@ public class Managed static extern bool MarshalStructAsParam_AsSeqByValSequentialAggregateSequentialWrapper(AggregateSequentialWrapper wrapper); [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(FixedBufferClassificationTest str, float f); + [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(FixedBufferClassificationTestBlittable str, float f); + [DllImport("MarshalStructAsParam")] + static extern bool MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(FixedArrayClassificationTest str, float f); + [DllImport("MarshalStructAsParam")] static extern int GetStringLength(AutoString str); [DllImport("MarshalStructAsParam")] static extern HFA GetHFA(float f1, float f2, float f3, float f4); + + [DllImport("MarshalStructAsParam")] + static extern float ProductHFA(HFA hfa); + + [DllImport("MarshalStructAsParam")] + static extern double ProductDoubleHFA(DoubleHFA hfa); [DllImport("MarshalStructAsParam")] static extern ManyInts GetMultiplesOf(int i); @@ -617,6 +632,96 @@ public class Managed failures++; } break; + case StructID.FixedBufferClassificationTestId: + Console.WriteLine("\tCalling MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest with nonblittable struct..."); + unsafe + { + FixedBufferClassificationTest str = new FixedBufferClassificationTest(); + str.arr[0] = 123456; + str.arr[1] = 78910; + str.arr[2] = 1234; + str.f = new NonBlittableFloat(56.789f); + if (!MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(str, str.f.F)) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest. Expected:True;Actual:False"); + failures++; + } + } + + Console.WriteLine("\tCalling MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest with blittable struct..."); + unsafe + { + FixedBufferClassificationTestBlittable str = new FixedBufferClassificationTestBlittable(); + str.arr[0] = 123456; + str.arr[1] = 78910; + str.arr[2] = 1234; + str.f = 56.789f; + if (!MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(str, str.f)) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest. Expected:True;Actual:False"); + failures++; + } + } + + Console.WriteLine("\tCalling MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest with fixed array..."); + FixedArrayClassificationTest fixedArrayTest = new FixedArrayClassificationTest + { + arr = new Int32Wrapper[3] + { + new Int32Wrapper { i = 123456 }, + new Int32Wrapper { i = 78910 }, + new Int32Wrapper { i = 1234 } + }, + f = 56.789f + }; + if (!MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(fixedArrayTest, fixedArrayTest.f)) + { + Console.WriteLine("\tFAILED! Managed to Native failed in MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest. Expected:True;Actual:False"); + failures++; + } + break; + case StructID.HFAId: + { + HFA hfa = new HFA + { + f1 = 2.0f, + f2 = 10.5f, + f3 = 15.2f, + f4 = 0.12f + }; + + float expected = hfa.f1 * hfa.f2 * hfa.f3 * hfa.f4; + float actual; + + Console.WriteLine("\tCalling ProductHFA with HFA."); + actual = ProductHFA(hfa); + if (expected != actual) + { + Console.WriteLine($"\tFAILED! Expected {expected}. Actual {actual}"); + failures++; + } + break; + } + case StructID.DoubleHFAId: + { + DoubleHFA doubleHFA = new DoubleHFA + { + d1 = 123.456, + d2 = 456.789 + }; + + double expected = doubleHFA.d1 * doubleHFA.d2; + double actual; + + Console.WriteLine("\tCalling ProductDoubleHFA."); + actual = ProductDoubleHFA(doubleHFA); + if (expected != actual) + { + Console.WriteLine($"\tFAILED! Expected {expected}. Actual {actual}"); + failures++; + } + break; + } default: Console.WriteLine("\tThere is not the struct id"); failures++; @@ -2239,6 +2344,9 @@ public class Managed MarshalStructAsParam_AsSeqByVal(StructID.SequentialWrapperId); MarshalStructAsParam_AsSeqByVal(StructID.SequentialDoubleWrapperId); MarshalStructAsParam_AsSeqByVal(StructID.AggregateSequentialWrapperId); + MarshalStructAsParam_AsSeqByVal(StructID.FixedBufferClassificationTestId); + MarshalStructAsParam_AsSeqByVal(StructID.HFAId); + MarshalStructAsParam_AsSeqByVal(StructID.DoubleHFAId); } [SecuritySafeCritical] diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp index 9de3f78..0f2582a 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.cpp @@ -1178,6 +1178,23 @@ extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE MarshalStructAsParam_AsExpByRefOutL return TRUE; } +////////////////////////////////////////////////////////////////////////////////////// +extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE MarshalStructAsParam_AsExpByValOverlappingLongFloat(OverlappingLongFloat str, LONG64 expected) +{ + return str.a == expected; +} + +extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE MarshalStructAsParam_AsExpByValOverlappingMultipleEightByte(OverlappingMultipleEightbyte str, float i1, float i2, float i3) +{ + return str.arr[0] == i1 && str.arr[1] == i2 && str.arr[2] == i3; +} + +extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE MarshalStructAsParam_AsSeqByValFixedBufferClassificationTest(FixedBufferClassificationTest str, float f) +{ + return str.f == f; +} + +//////////////////////////////////////////////////////////////////////////////////// extern "C" DLL_EXPORT int GetStringLength(AutoString str) { #ifdef _WIN32 @@ -1187,12 +1204,21 @@ extern "C" DLL_EXPORT int GetStringLength(AutoString str) #endif } +extern "C" DLL_EXPORT float STDMETHODCALLTYPE ProductHFA(HFA hfa) +{ + return hfa.f1 * hfa.f2 * hfa.f3 * hfa.f4; +} extern "C" DLL_EXPORT HFA STDMETHODCALLTYPE GetHFA(float f1, float f2, float f3, float f4) { return {f1, f2, f3, f4}; } +extern "C" DLL_EXPORT double STDMETHODCALLTYPE ProductDoubleHFA(DoubleHFA hfa) +{ + return hfa.d1 * hfa.d2; +} + extern "C" DLL_EXPORT ManyInts STDMETHODCALLTYPE GetMultiplesOf(int value) { ManyInts multiples = diff --git a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h index dcedc46..15d98ae 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h +++ b/tests/src/Interop/StructMarshalling/PInvoke/MarshalStructAsParamDLL.h @@ -56,12 +56,13 @@ typedef float FLOAT; typedef double DOUBLE; #endif -struct INNER2 // size = 12 bytes +struct INNER2 { INT f1; FLOAT f2; LPCSTR f3; }; + void ChangeINNER2(INNER2* p) { p->f1 = 77; @@ -161,7 +162,7 @@ bool IsCorrectInnerArraySequential(InnerArraySequential* p) } -union InnerArrayExplicit // size = 32 bytes +union InnerArrayExplicit { struct InnerSequential arr[2]; struct @@ -173,34 +174,48 @@ union InnerArrayExplicit // size = 32 bytes #ifdef WINDOWS - #ifdef _WIN64 - #pragma warning(push) - #pragma warning(disable: 4201) // nonstandard extension used: nameless struct/union - union OUTER3 // size = 32 bytes - { - struct InnerSequential arr[2]; - struct - { - CHAR _unused0[24]; - LPCSTR f4; - }; - }; - #pragma warning(pop) - #else - struct OUTER3 // size = 28 bytes - { - struct InnerSequential arr[2]; - LPCSTR f4; - }; - #endif +#ifdef _WIN64 +#pragma warning(push) +#pragma warning(disable: 4201) // nonstandard extension used: nameless struct/union +union OUTER3 +{ + struct InnerSequential arr[2]; + struct + { + CHAR _unused0[24]; + LPCSTR f4; + }; +}; +static_assert_no_msg(sizeof(OUTER3) == 32); +#pragma warning(pop) +#else +struct OUTER3 +{ + struct InnerSequential arr[2]; + LPCSTR f4; +}; +static_assert_no_msg(sizeof(OUTER3) == 28); +#endif +#else // WINDOWS +#if defined(__x86_64__) || defined(__aarch64__) +union OUTER3 +{ + struct InnerSequential arr[2]; + struct + { + CHAR _unused0[24]; + LPCSTR f4; + }; +}; +static_assert_no_msg(sizeof(OUTER3) == 32); +#else +struct OUTER3 +{ + struct InnerSequential arr[2]; + LPCSTR f4; +}; +static_assert_no_msg(sizeof(OUTER3) == 28); #endif - -#ifndef WINDOWS - struct OUTER3 // size = 28 bytes - { - struct InnerSequential arr[2]; - LPCSTR f4; - }; #endif void PrintOUTER3(OUTER3* p, char const * name) @@ -326,7 +341,7 @@ bool IsCorrectCharSetUnicodeSequential(CharSetUnicodeSequential* p) } -struct NumberSequential // size = 64 bytes +struct NumberSequential { LONG64 i64; ULONG64 ui64; @@ -383,7 +398,7 @@ bool IsCorrectNumberSequential(NumberSequential* p) return true; } -struct S3 // size = 1032 bytes +struct S3 { BOOL flag; LPCSTR str; @@ -434,7 +449,7 @@ bool IsCorrectS3(S3* p) return true; } -struct S4 // size = 8 bytes +struct S4 { INT age; LPCSTR name; @@ -444,7 +459,7 @@ enum Enum1 e1 = 1, e2 = 3 }; -struct S5 // size = 8 bytes +struct S5 { struct S4 s4; Enum1 ef; @@ -471,7 +486,7 @@ bool IsCorrectS5(S5* str) return true; } -struct StringStructSequentialAnsi // size = 8 bytes +struct StringStructSequentialAnsi { LPCSTR first; LPCSTR last; @@ -518,7 +533,7 @@ void ChangeStringStructSequentialAnsi(StringStructSequentialAnsi* str) str->last = newLast; } -struct StringStructSequentialUnicode // size = 8 bytes +struct StringStructSequentialUnicode { LPCWSTR first; LPCWSTR last; @@ -571,7 +586,7 @@ void ChangeStringStructSequentialUnicode(StringStructSequentialUnicode* str) } -struct S8 // size = 32 bytes +struct S8 { LPCSTR name; BOOL gender; @@ -614,7 +629,7 @@ void ChangeS8(S8* str) str->mySByte = 64; } #pragma pack (8) -struct S_int // size = 4 bytes +struct S_int { INT i; }; @@ -622,19 +637,19 @@ struct S_int // size = 4 bytes struct S9; typedef void (*TestDelegate1)(struct S9 myStruct); -struct S9 // size = 8 bytes +struct S9 { INT i32; TestDelegate1 myDelegate1; }; -struct S101 // size = 8 bytes +struct S101 { INT i; struct S_int s_int; }; -struct S10 // size = 8 bytes +struct S10 { struct S101 s; }; @@ -661,13 +676,13 @@ void ChangeS10(S10* str) typedef int* LPINT; #endif -struct S11 // size = 8 bytes +struct S11 { LPINT i32; INT i; }; -union U // size = 8 bytes +union U { INT i32; UINT ui32; @@ -724,7 +739,7 @@ bool IsCorrectU(U* p) return true; } -struct ByteStructPack2Explicit // size = 2 bytes +struct ByteStructPack2Explicit { BYTE b1; BYTE b2; @@ -749,7 +764,7 @@ bool IsCorrectByteStructPack2Explicit(ByteStructPack2Explicit* p) -struct ShortStructPack4Explicit // size = 4 bytes +struct ShortStructPack4Explicit { SHORT s1; SHORT s2; @@ -773,7 +788,7 @@ bool IsCorrectShortStructPack4Explicit(ShortStructPack4Explicit* p) } -struct IntStructPack8Explicit // size = 8 bytes +struct IntStructPack8Explicit { INT i1; INT i2; @@ -796,7 +811,7 @@ bool IsCorrectIntStructPack8Explicit(IntStructPack8Explicit* p) return true; } -struct LongStructPack16Explicit // size = 16 bytes +struct LongStructPack16Explicit { LONG64 l1; LONG64 l2; @@ -836,6 +851,12 @@ struct HFA float f4; }; +struct DoubleHFA +{ + double d1; + double d2; +}; + struct ManyInts { int i1; @@ -888,3 +909,51 @@ struct AggregateSequentialWrapper InnerSequential sequential; SequentialWrapper wrapper2; }; + +union OverlappingLongFloat +{ + LONG64 a; + struct + { + char unused[4]; + float f; + }; +}; + +struct FixedBufferClassificationTest +{ + int arr[3]; + float f; +}; + +// use float padding to ensure that we match the SystemV Classification +// as if this field was not here (the case in the managed representation). +union OverlappingMultipleEightbyte +{ + float arr[3]; + struct + { + float padding[2]; + int i; + }; +}; + +union OverlappingMultipleEightbyteFirst +{ + float arr[3]; + struct + { + float padding; + int i; + }; +}; + +union OverlappingMultipleEightbyteMultiple +{ + float arr[3]; + struct + { + float padding; + int i[3]; + }; +}; diff --git a/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs b/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs index 7ade4c3..59623ea 100644 --- a/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs +++ b/tests/src/Interop/StructMarshalling/PInvoke/Struct.cs @@ -313,6 +313,50 @@ public struct HFA public float f4; } +[StructLayout(LayoutKind.Explicit)] +public struct ExplicitHFA +{ + [FieldOffset(0)] + public float f1; + [FieldOffset(4)] + public float f2; + [FieldOffset(8)] + public float f3; + [FieldOffset(12)] + public float f4; +} + +[StructLayout(LayoutKind.Explicit)] +public unsafe struct ExplicitFixedHFA +{ + [FieldOffset(0)] + public float f1; + [FieldOffset(4)] + public float f2; + [FieldOffset(8)] + public fixed float fs[2]; +} + +[StructLayout(LayoutKind.Explicit)] +public struct OverlappingHFA +{ + [FieldOffset(0)] + public HFA hfa; + + [FieldOffset(0)] + public ExplicitHFA explicitHfa; + + [FieldOffset(0)] + public ExplicitFixedHFA explicitFixedHfa; +} + +[StructLayout(LayoutKind.Sequential)] +public struct DoubleHFA +{ + public double d1; + public double d2; +} + [StructLayout(LayoutKind.Sequential)] public struct ManyInts { @@ -369,3 +413,75 @@ public struct MultipleBool public bool b1; public bool b2; } + +[StructLayout(LayoutKind.Explicit)] +public struct OverlappingLongFloat +{ + [FieldOffset(0)] + public long l; + + [FieldOffset(4)] + public float f; +} + +[StructLayout(LayoutKind.Explicit)] +public struct OverlappingLongFloat2 +{ + [FieldOffset(4)] + public float f; + [FieldOffset(0)] + public long l; +} + +[StructLayout(LayoutKind.Explicit)] +public struct OverlappingMultipleEightbyte +{ + [FieldOffset(8)] + public int i; + [FieldOffset(0), MarshalAs(UnmanagedType.ByValArray, SizeConst = 3)] + public float[] arr; +} + +[StructLayout(LayoutKind.Sequential)] +public unsafe struct FixedBufferClassificationTestBlittable +{ + public fixed int arr[3]; + public float f; +} + +[StructLayout(LayoutKind.Sequential)] +public unsafe struct FixedBufferClassificationTest +{ + public fixed int arr[3]; + public NonBlittableFloat f; +} + +// A non-blittable wrapper for a float value. +// Used to force a type with a float field to be non-blittable +// and take a different code path. +[StructLayout(LayoutKind.Sequential)] +public struct NonBlittableFloat +{ + public NonBlittableFloat(float f) + { + arr = new []{f}; + } + + [MarshalAs(UnmanagedType.ByValArray, SizeConst = 1)] + private float[] arr; + + public float F => arr[0]; +} + +public struct Int32Wrapper +{ + public int i; +} + +[StructLayout(LayoutKind.Sequential)] +public unsafe struct FixedArrayClassificationTest +{ + [MarshalAs(UnmanagedType.ByValArray, SizeConst = 3)] + public Int32Wrapper[] arr; + public float f; +} -- 2.7.4