Skip to content

Commit aca6170

Browse files
author
Sergey
committed
Field seq for a LclFld always should be correct.
1 parent b5e8672 commit aca6170

File tree

6 files changed

+144
-46
lines changed

6 files changed

+144
-46
lines changed

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10266,7 +10266,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
1026610266
{
1026710267
// Create a CompAllocator that labels sub-structure with CMK_FieldSeqStore, and use that for allocation.
1026810268
CompAllocator ialloc(getAllocator(CMK_FieldSeqStore));
10269-
compRoot->m_fieldSeqStore = new (ialloc) FieldSeqStore(ialloc);
10269+
compRoot->m_fieldSeqStore = new (ialloc) FieldSeqStore(compRoot, ialloc);
1027010270
}
1027110271
return compRoot->m_fieldSeqStore;
1027210272
}

src/coreclr/jit/gentree.cpp

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18758,7 +18758,8 @@ void GenTree::LabelIndex(Compiler* comp, bool isConst)
1875818758
FieldSeqNode FieldSeqStore::s_notAField(nullptr, nullptr);
1875918759

1876018760
// FieldSeqStore methods.
18761-
FieldSeqStore::FieldSeqStore(CompAllocator alloc) : m_alloc(alloc), m_canonMap(new (alloc) FieldSeqNodeCanonMap(alloc))
18761+
FieldSeqStore::FieldSeqStore(Compiler* comp, CompAllocator alloc)
18762+
: m_compiler(comp), m_alloc(alloc), m_canonMap(new (alloc) FieldSeqNodeCanonMap(alloc))
1876218763
{
1876318764
}
1876418765

@@ -18809,6 +18810,8 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
1880918810
// We should never add a duplicate FieldSeqNode
1881018811
assert(a != b);
1881118812

18813+
bool lastA = (a->m_next == nullptr);
18814+
1881218815
FieldSeqNode* tmp = Append(a->m_next, b);
1881318816
FieldSeqNode fsn(a->m_fieldHnd, tmp);
1881418817
FieldSeqNode* res = nullptr;
@@ -18818,6 +18821,12 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b)
1881818821
}
1881918822
else
1882018823
{
18824+
if (lastA && !a->IsPseudoField())
18825+
{
18826+
CORINFO_CLASS_HANDLE nextStructHnd;
18827+
m_compiler->info.compCompHnd->getFieldType(a->m_fieldHnd, &nextStructHnd);
18828+
tmp->CheckFldBelongsToCls(m_compiler, nextStructHnd);
18829+
}
1882118830
res = m_alloc.allocate<FieldSeqNode>(1);
1882218831
*res = fsn;
1882318832
m_canonMap->Set(fsn, res);
@@ -18850,6 +18859,36 @@ bool FieldSeqNode::IsPseudoField() const
1885018859
return m_fieldHnd == FieldSeqStore::FirstElemPseudoField || m_fieldHnd == FieldSeqStore::ConstantIndexPseudoField;
1885118860
}
1885218861

18862+
//------------------------------------------------------------------------
18863+
// CheckFldBelongsToCls: Check that the field sequences belongs to the class.
18864+
//
18865+
// Arguments:
18866+
// comp - a compiler instance;
18867+
// clsHnd - the class handle.
18868+
//
18869+
// Return Value:
18870+
// True if the field sequence describe a correct field accesses of the class.
18871+
//
18872+
bool FieldSeqNode::CheckFldBelongsToCls(Compiler* comp, CORINFO_CLASS_HANDLE clsHnd) const
18873+
{
18874+
if (this == FieldSeqStore::NotAField())
18875+
{
18876+
return false;
18877+
}
18878+
if (clsHnd == NO_CLASS_HANDLE)
18879+
{
18880+
return false;
18881+
}
18882+
CORINFO_CLASS_HANDLE realClsHnd = comp->info.compCompHnd->getFieldClass(m_fieldHnd);
18883+
if (realClsHnd != clsHnd)
18884+
{
18885+
return false;
18886+
}
18887+
18888+
// The rest of FieldSeq should be already checked when it was created.
18889+
return true;
18890+
}
18891+
1885318892
#ifdef FEATURE_SIMD
1885418893
GenTreeSIMD* Compiler::gtNewSIMDNode(
1885518894
var_types type, GenTree* op1, SIMDIntrinsicID simdIntrinsicID, var_types baseType, unsigned size)
@@ -19654,6 +19693,19 @@ uint16_t GenTreeLclVarCommon::GetLclOffs() const
1965419693
}
1965519694
}
1965619695

