Add bound checking to accelerated intrinsics#23937
Open
IBMJimmyk wants to merge 1 commit into
Open
Conversation
IBMJimmyk
force-pushed
the
indexOfChk
branch
3 times, most recently
from
19bbb96 to
b2e97b6
Compare
Contributor
Author
|
After recognizedCallTransformer: Block 2 is split into Block 2 and 3. A check is added at the end of block 2 to make sure the input array is big enough for the inflate operation. If so, the check in block 6 checks the length of the output array. If that is also good, the fast path version of inflate is called in block 5. If either check fails, it jumps to block 4. Block 4 is the slow path and stays as a call out until the very end. This path should only be taken if the AbstractStringBuilder race condition is hit and messes up the array sizes. |
Contributor
Author
|
After recognizedCallTransformer: This is somewhat similar to how
|
IBMJimmyk
changed the title
Contributor
Author
|
The bulk of this change are common changes. But, there are some small codegen changes affecting Power, X, Z and Arm. So I need to tag a bunch of people here. @hzongaro Can you take a look at the Common/X changes? |
hzongaro
added
comp:jit
depends:omr
zl-wang
requested changes
May 27, 2026
zl-wang
left a comment
zl-wang
left a comment
Contributor
There was a problem hiding this comment.
High-level comment: directly adding BNDChk node vs. waiting for slowPath getting to it eventually. which one is better? maybe there are difficulties in creating the exception range(s) for the metaData?
| TR::Compilation *comp = cg->comp(); | ||
| TR::MethodSymbol *methodSymbol = node->getSymbol()->getMethodSymbol(); | ||
| static const bool disableCRC32 = feGetEnv("TR_aarch64DisableCRC32") != NULL; | ||
| static const bool disableStringIntrinsicFlagChk = feGetEnv("TR_DisableStringIntrinsicFlagChk") != NULL; |
Contributor
There was a problem hiding this comment.
a bracket around the boolean expression is preferable and more readable.
Contributor
Author
There was a problem hiding this comment.
I can change this.
| TR::RecognizedMethod rm = node->getSymbol()->castToMethodSymbol()->getMandatoryRecognizedMethod(); | ||
|
|
||
| bool isILGenPass = !getLastRun(); | ||
| static const bool disableStringIntrinsicFlagChk = feGetEnv("TR_DisableStringIntrinsicFlagChk") != NULL; |
| bool disableCAEInlining = !cg->getSupportsInlineUnsafeCompareAndExchange(); | ||
| static bool disableOSW = feGetEnv("TR_noPauseOnSpinWait") != NULL; | ||
| static const bool disableStringIntrinsicFlagChk = feGetEnv("TR_DisableStringIntrinsicFlagChk") != NULL; | ||
|
|
| } | ||
| } | ||
|
|
||
| static const bool disableStringIntrinsicFlagChk = feGetEnv("TR_DisableStringIntrinsicFlagChk") != NULL; |
| bool disableCASInlining = !cg->getSupportsInlineUnsafeCompareAndSet(); | ||
| bool disableCAEInlining = !cg->getSupportsInlineUnsafeCompareAndExchange(); | ||
|
|
||
| static const bool disableStringIntrinsicFlagChk = feGetEnv("TR_DisableStringIntrinsicFlagChk") != NULL; |
Contributor
Author
|
For the For the |
Prior to JDK25, due to a race condition in AbstractStringBuilder, it is possible for some accelerated String intrinsics to perform an out of bounds array access. This applies to: java/lang/StringLatin1.inflate([BI[CII)V java/lang/StringLatin1.indexOf([BI[BII)I java/lang/StringUTF16.indexOf([BI[BII)I To address this, bound checking has been added such that the safe slow path code is performed in the rares cases that an out of bounds array access may occur. Former behavior is restored via the TR_DisableStringIntrinsicFlagChk envvar. As the problem does not exist in JDK25, the bound checking is not added in JDK25 and onward. Issue: eclipse-openj9#23880 Signed-off-by: jimmyk <jimmyk@ca.ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prior to JDK25, due to a race condition in AbstractStringBuilder, it is possible for some accelerated String intrinsics to perform an out of bounds array access. This applies to:
To address this, bound checking has been added such that the safe slow path code is performed in the rares cases that an out of bounds array access may occur.
Former behavior is restored via the
TR_DisableStringIntrinsicFlagChkenvvar. As the problem does not exist in JDK25, the bound checking is not added in JDK25 and onward.Depends on:
eclipse-omr/omr#8255
Issue with more details here:
#23880
Original Issue the problem was found is here:
#23353