Skip to content

Fix JITEEInterface::IsValueClass to return false for pointers. #50749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 13, 2021
Merged

Fix JITEEInterface::IsValueClass to return false for pointers. #50749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d1f50e
lets VM return `isValueClass=false` for pointers.
Apr 5, 2021
a106c7f
fix assert and add a comment.
Apr 5, 2021
c38354c
fix diffs.
Apr 5, 2021
e1d07cf
fix crossgen2 part.
Apr 11, 2021
bcbb44a
Update JITEEVersionIdentifier.
Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/coreclr/inc/jiteeversionguid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// Note that this file is parsed by some tools, namely superpmi.py, so make sure the first line is exactly
// of the form:
//
// constexpr GUID JITEEVersionIdentifier = { /* a7bb194e-4e7c-4850-af12-ea9f30ea5a13 */
// constexpr GUID JITEEVersionIdentifier = { /* 1776ab48-edfa-49be-a11f-ec216b28174c */
//
// (without the leading slashes or spaces).
//
Expand All @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* a1f5e9a1-ee44-42f9-9319-e2a2dbf8c5c9 */
0xa1f5e9a1,
0xee44,
0x42f9,
{0x93, 0x19, 0xe2, 0xa2, 0xdb, 0xf8, 0xc5, 0xc9}
constexpr GUID JITEEVersionIdentifier = { /* 1776ab48-edfa-49be-a11f-ec216b28174c */
0x1776ab48,
0xedfa,
0x49be,
{0xa1, 0x1f, 0xec, 0x21, 0x6b, 0x28, 0x17, 0x4c}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
12 changes: 7 additions & 5 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16998,9 +16998,10 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
impBashVarAddrsToI(op2);
op2 = impImplicitIorI4Cast(op2, info.compRetType);
op2 = impImplicitR4orR8Cast(op2, info.compRetType);
// Note that we allow TYP_I_IMPL<->TYP_BYREF transformation, but only TYP_I_IMPL<-TYP_REF.
assertImp((genActualType(op2->TypeGet()) == genActualType(info.compRetType)) ||
((op2->TypeGet() == TYP_I_IMPL) && (info.compRetType == TYP_BYREF)) ||
((op2->TypeGet() == TYP_BYREF) && (info.compRetType == TYP_I_IMPL)) ||
((op2->TypeGet() == TYP_I_IMPL) && TypeIs(info.compRetType, TYP_BYREF)) ||
(op2->TypeIs(TYP_BYREF, TYP_REF) && (info.compRetType == TYP_I_IMPL)) ||
(varTypeIsFloating(op2->gtType) && varTypeIsFloating(info.compRetType)) ||
(varTypeIsStruct(op2) && varTypeIsStruct(info.compRetType)));

Expand Down Expand Up @@ -17049,9 +17050,10 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)

if (returnType != originalCallType)
{
// Allow TYP_BYREF to be returned as TYP_I_IMPL and vice versa
if (((returnType == TYP_BYREF) && (originalCallType == TYP_I_IMPL)) ||
((returnType == TYP_I_IMPL) && (originalCallType == TYP_BYREF)))
// Allow TYP_BYREF to be returned as TYP_I_IMPL and vice versa.
// Allow TYP_REF to be returned as TYP_I_IMPL and NOT vice verse.
if ((TypeIs(returnType, TYP_BYREF, TYP_REF) && (originalCallType == TYP_I_IMPL)) ||
((returnType == TYP_I_IMPL) && TypeIs(originalCallType, TYP_BYREF)))
{
JITDUMP("Allowing return type mismatch: have %s, needed %s\n", varTypeName(returnType),
varTypeName(originalCallType));
Expand Down
7 changes: 1 addition & 6 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,8 +1579,7 @@ private int appendClassName(char** ppBuf, ref int pnBufLen, CORINFO_CLASS_STRUCT

private bool isValueClass(CORINFO_CLASS_STRUCT_* cls)
{
TypeDesc type = HandleToObject(cls);
return type.IsValueType || type.IsPointer || type.IsFunctionPointer;
return HandleToObject(cls).IsValueType;
}

private CorInfoInlineTypeCheck canInlineTypeCheck(CORINFO_CLASS_STRUCT_* cls, CorInfoInlineTypeCheckSource source)
Expand All @@ -1602,10 +1601,6 @@ private uint getClassAttribsInternal(TypeDesc type)

CorInfoFlag result = (CorInfoFlag)0;

// CoreCLR uses UIntPtr in place of pointers here
if (type.IsPointer || type.IsFunctionPointer)
type = _compilation.TypeSystemContext.GetWellKnownType(WellKnownType.UIntPtr);

var metadataType = type as MetadataType;

// The array flag is used to identify the faked-up methods on
Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3772,11 +3772,7 @@ bool CEEInfo::isValueClass(CORINFO_CLASS_HANDLE clsHnd)

_ASSERTE(clsHnd);

// Note that clsHnd.IsValueType() would not return what the JIT expects
// for corner cases like ELEMENT_TYPE_FNPTR
TypeHandle VMClsHnd(clsHnd);
MethodTable * pMT = VMClsHnd.GetMethodTable();
ret = (pMT != NULL) ? pMT->IsValueType() : false;
ret = TypeHandle(clsHnd).IsValueType();

EE_TO_JIT_TRANSITION_LEAF();

Expand Down Expand Up @@ -3888,7 +3884,7 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd)
if (pMT->HasComponentSize())
ret |= CORINFO_FLG_VAROBJSIZE;

if (pMT->IsValueType())
if (VMClsHnd.IsValueType())
{
ret |= CORINFO_FLG_VALUECLASS;

Expand Down