Skip to content

Commit 79a6daa

Browse files
steve-ssteve-s
committed
[GR-31208] Fix unbounded recursive nodes and explode loops.
PullRequest: graalpython/1786
2 parents 876a674 + 5cd027d commit 79a6daa

File tree

13 files changed

+272
-142
lines changed

13 files changed

+272
-142
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_isinstance.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,46 @@ def test_isinstance_abstract():
229229
assert isinstance(AbstractChild(), AbstractSuper)
230230
assert not isinstance(AbstractChild(), Super)
231231
assert not isinstance(AbstractChild(), Child)
232+
233+
234+
def test_isinstance_recursive():
235+
called_instancecheck = 0
236+
expected_other = Child()
237+
238+
class UnrelatedMeta(type):
239+
def __instancecheck__(self, other):
240+
nonlocal called_instancecheck
241+
called_instancecheck += 1
242+
assert other == expected_other
243+
# Force it do read caller frame for some more fun
244+
# Note that we should be now in indirect call
245+
globals()
246+
return super(UnrelatedMeta, self).__instancecheck__(other)
247+
248+
class Unrelated(metaclass=UnrelatedMeta):
249+
pass
250+
251+
tpl = (Unrelated,)
252+
for i in range(1, 20):
253+
tpl = (tpl, tuple([Unrelated] * i))
254+
255+
def call_isinstance(expected, tpl):
256+
nonlocal called_instancecheck
257+
called_instancecheck = 0
258+
assert isinstance(expected, tpl) is False
259+
assert called_instancecheck == 191
260+
261+
# Call the test few times to also force the compilation with some luck...
262+
for i in range(1, 1000):
263+
call_isinstance(expected_other, tpl)
264+
265+
tpl = (Unrelated,)
266+
for i in range(1, 19):
267+
tpl = (tpl, tuple([Unrelated] * i))
268+
269+
# the very last item that isinstance should inspect should be Super
270+
tpl = (tpl, tuple([Unrelated] * 18 + [Super]))
271+
272+
called_instancecheck = 0
273+
assert isinstance(expected_other, tpl) is True
274+
assert called_instancecheck == 190

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@
267267
import com.oracle.truffle.api.interop.InteropLibrary;
268268
import com.oracle.truffle.api.interop.UnsupportedMessageException;
269269
import com.oracle.truffle.api.library.CachedLibrary;
270-
import com.oracle.truffle.api.nodes.ExplodeLoop;
271270
import com.oracle.truffle.api.nodes.UnexpectedResultException;
272271
import com.oracle.truffle.api.object.HiddenKey;
273272
import com.oracle.truffle.api.profiles.BranchProfile;
@@ -2186,14 +2185,9 @@ public PFunction function(Object cls, PCode code, PDict globals, String name, PT
21862185
return factory().createFunction(name, getTypeName(cls), code, globals, getObjectArrayNode.execute(defaultArgs), null, getClosure(getObjectArrayNode.execute(closure)));
21872186
}
21882187

2189-
@ExplodeLoop
21902188
private static PCell[] getClosure(Object[] closure) {
2191-
assert closure != null;
21922189
PCell[] cells = new PCell[closure.length];
2193-
for (int i = 0; i < closure.length; i++) {
2194-
assert closure[i] instanceof PCell;
2195-
cells[i] = (PCell) closure[i];
2196-
}
2190+
PythonUtils.arraycopy(closure, 0, cells, 0, closure.length);
21972191
return cells;
21982192
}
21992193

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java

Lines changed: 114 additions & 92 deletions
Large diffs are not rendered by default.

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/io/IONodes.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
5555

5656
import com.oracle.graal.python.annotations.ClinicConverterFactory;
57+
import com.oracle.graal.python.builtins.modules.MathGuards;
5758
import com.oracle.graal.python.builtins.modules.WarningsModuleBuiltins;
5859
import com.oracle.graal.python.builtins.objects.PNone;
5960
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes;
@@ -441,6 +442,7 @@ String str(Object s,
441442
}
442443

