Skip to content

Commit c08e34c

Browse files
committed
Use PySequence_GetItem in sequence iterator next
1 parent 17734a2 commit c08e34c

File tree

5 files changed

+155
-9
lines changed

5 files changed

+155
-9
lines changed

graalpython/com.oracle.graal.python.cext/src/abstract.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,31 @@ int PyTruffle_PySequence_Check(PyObject *s) {
228228
return seq && seq->sq_item != NULL;
229229
}
230230

231+
// downcall for native python objects
232+
// partially taken from CPython "Objects/abstract.c/PySequence_GetItem"
233+
PyObject* PyTruffle_PySequence_GetItem(PyObject *s, Py_ssize_t i)
234+
{
235+
PySequenceMethods *m = Py_TYPE(s)->tp_as_sequence;
236+
if (m && m->sq_item) {
237+
if (i < 0) {
238+
if (m->sq_length) {
239+
Py_ssize_t l = (*m->sq_length)(s);
240+
if (l < 0) {
241+
return NULL;
242+
}
243+
i += l;
244+
}
245+
}
246+
PyObject *res = m->sq_item(s, i);
247+
return res;
248+
}
249+
250+
if (Py_TYPE(s)->tp_as_mapping && Py_TYPE(s)->tp_as_mapping->mp_subscript) {
251+
return PyErr_Format(PyExc_TypeError, "%.200s is not a sequence", Py_TYPE(s)->tp_name);
252+
}
253+
return PyErr_Format(PyExc_TypeError, "'%.200s' object does not support indexing", Py_TYPE(s)->tp_name);
254+
}
255+
231256
// downcall for native python objects
232257
// taken from CPython "Objects/abstract.c/Py_Sequence_Size"
233258
Py_ssize_t PyTruffle_PySequence_Size(PyObject *s) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextAbstractBuiltins.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.builtins.modules.cext;
4242