19696+
void GenTreeLclFld::SetFieldSeq(FieldSeqNode* fieldSeq)
19697+
{
19698+
#if defined(DEBUG)
19699+
if (fieldSeq != FieldSeqStore::NotAField())
19700+
{
19701+
Compiler* comp = JitTls::GetCompiler();
19702+
const LclVarDsc* varDsc = comp->lvaGetDesc(GetLclNum());
19703+
assert(fieldSeq->CheckFldBelongsToCls(comp, varDsc->GetStructHnd()));
19704+
}
19705+
#endif // DEBUG
19706+
m_fieldSeq = fieldSeq;
19707+
}
19708+
1965719709
#ifdef TARGET_ARM
1965819710
//------------------------------------------------------------------------
1965919711
// IsOffsetMisaligned: check if the field needs a special handling on arm.

src/coreclr/jit/gentree.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ struct FieldSeqNode
266266
return tail;
267267
}
268268

269+
bool CheckFldBelongsToCls(Compiler* comp, CORINFO_CLASS_HANDLE clsHnd) const;
270+
269271
// Make sure this provides methods that allow it to be used as a KeyFuncs type in SimplerHash.
270272
static int GetHashCode(FieldSeqNode fsn)
271273
{
@@ -284,6 +286,7 @@ class FieldSeqStore
284286
{
285287
typedef JitHashTable<FieldSeqNode, /*KeyFuncs*/ FieldSeqNode, FieldSeqNode*> FieldSeqNodeCanonMap;
286288

289+
Compiler* m_compiler;
287290
CompAllocator m_alloc;
288291
FieldSeqNodeCanonMap* m_canonMap;
289292

@@ -294,7 +297,7 @@ class FieldSeqStore
294297
static int ConstantIndexPseudoFieldStruct;
295298

296299
public:
297-
FieldSeqStore(CompAllocator alloc);
300+
FieldSeqStore(Compiler* comp, CompAllocator alloc);
298301

299302
// Returns the (canonical in the store) singleton field sequence for the given handle.
300303
FieldSeqNode* CreateSingleton(CORINFO_FIELD_HANDLE fieldHnd);
@@ -3433,10 +3436,7 @@ struct GenTreeLclFld : public GenTreeLclVarCommon
34333436
return m_fieldSeq;
34343437
}
34353438

3436-
void SetFieldSeq(FieldSeqNode* fieldSeq)
3437-
{
3438-
m_fieldSeq = fieldSeq;
3439-
}
3439+
void SetFieldSeq(FieldSeqNode* fieldSeq);
34403440

34413441
#ifdef TARGET_ARM
34423442
bool IsOffsetMisaligned() const;

src/coreclr/jit/lclmorph.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,14 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
863863
addr->ChangeOper(GT_LCL_FLD_ADDR);
864864
addr->AsLclFld()->SetLclNum(val.LclNum());
865865
addr->AsLclFld()->SetLclOffs(val.Offset());
866-
addr->AsLclFld()->SetFieldSeq(val.FieldSeq());
866+
if (varTypeIsStruct(varDsc) && val.FieldSeq()->CheckFldBelongsToCls(m_compiler, varDsc->GetStructHnd()))
867+
{
868+
addr->AsLclFld()->SetFieldSeq(val.FieldSeq());
869+
}
870+
else
871+
{
872+
addr->AsLclFld()->SetFieldSeq(FieldSeqStore::NotAField());
873+
}
867874
}
868875
else
869876
{
@@ -1032,6 +1039,10 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
10321039
indir->ChangeOper(GT_LCL_FLD);
10331040
indir->AsLclFld()->SetLclNum(val.LclNum());
10341041
indir->AsLclFld()->SetLclOffs(val.Offset());
1042+
if (fieldSeq != nullptr && !fieldSeq->CheckFldBelongsToCls(m_compiler, varDsc->GetStructHnd()))
1043+
{
1044+
fieldSeq = nullptr;
1045+
}
10351046
indir->AsLclFld()->SetFieldSeq(fieldSeq == nullptr ? FieldSeqStore::NotAField() : fieldSeq);
10361047

10371048
// Promoted struct vars aren't currently handled here so the created LCL_FLD can't be

src/coreclr/jit/morph.cpp

Lines changed: 72 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11591,38 +11591,39 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1159111591
}
1159211592
else
1159311593
{
11594+
GenTree* dstFldAddr;
1159411595
if (addrSpill)
1159511596
{
1159611597
assert(addrSpillTemp != BAD_VAR_NUM);
11597-
dstFld = gtNewLclvNode(addrSpillTemp, TYP_BYREF);
11598+
dstFldAddr = gtNewLclvNode(addrSpillTemp, TYP_BYREF);
1159811599
}
1159911600
else
1160011601
{
1160111602
if (i == 0)
1160211603
{
1160311604
// Use the orginal destAddr tree when i == 0
11604-
dstFld = destAddr;
11605+
dstFldAddr = destAddr;
1160511606
}
1160611607
else
1160711608
{
1160811609
// We can't clone multiple copies of a tree with persistent side effects
1160911610
noway_assert((destAddr->gtFlags & GTF_PERSISTENT_SIDE_EFFECTS) == 0);
1161011611

11611-
dstFld = gtCloneExpr(destAddr);
11612-
noway_assert(dstFld != nullptr);
11612+
dstFldAddr = gtCloneExpr(destAddr);
11613+
noway_assert(dstFldAddr != nullptr);
1161311614

11614-
JITDUMP("dstFld - Multiple Fields Clone created:\n");
11615-
DISPTREE(dstFld);
11615+
JITDUMP("dstFldAddr - Multiple Fields Clone created:\n");
11616+
DISPTREE(dstFldAddr);
1161611617

1161711618
// Morph the newly created tree
11618-
dstFld = fgMorphTree(dstFld);
11619+
dstFldAddr = fgMorphTree(dstFldAddr);
1161911620
}
1162011621

1162111622
// Is the address of a local?
1162211623
GenTreeLclVarCommon* lclVarTree = nullptr;
1162311624
bool isEntire = false;
1162411625
bool* pIsEntire = (blockWidthIsConst ? &isEntire : nullptr);
11625-
if (dstFld->DefinesLocalAddr(this, blockWidth, &lclVarTree, pIsEntire))
11626+
if (dstFldAddr->DefinesLocalAddr(this, blockWidth, &lclVarTree, pIsEntire))
1162611627
{
1162711628
lclVarTree->gtFlags |= GTF_VAR_DEF;
1162811629
if (!isEntire)
@@ -11636,33 +11637,44 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1163611637
unsigned srcFieldLclNum = srcVarDsc->lvFieldLclStart + i;
1163711638
LclVarDsc* srcFieldVarDsc = lvaGetDesc(srcFieldLclNum);
1163811639

11639-
// Have to set the field sequence -- which means we need the field handle.
11640-
CORINFO_CLASS_HANDLE classHnd = srcVarDsc->GetStructHnd();
11641-
CORINFO_FIELD_HANDLE fieldHnd =
11642-
info.compCompHnd->getFieldInClass(classHnd, srcFieldVarDsc->lvFldOrdinal);
11643-
FieldSeqNode* curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd);
11640+
CORINFO_CLASS_HANDLE srcClassHnd = srcVarDsc->GetStructHnd();
11641+
CORINFO_CLASS_HANDLE dstClassHnd = gtGetStructHandleIfPresent(dest);
11642+
11643+
FieldSeqNode* curFieldSeq;
11644+
11645+
if (srcClassHnd == dstClassHnd)
11646+
{
11647+
CORINFO_FIELD_HANDLE fieldHnd =
11648+
info.compCompHnd->getFieldInClass(dstClassHnd, srcFieldVarDsc->lvFldOrdinal);
11649+
curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd);
11650+
}
11651+
else
11652+
{
11653+
// source field handle won't match an actual field in the dst.
11654+
curFieldSeq = FieldSeqStore::NotAField();
11655+
}
1164411656

1164511657
unsigned srcFieldOffset = lvaGetDesc(srcFieldLclNum)->lvFldOffset;
1164611658

1164711659
if (srcFieldOffset == 0)
1164811660
{
11649-
fgAddFieldSeqForZeroOffset(dstFld, curFieldSeq);
11661+
fgAddFieldSeqForZeroOffset(dstFldAddr, curFieldSeq);
1165011662
}
1165111663
else
1165211664
{
1165311665
GenTree* fieldOffsetNode = gtNewIconNode(srcFieldVarDsc->lvFldOffset, curFieldSeq);
11654-
dstFld = gtNewOperNode(GT_ADD, TYP_BYREF, dstFld, fieldOffsetNode);
11666+
dstFldAddr = gtNewOperNode(GT_ADD, TYP_BYREF, dstFldAddr, fieldOffsetNode);
1165511667
}
1165611668

11657-
dstFld = gtNewIndir(srcFieldVarDsc->TypeGet(), dstFld);
11669+
dstFld = gtNewIndir(srcFieldVarDsc->TypeGet(), dstFldAddr);
1165811670

1165911671
// !!! The destination could be on stack. !!!
1166011672
// This flag will let us choose the correct write barrier.
1166111673
dstFld->gtFlags |= GTF_IND_TGTANYWHERE;
1166211674
}
1166311675
}
1166411676

