Skip to content
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

Jump to bottom

Throw ArrayStoreException if null is stored in non-nullable array #20291

Merged
merged 1 commit into from
Oct 8, 2024
+65 −63
Merged

Throw ArrayStoreException if null is stored in non-nullable array #20291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 runtime/compiler/env/VMJ9.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2937,6 +2937,12 @@ TR_J9VMBase::testIsClassArrayType(TR::Node *j9ClassRefNode)
return testAreSomeClassDepthAndFlagsSet(j9ClassRefNode, getFlagValueForArrayCheck());
}

TR::Node *
TR_J9VMBase::testIsArrayClassNullRestrictedType(TR::Node *j9ClassRefNode)
{
return testAreSomeClassFlagsSet(j9ClassRefNode, J9ClassArrayIsNullRestricted);
}

TR::Node *
TR_J9VMBase::loadArrayClassComponentType(TR::Node *j9ClassRefNode)
{
Expand All @@ -2959,12 +2965,6 @@ TR_J9VMBase::checkSomeArrayCompClassFlags(TR::Node *arrayBaseAddressNode, TR::IL
return ifNode;
}

TR::Node *
TR_J9VMBase::checkArrayCompClassPrimitiveValueType(TR::Node *arrayBaseAddressNode, TR::ILOpCodes ifCmpOp)
{
return checkSomeArrayCompClassFlags(arrayBaseAddressNode, ifCmpOp, J9ClassIsPrimitiveValueType);
}

TR::Node *
TR_J9VMBase::checkArrayCompClassValueType(TR::Node *arrayBaseAddressNode, TR::ILOpCodes ifCmpOp)
{
Expand Down
17 changes: 9 additions & 8 deletions runtime/compiler/env/VMJ9.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,15 @@ class TR_J9VMBase : public TR_FrontEnd
*/
TR::Node * testIsClassArrayType(TR::Node *j9ClassRefNode);

/**
* \brief Generate IL to test whether the array's class is null-restricted
* \param j9ClassRefNode A node representing a reference to a \ref J9Class
* \return \ref TR::Node that tests whether the array's class flags has the
* \ref J9ClassArrayIsNullRestricted flag set, yielding a non-zero result if the
* flag is set, or zero otherwise.
*/
TR::Node * testIsArrayClassNullRestrictedType(TR::Node *j9ClassRefNode);

/**
* \brief Test whether any of the specified flags is set on the array's component class
* \param arrayBaseAddressNode A node representing a reference to the array base address
Expand All @@ -1338,14 +1347,6 @@ class TR_J9VMBase : public TR_FrontEnd
*/
TR::Node * checkArrayCompClassValueType(TR::Node *arrayBaseAddressNode, TR::ILOpCodes ifCmpOp);

/**
* \brief Check whether or not the array component class is a primitive value type
* \param arrayBaseAddressNode A node representing a reference to the array base address
* \param ifCmpOp If comparison opCode such as ificmpeq or ificmpne
* \return \ref TR::Node that compares the array component class J9ClassIsPrimitiveValueType flag to a zero integer
*/
TR::Node * checkArrayCompClassPrimitiveValueType(TR::Node *arrayBaseAddressNode, TR::ILOpCodes ifCmpOp);

virtual J9JITConfig *getJ9JITConfig() { return _jitConfig; }

virtual int32_t getCompThreadIDForVMThread(void *vmThread);
Expand Down
5 changes: 3 additions & 2 deletions runtime/compiler/optimizer/J9ValuePropagation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2653,8 +2653,9 @@ J9::ValuePropagation::transformFlattenedArrayElementStore(TR_OpaqueClassBlock *a
// The value that is being stored into the array element has to be non null.
if (needsNullValueCheck)
{
TR::Node *passThru = TR::Node::create(callNode, TR::PassThrough, 1, valueNode);
TR::Node *nullCheck = TR::Node::createWithSymRef(callNode, TR::NULLCHK, 1, passThru, comp()->getSymRefTab()->findOrCreateNullCheckSymbolRef(comp()->getMethodSymbol()));
TR::Node *isNonNull = TR::Node::create(callNode, TR::acmpne, 2, valueNode, TR::Node::aconst(0));
TR::Node *nullCheck = TR::Node::createWithSymRef(callNode, TR::ZEROCHK, 1, isNonNull,
comp()->getSymRefTab()->findOrCreateArrayStoreExceptionSymbolRef(comp()->getMethodSymbol()));
callTree->insertBefore(TR::TreeTop::create(comp(), nullCheck));
if (trace())
{
Expand Down
94 changes: 47 additions & 47 deletions runtime/compiler/optimizer/TreeLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,20 +690,14 @@ NonNullableArrayNullStoreCheckTransformer::lower(TR::Node* const node, TR::TreeT
// | BBEnd | |
// +--------------------------------+ |
// | BBStart (Extension) | |
// | ificmpeq -->------------------*---------+
// | iand | |
// | iloadi <isClassFlags> | |
// | aloadi <componentClass> | |
// | ZEROCHK jitArrayStoreException | |
// | icmpeq | |
// | iand | |
// | iloadi <isClassFlags> | |
// | aloadi <vft-symbol> | |
// | ==><array-reference> | |
// | iconst J9ClassIsPrimitiveValueType |
// | iconst 0 | |
// | BBEnd | |
// +--------------------------------+ |
// | BBStart (Extension) | |
// | NULLCHK | |
// | Passthrough | |
// | ==><value-reference> | |
// | iconst J9ClassArrayIsNullRestricted|
// | iconst 0 | |
// | BBEnd | |
// +--------------------------------+ |
// | |
Expand Down Expand Up @@ -731,17 +725,18 @@ NonNullableArrayNullStoreCheckTransformer::lower(TR::Node* const node, TR::TreeT
tt->getPrevTreeTop()->join(nextTT);
TR::Block *nextBlock = prevBlock->splitPostGRA(nextTT, cfg);

TR::Node *ifNode = comp()->fej9()->checkArrayCompClassPrimitiveValueType(destChild, TR::ificmpeq);
TR::SymbolReference* const vftSymRef = comp()->getSymRefTab()->findOrCreateVftSymbolRef();

ifNode->setBranchDestination(nextBlock->getEntry());
TR::Node *vftNode = TR::Node::createWithSymRef(node, TR::aloadi, 1, destChild, vftSymRef);

// Copy register dependencies from the end of the block split
// to the ificmpeq, which checks for a value type, that's being
// added to the end of that block
//
copyRegisterDependency(prevBlock->getExit()->getNode(), ifNode);
TR::Node *testIsNullRestrictedArray = comp()->fej9()->testIsArrayClassNullRestrictedType(vftNode);
TR::Node *testIsNotNullRestrictedArray = TR::Node::create(TR::icmpeq, 2, testIsNullRestrictedArray, TR::Node::iconst(0));

TR::TreeTop *ifArrayCompClassPrimitiveValueTypeTT = prevBlock->append(TR::TreeTop::create(comp(), ifNode));
TR::ResolvedMethodSymbol *currentMethod = comp()->getMethodSymbol();

TR::SymbolReference *jitThrowArrayStoreException = comp()->getSymRefTab()->findOrCreateArrayStoreExceptionSymbolRef(currentMethod);
TR::Node *checkNotNullRestrictedArray = TR::Node::createWithSymRef(TR::ZEROCHK, 1, 1, testIsNotNullRestrictedArray, jitThrowArrayStoreException);
TR::TreeTop *checkNotNullRestrictedArrayTT = prevBlock->append(TR::TreeTop::create(comp(), checkNotNullRestrictedArray));

bool enableTrace = trace();
auto* const nullConst = TR::Node::aconst(0);
Expand All @@ -753,35 +748,24 @@ NonNullableArrayNullStoreCheckTransformer::lower(TR::Node* const node, TR::TreeT
//
copyRegisterDependency(prevBlock->getExit()->getNode(), checkValueNull);

TR::TreeTop *checkValueNullTT = ifArrayCompClassPrimitiveValueTypeTT->insertBefore(TR::TreeTop::create(comp(), checkValueNull));
TR::TreeTop *checkValueNullTT = checkNotNullRestrictedArrayTT->insertBefore(TR::TreeTop::create(comp(), checkValueNull));

if (enableTrace)
{
traceMsg(comp(),"checkValueNull n%dn is inserted before n%dn in prevBlock %d\n", checkValueNull->getGlobalIndex(), ifNode->getGlobalIndex(), prevBlock->getNumber());
traceMsg(comp(),"checkValueNull n%dn is inserted before n%dn in prevBlock %d\n", checkValueNull->getGlobalIndex(), checkNotNullRestrictedArray->getGlobalIndex(), prevBlock->getNumber());
}

TR::Block *compTypeTestBlock = prevBlock->split(ifArrayCompClassPrimitiveValueTypeTT, cfg);
compTypeTestBlock->setIsExtensionOfPreviousBlock(true);
TR::Block *checkNotNullRestrictedBlock = prevBlock->split(checkNotNullRestrictedArrayTT, cfg);
checkNotNullRestrictedBlock->setIsExtensionOfPreviousBlock(true);

cfg->addEdge(prevBlock, nextBlock);

if (enableTrace)
{
traceMsg(comp(),"ifArrayCompClassValueTypeTT n%dn is isolated in compTypeTestBlock %d\n", ifNode->getGlobalIndex(), compTypeTestBlock->getNumber());
traceMsg(comp(),"checkNotNullRestrictedArray n%dn is isolated in checkNotNullRestrictedBlock %d\n", checkNotNullRestrictedArray->getGlobalIndex(), checkNotNullRestrictedBlock->getNumber());
}

TR::ResolvedMethodSymbol *currentMethod = comp()->getMethodSymbol();

TR::Node *passThru = TR::Node::create(node, TR::PassThrough, 1, sourceChild);
TR::Node *nullCheck = TR::Node::createWithSymRef(node, TR::NULLCHK, 1, passThru,
comp()->getSymRefTab()->findOrCreateNullCheckSymbolRef(currentMethod));
TR::TreeTop *nullCheckTT = compTypeTestBlock->append(TR::TreeTop::create(comp(), nullCheck));

TR::Block *nullCheckBlock = compTypeTestBlock->split(nullCheckTT, cfg);

nullCheckBlock->setIsExtensionOfPreviousBlock(true);

cfg->addEdge(compTypeTestBlock, nextBlock);
cfg->addEdge(checkNotNullRestrictedBlock, nextBlock);
}
else
{
Expand Down Expand Up @@ -901,6 +885,9 @@ static bool skipBoundChecks(TR::Compilation *comp, TR::Node *node)
*
Before:
+----------------------------------------+
|NULLCHK |
| PassThrough |
| ==>iRegLoad (array address) |
|treetop |
| acall jitLoadFlattenableArrayElement |
| ==>iRegLoad |
Expand All @@ -911,15 +898,16 @@ static bool skipBoundChecks(TR::Compilation *comp, TR::Node *node)
After:
+------------------------------------------+
|BBStart |
|treetop |
| ==>iRegLoad |
|NULLCHK |
| PassThrough |
| ==>iRegLoad (array address) |
|treetop |
| ==>aRegLoad |
|ificmpne ----->---------------------------+-----------+
| iand | |
| iloadi <isClassFlags> | |
| ... | |
| iconst J9ClassIsPrimitiveValueType | |
| iconst J9ClassArrayIsNullRestricted | |
| iconst 0 | |
| GlRegDeps () | |
| ==>aRegLoad | |
Expand Down Expand Up @@ -1120,8 +1108,13 @@ LoadArrayElementTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
///////////////////////////////////////
// 9. Create ificmpne node that checks classFlags

TR::SymbolReference* const vftSymRef = comp->getSymRefTab()->findOrCreateVftSymbolRef();

TR::Node *vftNode = TR::Node::createWithSymRef(node, TR::aloadi, 1, anchoredArrayBaseAddressNode, vftSymRef);
TR::Node *testIsArrayClassNullRestrictedNode = comp->fej9()->testIsArrayClassNullRestrictedType(vftNode);

// The branch destination will be set up later
TR::Node *ifNode = comp->fej9()->checkArrayCompClassPrimitiveValueType(anchoredArrayBaseAddressNode, TR::ificmpne);
TR::Node *ifNode = TR::Node::createif(TR::ificmpne, testIsArrayClassNullRestrictedNode, TR::Node::iconst(0));
hzongaro marked this conversation as resolved.
Show resolved Hide resolved

// Copy register dependency to the ificmpne node that's being appended to the current block
copyRegisterDependency(arrayElementLoadBlock->getExit()->getNode(), ifNode);
Expand Down Expand Up @@ -1226,6 +1219,9 @@ static bool skipArrayStoreChecks(TR::Compilation *comp, TR::Node *node)
*
Before:
+-------------------------------------------+
|NULLCHK |
| PassThrough |
| aload <ArrayAddress> |
|treetop |
| acall jitStoreFlattenableArrayElement |
| aload <value> |
Expand All @@ -1237,8 +1233,9 @@ static bool skipArrayStoreChecks(TR::Compilation *comp, TR::Node *node)
After:
+-------------------------------------------+
|BBStart |
|treetop |
| aload <ArrayAddress> |
|NULLCHK |
| PassThrough |
| aload <ArrayAddress> |
|treetop |
| aload <index> |
|treetop |
Expand All @@ -1247,7 +1244,7 @@ static bool skipArrayStoreChecks(TR::Compilation *comp, TR::Node *node)
| iand | |
| iloadi <isClassFlags> | |
| ... | |
| iconst J9ClassIsPrimitiveValueType | |
| iconst J9ClassArrayIsNullRestricted | |
| iconst 0 | |
| GlRegDeps () | |
| PassThrough rcx | |
Expand All @@ -1260,8 +1257,6 @@ static bool skipArrayStoreChecks(TR::Compilation *comp, TR::Node *node)
+-------------------------------------------+ |
+-------------------------------------------+ |
|BBStart (extension of previous block) | |
|NULLCHK on n82n [if required] | |
| ... | |
|BNDCHK | |
| ... | |
|treetop | |
Expand Down Expand Up @@ -1476,8 +1471,13 @@ StoreArrayElementTransformer::lower(TR::Node* const node, TR::TreeTop* const tt)
///////////////////////////////////////
// 8. Create the ificmpne node that checks classFlags

TR::SymbolReference* const vftSymRef = comp->getSymRefTab()->findOrCreateVftSymbolRef();

TR::Node *vftNode = TR::Node::createWithSymRef(node, TR::aloadi, 1, anchoredArrayBaseAddressNode, vftSymRef);
TR::Node *testIsArrayClassNullRestrictedNode = comp->fej9()->testIsArrayClassNullRestrictedType(vftNode);

// The branch destination will be set up later
TR::Node *ifNode = comp->fej9()->checkArrayCompClassPrimitiveValueType(anchoredArrayBaseAddressNode, TR::ificmpne);
TR::Node *ifNode = TR::Node::createif(TR::ificmpne, testIsArrayClassNullRestrictedNode, TR::Node::iconst(0));
hzongaro marked this conversation as resolved.
Show resolved Hide resolved

// Copy register dependency to the ificmpne node that's being appended to the current block
copyRegisterDependency(arrayElementStoreBlock->getExit()->getNode(), ifNode);
Expand Down