443444
@TypeSystemReference(PythonArithmeticTypes.class)
445+
@ImportStatic(MathGuards.class)
444446
public abstract static class SeekPosNode extends ArgumentCastNode.ArgumentCastNodeWithRaise {
445447

446448
protected static final int MAX = Integer.MAX_VALUE;
@@ -475,7 +477,7 @@ public int toInt(PInt x) {
475477
return i;
476478
}
477479

478-
@Specialization(limit = "3")
480+
@Specialization(limit = "3", guards = "!isInteger(value)")
479481
Object doOthers(VirtualFrame frame, Object value,
480482
@Cached SeekPosNode rec,
481483
@CachedLibrary("value") PythonObjectLibrary lib) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/zlib/ZLibModuleBuiltins.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.oracle.graal.python.builtins.CoreFunctions;
6464
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
6565
import com.oracle.graal.python.builtins.PythonBuiltins;
66+
import com.oracle.graal.python.builtins.modules.MathGuards;
6667
import com.oracle.graal.python.builtins.objects.PNone;
6768
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes;
6869
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes.ToBytesNode;
@@ -210,6 +211,7 @@ private static String asString(Object o) {
210211
return null;
211212
}
212213

214+
@ImportStatic(MathGuards.class)
213215
public abstract static class ExpectIntNode extends ArgumentCastNode.ArgumentCastNodeWithRaise {
214216
private final Object defaultValue;
215217

@@ -231,6 +233,11 @@ static int doInt(int i) {
231233
return i;
232234
}
233235

236+
@Specialization
237+
static int doBool(boolean b) {
238+
return PInt.intValue(b);
239+
}
240+
234241
@Specialization
235242
public int toInt(long x) {
236243
// lost magnitude is ok here.
@@ -243,7 +250,7 @@ public int toInt(PInt x) {
243250
return x.intValue();
244251
}
245252

246-
@Specialization(guards = "!isPNone(value)", limit = "3")
253+
@Specialization(guards = {"!isPNone(value)", "!isInteger(value)"}, limit = "3")
247254
Object doOthers(VirtualFrame frame, Object value,
248255
@Cached("createRec()") ExpectIntNode rec,
249256
@CachedLibrary("value") PythonObjectLibrary lib) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/array/ArrayBuiltins.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,14 +1180,18 @@ static Object byteswap8(PArray self) {
11801180
return PNone.NONE;
11811181
}
11821182

1183-
@ExplodeLoop
11841183
private static void doByteSwapExploded(PArray self, int itemsize, byte[] buffer) {
11851184
for (int i = 0; i < self.getLength() * itemsize; i += itemsize) {
1186-
for (int j = 0; j < itemsize / 2; j++) {
1187-
byte b = buffer[i + j];
1188-
buffer[i + j] = buffer[i + itemsize - j - 1];
1189-
buffer[i + itemsize - j - 1] = b;
1190-
}
1185+
doByteSwapExplodedInnerLoop(buffer, itemsize, i);
1186+
}
1187+
}
1188+
1189+
@ExplodeLoop
1190+
private static void doByteSwapExplodedInnerLoop(byte[] buffer, int itemsize, int i) {
1191+
for (int j = 0; j < itemsize / 2; j++) {
1192+
byte b = buffer[i + j];
1193+
buffer[i + j] = buffer[i + itemsize - j - 1];
1194+
buffer[i + itemsize - j - 1] = b;
11911195
}
11921196
}
11931197
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/KeywordsStorage.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ private HashingStorage generalize(HashingStorageLibrary lib, PythonLanguage lang
214214

215215
@ExportMessage
216216
static class ForEachUntyped {
217-
@Specialization(guards = "self.length() == cachedLen", limit = "1")
217+
@Specialization(guards = {"self.length() == cachedLen", "cachedLen <= 32"}, limit = "1")
218218
@ExplodeLoop
219219
static Object cached(KeywordsStorage self, ForEachNode<Object> node, Object arg,
220220
@Exclusive @Cached("self.length()") int cachedLen) {
@@ -239,7 +239,7 @@ static Object generic(KeywordsStorage self, ForEachNode<Object> node, Object arg
239239

240240
@ExportMessage
241241
public static class AddAllToOther {
242-
@Specialization(guards = "self.length() == cachedLen", limit = "1")
242+
@Specialization(guards = {"self.length() == cachedLen", "cachedLen <= 32"}, limit = "1")
243243
@ExplodeLoop
244244
static HashingStorage cached(KeywordsStorage self, HashingStorage other,
245245
@Exclusive @Cached("self.length()") int cachedLen,

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetBaseClassNodeGen;
106106
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetBaseClassesNodeGen;
107107
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetInstanceShapeNodeGen;
108+
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetItemsizeNodeGen;
108109
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetMroStorageNodeGen;
109110
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetNameNodeGen;
110111
import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.GetSolidBaseNodeGen;
@@ -128,6 +129,7 @@
128129
import com.oracle.graal.python.nodes.truffle.PythonTypes;
129130
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
130131
import com.oracle.graal.python.runtime.PythonContext;
132+
import com.oracle.graal.python.runtime.PythonOptions;
131133
import com.oracle.graal.python.runtime.exception.PException;
132134
import com.oracle.graal.python.runtime.sequence.PSequence;
133135
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
@@ -1498,41 +1500,58 @@ public static GetInstanceShape create() {
14981500
}
14991501

15001502
@GenerateUncached
1503+
@ImportStatic(PythonOptions.class)
15011504
public abstract static class GetItemsizeNode extends Node {
1505+
// We avoid PythonOptions here because it would require language reference
1506+
static final int MAX_RECURSION_DEPTH = 4;
15021507

1503-
public abstract Long execute(Object cls);
1508+
public final long execute(Object cls) {
1509+
return execute(cls, 0);
1510+
}
1511+
1512+
public abstract long execute(Object cls, int depth);
15041513

15051514
@Specialization
1506-
static Long getItemsizeType(PythonBuiltinClassType cls) {
1515+
static long getItemsizeType(PythonBuiltinClassType cls, @SuppressWarnings("unused") int depth) {
15071516
return getBuiltinTypeItemsize(cls);
15081517
}
15091518

15101519
@Specialization
1511-
static Long getItemsizeManaged(PythonBuiltinClass cls) {
1512-
return getItemsizeType(cls.getType());
1520+
static long getItemsizeManaged(PythonBuiltinClass cls, @SuppressWarnings("unused") int depth) {
1521+
return getItemsizeType(cls.getType(), depth);
15131522
}
15141523

1515-
@Specialization
1516-
static Long getItemsizeManaged(PythonClass cls,
1517-
@Cached ConditionProfile hasValueProfile,
1518-
@Cached ReadAttributeFromObjectNode readNode,
1519-
@Cached WriteAttributeToObjectNode writeNode,
1520-
@Cached GetBaseClassNode getBaseNode,
1524+
@Specialization(guards = "depth < MAX_RECURSION_DEPTH")
1525+
static long getItemsizeManagedRecursiveNode(PythonClass cls, int depth,
1526+
@Shared("hasVal") @Cached ConditionProfile hasValueProfile,
1527+
@Shared("read") @Cached ReadAttributeFromObjectNode readNode,
1528+
@Shared("write") @Cached WriteAttributeToObjectNode writeNode,
1529+
@Shared("getBase") @Cached GetBaseClassNode getBaseNode,
15211530
@Cached GetItemsizeNode baseItemsizeNode) {
1531+
CompilerAsserts.partialEvaluationConstant(depth);
15221532
Object itemsize = readNode.execute(cls, TYPE_ITEMSIZE);
15231533
if (hasValueProfile.profile(itemsize != PNone.NO_VALUE)) {
15241534
return (long) itemsize;
15251535
}
15261536

15271537
Object base = getBaseNode.execute(cls);
15281538
assert base != null;
1529-
itemsize = baseItemsizeNode.execute(base);
1539+
itemsize = baseItemsizeNode.execute(base, depth + 1);
15301540
writeNode.execute(cls, TYPE_ITEMSIZE, itemsize);
15311541
return (long) itemsize;
15321542
}
15331543

1544+
@Specialization(guards = "depth >= MAX_RECURSION_DEPTH")
1545+
static long getItemsizeManagedRecursiveCall(PythonClass cls, int depth,
1546+
@Shared("hasVal") @Cached ConditionProfile hasValueProfile,
1547+
@Shared("read") @Cached ReadAttributeFromObjectNode readNode,
1548+
@Shared("write") @Cached WriteAttributeToObjectNode writeNode,
1549+
@Shared("getBase") @Cached GetBaseClassNode getBaseNode) {
1550+
return getItemsizeManagedRecursiveNode(cls, depth, hasValueProfile, readNode, writeNode, getBaseNode, GetItemsizeNodeGen.getUncached());
1551+
}
1552+
15341553
@Specialization
1535-
static Long getNative(PythonNativeClass cls,
1554+
static long getNative(PythonNativeClass cls, @SuppressWarnings("unused") int depth,
15361555
@Cached GetTypeMemberNode getTpDictoffsetNode) {
15371556
return (long) getTpDictoffsetNode.execute(cls, NativeMember.TP_ITEMSIZE);
15381557
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/CreateArgumentsNode.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@
6060
import com.oracle.graal.python.nodes.PNodeWithContext;
6161
import com.oracle.graal.python.nodes.PRaiseNode;
6262
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.ApplyKeywordsNodeGen;
63-
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.ApplyKeywordsNodeGen.SearchNamedParameterNodeGen;
6463
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.ApplyPositionalArgumentsNodeGen;
6564
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.CreateAndCheckArgumentsNodeGen;
6665
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.FillDefaultsNodeGen;
6766
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.FillKwDefaultsNodeGen;
6867
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.FindKwDefaultNodeGen;
6968
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.HandleTooManyArgumentsNodeGen;
69+
import com.oracle.graal.python.nodes.argument.CreateArgumentsNodeGen.ApplyKeywordsNodeGen.SearchNamedParameterNodeGen;
7070
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetDefaultsNode;
7171
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetKeywordDefaultsNode;
7272
import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetSignatureNode;
@@ -375,7 +375,7 @@ protected abstract static class HandleTooManyArgumentsNode extends PNodeWithCont
375375
public abstract PException execute(Object[] scope_w, Object callable, Signature signature, int co_argcount, int co_kwonlyargcount, int ndefaults, int avail, boolean methodcall,
376376
int adjustCount);
377377

378-
@Specialization(guards = {"co_kwonlyargcount == cachedKwOnlyArgCount"})
378+
@Specialization(guards = {"co_kwonlyargcount == cachedKwOnlyArgCount", "cachedKwOnlyArgCount <= 32"})
379379
@ExplodeLoop
380380
static PException doCached(Object[] scope_w, Object callable, Signature signature, int co_argcount, @SuppressWarnings("unused") int co_kwonlyargcount, int ndefaults, int avail,
381381
boolean methodcall, int adjustCount,
@@ -543,7 +543,7 @@ int getUserArgumentLength(Object[] arguments) {
543543
return PArguments.getUserArgumentLength(arguments);
544544
}
545545

546-
@Specialization(guards = {"kwLen == keywords.length", "calleeSignature == cachedSignature"})
546+
@Specialization(guards = {"kwLen == keywords.length", "calleeSignature == cachedSignature", "kwLen <= 32"})
547547
@ExplodeLoop
548548
Object[] applyCached(Object callee, @SuppressWarnings("unused") Signature calleeSignature, Object[] arguments, PKeyword[] keywords,
549549
@Cached PRaiseNode raise,
@@ -687,7 +687,7 @@ private static String joinNames(List<String> names) {
687687
protected abstract static class SearchNamedParameterNode extends Node {
688688
public abstract int execute(String[] parameters, String name);
689689

690-
@Specialization(guards = "cachedLen == parameters.length")
690+
@Specialization(guards = {"cachedLen == parameters.length", "cachedLen <= 32"})
691691
@ExplodeLoop
692692
int cached(String[] parameters, String name,
693693
@Cached("parameters.length") int cachedLen) {
@@ -877,7 +877,7 @@ protected abstract static class FindKwDefaultNode extends Node {
877877

878878
public abstract PKeyword execute(PKeyword[] kwdefaults, String kwname);
879879

880-
@Specialization(guards = {"kwdefaults.length == cachedLength", "kwdefaults.length < 32"})
880+
@Specialization(guards = {"kwdefaults.length == cachedLength", "cachedLength < 32"})
881881
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN)
882882
PKeyword doCached(PKeyword[] kwdefaults, String kwname,
883883
@Cached("kwdefaults.length") int cachedLength) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/CompactKeywordsNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
abstract class CompactKeywordsNode extends Node {
5454
public abstract PKeyword[] execute(PKeyword[] keys, int reshape);
5555

56-
@Specialization(guards = {"cachedLen == keys.length", "reshape == cachedReshape"}, limit = "getVariableArgumentInlineCacheLimit()")
56+
@Specialization(guards = {"cachedLen == keys.length", "reshape == cachedReshape", "cachedLen <= 32"}, limit = "getVariableArgumentInlineCacheLimit()")
5757
@ExplodeLoop
5858
PKeyword[] cached(PKeyword[] keys, @SuppressWarnings("unused") int reshape,
5959
@Cached("keys.length") int cachedLen,

0 commit comments

Comments
 (0)