11665-
GenTree* srcFld;
11677+
GenTree* srcFld = DUMMY_INIT(NULL);
1166611678
if (srcDoFldAsg)
1166711679
{
1166811680
noway_assert(srcLclNum != BAD_VAR_NUM);
@@ -11688,39 +11700,53 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1168811700
}
1168911701
else
1169011702
{
11703+
GenTree* srcFldAddr = nullptr;
1169111704
if (addrSpill)
1169211705
{
1169311706
assert(addrSpillTemp != BAD_VAR_NUM);
11694-
srcFld = gtNewLclvNode(addrSpillTemp, TYP_BYREF);
11707+
srcFldAddr = gtNewLclvNode(addrSpillTemp, TYP_BYREF);
1169511708
}
1169611709
else
1169711710
{
1169811711
if (i == 0)
1169911712
{
1170011713
// Use the orginal srcAddr tree when i == 0
11701-
srcFld = srcAddr;
11714+
srcFldAddr = srcAddr;
1170211715
}
1170311716
else
1170411717
{
1170511718
// We can't clone multiple copies of a tree with persistent side effects
1170611719
noway_assert((srcAddr->gtFlags & GTF_PERSISTENT_SIDE_EFFECTS) == 0);
1170711720

11708-
srcFld = gtCloneExpr(srcAddr);
11709-
noway_assert(srcFld != nullptr);
11721+
srcFldAddr = gtCloneExpr(srcAddr);
11722+
noway_assert(srcFldAddr != nullptr);
1171011723

1171111724
JITDUMP("srcFld - Multiple Fields Clone created:\n");
11712-
DISPTREE(srcFld);
11725+
DISPTREE(srcFldAddr);
1171311726

1171411727
// Morph the newly created tree
11715-
srcFld = fgMorphTree(srcFld);
11728+
srcFldAddr = fgMorphTree(srcFldAddr);
1171611729
}
1171711730
}
1171811731

11719-
CORINFO_CLASS_HANDLE classHnd = lvaTable[destLclNum].GetStructHnd();
11720-
CORINFO_FIELD_HANDLE fieldHnd =
11721-
info.compCompHnd->getFieldInClass(classHnd, lvaTable[dstFieldLclNum].lvFldOrdinal);
11722-
FieldSeqNode* curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd);
11723-
var_types destType = lvaGetDesc(dstFieldLclNum)->lvType;
11732+
CORINFO_CLASS_HANDLE dstClassHnd = lvaTable[destLclNum].GetStructHnd();
11733+
CORINFO_CLASS_HANDLE srcClassHnd = gtGetStructHandleIfPresent(src);
11734+
11735+
FieldSeqNode* curFieldSeq;
11736+
11737+
if (srcClassHnd == dstClassHnd)
11738+
{
11739+
CORINFO_FIELD_HANDLE fieldHnd =
11740+
info.compCompHnd->getFieldInClass(dstClassHnd, lvaTable[dstFieldLclNum].lvFldOrdinal);
11741+
curFieldSeq = GetFieldSeqStore()->CreateSingleton(fieldHnd);
11742+
}
11743+
else
11744+
{
11745+
// source field handle won't match an actual field in the dst.
11746+
curFieldSeq = FieldSeqStore::NotAField();
11747+
}
11748+
11749+
var_types destType = lvaGetDesc(dstFieldLclNum)->lvType;
1172411750

