diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py index 383ea8038b..ca3cc6cb29 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py +++ b/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: ( diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_deque.py b/graalpython/com.oracle.graal.python.test/src/tests/test_deque.py index 86b7069d3f..b5fa502779 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_deque.py +++ b/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]) diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_list.py b/graalpython/com.oracle.graal.python.test/src/tests/test_list.py index 9bfbeaf88c..09a659d784 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_list.py +++ b/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) diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_mmap.py b/graalpython/com.oracle.graal.python.test/src/tests/test_mmap.py index 082966834a..150333a6ae 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_mmap.py +++ b/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) diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/deque/DequeBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/deque/DequeBuiltins.java index 57e83e896b..936428ccc3 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/deque/DequeBuiltins.java +++ b/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); } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java index a8875923f5..2e45c04dbc 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java +++ b/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); } } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mmap/MMapBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mmap/MMapBuiltins.java index c3c7a364cb..81067e74c6 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mmap/MMapBuiltins.java +++ b/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); } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSqAssItem.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSqAssItem.java index 0d82ad6732..bc75b1c09c 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSqAssItem.java +++ b/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; } }