[GR-42807] Ensure deleted elements are filled with hole values.

woess · woess · commit 6b78b316db52 · 2022-12-09T22:41:30.000Z
PullRequest: js/2676
diff --git a/graal-js/src/com.oracle.truffle.js.test/js/GR-42807.js b/graal-js/src/com.oracle.truffle.js.test/js/GR-42807.js
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
+ */
+
+var s2 = "";
+var a0 = [];
+var a1 = [];
+var a2 = [];
+var a3 = [];
+var a4 = [];
+var o1 = {};
+for (let i = 0; i < 5; i++) {
+    Array.prototype.shift.apply(a1);
+    a1.splice(NaN, -2044533922.5, a0, s2);
+}
+for (let i = 0; i < 5; i++) {
+    Array.prototype.shift.apply(a2);
+    a2.splice(NaN, -2044533922.5, 3, 4);
+}
+for (let i = 0; i < 5; i++) {
+    Array.prototype.shift.apply(a3);
+    a3.splice(NaN, -2044533922.5, 3.1, 4.15);
+}
+for (let i = 0; i < 5; i++) {
+    Array.prototype.shift.apply(a4);
+    a4.splice(NaN, -2044533922.5, o1, o1);
+}
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractJSObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractJSObjectArray.java
@@ -140,6 +140,7 @@ protected final void fillHoles(JSDynamicObject object, int internalIndex, int gr
         if (grown != 0) {
             incrementHolesCount(object, Math.abs(grown) - 1);
         }
+        assert checkFillHoles(object, internalIndex, grown);
     }
 
     @Override
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractObjectArray.java
@@ -142,6 +142,7 @@ protected final void fillHoles(JSDynamicObject object, int internalIndex, int gr
         if (grown != 0) {
             incrementHolesCount(object, Math.abs(grown) - 1);
         }
+        assert checkFillHoles(object, internalIndex, grown);
     }
 
     @Override
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractWritableArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/AbstractWritableArray.java
@@ -199,8 +199,9 @@ protected final int prepareSupportedHoles(JSDynamicObject object, int index, Pro
     }
 
     @SuppressWarnings("unused")
-    protected void incrementHolesCount(JSDynamicObject object, int offset) {
-        throw Errors.shouldNotReachHere();
+    protected final void incrementHolesCount(JSDynamicObject object, int offset) {
+        assert isHolesType();
+        arraySetHoleCount(object, arrayGetHoleCount(object) + offset);
     }
 
     protected abstract void setHoleValue(JSDynamicObject object, int index);
@@ -348,6 +349,27 @@ protected void fillHoles(JSDynamicObject object, int internalIndex, int grown, P
         }
     }
 
