Skip to content

Commit 1efc56f

Browse files
steve-ssteve-s
committed
[GR-32131] Slots are not correctly updated when NO_VALUE is written.
PullRequest: graalpython/1855
2 parents 906d989 + 3300d11 commit 1efc56f

File tree

6 files changed

+86
-37
lines changed

6 files changed

+86
-37
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/BuiltinMethodDescriptor.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,22 @@ public static BuiltinMethodDescriptor get(NodeFactory<? extends PythonBuiltinBas
9999
Class<? extends PythonBuiltinBaseNode> nodeClass = factory.getNodeClass();
100100
BuiltinMethodDescriptor result = null;
101101
if (PythonUnaryBuiltinNode.class.isAssignableFrom(nodeClass)) {
102-
result = new UnaryBuiltinInfo(factory, type);
102+
result = new UnaryBuiltinDescriptor(factory, type);
103103
} else if (PythonBinaryBuiltinNode.class.isAssignableFrom(nodeClass)) {
104-
result = new BinaryBuiltinInfo(factory, type);
104+
result = new BinaryBuiltinDescriptor(factory, type);
105105
} else if (PythonTernaryBuiltinNode.class.isAssignableFrom(nodeClass)) {
106-
result = new TernaryBuiltinInfo(factory, type);
106+
result = new TernaryBuiltinDescriptor(factory, type);
107107
}
108108
if (result != null) {
109109
return CACHE.computeIfAbsent(result, x -> x);
110110
}
111111
return null;
112112
}
113113

