From 85d16e5b20550df840de85eaf3836eafcc7e2632 Mon Sep 17 00:00:00 2001 From: Zac Tawfick Date: Fri, 5 Jun 2026 14:54:13 -0700 Subject: [PATCH] Raise IndexError for deque indices below -len. Fixes oracle/graalpython#923 Signed-off-by: Zac Tawfick --- .../src/tests/test_deque.py | 13 +++++++++++- .../builtins/objects/deque/DequeBuiltins.java | 21 ++++++++++++------- .../objects/type/slots/TpSlotSizeArgFun.java | 14 ++++++------- .../objects/type/slots/TpSlotSqAssItem.java | 4 ++-- 4 files changed, 34 insertions(+), 18 deletions(-) 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 251eae520e..86b7069d3f 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 @@ -1,4 +1,4 @@ -# Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # The Universal Permissive License (UPL), Version 1.0 @@ -394,6 +394,17 @@ def test_delitem(self): self.assertTrue(val not in d) self.assertEqual(len(d), 0) + 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]) + i = -len(d) - 1 + with self.assertRaises(IndexError): + d[i] + with self.assertRaises(IndexError): + d[i] = 0 + with self.assertRaises(IndexError): + del d[i] + def test_reverse(self): n = 500 # O(n**2) test, don't make this too big data = [random.random() for i in range(n)] 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 2e35ba8f06..57e83e896b 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * The Universal Permissive License (UPL), Version 1.0 @@ -76,7 +76,6 @@ import com.oracle.graal.python.builtins.PythonBuiltins; import com.oracle.graal.python.builtins.objects.PNone; import com.oracle.graal.python.builtins.objects.PNotImplemented; -import com.oracle.graal.python.builtins.objects.common.IndexNodes.NormalizeIndexCustomMessageNode; import com.oracle.graal.python.builtins.objects.deque.DequeBuiltinsClinicProviders.DequeInsertNodeClinicProviderGen; import com.oracle.graal.python.builtins.objects.deque.DequeBuiltinsClinicProviders.DequeRotateNodeClinicProviderGen; import com.oracle.graal.python.builtins.objects.function.PKeyword; @@ -824,9 +823,12 @@ public abstract static class DequeGetItemNode extends SqItemBuiltinNode { @Specialization @TruffleBoundary Object doGeneric(PDeque self, int idx, - @Cached NormalizeIndexCustomMessageNode normalizeIndexNode) { - int normIdx = normalizeIndexNode.execute(idx, self.getSize(), ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE); - return doGetItem(self, normIdx); + @Bind Node inliningTarget, + @Cached PRaiseNode raiseNode) { + if (idx < 0 || idx >= self.getSize()) { + throw raiseNode.raise(inliningTarget, IndexError, ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE); + } + return doGetItem(self, idx); } @TruffleBoundary @@ -846,9 +848,12 @@ public abstract static class DequeSetItemNode extends SqAssItemBuiltinNode { @Specialization static void setOrDel(PDeque self, int idx, Object value, - @Cached NormalizeIndexCustomMessageNode normalizeIndexNode) { - int normIdx = normalizeIndexNode.execute(idx, self.getSize(), ErrorMessages.DEQUE_INDEX_OUT_OF_RANGE); - self.setItem(normIdx, value != PNone.NO_VALUE ? value : null); + @Bind Node inliningTarget, + @Cached PRaiseNode raiseNode) { + if (idx < 0 || idx >= self.getSize()) { + throw raiseNode.raise(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/type/slots/TpSlotSizeArgFun.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSizeArgFun.java index 6be12fad54..451539aa2d 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSizeArgFun.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotSizeArgFun.java @@ -174,7 +174,7 @@ Object doGeneric(VirtualFrame frame, Object self, Object index, CompilerDirectives.transferToInterpreterAndInvalidate(); fixNegativeIndex = insert(FixNegativeIndex.create()); } - size = fixNegativeIndex.execute(frame, size, index); + size = fixNegativeIndex.execute(frame, size, self); } return slotNode.execute(frame, self, size); } @@ -210,17 +210,17 @@ Object doGeneric(VirtualFrame frame, Object self, Object index, @GenerateInline(false) // lazy public abstract static class FixNegativeIndex extends Node { - public abstract int execute(VirtualFrame frame, int indexAsSize, Object indexObj); + public abstract int execute(VirtualFrame frame, int indexAsSize, Object self); @Specialization - static int doIt(VirtualFrame frame, int indexAsSize, Object indexObj, + static int doIt(VirtualFrame frame, int indexAsSize, Object self, @Bind Node inliningTarget, - @Cached GetObjectSlotsNode getIndexSlots, + @Cached GetObjectSlotsNode getSelfSlots, @Cached CallSlotLenNode callSlotLen) { assert indexAsSize < 0; - TpSlots indexSlots = getIndexSlots.execute(inliningTarget, indexObj); - if (indexSlots.sq_length() != null) { - int len = callSlotLen.execute(frame, inliningTarget, indexSlots.sq_length(), indexObj); + TpSlots selfSlots = getSelfSlots.execute(inliningTarget, self); + if (selfSlots.sq_length() != null) { + int len = callSlotLen.execute(frame, inliningTarget, selfSlots.sq_length(), self); return indexAsSize + len; } return indexAsSize; 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 c4933fabc7..25264e8604 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 @@ -174,7 +174,7 @@ Object doGeneric(VirtualFrame frame, Object self, Object index, Object value, CompilerDirectives.transferToInterpreterAndInvalidate(); fixNegativeIndex = insert(FixNegativeIndex.create()); } - size = fixNegativeIndex.execute(frame, size, index); + size = fixNegativeIndex.execute(frame, size, self); } slotNode.executeIntKey(frame, self, size, value); return PNone.NONE; @@ -209,7 +209,7 @@ Object doGeneric(VirtualFrame frame, Object self, Object index, CompilerDirectives.transferToInterpreterAndInvalidate(); fixNegativeIndex = insert(FixNegativeIndex.create()); } - size = fixNegativeIndex.execute(frame, size, index); + size = fixNegativeIndex.execute(frame, size, self); } slotNode.executeIntKey(frame, self, size, PNone.NO_VALUE); return PNone.NONE;