Skip to content

Commit fa07fff

Browse files
committed
[GR-52946] Backport to 24.0 : GH-8638: Correctness problem when System.arraycopy a byte array.
PullRequest: graal/17390
2 parents 83f9932 + e9edc20 commit fa07fff

File tree

1 file changed

+36
-0
lines changed

1 file changed

+36
-0
lines changed

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/nodes/BasicArrayCopyNode.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,39 @@ public boolean isExact() {
255255
return false;
256256
}
257257

258+
/**
259+
* When dealing with arrays of primitives it's possible to store a larger value into a smaller
260+
* component type array. For example a long value can be written into a byte array by storing
261+
* the larger value in the first entry and storing {@code Illegal} in the slots whose value
262+
* comes from the long value. So a long in a byte array would be a long value followed by 7
263+
* {@code Illegal} values. That also means such a write much be treated as a group by arraycopy
264+
* so reads and writes can't be performed in the middle of the illegal values.
265+
* <p>
266+
* So to ensure that an arraycopy properly treats these kinds of writes as group, it's
267+
* sufficient to check that the first value is not {@code Illegal} and that the next value after
268+
* the end is not {@code Illegal} since {@code Illegal} values only appear in the tail of these
269+
* groups.
270+
*/
271+
private static boolean ensureIllegalValuesCanBeRepresented(int length, int srcPos, int srcLength, int dstPos, int dstLength, VirtualObjectNode src, VirtualObjectNode dst, VirtualizerTool tool) {
272+
if (length <= 0) {
273+
return true;
274+
}
275+
// check source
276+
ValueNode firstSrcEntry = tool.getEntry(src, srcPos);
277+
ValueNode followingSrcEntry = srcPos + length < srcLength ? tool.getEntry(src, srcPos + length) : null;
278+
if (firstSrcEntry.isIllegalConstant() || followingSrcEntry != null && followingSrcEntry.isIllegalConstant()) {
279+
return false;
280+
}
281+
282+
// check dst
283+
ValueNode firstDstEntry = tool.getEntry(dst, dstPos);
284+
ValueNode followingDstEntry = dstPos + length < dstLength ? tool.getEntry(dst, dstPos + length) : null;
285+
if (firstDstEntry.isIllegalConstant() || followingDstEntry != null && followingDstEntry.isIllegalConstant()) {
286+
return false;
287+
}
288+
return true;
289+
}
290+
258291
@Override
259292
public void virtualize(VirtualizerTool tool) {
260293
ValueNode sourcePosition = tool.getAlias(getSourcePosition());
@@ -288,6 +321,9 @@ public void virtualize(VirtualizerTool tool) {
288321
if (!checkEntryTypes(srcPosInt, len, srcVirtual, destVirtual.type().getComponentType(), tool)) {
289322
return;
290323
}
324+
if (!ensureIllegalValuesCanBeRepresented(len, srcPosInt, srcVirtual.entryCount(), destPosInt, destVirtual.entryCount(), srcVirtual, destVirtual, tool)) {
325+
return;
326+
}
291327
if (srcVirtual == destVirtual && srcPosInt < destPosInt) {
292328
// must copy backwards to avoid losing elements
293329
for (int i = len - 1; i >= 0; i--) {

0 commit comments

Comments
 (0)