1172511751
bool done = false;
1172611752
if (lvaGetDesc(dstFieldLclNum)->lvFldOffset == 0)
@@ -11740,7 +11766,7 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1174011766
srcLclVarTree->gtFlags |= GTF_VAR_CAST;
1174111767
srcLclVarTree->ChangeOper(GT_LCL_FLD);
1174211768
srcLclVarTree->gtType = destType;
11743-
srcLclVarTree->AsLclFld()->SetFieldSeq(curFieldSeq);
11769+
srcLclVarTree->AsLclFld()->SetFieldSeq(FieldSeqStore::NotAField());
1174411770
srcFld = srcLclVarTree;
1174511771
done = true;
1174611772
}
@@ -11751,14 +11777,14 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
1175111777
unsigned fldOffset = lvaGetDesc(dstFieldLclNum)->lvFldOffset;
1175211778
if (fldOffset == 0)
1175311779
{
11754-
fgAddFieldSeqForZeroOffset(srcFld, curFieldSeq);
11780+
fgAddFieldSeqForZeroOffset(srcFldAddr, curFieldSeq);
1175511781
}
1175611782
else
1175711783
{
1175811784
GenTreeIntCon* fldOffsetNode = gtNewIconNode(fldOffset, curFieldSeq);
11759-
srcFld = gtNewOperNode(GT_ADD, TYP_BYREF, srcFld, fldOffsetNode);
11785+
srcFldAddr = gtNewOperNode(GT_ADD, TYP_BYREF, srcFldAddr, fldOffsetNode);
1176011786
}
11761-
srcFld = gtNewIndir(destType, srcFld);
11787+
srcFld = gtNewIndir(destType, srcFldAddr);
1176211788
}
1176311789
}
1176411790
}
@@ -14604,19 +14630,29 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
1460414630
lclFld = temp->AsLclFld();
1460514631
lclFld->SetLclOffs(static_cast<unsigned>(ival1));
1460614632