43+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.OverflowError;
4344
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
4445
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
4546
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
@@ -113,9 +114,11 @@
113114
import com.oracle.graal.python.lib.PyObjectSizeNode;
114115
import com.oracle.graal.python.lib.PySequenceCheckNode;
115116
import com.oracle.graal.python.lib.PySequenceContainsNode;
117+
import com.oracle.graal.python.lib.PySequenceGetItemNode;
116118
import com.oracle.graal.python.lib.PySequenceIterSearchNode;
117119
import com.oracle.graal.python.lib.PySliceNew;
118120
import com.oracle.graal.python.nodes.ErrorMessages;
121+
import com.oracle.graal.python.nodes.PRaiseNode;
119122
import com.oracle.graal.python.nodes.attributes.WriteAttributeToDynamicObjectNode;
120123
import com.oracle.graal.python.nodes.attributes.WriteAttributeToObjectNode;
121124
import com.oracle.graal.python.nodes.builtins.ListNodes.ConstructListNode;
@@ -662,13 +665,12 @@ static int check(Object object,
662665
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject, Py_ssize_t}, call = Direct)
663666
abstract static class PySequence_GetItem extends CApiBinaryBuiltinNode {
664667
@Specialization
665-
Object doManaged(Object delegate, Object position,
666-
@Cached PySequenceCheckNode pySequenceCheck,
667-
@Cached PyObjectGetItem getItem) {
668-
if (!pySequenceCheck.execute(delegate)) {
669-
throw raise(TypeError, ErrorMessages.OBJ_DOES_NOT_SUPPORT_INDEXING, delegate);
668+
Object doManaged(Object delegate, long position,
669+
@Cached PySequenceGetItemNode getItemNode) {
670+
if ((int) position != position) {
671+
throw PRaiseNode.raiseUncached(this, OverflowError, ErrorMessages.CANNOT_FIT_P_INTO_INDEXSIZED_INT, position);
670672
}
671-
return getItem.execute(null, delegate, position);
673+
return getItemNode.execute(null, delegate, (int) position);
672674
}
673675
}
674676

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ public enum NativeCAPISymbol implements NativeCExtSymbol {
263263
FUN_PY_TRUFFLE_RELEASE_BUFFER("PyTruffle_ReleaseBuffer"),
264264
FUN_PY_TRUFFLE_PY_SEQUENCE_CHECK("PyTruffle_PySequence_Check"),
265265
FUN_PY_TRUFFLE_PY_SEQUENCE_SIZE("PyTruffle_PySequence_Size"),
266+
FUN_PY_TRUFFLE_PY_SEQUENCE_GET_ITEM("PyTruffle_PySequence_GetItem"),
266267
FUN_GET_PY_METHOD_DEF_TYPEID("get_PyMethodDef_typeid"),
267268
FUN_GET_INT_T_TYPEID("get_int_t_typeid"),
268269
FUN_GET_INT8_T_TYPEID("get_int8_t_typeid"),

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/iterator/IteratorBuiltins.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@
6262
import com.oracle.graal.python.lib.PyNumberAsSizeNode;
6363
import com.oracle.graal.python.lib.PyObjectGetAttr;
6464
import com.oracle.graal.python.lib.PyObjectSizeNode;
65+
import com.oracle.graal.python.lib.PySequenceGetItemNode;
6566
import com.oracle.graal.python.nodes.ErrorMessages;
66-
import com.oracle.graal.python.nodes.call.special.LookupAndCallBinaryNode;
6767
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
6868
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
6969
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
@@ -245,10 +245,17 @@ Object next(PSequenceIterator self,
245245
@Specialization(guards = {"!self.isExhausted()", "!self.isPSequence()"})
246246
Object next(VirtualFrame frame, PSequenceIterator self,
247247
@Bind("this") Node inliningTarget,
248-
@Cached("create(GetItem)") LookupAndCallBinaryNode callGetItem,
248+
@Cached PySequenceGetItemNode getItem,
249249
@Cached IsBuiltinObjectProfile profile) {
250250
try {
251-
return callGetItem.executeObject(frame, self.getObject(), self.index++);
251+
/*
252+
* This must use PySequence_GetItem and not any other get item nodes. The reason is
253+
* that other get item nodes call mp_subscript. Some extensions iterate self in
254+
* their mp_getitem which could result in infinite recursion.
255+
*
256+
* Example psycopg2:column_type.c:column_subscript
257+
*/
258+
return getItem.execute(frame, self.getObject(), self.index++);
252259
} catch (PException e) {
253260
e.expectIndexError(inliningTarget, profile);
254261
return stopIteration(self);
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.lib;
42+
43+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
44+
45+
import com.oracle.graal.python.builtins.objects.PNone;
46+
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
47+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes;
48+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
49+
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
51+
import com.oracle.graal.python.nodes.ErrorMessages;
52+
import com.oracle.graal.python.nodes.PNodeWithRaiseAndIndirectCall;
53+
import com.oracle.graal.python.nodes.PRaiseNode;
54+
import com.oracle.graal.python.nodes.call.special.CallBinaryMethodNode;
55+
import com.oracle.graal.python.nodes.call.special.LookupSpecialMethodSlotNode;
56+
import com.oracle.graal.python.nodes.object.InlinedGetClassNode;
57+
import com.oracle.graal.python.runtime.ExecutionContext;
58+
import com.oracle.graal.python.runtime.PythonContext;
59+
import com.oracle.truffle.api.dsl.Bind;
60+
import com.oracle.truffle.api.dsl.Cached;
61+
import com.oracle.truffle.api.dsl.Specialization;
62+
import com.oracle.truffle.api.frame.Frame;
63+
import com.oracle.truffle.api.frame.VirtualFrame;
64+
import com.oracle.truffle.api.nodes.Node;
65+
66+
/**
67+
* Equivalent of CPython's {@code PySequence_GetItem}. For native object it would only call
68+
* {@code sq_item} and never {@code mp_item}.
69+
*/
70+
public abstract class PySequenceGetItemNode extends PNodeWithRaiseAndIndirectCall {
71+
public abstract Object execute(Frame frame, Object object, int index);
72+
73+
@Specialization(guards = "!isNativeObject(object)")
74+
static Object doGenericManaged(VirtualFrame frame, Object object, int index,
75+
@Bind("this") Node inliningTarget,
76+
@Cached InlinedGetClassNode getClassNode,
77+
@Cached PySequenceCheckNode sequenceCheckNode,
78+
@Cached PyMappingCheckNode mappingCheckNode,
79+
@Cached(parameters = "GetItem") LookupSpecialMethodSlotNode lookupGetItem,
80+
@Cached CallBinaryMethodNode callGetItem,
81+
@Cached PRaiseNode.Lazy raise) {
82+
if (sequenceCheckNode.execute(object)) {
83+
Object type = getClassNode.execute(inliningTarget, object);
84+
Object getItem = lookupGetItem.execute(frame, type, object);
85+
assert getItem != PNone.NO_VALUE;
86+
return callGetItem.executeObject(frame, getItem, object, index);
87+
}
88+
if (mappingCheckNode.execute(object)) {
89+
throw raise.get(inliningTarget).raise(TypeError, ErrorMessages.IS_NOT_A_SEQUENCE, object);
90+
} else {
91+
throw raise.get(inliningTarget).raise(TypeError, ErrorMessages.OBJ_DOES_NOT_SUPPORT_INDEXING, object);
92+
}
93+
}
94+
95+
@Specialization
96+
Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index,
97+
@Bind("this") Node inliningTarget,
98+
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
99+
@Cached CApiTransitions.NativeToPythonNode toPythonNode,
100+
@Cached CExtNodes.PCallCapiFunction callGetItem,
101+
@Cached ExternalFunctionNodes.DefaultCheckFunctionResultNode checkFunctionResultNode) {
102+
Object savedState = ExecutionContext.IndirectCallContext.enter(frame, this);
103+
try {
104+
Object nativeResult = callGetItem.call(NativeCAPISymbol.FUN_PY_TRUFFLE_PY_SEQUENCE_GET_ITEM, toNativeNode.execute(object), index);
105+
checkFunctionResultNode.execute(PythonContext.get(inliningTarget), NativeCAPISymbol.FUN_PY_TRUFFLE_PY_SEQUENCE_GET_ITEM.getTsName(), nativeResult);
106+
return toPythonNode.execute(nativeResult);
107+
} finally {
108+
ExecutionContext.IndirectCallContext.exit(frame, this, savedState);
109+
}
110+
}
111+
}

0 commit comments

Comments
 (0)