+    protected final boolean checkFillHoles(JSDynamicObject object, int internalIndex, int grown) {
+        int start;
+        int end;
+        if (grown > 1) {
+            start = internalIndex - grown + 1;
+            end = internalIndex;
+        } else if (grown < -1) {
+            start = internalIndex + 1;
+            end = internalIndex - grown;
+        } else {
+            return true;
+        }
+
+        for (int i = start; i < end; i++) {
+            if (!isHolePrepared(object, i)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     public abstract AbstractWritableArray toDouble(JSDynamicObject object, long index, double value);
 
     public abstract AbstractWritableArray toObject(JSDynamicObject object, long index, Object value);
@@ -520,7 +542,7 @@ protected final void setLengthLessContiguous(JSDynamicObject object, long length
             if (SET_LENGTH_PROFILE.contiguousShrinkUsed(profile, newUsedLength < oldUsed)) {
                 if (isHolesType()) {
                     incrementHolesCount(object, -countHolesPrepared(object, arrayOffset + newUsedLength, arrayOffset + oldUsed));
-                    assert arrayGetHoleCount(object) == countHoles(object);
+                    assert assertHoleCount(object);
                 }
 
                 // use old arrayOffset
@@ -614,7 +636,7 @@ protected final ScriptArray deleteElementHoles(JSDynamicObject object, long inde
                 }
             }
         }
-        assert arrayGetHoleCount(object) == countHoles(object);
+        assert assertHoleCount(object);
         return this;
     }
 
@@ -806,6 +828,14 @@ private int countHolesPrepared(JSDynamicObject object, int start, int end) {
         return holeCount;
     }
 
+    protected final boolean assertHoleCount(JSDynamicObject object) {
+        assert isHolesType();
+        int holeCount = arrayGetHoleCount(object);
+        int countedHoles = countHoles(object);
+        assert holeCount == countedHoles : String.format("holeCount, %d, differs from the actual count, %d", holeCount, countedHoles);
+        return true;
+    }
+
     /**
      * Move {@code len} elements from {@code src} to {@code dst}.
      */
@@ -837,6 +867,20 @@ public ScriptArray shiftRangeImpl(JSDynamicObject object, long from) {
         return removeRangeImpl(object, 0, from);
     }
 
+    protected static boolean unusedElementsAreHoles(Object[] array, int usedStart, int usedLength) {
+        for (int i = 0; i < usedStart; i++) {
+            if (array[i] != null) {
+                return false;
+            }
+        }
+        for (int i = usedStart + usedLength; i < array.length; i++) {
+            if (array[i] != null) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     protected interface SetSupportedProfileAccess extends ProfileAccess {
         default boolean ensureCapacityGrow(ProfileHolder profile, boolean condition) {
             return profile.profile(this, 0, condition);
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ContiguousJSObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ContiguousJSObjectArray.java
@@ -52,6 +52,7 @@ public static ContiguousJSObjectArray makeContiguousJSObjectArray(JSDynamicObjec
                     int integrityLevel) {
         ContiguousJSObjectArray arrayType = createContiguousJSObjectArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
+        assert unusedElementsAreHoles(array, arrayOffset, usedLength);
         return arrayType;
     }
 
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ContiguousObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ContiguousObjectArray.java
@@ -51,6 +51,7 @@ public final class ContiguousObjectArray extends AbstractContiguousObjectArray {
     public static ContiguousObjectArray makeContiguousObjectArray(JSDynamicObject object, long length, Object[] array, long indexOffset, int arrayOffset, int usedLength, int integrityLevel) {
         ContiguousObjectArray arrayType = createContiguousObjectArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
+        assert unusedElementsAreHoles(array, arrayOffset, usedLength);
         return arrayType;
     }
 
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesDoubleArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesDoubleArray.java
@@ -65,7 +65,7 @@ public static HolesDoubleArray makeHolesDoubleArray(JSDynamicObject object, int
         HolesDoubleArray arrayType = createHolesDoubleArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
         arraySetHoleCount(object, holeCount);
-        assert holeCount == arrayType.countHoles(object);
+        assert arrayType.assertHoleCount(object);
         return arrayType;
     }
 
@@ -142,11 +142,6 @@ public AbstractDoubleArray toNonHoles(JSDynamicObject object, long index, Object
         return newArray;
     }
 
-    @Override
-    protected void incrementHolesCount(JSDynamicObject object, int offset) {
-        arraySetHoleCount(object, arrayGetHoleCount(object) + offset);
-    }
-
     @Override
     public boolean isSupported(JSDynamicObject object, long index) {
         return isSupportedHoles(object, index);
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesIntArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesIntArray.java
@@ -64,7 +64,7 @@ public static HolesIntArray makeHolesIntArray(JSDynamicObject object, int length
         HolesIntArray arrayType = createHolesIntArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
         arraySetHoleCount(object, holeCount);
-        assert holeCount == arrayType.countHoles(object) : String.format("holeCount, %d, differs from the actual count, %d", holeCount, arrayType.countHoles(object));
+        assert arrayType.assertHoleCount(object);
         return arrayType;
     }
 
@@ -160,11 +160,6 @@ public Object getInBoundsFast(JSDynamicObject object, int index) {
         return value;
     }
 
-    @Override
-    protected void incrementHolesCount(JSDynamicObject object, int offset) {
-        arraySetHoleCount(object, arrayGetHoleCount(object) + offset);
-    }
-
     @Override
     public HolesIntArray toHoles(JSDynamicObject object, long index, Object value) {
         return this;
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesJSObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesJSObjectArray.java
@@ -64,7 +64,7 @@ public static HolesJSObjectArray makeHolesJSObjectArray(JSDynamicObject object,
         HolesJSObjectArray arrayType = createHolesJSObjectArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
         arraySetHoleCount(object, holeCount);
-        assert holeCount == arrayType.countHoles(object);
+        assert arrayType.assertHoleCount(object);
         return arrayType;
     }
 
@@ -153,11 +153,6 @@ public AbstractWritableArray toObject(JSDynamicObject object, long index, Object
         return newArray;
     }
 
-    @Override
-    protected void incrementHolesCount(JSDynamicObject object, int offset) {
-        arraySetHoleCount(object, arrayGetHoleCount(object) + offset);
-    }
-
     @Override
     public int prepareInBounds(JSDynamicObject object, int index, ProfileHolder profile) {
         return prepareInBoundsHoles(object, index, profile);
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/HolesObjectArray.java
@@ -63,7 +63,7 @@ public static HolesObjectArray makeHolesObjectArray(JSDynamicObject object, int
         HolesObjectArray arrayType = createHolesObjectArray().setIntegrityLevel(integrityLevel);
         setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);
         arraySetHoleCount(object, holeCount);
-        assert holeCount == arrayType.countHoles(object) : String.format("holeCount, %d, differs from the actual count, %d", holeCount, arrayType.countHoles(object));
+        assert arrayType.assertHoleCount(object);
         return arrayType;
     }
 
@@ -135,11 +135,6 @@ public AbstractObjectArray toNonHoles(JSDynamicObject object, long index, Object
         return newArray;
     }
 
-    @Override
-    protected void incrementHolesCount(JSDynamicObject object, int offset) {
-        arraySetHoleCount(object, arrayGetHoleCount(object) + offset);
-    }
-
     @Override
     public int prepareInBounds(JSDynamicObject object, int index, ProfileHolder profile) {
         return prepareInBoundsHoles(object, index, profile);
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ZeroBasedJSObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ZeroBasedJSObjectArray.java
@@ -177,7 +177,9 @@ public ScriptArray removeRangeImpl(JSDynamicObject object, long start, long end)
     public ScriptArray shiftRangeImpl(JSDynamicObject object, long from) {
         int usedLength = getUsedLength(object);
         if (from < usedLength) {
-            return ContiguousJSObjectArray.makeContiguousJSObjectArray(object, lengthInt(object) - from, getArray(object), -from, (int) from, (int) (usedLength - from), integrityLevel);
+            JSDynamicObject[] array = getArray(object);
+            Arrays.fill(array, 0, (int) from, null);
+            return ContiguousJSObjectArray.makeContiguousJSObjectArray(object, lengthInt(object) - from, array, -from, (int) from, (int) (usedLength - from), integrityLevel);
         } else {
             return removeRangeImpl(object, 0, from);
         }
diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ZeroBasedObjectArray.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/array/dyn/ZeroBasedObjectArray.java
@@ -166,7 +166,9 @@ public ScriptArray removeRangeImpl(JSDynamicObject object, long start, long end)
     public ScriptArray shiftRangeImpl(JSDynamicObject object, long from) {
         int usedLength = getUsedLength(object);
         if (from < usedLength) {
-            return ContiguousObjectArray.makeContiguousObjectArray(object, lengthInt(object) - from, getArray(object), -from, (int) from, (int) (usedLength - from), integrityLevel);
+            Object[] array = getArray(object);
+            Arrays.fill(array, 0, (int) from, null);
+            return ContiguousObjectArray.makeContiguousObjectArray(object, lengthInt(object) - from, array, -from, (int) from, (int) (usedLength - from), integrityLevel);
         } else {
             return removeRangeImpl(object, 0, from);
         }

Original file line number	Diff line number	Diff line change
`@@ -140,6 +140,7 @@ protected final void fillHoles(JSDynamicObject object, int internalIndex, int gr`
`140`	`140`	`if (grown != 0) {`
`141`	`141`	`incrementHolesCount(object, Math.abs(grown) - 1);`
`142`	`142`	`}`
	`143`	`+ assert checkFillHoles(object, internalIndex, grown);`
`143`	`144`	`}`
`144`	`145`
`145`	`146`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -142,6 +142,7 @@ protected final void fillHoles(JSDynamicObject object, int internalIndex, int gr`
`142`	`142`	`if (grown != 0) {`
`143`	`143`	`incrementHolesCount(object, Math.abs(grown) - 1);`
`144`	`144`	`}`
	`145`	`+ assert checkFillHoles(object, internalIndex, grown);`
`145`	`146`	`}`
`146`	`147`
`147`	`148`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@ public static ContiguousJSObjectArray makeContiguousJSObjectArray(JSDynamicObjec`
`52`	`52`	`int integrityLevel) {`
`53`	`53`	`ContiguousJSObjectArray arrayType = createContiguousJSObjectArray().setIntegrityLevel(integrityLevel);`
`54`	`54`	`setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);`
	`55`	`+ assert unusedElementsAreHoles(array, arrayOffset, usedLength);`
`55`	`56`	`return arrayType;`
`56`	`57`	`}`
`57`	`58`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,7 @@ public final class ContiguousObjectArray extends AbstractContiguousObjectArray {`
`51`	`51`	`public static ContiguousObjectArray makeContiguousObjectArray(JSDynamicObject object, long length, Object[] array, long indexOffset, int arrayOffset, int usedLength, int integrityLevel) {`
`52`	`52`	`ContiguousObjectArray arrayType = createContiguousObjectArray().setIntegrityLevel(integrityLevel);`
`53`	`53`	`setArrayProperties(object, array, length, usedLength, indexOffset, arrayOffset);`
	`54`	`+ assert unusedElementsAreHoles(array, arrayOffset, usedLength);`
`54`	`55`	`return arrayType;`
`55`	`56`	`}`
`56`	`57`