Skip to content

Commit bd608d8

Browse files
tomasstupkatomasstupka
committed
should not use DynamicObjectStorage when class attribute has a non string key
1 parent 7d331c6 commit bd608d8

File tree

2 files changed

+39
-19
lines changed

2 files changed

+39
-19
lines changed

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

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,8 +2198,8 @@ Object type(Object cls, Object obj, PNone bases, PNone dict, PKeyword[] kwds,
21982198

21992199
@Specialization(guards = "isString(wName)")
22002200
Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict namespaceOrig, PKeyword[] kwds,
2201-
@CachedLibrary(limit = "4") PythonObjectLibrary lib,
2202-
@CachedLibrary(limit = "2") HashingStorageLibrary nslib,
2201+
@CachedLibrary(limit = "5") PythonObjectLibrary lib,
2202+
@CachedLibrary(limit = "3") HashingStorageLibrary hashingStoragelib,
22032203
@Cached BranchProfile updatedStorage,
22042204
@Cached("create(__NEW__)") LookupInheritedAttributeNode getNewFuncNode,
22052205
@Cached("create(__INIT_SUBCLASS__)") GetAttributeNode getInitSubclassNode,
@@ -2229,9 +2229,9 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
22292229

22302230
try {
22312231
PDict namespace = (PDict) copyDict.call(frame, namespaceOrig);
2232-
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, nslib, getDictAttrNode, getWeakRefAttrNode, getBestBaseNode, getItemSize, writeItemSize);
2232+
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, lib, hashingStoragelib, getDictAttrNode, getWeakRefAttrNode, getBestBaseNode, getItemSize, writeItemSize);
22332233