114+
public static boolean isInstance(Object obj) {
115+
return obj instanceof UnaryBuiltinDescriptor || obj instanceof BinaryBuiltinDescriptor || obj instanceof TernaryBuiltinDescriptor;
116+
}
117+
114118
private static boolean needsFrame(NodeFactory<? extends PythonBuiltinBaseNode> factory) {
115119
for (Builtin builtin : factory.getNodeClass().getAnnotationsByType(Builtin.class)) {
116120
if (builtin.needsFrame()) {
@@ -152,8 +156,8 @@ public int hashCode() {
152156
// Note: manually written subclass for each builtin works better with Truffle DSL than one
153157
// generic class that would parametrize the 'factory' field
154158

155-
public static final class UnaryBuiltinInfo extends BuiltinMethodDescriptor {
156-
public UnaryBuiltinInfo(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
159+
public static final class UnaryBuiltinDescriptor extends BuiltinMethodDescriptor {
160+
public UnaryBuiltinDescriptor(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
157161
super(factory, type);
158162
}
159163

@@ -162,8 +166,8 @@ public PythonUnaryBuiltinNode createNode() {
162166
}
163167
}
164168

165-
public static final class BinaryBuiltinInfo extends BuiltinMethodDescriptor {
166-
public BinaryBuiltinInfo(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
169+
public static final class BinaryBuiltinDescriptor extends BuiltinMethodDescriptor {
170+
public BinaryBuiltinDescriptor(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
167171
super(factory, type);
168172
}
169173

@@ -172,8 +176,8 @@ public PythonBinaryBuiltinNode createNode() {
172176
}
173177
}
174178

175-
public static final class TernaryBuiltinInfo extends BuiltinMethodDescriptor {
176-
public TernaryBuiltinInfo(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
179+
public static final class TernaryBuiltinDescriptor extends BuiltinMethodDescriptor {
180+
public TernaryBuiltinDescriptor(NodeFactory<? extends PythonBuiltinBaseNode> factory, PythonBuiltinClassType type) {
177181
super(factory, type);
178182
}
179183

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,13 +520,14 @@ private static void fixupSpecialMethodSlot(PythonManagedClass klass, SpecialMeth
520520
Object value = ReadAttributeFromObjectNode.getUncachedForceType().execute(kls, slot.getName());
521521
if (value != PNone.NO_VALUE) {
522522
currentNewValue = value;
523-
slot.setValue(klass, value);
524523
break;
525524
}
526525
}
527-
assert newValue == null || newValue == PNone.NO_VALUE || slot.getValue(klass) != PNone.NO_VALUE;
526+
// If we did not find it at all, it is OK as long as the new value is NO_VALUE
527+
assert newValue == PNone.NO_VALUE || currentNewValue != PNone.NO_VALUE;
528528
if (currentOldValue != currentNewValue) {
529529
// Something actually changed, fixup subclasses...
530+
slot.setValue(klass, currentNewValue);
530531
fixupSpecialMethodInSubClasses(klass.getSubClasses(), slot, originalValue, newValue);
531532
} else {
532533
// We assume no other changes in MRO, so we must have either overridden the slot with

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PGuards.java

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,10 @@
5757
import com.oracle.graal.python.builtins.objects.ellipsis.PEllipsis;
5858
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
5959
import com.oracle.graal.python.builtins.objects.floats.PFloat;
60-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinInfo;
61-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinInfo;
62-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.UnaryBuiltinInfo;
60+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor;
61+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinDescriptor;
62+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinDescriptor;
63+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.UnaryBuiltinDescriptor;
6364
import com.oracle.graal.python.builtins.objects.function.PArguments;
6465
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
6566
import com.oracle.graal.python.builtins.objects.function.PFunction;
@@ -168,6 +169,19 @@ public static boolean isCallable(Object value) {
168169
return value instanceof PBuiltinFunction || value instanceof PFunction || value instanceof PBuiltinMethod || value instanceof PMethod;
169170
}
170171

172+
/**
173+
* Use instead of {@link #isCallable(Object)} for objects coming from
174+
* {@link com.oracle.graal.python.nodes.attributes.LookupCallableSlotInMRONode}. Note that such
175+
* objects can be then forwarded only to call nodes that support them.
176+
*/
177+
public static boolean isCallableOrDescriptor(Object value) {
178+
return isCallable(value) || BuiltinMethodDescriptor.isInstance(value);
179+
}
180+
181+
public static boolean isBuiltinDescriptor(Object value) {
182+
return BuiltinMethodDescriptor.isInstance(value);
183+
}
184+
171185
public static boolean isClass(Object obj, InteropLibrary lib) {
172186
try {
173187
return lib.isMetaObject(obj) && lib.hasLanguage(obj) && lib.getLanguage(obj) == PythonLanguage.class;
@@ -513,15 +527,15 @@ public static boolean isKindOfBuiltinClass(Object clazz) {
513527
return clazz instanceof PythonBuiltinClassType || clazz instanceof PythonBuiltinClass;
514528
}
515529

516-
public static boolean isUnaryBuiltinInfo(Object value) {
517-
return value instanceof UnaryBuiltinInfo;
530+
public static boolean isUnaryBuiltinDescriptor(Object value) {
531+
return value instanceof UnaryBuiltinDescriptor;
518532
}
519533

520-
public static boolean isBinaryBuiltinInfo(Object value) {
521-
return value instanceof BinaryBuiltinInfo;
534+
public static boolean isBinaryBuiltinDescriptor(Object value) {
535+
return value instanceof BinaryBuiltinDescriptor;
522536
}
523537

524-
public static boolean isTernaryBuiltinInfo(Object value) {
525-
return value instanceof TernaryBuiltinInfo;
538+
public static boolean isTernaryBuiltinDescriptor(Object value) {
539+
return value instanceof TernaryBuiltinDescriptor;
526540
}
527541
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallBinaryMethodNode.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@
4242

4343
import com.oracle.graal.python.PythonLanguage;
4444
import com.oracle.graal.python.builtins.objects.PNone;
45-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinInfo;
45+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinDescriptor;
46+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinDescriptor;
4647
import com.oracle.graal.python.builtins.objects.function.PArguments;
4748
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
4849
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -87,14 +88,25 @@ public final Object executeObject(Object callable, Object arg1, Object arg2) {
8788
}
8889

8990
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
90-
Object callSpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") BinaryBuiltinInfo info, Object arg1, Object arg2,
91-
@SuppressWarnings("unused") @Cached("info") BinaryBuiltinInfo cachedInfo,
91+
Object callBinarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") BinaryBuiltinDescriptor info, Object arg1, Object arg2,
92+
@SuppressWarnings("unused") @Cached("info") BinaryBuiltinDescriptor cachedInfo,
9293
@Cached("cachedInfo.createNode()") PythonBinaryBuiltinNode node) {
9394
return node.call(frame, arg1, arg2);
9495
}
9596

96-
@Specialization(replaces = "callSpecialMethodSlotInlined")
97-
Object callSpecialMethodSlotCallTarget(VirtualFrame frame, BinaryBuiltinInfo info, Object arg1, Object arg2,
97+
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
98+
Object callTernarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object arg1, Object arg2,
99+
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
100+
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
101+
return node.call(frame, arg1, arg2, PNone.NO_VALUE);
102+
}
103+
104+
protected static boolean isBinaryOrTernaryBuiltinDescriptor(Object value) {
105+
return value instanceof BinaryBuiltinDescriptor || value instanceof TernaryBuiltinDescriptor;
106+
}
107+
108+
@Specialization(guards = "isBinaryOrTernaryBuiltinDescriptor(info)", replaces = {"callBinarySpecialMethodSlotInlined", "callTernarySpecialMethodSlotInlined"})
109+
Object callSpecialMethodSlotCallTarget(VirtualFrame frame, BinaryBuiltinDescriptor info, Object arg1, Object arg2,
98110
@CachedLanguage PythonLanguage language,
99111
@Cached GenericInvokeNode invokeNode) {
100112
RootCallTarget callTarget = language.getDescriptorCallTarget(info);
@@ -402,7 +414,7 @@ static Object callQuaternaryFunction(VirtualFrame frame, @SuppressWarnings("unus
402414
return builtinNode.call(frame, arg1, arg2, PNone.NO_VALUE, PNone.NO_VALUE);
403415
}
404416

405-
@Specialization(guards = "!isBinaryBuiltinInfo(func)", //
417+
@Specialization(guards = "!isBinaryOrTernaryBuiltinDescriptor(func)", //
406418
replaces = {"callBoolSingle", "callBool", "callIntSingle", "callInt", "callBoolIntSingle", "callBoolInt", "callLongSingle", "callLong", "callBoolLongSingle",
407419
"callBoolLong", "callDoubleSingle", "callDouble", "callBoolDoubleSingle", "callBoolDouble", "callObjectSingleContext", "callObject",
408420
"callMethodSingleContext", "callSelfMethodSingleContext", "callMethod", "callSelfMethod", "callTernaryFunction", "callQuaternaryFunction"})

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallTernaryMethodNode.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
package com.oracle.graal.python.nodes.call.special;
4242

4343
import com.oracle.graal.python.PythonLanguage;
44-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinInfo;
44+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinDescriptor;
4545
import com.oracle.graal.python.builtins.objects.function.PArguments;
4646
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
4747
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -76,14 +76,14 @@ public static CallTernaryMethodNode getUncached() {
7676
public abstract Object execute(Frame frame, Object callable, Object arg1, Object arg2, Object arg3);
7777

7878
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
79-
static Object callSpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinInfo info, Object arg1, Object arg2, Object arg3,
80-
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinInfo cachedInfo,
79+
static Object callSpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object arg1, Object arg2, Object arg3,
80+
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
8181
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
8282
return node.call(frame, arg1, arg2, arg3);
8383
}
8484

8585
@Specialization(replaces = "callSpecialMethodSlotInlined")
86-
static Object callSpecialMethodSlotCallTarget(VirtualFrame frame, TernaryBuiltinInfo info, Object arg1, Object arg2, Object arg3,
86+
static Object callSpecialMethodSlotCallTarget(VirtualFrame frame, TernaryBuiltinDescriptor info, Object arg1, Object arg2, Object arg3,
8787
@CachedLanguage PythonLanguage language,
8888
@Cached GenericInvokeNode invokeNode) {
8989
RootCallTarget callTarget = language.getDescriptorCallTarget(info);
@@ -227,7 +227,7 @@ static Object callSelfMethod(VirtualFrame frame, @SuppressWarnings("unused") PBu
227227
return builtinNode.call(frame, func.getSelf(), arg1, arg2, arg3);
228228
}
229229

230-
@Specialization(guards = "!isTernaryBuiltinInfo(func)", replaces = {"doBuiltinFunctionOIOCached", "doBuiltinFunctionCached", "doBuiltinFunctionOIOCtCached", "doBuiltinFunctionCtCached",
230+
@Specialization(guards = "!isTernaryBuiltinDescriptor(func)", replaces = {"doBuiltinFunctionOIOCached", "doBuiltinFunctionCached", "doBuiltinFunctionOIOCtCached", "doBuiltinFunctionCtCached",
231231
"doBuiltinFunctionOIOCachedReverse", "doBuiltinFunctionCachedReverse", "doBuiltinFunctionOIOCtCachedReverse", "doBuiltinFunctionCtCachedReverse",
232232
"doBuiltinMethodOIOCached", "doBuiltinMethodCached", "doBuiltinMethodOIOCtCached", "doBuiltinMethodCtCached", "callSelfMethodSingleContext",
233233
"callSelfMethod"})

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/CallUnaryMethodNode.java

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@
4242

4343
import com.oracle.graal.python.PythonLanguage;
4444
import com.oracle.graal.python.builtins.objects.PNone;
45-
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.UnaryBuiltinInfo;
45+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor;
46+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinDescriptor;
47+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.TernaryBuiltinDescriptor;
48+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.UnaryBuiltinDescriptor;
4649
import com.oracle.graal.python.builtins.objects.function.PArguments;
4750
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
4851
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -51,6 +54,7 @@
5154
import com.oracle.graal.python.nodes.call.GenericInvokeNode;
5255
import com.oracle.graal.python.nodes.call.special.MaybeBindDescriptorNode.BoundDescriptor;
5356
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
57+
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
5458
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
5559
import com.oracle.graal.python.util.PythonUtils;
5660
import com.oracle.truffle.api.RootCallTarget;
@@ -104,14 +108,28 @@ public final Object executeObject(Object callable, Object receiver) {
104108
}
105109

106110
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
107-
Object callSpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") UnaryBuiltinInfo info, Object receiver,
108-
@SuppressWarnings("unused") @Cached("info") UnaryBuiltinInfo cachedInfo,
111+
Object callUnarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") UnaryBuiltinDescriptor info, Object receiver,
112+
@SuppressWarnings("unused") @Cached("info") UnaryBuiltinDescriptor cachedInfo,
109113
@Cached("cachedInfo.createNode()") PythonUnaryBuiltinNode node) {
110114
return node.call(frame, receiver);
111115
}
112116

113-
@Specialization(replaces = "callSpecialMethodSlotInlined")
114-
Object callSpecialMethodSlotCallTarget(VirtualFrame frame, UnaryBuiltinInfo info, Object receiver,
117+
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
118+
Object callBinarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") BinaryBuiltinDescriptor info, Object receiver,
119+
@SuppressWarnings("unused") @Cached("info") BinaryBuiltinDescriptor cachedInfo,
120+
@Cached("cachedInfo.createNode()") PythonBinaryBuiltinNode node) {
121+
return node.call(frame, receiver, PNone.NO_VALUE);
122+
}
123+
124+
@Specialization(guards = "cachedInfo == info", limit = "getCallSiteInlineCacheMaxDepth()")
125+
Object callTernarySpecialMethodSlotInlined(VirtualFrame frame, @SuppressWarnings("unused") TernaryBuiltinDescriptor info, Object receiver,
126+
@SuppressWarnings("unused") @Cached("info") TernaryBuiltinDescriptor cachedInfo,
127+
@Cached("cachedInfo.createNode()") PythonTernaryBuiltinNode node) {
128+
return node.call(frame, receiver, PNone.NO_VALUE, PNone.NO_VALUE);
129+
}
130+
131+
@Specialization(guards = "isBuiltinDescriptor(info)", replaces = {"callUnarySpecialMethodSlotInlined", "callBinarySpecialMethodSlotInlined", "callTernarySpecialMethodSlotInlined"})
132+
Object callSpecialMethodSlotCallTarget(VirtualFrame frame, BuiltinMethodDescriptor info, Object receiver,
115133
@CachedLanguage PythonLanguage language,
116134
@Cached GenericInvokeNode invokeNode) {
117135
RootCallTarget callTarget = language.getDescriptorCallTarget(info);
@@ -281,7 +299,7 @@ static Object callBinaryMethod(VirtualFrame frame, @SuppressWarnings("unused") P
281299
return builtinNode.call(frame, arg, PNone.NO_VALUE);
282300
}
283301

284-
@Specialization(guards = "!isUnaryBuiltinInfo(func)", replaces = {"callIntSingle", "callInt", "callLongSingle", "callLong", "callDoubleSingle", "callDouble", "callBoolSingle", "callBool",
302+
@Specialization(guards = "!isBuiltinDescriptor(func)", replaces = {"callIntSingle", "callInt", "callLongSingle", "callLong", "callDoubleSingle", "callDouble", "callBoolSingle", "callBool",
285303
"callObjectSingle", "callObject",
286304
"callMethodSingleContext", "callSelfMethodSingleContext", "callMethod", "callSelfMethod", "callBinaryMethodSingleContext", "callBinaryMethod"})
287305
@Megamorphic

0 commit comments

Comments
 (0)