Skip to content

Commit

Permalink
Ensure the layout of StackAllocatedBox<T> matches boxed layouts (#1…
Browse files Browse the repository at this point in the history
…07050)

The boxed layout of a struct always has its data at +8, as evidenced by
`Object::UnBox`. This means that `StackAllocatedBox<T>` should have
`Pack = 1`, otherwise this may not be the case. In the test failure we
had a `StackAllocatedBox<Int128>` which had its `_value` field at offset
16. After object stack allocation this meant that we were saving data in
padding of the structure, which promotion does not guarantee to
preserve.

Fix #106947
  • Loading branch information
jakobbotsch authored Aug 28, 2024
1 parent c1a9f26 commit 7ff684a
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 2 deletions.
4 changes: 3 additions & 1 deletion src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10299,7 +10299,9 @@ void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType)
int simdSize = ConstantValue<int>(simdType->m_args[0]);
CorInfoType baseJitType = (CorInfoType)ConstantValue<int>(simdType->m_args[1]);

printf("%s(simd%d, %s)", VNFuncName(simdType->m_func), simdSize, varTypeName(JitType2PreciseVarType(baseJitType)));
printf("%s(simd%d, %s)", VNFuncName(simdType->m_func), simdSize,
baseJitType == CORINFO_TYPE_UNDEF ? varTypeName(TYP_UNDEF)
: varTypeName(JitType2PreciseVarType(baseJitType)));
}
#endif // FEATURE_SIMD

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,7 @@ DEFINE_FIELD_U(_condition, ContractExceptionObject, _Condition)
DEFINE_CLASS(MODULEBASE, Reflection, Module)

DEFINE_CLASS(STACKALLOCATEDBOX, CompilerServices, StackAllocatedBox`1)
DEFINE_FIELD(STACKALLOCATEDBOX, VALUE, _value)

DEFINE_CLASS(UTF8STRINGMARSHALLER, Marshalling, Utf8StringMarshaller)
DEFINE_METHOD(UTF8STRINGMARSHALLER, CONVERT_TO_MANAGED, ConvertToManaged, SM_PtrByte_RetStr)
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6168,6 +6168,12 @@ CORINFO_CLASS_HANDLE CEEInfo::getTypeForBoxOnStack(CORINFO_CLASS_HANDLE cls)
TypeHandle stackAllocatedBox = CoreLibBinder::GetClass(CLASS__STACKALLOCATEDBOX);
TypeHandle stackAllocatedBoxInst = stackAllocatedBox.Instantiate(boxedFieldsInst);
result = static_cast<CORINFO_CLASS_HANDLE>(stackAllocatedBoxInst.AsPtr());

#ifdef _DEBUG
FieldDesc* pValueFD = CoreLibBinder::GetField(FIELD__STACKALLOCATEDBOX__VALUE);
DWORD index = pValueFD->GetApproxEnclosingMethodTable()->GetIndexForFieldDesc(pValueFD);
_ASSERTE(stackAllocatedBoxInst.GetMethodTable()->GetFieldDescByIndex(index)->GetOffset() == TARGET_POINTER_SIZE);
#endif
}

EE_TO_JIT_TRANSITION();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
namespace System.Runtime.CompilerServices
{
[NonVersionable]
[StructLayout(LayoutKind.Sequential)]
[StructLayout(LayoutKind.Sequential, Pack = 1)]
internal unsafe struct StackAllocatedBox<T>
{
// These fields are only accessed from jitted code
Expand Down

0 comments on commit 7ff684a

Please sign in to comment.