[GR-76286] Fix negative sequence slot index handling also for list and mmap.

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py

@@ -211,6 +211,15 @@ def _reference_setitem(args):
     return seq
 
 
+def _reference_delitem(args):
+    seq = args[0]
+    idx = args[1]
+    if not hasattr(seq, '__delitem__'):
+        raise TypeError
+    seq.__delitem__(idx)
+    return seq
+
+
 def _reference_fast(args):
     obj = args[0]
     if isinstance(obj, tuple) or isinstance(obj, list):
@@ -1469,6 +1478,49 @@ def _reference_delslice(args):
         cmpfunc=unhandled_error_compare
     )
 
+    test_PySequence_SetItem_list_negative_boundary = test_PySequence_SetItem.copy()
+    test_PySequence_SetItem_list_negative_boundary.pfunc = _reference_setitem
+    test_PySequence_SetItem_list_negative_boundary.parameters = lambda: (
+        (['a', 'b', 'c'], -4, 'z'),
+    )
+
+    test_PySequence_SetItem_mmap_negative_boundary = test_PySequence_SetItem.copy()
+    test_PySequence_SetItem_mmap_negative_boundary.pfunc = _reference_setitem
+    test_PySequence_SetItem_mmap_negative_boundary.parameters = lambda: (
+        (create_anon_mmap(b'hello world!'), -14, b'z'),
+    )
+
+    test_PySequence_DelItem = CPyExtFunction(
+        _reference_delitem,
+        lambda: (
+            ([1, 2, 3], 1),
+        ),
+        code=''' PyObject* wrap_PySequence_DelItem(PyObject* sequence, Py_ssize_t idx) {
+            if (PySequence_DelItem(sequence, idx) < 0) {
+                return NULL;
+            }
+            return sequence;
+        }
+        ''',
+        resultspec="O",
+        argspec='On',
+        arguments=["PyObject* sequence", "Py_ssize_t idx"],
+        callfunction="wrap_PySequence_DelItem",
+        cmpfunc=unhandled_error_compare
+    )
+
+    test_PySequence_DelItem_list_negative_boundary = test_PySequence_DelItem.copy()
+    test_PySequence_DelItem_list_negative_boundary.pfunc = _reference_delitem
+    test_PySequence_DelItem_list_negative_boundary.parameters = lambda: (
+        ([1, 2, 3], -4),
+    )
+
+    test_PySequence_DelItem_mmap_negative_boundary = test_PySequence_DelItem.copy()
+    test_PySequence_DelItem_mmap_negative_boundary.pfunc = _reference_delitem
+    test_PySequence_DelItem_mmap_negative_boundary.parameters = lambda: (
+        (create_anon_mmap(b'hello world!'), -14),
+    )
+
     test_PySequence_Tuple = CPyExtFunction(
         lambda args: tuple(args[0]),
         lambda: (

graalpython/com.oracle.graal.python.test/src/tests/test_deque.py

@@ -263,6 +263,15 @@ def test_getitem(self):
         d = deque('superman')
         self.assertEqual(d[0], 's')
         self.assertEqual(d[-1], 'n')
+
+        d = deque([10, 20, 30, 40, 50])
+        self.assertEqual(d[-len(d)], 10)
+        self.assertEqual(d.__getitem__(-len(d)), 10)
+        for i in range(-len(d) - 1, -2 * len(d) - 1, -1):
+            with self.assertRaises(IndexError):
+                d[i]
+            self.assertRaises(IndexError, d.__getitem__, i)
+
         d = deque()
         self.assertRaises(IndexError, d.__getitem__, 0)
         self.assertRaises(IndexError, d.__getitem__, -1)
@@ -380,6 +389,16 @@ def test_setitem(self):
             l[i] = 7*i
         self.assertEqual(list(d), l)
 
+        d = deque([10, 20, 30, 40, 50])
+        d[-len(d)] = 11
+        d.__setitem__(-1, 55)
+        self.assertEqual(list(d), [11, 20, 30, 40, 55])
+        for i in range(-len(d) - 1, -2 * len(d) - 1, -1):
+            with self.assertRaises(IndexError):
+                d[i] = 99
+            self.assertRaises(IndexError, d.__setitem__, i, 99)
+        self.assertEqual(list(d), [11, 20, 30, 40, 55])
+
     def test_delitem(self):
         n = 500         # O(n**2) test, don't make this too big
         d = deque(range(n))
@@ -394,6 +413,20 @@ def test_delitem(self):
             self.assertTrue(val not in d)
         self.assertEqual(len(d), 0)
 
+        data = [10, 20, 30, 40, 50]
+        d = deque(data)
+        del d[-len(d)]
+        self.assertEqual(list(d), [20, 30, 40, 50])
+        d = deque(data)
+        d.__delitem__(-1)
+        self.assertEqual(list(d), [10, 20, 30, 40])
+        for i in range(-len(data) - 1, -2 * len(data) - 1, -1):
+            d = deque(data)
+            with self.assertRaises(IndexError):
+                del d[i]
+            self.assertRaises(IndexError, d.__delitem__, i)
+            self.assertEqual(list(d), data)
+
     def test_negative_index_out_of_range(self):
         # GH-923: an index < -len must raise IndexError, not wrap around twice
         d = deque([10, 20, 30, 40, 50])

graalpython/com.oracle.graal.python.test/src/tests/test_list.py

@@ -168,6 +168,25 @@ def __index__(self):
         cpy = [l[idxObj], l[idxObj], l[idxObj]]
         self.assertEqual(cpy, l)
 
+        l = [10, 20, 30, 40, 50]
+        self.assertEqual(l[-len(l)], 10)
+        self.assertEqual(l.__getitem__(-len(l)), 10)
+        for i in range(-len(l) - 1, -2 * len(l) - 1, -1):
+            with self.assertRaises(IndexError):
+                l[i]
+            self.assertRaises(IndexError, l.__getitem__, i)
+
+    def test_setitem_negative_index_boundary(self):
+        l = [10, 20, 30, 40, 50]
+        l[-len(l)] = 11
+        l.__setitem__(-1, 55)
+        self.assertEqual(l, [11, 20, 30, 40, 55])
+        for i in range(-len(l) - 1, -2 * len(l) - 1, -1):
+            with self.assertRaises(IndexError):
+                l[i] = 99
+            self.assertRaises(IndexError, l.__setitem__, i, 99)
+        self.assertEqual(l, [11, 20, 30, 40, 55])
+
     def pop_all_list(self, list):
         size = len(list)
         self.assertRaises(IndexError, list.pop, size)
@@ -248,6 +267,20 @@ def test_del_item(self):
         del l[4]
         self.assertEqual(["1", "2", "3", "4", "6"], l)
 
+        data = [10, 20, 30, 40, 50]
+        l = list(data)
+        del l[-len(l)]
+        self.assertEqual(l, [20, 30, 40, 50])
+        l = list(data)
+        l.__delitem__(-1)
+        self.assertEqual(l, [10, 20, 30, 40])
+        for i in range(-len(data) - 1, -2 * len(data) - 1, -1):
+            l = list(data)
+            with self.assertRaises(IndexError):
+                del l[i]
+            self.assertRaises(IndexError, l.__delitem__, i)
+            self.assertEqual(l, data)
+
     def test_del_border(self):
         l = [1, 2, 3]
         self.assertRaises(IndexError, l.__delitem__, 3)

graalpython/com.oracle.graal.python.test/src/tests/test_mmap.py

@@ -87,6 +87,76 @@ def test_getitem():
         m[i] = i
     assert m[slice(-10, 100)] == b'\x02\x03\x04\x05\x06\x07\x08\t\n\x0b'
 
+    assert m[-len(m)] == 0
+    assert m.__getitem__(-len(m)) == 0
+    for i in range(-len(m) - 1, -2 * len(m) - 1, -1):
+        try:
+            m[i]
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+        try:
+            m.__getitem__(i)
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+
+
+def test_setitem_negative_index_boundary():
+    m = mmap.mmap(-1, 5)
+    m[:] = b'abcde'
+    m[-len(m)] = ord('A')
+    m.__setitem__(-1, ord('E'))
+    assert m[:] == b'AbcdE'
+    for i in range(-len(m) - 1, -2 * len(m) - 1, -1):
+        try:
+            m[i] = ord('x')
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+        try:
+            m.__setitem__(i, ord('x'))
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+    assert m[:] == b'AbcdE'
+
+
+def test_delitem_negative_index_boundary():
+    m = mmap.mmap(-1, 5)
+    m[:] = b'abcde'
+    try:
+        del m[-len(m)]
+    except TypeError:
+        pass
+    else:
+        assert False, "expected TypeError"
+    try:
+        m.__delitem__(-1)
+    except TypeError:
+        pass
+    else:
+        assert False, "expected TypeError"
+
+    for i in range(-len(m) - 1, -2 * len(m) - 1, -1):
+        try:
+            del m[i]
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+        try:
+            m.__delitem__(i)
+        except IndexError:
+            pass
+        else:
+            assert False, "expected IndexError"
+    assert m[:] == b'abcde'
+
 
 def test_readline():
     m = mmap.mmap(-1, 9)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/deque/DequeBuiltins.java

@@ -823,10 +823,9 @@ public abstract static class DequeGetItemNode extends SqItemBuiltinNode {
         @Specialization
         @TruffleBoundary
         Object doGeneric(PDeque self, int idx,
-                        @Bind Node inliningTarget,
-                        @Cached PRaiseNode raiseNode) {
+                        @Bind Node inliningTarget) {
             if (idx < 0 || idx >= self.getSize()) {
-                throw raiseNode.raise(inliningTarget, IndexError, ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE);
+                throw PRaiseNode.raiseStatic(inliningTarget, IndexError, ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE);
             }
             return doGetItem(self, idx);
         }
@@ -846,12 +845,12 @@ Object doGetItem(PDeque self, int idx) {
     @GenerateNodeFactory
     public abstract static class DequeSetItemNode extends SqAssItemBuiltinNode {
 
+        @TruffleBoundary
         @Specialization
         static void setOrDel(PDeque self, int idx, Object value,
-                        @Bind Node inliningTarget,
-                        @Cached PRaiseNode raiseNode) {
+                        @Bind Node inliningTarget) {
             if (idx < 0 || idx >= self.getSize()) {
-                throw raiseNode.raise(inliningTarget, IndexError, ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE);
+                throw PRaiseNode.raiseStatic(inliningTarget, IndexError, ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE);
             }
             self.setItem(idx, value != PNone.NO_VALUE ? value : null);
         }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

@@ -25,6 +25,7 @@
  */
 package com.oracle.graal.python.builtins.objects.list;
 
+import static com.oracle.graal.python.builtins.objects.common.IndexNodes.checkBounds;
 import static com.oracle.graal.python.nodes.BuiltinNames.J_APPEND;
 import static com.oracle.graal.python.nodes.BuiltinNames.J_EXTEND;
 import static com.oracle.graal.python.nodes.BuiltinNames.J_LIST;
@@ -340,9 +341,11 @@ static void doInt(Object self, int index, Object value,
                         @Bind Node inliningTarget,
                         @Exclusive @Cached GetListStorageNode getStorageNode,
                         @Cached ListNodes.UpdateListStorageNode updateStorageNode,
-                        @Cached("createForList()") SequenceStorageNodes.SetItemNode setItemNode) {
+                        @Cached SequenceStorageNodes.SetItemScalarGeneralizingNode setItemNode,
+                        @Exclusive @Cached PRaiseNode raiseNode) {
             var sequenceStorage = getStorageNode.execute(inliningTarget, self);
-            var newStorage = setItemNode.execute(sequenceStorage, index, value);
+            checkBounds(inliningTarget, raiseNode, ErrorMessages.LIST_ASSIGMENT_INDEX_OUT_OF_RANGE, index, sequenceStorage.length());
+            var newStorage = setItemNode.execute(inliningTarget, sequenceStorage, index, value, SequenceStorageNodes.ListGeneralizationNode.SUPPLIER);
             updateStorageNode.execute(inliningTarget, self, sequenceStorage, newStorage);
         }
 
@@ -351,10 +354,10 @@ static void doInt(Object self, int index, Object value,
         static void doGeneric(Object list, int index, @SuppressWarnings("unused") Object value,
                         @Bind Node inliningTarget,
                         @Exclusive @Cached GetListStorageNode getStorageNode,
-                        @Cached NormalizeIndexNode normalizeIndexNode,
-                        @Cached SequenceStorageNodes.DeleteItemNode deleteItemNode) {
+                        @Cached SequenceStorageNodes.DeleteItemNode deleteItemNode,
+                        @Exclusive @Cached PRaiseNode raiseNode) {
             var sequenceStorage = getStorageNode.execute(inliningTarget, list);
-            index = normalizeIndexNode.execute(index, sequenceStorage.length());
+            checkBounds(inliningTarget, raiseNode, ErrorMessages.LIST_ASSIGMENT_INDEX_OUT_OF_RANGE, index, sequenceStorage.length());
             deleteItemNode.execute(inliningTarget, sequenceStorage, index);
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mmap/MMapBuiltins.java

@@ -398,10 +398,7 @@ static void doSingle(VirtualFrame frame, PMMap self, int index, Object val,
                 throw raiseNode.raise(inliningTarget, PythonBuiltinClassType.ValueError, MMAP_CLOSED_OR_INVALID);
             }
             long len = self.getLength();
-            long idx = index < 0 ? index + len : index;
-            if (idx < 0 || idx >= len) {
-                throw raiseNode.raise(inliningTarget, PythonBuiltinClassType.IndexError, MMAP_INDEX_OUT_OF_RANGE);
-            }
+            checkBounds(inliningTarget, raiseNode, MMAP_INDEX_OUT_OF_RANGE, index, len);
             if (val == PNone.NO_VALUE) {
                 throw raiseNode.raise(inliningTarget, PythonBuiltinClassType.TypeError, MMAP_OBJECT_DOESNT_SUPPORT_ITEM_DELETION);
             }
@@ -413,7 +410,7 @@ static void doSingle(VirtualFrame frame, PMMap self, int index, Object val,
             }
             byte b = bufferLib.readByte(val, 0);
             try {
-                posixSupportLib.mmapWriteByte(context.getPosixSupport(), self.getPosixSupportHandle(), idx, b);
+                posixSupportLib.mmapWriteByte(context.getPosixSupport(), self.getPosixSupportHandle(), index, b);
             } catch (PosixException ex) {
                 throw constructAndRaiseNode.get(inliningTarget).raiseOSErrorFromPosixException(frame, ex);
             }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSqAssItem.java

@@ -51,10 +51,9 @@
 import com.oracle.graal.python.PythonLanguage;
 import com.oracle.graal.python.builtins.Python3Core;
 import com.oracle.graal.python.builtins.PythonBuiltinClassType;
-import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionInvoker;
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.EnsurePythonObjectNode;
-import com.oracle.graal.python.builtins.objects.type.slots.TpSlotInquiry.CheckInquiryResultNode;
+import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionInvoker;
 import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.PExternalFunctionWrapper;
 import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
 import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeInternalNode;
@@ -67,6 +66,7 @@
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlot.TpSlotManaged;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlot.TpSlotNative;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlot.TpSlotPython;
+import com.oracle.graal.python.builtins.objects.type.slots.TpSlotInquiry.CheckInquiryResultNode;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSizeArgFun.FixNegativeIndex;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSqAssItemFactory.CallSlotSqAssItemNodeGen;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSqAssItemFactory.WrapSqDelItemBuiltinNodeGen;
@@ -203,15 +203,15 @@ Object doIntIndex(VirtualFrame frame, Object self, int index) {
         Object doGeneric(VirtualFrame frame, Object self, Object index,
                         @Bind Node inliningTarget,
                         @Cached PyNumberAsSizeNode asSizeNode) {
-            int size = asSizeNode.executeExact(frame, inliningTarget, index, PythonBuiltinClassType.OverflowError);
-            if (size < 0) {
+            int indexAsSize = asSizeNode.executeExact(frame, inliningTarget, index, PythonBuiltinClassType.OverflowError);
+            if (indexAsSize < 0) {
                 if (fixNegativeIndex == null) {
                     CompilerDirectives.transferToInterpreterAndInvalidate();
                     fixNegativeIndex = insert(FixNegativeIndex.create());
                 }
-                size = fixNegativeIndex.execute(frame, size, self);
+                indexAsSize = fixNegativeIndex.execute(frame, indexAsSize, self);
             }
-            slotNode.executeIntKey(frame, self, size, PNone.NO_VALUE);
+            slotNode.executeIntKey(frame, self, indexAsSize, PNone.NO_VALUE);
             return PNone.NONE;
         }
     }