14633+
FieldSeqNode* newFldSeq;
14634+
1460714635
if (lclFld->GetFieldSeq() == FieldSeqStore::NotAField())
1460814636
{
1460914637
if (fieldSeq != nullptr)
1461014638
{
14611-
// If it does represent a field, note that.
14612-
lclFld->SetFieldSeq(fieldSeq);
14639+
newFldSeq = fieldSeq;
14640+
}
14641+
else
14642+
{
14643+
newFldSeq = FieldSeqStore::NotAField();
1461314644
}
1461414645
}
1461514646
else
1461614647
{
14617-
// Append 'fieldSeq' to the existing one
14618-
lclFld->SetFieldSeq(GetFieldSeqStore()->Append(lclFld->GetFieldSeq(), fieldSeq));
14648+
newFldSeq = GetFieldSeqStore()->Append(lclFld->GetFieldSeq(), fieldSeq);
14649+
}
14650+
const LclVarDsc* varDsc = lvaGetDesc(lclFld);
14651+
if (!varTypeIsStruct(varDsc) || !newFldSeq->CheckFldBelongsToCls(this, varDsc->GetStructHnd()))
14652+
{
14653+
newFldSeq = FieldSeqStore::NotAField();
1461914654
}
14655+
lclFld->SetFieldSeq(newFldSeq);
1462014656
}
1462114657
temp->gtType = tree->gtType;
1462214658
foldAndReturnTemp = true;

0 commit comments

Comments
 (0)