Skip to content

Commit 27c88bd

Browse files
msimacekmsimacek
committed
Handle MRO change corner cases
1 parent 3153507 commit 27c88bd

File tree

5 files changed

+55
-26
lines changed

5 files changed

+55
-26
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ abstract static class SpecializedSetStringKey extends Node {
232232
}
233233

234234
static EconomicMapStorage dynamicObjectStorageToEconomicMap(Node inliningTarget, DynamicObjectStorage s, DynamicObjectLibrary dylib, PyObjectHashNode hashNode, ObjectHashMap.PutNode putNode) {
235-
// TODO: shouldn't we invalidate all MRO assumptions in this case?
236235
DynamicObject store = s.store;
237236
EconomicMapStorage result = EconomicMapStorage.create(dylib.getShape(store).getPropertyCount());
238237
ObjectHashMap resultMap = result.map;
@@ -542,7 +541,6 @@ static Object domStringKey(Frame frame, Node inliningTarget, DynamicObjectStorag
542541
@Cached PyUnicodeCheckExactNode isBuiltinString,
543542
@Cached CastBuiltinStringToTruffleStringNode castStr,
544543
@Exclusive @Cached PyObjectHashNode hashNode,
545-
@Exclusive @Cached InlinedBranchProfile invalidateMroProfile,
546544
@CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
547545
if (!isBuiltinString.execute(inliningTarget, keyObj)) {
548546
// Just for the potential side effects

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ Object getBase() {
165165
}
166166

167167
public void setMRO(PythonAbstractClass[] mro) {
168+
if (methodResolutionOrder != null) {
169+
methodResolutionOrder.lookupChanged();
170+
}
168171
methodResolutionOrder = new MroSequenceStorage(name, mro);
169172
}
170173

@@ -303,12 +306,10 @@ public final void setBases(Node node, Object newBaseClass, PythonAbstractClass[]
303306
// for what follows see also typeobject.c#type_set_bases()
304307
this.base = newBaseClass;
305308
this.baseClasses = newBaseClasses;
306-
this.methodResolutionOrder.lookupChanged();
307309
this.setMRO(ComputeMroNode.doSlowPath(node, this));
308310

309311
for (PythonAbstractClass scls : subclassesArray) {
310312
if (scls instanceof PythonManagedClass pmc) {
311-
pmc.methodResolutionOrder.lookupChanged();
312313
pmc.setMRO(ComputeMroNode.doSlowPath(node, scls));
313314
}
314315
}
@@ -321,14 +322,12 @@ public final void setBases(Node node, Object newBaseClass, PythonAbstractClass[]
321322
this.base = oldBase;
322323
this.baseClasses = oldBaseClasses;
323324
}
324-
this.methodResolutionOrder.lookupChanged();
325325
this.setMRO(oldMRO);
326326

327327
for (int i = 0; i < subclassesArray.length; i++) {
328328
PythonAbstractClass scls = subclassesArray[i];
329329
if (oldSubClasssMROs[i] != null) {
330330
PythonManagedClass pmc = (PythonManagedClass) scls;
331-
pmc.methodResolutionOrder.lookupChanged();
332331
pmc.setMRO(oldSubClasssMROs[i]);
333332
}
334333
}

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@
190190
import com.oracle.graal.python.nodes.attributes.GetFixedAttributeNode;
191191
import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
192192
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
193+
import com.oracle.graal.python.nodes.builtins.TupleNodes;
193194
import com.oracle.graal.python.nodes.call.CallNode;
194195
import com.oracle.graal.python.nodes.call.special.CallUnaryMethodNode;
195196
import com.oracle.graal.python.nodes.call.special.LookupSpecialMethodNode;
@@ -199,6 +200,7 @@
199200
import com.oracle.graal.python.nodes.expression.CastToListExpressionNode.CastToListNode;
200201
import com.oracle.graal.python.nodes.frame.ReadCallerFrameNode;
201202
import com.oracle.graal.python.nodes.function.BuiltinFunctionRootNode;
203+
import com.oracle.graal.python.nodes.object.BuiltinClassProfiles;
202204
import com.oracle.graal.python.nodes.object.BuiltinClassProfiles.IsBuiltinClassProfile;
203205
import com.oracle.graal.python.nodes.object.GetClassNode;
204206
import com.oracle.graal.python.nodes.object.GetClassNode.GetPythonObjectClassNode;
@@ -576,7 +578,7 @@ private static MroSequenceStorage doPythonClass(PythonManagedClass obj, Node inl
576578
return obj.getMethodResolutionOrder();
577579
}
578580

579-
@InliningCutoff
581+
@TruffleBoundary
580582
private static void initializeMRO(PythonManagedClass obj, Node inliningTarget, InlinedConditionProfile isPythonClass, PythonLanguage language) {
581583
PythonAbstractClass[] mro = ComputeMroNode.doSlowPath(inliningTarget, obj, false);
582584
if (isPythonClass.profile(inliningTarget, obj instanceof PythonClass)) {
@@ -1623,23 +1625,22 @@ public static PythonAbstractClass[] doSlowPath(Node node, PythonAbstractClass cl
16231625
return computeMethodResolutionOrder(node, cls, invokeMro);
16241626
}
16251627

1626-
// No BoundaryCallContext: TODO: unlikely, but __mro__ lookup and call can run arbitrary
1627-
// code
16281628
@TruffleBoundary
16291629
static PythonAbstractClass[] invokeMro(Node node, PythonAbstractClass cls) {
16301630
Object type = GetClassNode.executeUncached(cls);
1631-
if (IsTypeNode.executeUncached(type) && type instanceof PythonClass) {
1632-
Object mroMeth = LookupAttributeInMRONode.Dynamic.getUncached().execute(type, T_MRO);
1633-
if (mroMeth instanceof PFunction) {
1634-
Object mroObj = CallUnaryMethodNode.getUncached().executeObject(mroMeth, cls);
1635-
if (mroObj instanceof PSequence mroSequence) {
1636-
SequenceStorage mroStorage = mroSequence.getSequenceStorage();
1637-
return mroCheck(node, cls, GetInternalObjectArrayNode.executeUncached(mroStorage), mroStorage);
1638-
}
1639-
throw PRaiseNode.raiseStatic(node, TypeError, ErrorMessages.OBJ_NOT_ITERABLE, cls);
1640-
}
1631+
if (BuiltinClassProfiles.IsBuiltinClassExactProfile.profileClassSlowPath(type, PythonBuiltinClassType.PythonClass)) {
1632+
// Default type.mro
1633+
return null;
16411634
}
1642-
return null;
1635+
Object mroMeth = LookupSpecialMethodNode.Dynamic.executeUncached(type, T_MRO, cls);
1636+
if (mroMeth instanceof PBuiltinFunction f && f.getNodeClass() != null && TypeBuiltins.MroNode.class.isAssignableFrom(f.getNodeClass())) {
1637+
// Default type.mro
1638+
return null;
1639+
}
1640+
Object mroObj = CallUnaryMethodNode.getUncached().executeObject(mroMeth, cls);
1641+
PTuple mroTuple = TupleNodes.ConstructTupleNode.getUncached().execute(null, mroObj);
1642+
SequenceStorage mroStorage = mroTuple.getSequenceStorage();
1643+
return mroCheck(node, cls, GetInternalObjectArrayNode.executeUncached(mroStorage), mroStorage);
16431644
}
16441645

16451646
private static PythonAbstractClass[] computeMethodResolutionOrder(Node node, PythonAbstractClass cls, boolean invokeMro) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.oracle.graal.python.nodes.PNodeWithContext;
5555
import com.oracle.graal.python.runtime.PythonContext;
5656
import com.oracle.graal.python.runtime.PythonOptions;
57+
import com.oracle.graal.python.runtime.exception.StacktracelessCheckedException;
5758
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
5859
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage.FinalAttributeAssumptionNode;
5960
import com.oracle.truffle.api.Assumption;
@@ -80,7 +81,17 @@
8081
@ImportStatic(PythonOptions.class)
8182
public abstract class LookupAttributeInMRONode extends PNodeWithContext {
8283

83-
public abstract Object execute(Object klass);
84+
public final Object execute(Object klass) {
85+
try {
86+
return executeInternal(klass);
87+
} catch (MROChangedException e) {
88+
// This exception can occur only during specialization
89+
CompilerDirectives.transferToInterpreterAndInvalidate();
90+
return e.result;
91+
}
92+
}
93+
94+
protected abstract Object executeInternal(Object klass) throws MROChangedException;
8495

8596
@GenerateUncached
8697
@GenerateInline(false) // footprint reduction 36 -> 17
@@ -181,7 +192,16 @@ private static boolean skipNonStaticBase(Object clsObj, boolean skipNonStaticBas
181192
return skipNonStaticBases && clsObj instanceof PythonClass && !((PythonClass) clsObj).isStaticBase();
182193
}
183194

184-
FinalAttributeAssumptionNode findAttrAndAssumptionInMRO(Object klass) {
195+
@SuppressWarnings("serial")
196+
public static class MROChangedException extends StacktracelessCheckedException {
197+
final Object result;
198+
199+
public MROChangedException(Object result) {
200+
this.result = result;
201+
}
202+
}
203+
204+
FinalAttributeAssumptionNode findAttrAndAssumptionInMRO(Object klass) throws MROChangedException {
185205
CompilerAsserts.neverPartOfCompilation();
186206
// Regarding potential side effects to MRO caused by __eq__ of the keys in the dicts that we
187207
// search through: CPython seems to read the MRO once and then compute the result also
@@ -193,6 +213,9 @@ FinalAttributeAssumptionNode findAttrAndAssumptionInMRO(Object klass) {
193213
if (assumptionNode != null) {
194214
return assumptionNode;
195215
}
216+
// Put a new assumption in place in case the MRO changes during the lookup
217+
FinalAttributeAssumptionNode node = new FinalAttributeAssumptionNode();
218+
mro.putFinalAttributeAssumption(key, node);
196219
Object result = PNone.NO_VALUE;
197220
for (int i = 0; i < mro.length(); i++) {
198221
PythonAbstractClass clsObj = mro.getPythonClassItemNormalized(i);
@@ -205,8 +228,12 @@ FinalAttributeAssumptionNode findAttrAndAssumptionInMRO(Object klass) {
205228
break;
206229
}
207230
}
208-
FinalAttributeAssumptionNode node = new FinalAttributeAssumptionNode(result);
209-
mro.putFinalAttributeAssumption(key, node);
231+
if (node.getAssumption() == null) {
232+
// MRO changed during lookup. To avoid reexecuting the side-effects, return via
233+
// exception. This should abort the specialization
234+
throw new MROChangedException(result);
235+
}
236+
node.setValue(result);
210237
return node;
211238
}
212239

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ public static final class FinalAttributeAssumptionNode {
7373
@CompilationFinal private Assumption assumption;
7474
@CompilationFinal private Object value;
7575

76-
public FinalAttributeAssumptionNode(Object value) {
77-
this.value = value;
76+
public FinalAttributeAssumptionNode() {
7877
this.assumption = Truffle.getRuntime().createAssumption("attribute in MRO final");
7978
}
8079

@@ -86,10 +85,15 @@ public void invalidate() {
8685
value = null;
8786
}
8887

88+
@NonIdempotent
8989
public Object getValue() {
9090
return value;
9191
}
9292

93+
public void setValue(Object value) {
94+
this.value = value;
95+
}
96+
9397
@NonIdempotent
9498
public Assumption getAssumption() {
9599
return assumption;

0 commit comments

Comments
 (0)