2234-
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
2234+
for (DictEntry entry : hashingStoragelib.entries(namespace.getDictStorage())) {
22352235
Object setName = getSetNameNode.execute(entry.value);
22362236
if (setName != PNone.NO_VALUE) {
22372237
try {
@@ -2253,32 +2253,32 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
22532253
PFrame callerFrame = getReadCallerFrameNode().executeWith(frame, 0);
22542254
PythonObject globals = callerFrame.getGlobals();
22552255
if (globals != null) {
2256-
String moduleName = getModuleNameFromGlobals(globals, nslib);
2256+
String moduleName = getModuleNameFromGlobals(globals, hashingStoragelib);
22572257
if (moduleName != null) {
22582258
ensureWriteAttrNode().execute(frame, newType, __MODULE__, moduleName);
22592259
}
22602260
}
22612261
}
22622262

22632263
// delete __qualname__ from namespace
2264-
if (nslib.hasKey(namespace.getDictStorage(), __QUALNAME__)) {
2265-
HashingStorage newStore = nslib.delItem(namespace.getDictStorage(), __QUALNAME__);
2264+
if (hashingStoragelib.hasKey(namespace.getDictStorage(), __QUALNAME__)) {
2265+
HashingStorage newStore = hashingStoragelib.delItem(namespace.getDictStorage(), __QUALNAME__);
22662266
if (newStore != namespace.getDictStorage()) {
22672267
updatedStorage.enter();
22682268
namespace.setDictStorage(newStore);
22692269
}
22702270
}
22712271

22722272
// set __class__ cell contents
2273-
Object classcell = nslib.getItem(namespace.getDictStorage(), __CLASSCELL__);
2273+
Object classcell = hashingStoragelib.getItem(namespace.getDictStorage(), __CLASSCELL__);
22742274
if (classcell != null) {
22752275
if (classcell instanceof PCell) {
22762276
((PCell) classcell).setRef(newType);
22772277
} else {
22782278
raise(TypeError, ErrorMessages.MUST_BE_A_CELL, "__classcell__");
22792279
}
2280-
if (nslib.hasKey(namespace.getDictStorage(), __CLASSCELL__)) {
2281-
HashingStorage newStore = nslib.delItem(namespace.getDictStorage(), __CLASSCELL__);
2280+
if (hashingStoragelib.hasKey(namespace.getDictStorage(), __CLASSCELL__)) {
2281+
HashingStorage newStore = hashingStoragelib.delItem(namespace.getDictStorage(), __CLASSCELL__);
22822282
if (newStore != namespace.getDictStorage()) {
22832283
updatedStorage.enter();
22842284
namespace.setDictStorage(newStore);
@@ -2324,8 +2324,8 @@ private String getModuleNameFromGlobals(PythonObject globals, HashingStorageLibr
23242324
}
23252325

23262326
private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases, PDict namespace, Object metaclass,
2327-
HashingStorageLibrary nslib, LookupAttributeInMRONode getDictAttrNode, LookupAttributeInMRONode getWeakRefAttrNode, GetBestBaseClassNode getBestBaseNode,
2328-
GetItemsizeNode getItemSize, WriteAttributeToObjectNode writeItemSize) {
2327+
PythonObjectLibrary lib, HashingStorageLibrary hashingStorageLib, LookupAttributeInMRONode getDictAttrNode,
2328+
LookupAttributeInMRONode getWeakRefAttrNode, GetBestBaseClassNode getBestBaseNode, GetItemsizeNode getItemSize, WriteAttributeToObjectNode writeItemSize) {
23292329
Object[] array = ensureGetObjectArrayNode().execute(bases);
23302330

23312331
PythonAbstractClass[] basesArray;
@@ -2359,7 +2359,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23592359
// 2.) copy the dictionary slots
23602360
Object[] slots = new Object[1];
23612361
boolean[] qualnameSet = new boolean[]{false};
2362-
copyDictSlots(pythonClass, namespace, nslib, slots, qualnameSet);
2362+
copyDictSlots(pythonClass, namespace, lib, hashingStorageLib, slots, qualnameSet);
23632363
if (!qualnameSet[0]) {
23642364
pythonClass.setQualName(name);
23652365
}
@@ -2369,9 +2369,9 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23692369

23702370
// CPython masks the __hash__ method with None when __eq__ is overriden, but __hash__ is
23712371
// not
2372-
Object hashMethod = nslib.getItem(namespace.getDictStorage(), __HASH__);
2372+
Object hashMethod = hashingStorageLib.getItem(namespace.getDictStorage(), __HASH__);
23732373
if (hashMethod == null) {
2374-
Object eqMethod = nslib.getItem(namespace.getDictStorage(), __EQ__);
2374+
Object eqMethod = hashingStorageLib.getItem(namespace.getDictStorage(), __EQ__);
23752375
if (eqMethod != null) {
23762376
pythonClass.setAttribute(__HASH__, PNone.NONE);
23772377
}
@@ -2455,7 +2455,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
24552455
}
24562456

24572457
// checks for some name errors too
2458-
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, addWeakRef, namespace, nslib);
2458+
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, addWeakRef, namespace, hashingStorageLib);
24592459

24602460
// add native slot descriptors
24612461
if (pythonClass.needsNativeAllocation()) {
@@ -2515,10 +2515,11 @@ private static boolean hasPythonClassBases(PythonAbstractClass[] basesArray) {
25152515
return false;
25162516
}
25172517

2518-
private void copyDictSlots(PythonClass pythonClass, PDict namespace, HashingStorageLibrary nslib, Object[] slots, boolean[] qualnameSet) {
2518+
private void copyDictSlots(PythonClass pythonClass, PDict namespace, PythonObjectLibrary lib, HashingStorageLibrary hashingStorageLib, Object[] slots, boolean[] qualnameSet) {
25192519
// copy the dictionary slots over, as CPython does through PyDict_Copy
25202520
// Also check for a __slots__ sequence variable in dict
2521-
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
2521+
PDict typeDict = null;
2522+
for (DictEntry entry : hashingStorageLib.entries(namespace.getDictStorage())) {
25222523
Object key = entry.getKey();
25232524
Object value = entry.getValue();
25242525
if (__SLOTS__.equals(key)) {
@@ -2565,8 +2566,25 @@ private void copyDictSlots(PythonClass pythonClass, PDict namespace, HashingStor
25652566
}
25662567
} else if (SpecialAttributeNames.__CLASSCELL__.equals(key)) {
25672568
// don't populate this attribute
2568-
} else {
2569+
} else if (key instanceof String && typeDict == null) {
25692570
pythonClass.setAttribute(key, value);
2571+
} else {
2572+
// DynamicObjectStorage ignores non-string keys
2573+
typeDict = lib.getDict(pythonClass);
2574+
if (typeDict == null) {
2575+
// 1.) create DynamicObjectStorage based dict from pythonClass
2576+
typeDict = PythonObjectFactory.getUncached().createDictFixedStorage(pythonClass);
2577+
try {
2578+
lib.setDict(pythonClass, typeDict);
2579+
} catch (UnsupportedMessageException ex) {
2580+
CompilerDirectives.transferToInterpreterAndInvalidate();
2581+
throw new IllegalStateException("can't set dict into " + pythonClass, ex);
2582+
}
2583+
}
2584+
// 2.) writing a non string key converts DynamicObjectStorage to
2585+
// EconomicMapStorage
2586+
HashingStorage updatedStore = hashingStorageLib.setItem(typeDict.getDictStorage(), key, value);
2587+
typeDict.setDictStorage(updatedStore);
25702588
}
25712589
}
25722590
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/PythonObject.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.oracle.graal.python.builtins.objects.PNone;
3535
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
3636
import com.oracle.graal.python.builtins.objects.dict.PDict;
37+
import com.oracle.graal.python.builtins.objects.getsetdescriptor.HiddenPythonKey;
3738
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
3839
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
3940
import com.oracle.graal.python.nodes.HiddenAttributes;
@@ -127,6 +128,7 @@ public final Object getAttribute(Object key) {
127128
@SuppressWarnings("deprecation")
128129
@TruffleBoundary
129130
public void setAttribute(Object name, Object value) {
131+
assert name instanceof String || name instanceof HiddenPythonKey || name instanceof HiddenKey : name.getClass().getSimpleName();
130132
CompilerAsserts.neverPartOfCompilation();
131133
DynamicObjectLibrary.getUncached().putWithFlags(getStorage(), name, value, 0);
132134
}

0 commit comments

Comments
 (0)