Skip to content

Commit a63c3b3

Browse files
tomasstupkatomasstupka
committed
ignore __dict__ and __wekaref__ offsets when evaluating solid base and
check for too many dict/weakref slots in class bases
1 parent 1506e43 commit a63c3b3

File tree

4 files changed

+115
-23
lines changed

4 files changed

+115
-23
lines changed

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,62 @@ class C:
9494
except ValueError:
9595
raised = True
9696
assert raised
97+
98+
def test_bases_have_class_layout_conflict(self):
99+
class A: __slots__ = ["a"]
100+
class B: __slots__ = ["b"]
101+
with self.assertRaisesRegex(TypeError, 'multiple bases have instance lay-out conflict'):
102+
class C(A, B): pass
103+
with self.assertRaisesRegex(TypeError, 'multiple bases have instance lay-out conflict'):
104+
class C(A, B): __slots__ = ["a"]
105+
class B: __slots__ = ["a"]
106+
with self.assertRaisesRegex(TypeError, 'multiple bases have instance lay-out conflict'):
107+
class C(A, B): pass
108+
with self.assertRaisesRegex(TypeError, 'multiple bases have instance lay-out conflict'):
109+
class C(A, B): __slots__ = ["a"]
110+
with self.assertRaisesRegex(TypeError, 'multiple bases have instance lay-out conflict'):
111+
class C(A, B): __slots__ = ["a", "a"]
112+
113+
def test_no_bases_have_class_layout_conflict(self):
114+
class A: __slots__ = ["__dict__"]
115+
class B: __slots__ = ["__dict__"]
116+
class C(A, B): pass
117+
class C(A, B): __slots__ = ["a"]
118+
class C(A, B): __slots__ = ["__weakref__"]
119+
120+
class A: __slots__ = ["__weakref__"]
121+
class B: __slots__ = ["__weakref__"]
122+
class C(A, B): pass
123+
class C(A, B): __slots__ = ["a"]
124+
class C(A, B): __slots__ = ["_dict_"]
125+
126+
def test_slot_disallowed(self):
127+
class A: __slots__ = ["__dict__"]
128+
class B: __slots__ = ["__dict__"]
129+
with self.assertRaisesRegex(TypeError, '__dict__ slot disallowed: we already got one'):
130+
class C(A, B): __slots__ = ["__dict__"]
131+
132+
class A: pass
133+
class B: __slots__ = ["__dict__"]
134+
with self.assertRaisesRegex(TypeError, '__dict__ slot disallowed: we already got one'):
135+
class C(A, B): __slots__ = ["__dict__"]
97136

137+
class A: __slots__ = ["__weakref__"]
138+
class B: __slots__ = ["__weakref__"]
139+
with self.assertRaisesRegex(TypeError, '__weakref__ slot disallowed: either we already got one, or __itemsize__ != 0'):
140+
class C(A, B): __slots__ = ["__weakref__"]
141+
142+
class A: pass
143+
class B: __slots__ = ["__weakref__"]
144+
with self.assertRaisesRegex(TypeError, '__weakref__ slot disallowed: either we already got one, or __itemsize__ != 0'):
145+
class C(A, B): __slots__ = ["__weakref__"]
146+
147+
class A: pass
148+
class B: pass
149+
with self.assertRaisesRegex(TypeError, '__dict__ slot disallowed: we already got one'):
150+
class C(A, B): __slots__ = ["__dict__", "__dict__"]
151+
with self.assertRaisesRegex(TypeError, '__weakref__ slot disallowed: either we already got one, or __itemsize__ != 0'):
152+
class C(A, B): __slots__ = ["__weakref__", "__weakref__"]
153+
98154
if __name__ == "__main__":
99155
unittest.main()

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,6 +2089,7 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
20892089
@Cached CallNode callInitSubclassNode,
20902090
@Cached CallNode callNewFuncNode,
20912091
@Cached("create(__DICT__)") LookupAttributeInMRONode getDictAttrNode,
2092+
@Cached("create(__WEAKREF__)") LookupAttributeInMRONode getWeakRefAttrNode,
20922093
@Cached GetBestBaseClassNode getBestBaseNode,
20932094
@Cached DictBuiltins.CopyNode copyDict) {
20942095
// Determine the proper metatype to deal with this
@@ -2106,7 +2107,7 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
21062107

21072108
try {
21082109
PDict namespace = (PDict) copyDict.call(frame, namespaceOrig);
2109-
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, nslib, getDictAttrNode, getBestBaseNode);
2110+
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, nslib, getDictAttrNode, getWeakRefAttrNode, getBestBaseNode);
21102111

21112112
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
21122113
Object setName = getSetNameNode.execute(entry.value);
@@ -2200,8 +2201,8 @@ private String getModuleNameFromGlobals(PythonObject globals, HashingStorageLibr
22002201
}
22012202
}
22022203

2203-
private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases, PDict namespace, Object metaclass, HashingStorageLibrary nslib, LookupAttributeInMRONode getDictAttrNode,
2204-
GetBestBaseClassNode getBestBaseNode) {
2204+
private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases, PDict namespace, Object metaclass,
2205+
HashingStorageLibrary nslib, LookupAttributeInMRONode getDictAttrNode, LookupAttributeInMRONode getWeakRefAttrNode, GetBestBaseClassNode getBestBaseNode) {
22052206
Object[] array = ensureGetObjectArrayNode().execute(bases);
22062207

22072208
PythonAbstractClass[] basesArray;
@@ -2220,7 +2221,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
22202221
}
22212222
}
22222223
// check for possible layout conflicts
2223-
getBestBaseNode.execute(basesArray);
2224+
Object base = getBestBaseNode.execute(basesArray);
22242225

22252226
assert metaclass != null;
22262227

@@ -2254,6 +2255,13 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
22542255
}
22552256

22562257
boolean addDict = false;
2258+
boolean addWeakRef = false;
2259+
// may_add_dict = base->tp_dictoffset == 0
2260+
boolean mayAddDict = getDictAttrNode.execute(base) == PNone.NO_VALUE;
2261+
// may_add_weak = base->tp_weaklistoffset == 0 && base->tp_itemsize == 0
2262+
// TODO: maybe also slots size as equivalent of tp_itemsize?
2263+
boolean mayAddWeakRef = getWeakRefAttrNode.execute(base) == PNone.NO_VALUE;
2264+
22572265
if (slots[0] == null) {
22582266
// takes care of checking if we may_add_dict and adds it if needed
22592267
addDictIfNative(frame, pythonClass);
@@ -2292,12 +2300,17 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
22922300
throw raise(TypeError, ErrorMessages.MUST_BE_STRINGS_NOT_P, "__slots__ items", element);
22932301
}
22942302
if (__DICT__.equals(slotName)) {
2295-
// check that the native base does not already have tp_dictoffset
2296-
if (addDictIfNative(frame, pythonClass)) {
2297-
throw raise(TypeError, ErrorMessages.SLOT_DISALLOWED_WE_GOT_ONE, "__dict__");
2303+
if (!mayAddDict || addDict || addDictIfNative(frame, pythonClass)) {
2304+
throw raise(TypeError, ErrorMessages.DICT_SLOT_DISALLOWED_WE_GOT_ONE);
22982305
}
22992306
addDict = true;
23002307
addDictDescrAttribute(basesArray, getDictAttrNode, pythonClass);
2308+
} else if (__WEAKREF__.equals(slotName)) {
2309+
if (!mayAddWeakRef || addWeakRef) {
2310+
throw raise(TypeError, ErrorMessages.WEAKREF_SLOT_DISALLOWED_WE_GOT_ONE);
2311+
}
2312+
addWeakRef = true;
2313+
addWeakrefDescrAttribute(pythonClass);
23012314
} else {
23022315
// TODO: check for __weakref__
23032316
// TODO avoid if native slots are inherited
@@ -2317,7 +2330,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
23172330
}
23182331

23192332
// checks for some name errors too
2320-
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, false, namespace, nslib);
2333+
PTuple newSlots = copySlots(name, slotsStorage, slotlen, addDict, addWeakRef, namespace, nslib);
23212334

23222335
// add native slot descriptors
23232336
if (pythonClass.needsNativeAllocation()) {
@@ -2558,6 +2571,9 @@ private CastToListNode getCastToListNode() {
25582571
return castToList;
25592572
}
25602573

2574+
/**
2575+
* check that the native base does not already have tp_dictoffset
2576+
*/
25612577
private boolean addDictIfNative(VirtualFrame frame, PythonManagedClass pythonClass) {
25622578
boolean addedNewDict = false;
25632579
if (pythonClass.needsNativeAllocation()) {

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

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
import com.oracle.graal.python.nodes.PGuards;
112112
import com.oracle.graal.python.nodes.PNodeWithContext;
113113
import com.oracle.graal.python.nodes.PRaiseNode;
114+
import static com.oracle.graal.python.nodes.SpecialAttributeNames.__WEAKREF__;
114115
import com.oracle.graal.python.nodes.SpecialMethodNames;
115116
import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
116117
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
@@ -1010,38 +1011,41 @@ protected Object exec(Object type,
10101011
@Cached CallBinaryMethodNode callGetAttr,
10111012
@Cached GetDictStorageNode getDictStorageNode,
10121013
@CachedLibrary(limit = "4") HashingStorageLibrary storageLibrary,
1013-
@CachedLibrary(limit = "6") PythonObjectLibrary objectLibrary) {
1014-
return solidBase(type, getBaseClassNode, context, lookupGetAttribute, callGetAttr, getDictStorageNode, storageLibrary, objectLibrary);
1014+
@CachedLibrary(limit = "6") PythonObjectLibrary objectLibrary,
1015+
@Cached GetInternalObjectArrayNode getArrayNode) {
1016+
return solidBase(type, getBaseClassNode, context, lookupGetAttribute, callGetAttr, getDictStorageNode, storageLibrary, objectLibrary, getArrayNode);
10151017
}
10161018

10171019
private Object solidBase(Object type, GetBaseClassNode getBaseClassNode, PythonContext context, LookupSpecialMethodNode.Dynamic lookupGetAttribute,
10181020
CallBinaryMethodNode callGetAttr, GetDictStorageNode getDictStorageNode, HashingStorageLibrary storageLibrary,
1019-
PythonObjectLibrary objectLibrary) {
1021+
PythonObjectLibrary objectLibrary, GetInternalObjectArrayNode getArrayNode) {
10201022
Object base = getBaseClassNode.execute(type);
10211023

10221024
if (base != null) {
1023-
base = solidBase(base, getBaseClassNode, context, lookupGetAttribute, callGetAttr, getDictStorageNode, storageLibrary, objectLibrary);
1025+
base = solidBase(base, getBaseClassNode, context, lookupGetAttribute, callGetAttr, getDictStorageNode, storageLibrary, objectLibrary, getArrayNode);
10241026
} else {
10251027
base = context.getCore().lookupType(PythonBuiltinClassType.PythonObject);
10261028
}
10271029

1028-
if (type != base && extraivars(type, base, lookupGetAttribute, callGetAttr, getDictStorageNode, storageLibrary, objectLibrary)) {
1030+
if (type == base) {
10291031
return type;
1030-
} else {
1031-
return base;
10321032
}
1033-
}
10341033

1035-
private boolean extraivars(Object type, Object base, LookupSpecialMethodNode.Dynamic lookupGetAttribute, CallBinaryMethodNode callGetAttr,
1036-
GetDictStorageNode getDictStorageNode, HashingStorageLibrary storageLibrary, PythonObjectLibrary objectLibrary) {
10371034
Object typeSlots = getSlotsFromDict(type, lookupGetAttribute, callGetAttr, getDictStorageNode, objectLibrary, storageLibrary);
10381035
Object baseSlots = getSlotsFromDict(base, lookupGetAttribute, callGetAttr, getDictStorageNode, objectLibrary, storageLibrary);
1036+
if (extraivars(type, base, typeSlots, baseSlots, objectLibrary, getArrayNode)) {
1037+
return type;
1038+
} else {
1039+
return base;
1040+
}
1041+
}
10391042

1040-
if (typeSlots == null && baseSlots != null && objectLibrary.length(baseSlots) != 0 ||
1041-
baseSlots == null && typeSlots != null && objectLibrary.length(typeSlots) != 0) {
1043+
@TruffleBoundary
1044+
private boolean extraivars(Object type, Object base, Object typeSlots, Object baseSlots, PythonObjectLibrary objectLibrary, GetInternalObjectArrayNode getArrayNode) {
1045+
if (typeSlots == null && baseSlots != null && length(((PSequence) baseSlots).getSequenceStorage(), getArrayNode) != 0 ||
1046+
baseSlots == null && typeSlots != null && length(((PSequence) typeSlots).getSequenceStorage(), getArrayNode) != 0) {
10421047
return true;
10431048
}
1044-
10451049
Object typeNewMethod = LookupAttributeInMRONode.lookupSlow(type, __NEW__, GetMroStorageNode.getUncached(), ReadAttributeFromObjectNode.getUncached(), true);
10461050
Object baseNewMethod = LookupAttributeInMRONode.lookupSlow(base, __NEW__, GetMroStorageNode.getUncached(), ReadAttributeFromObjectNode.getUncached(), true);
10471051
if (typeNewMethod != baseNewMethod) {
@@ -1050,7 +1054,22 @@ private boolean extraivars(Object type, Object base, LookupSpecialMethodNode.Dyn
10501054
return hasDict(base, objectLibrary) != hasDict(type, objectLibrary);
10511055
}
10521056

1053-
protected Object getSlotsFromDict(Object type, LookupSpecialMethodNode.Dynamic lookupGetAttribute, CallBinaryMethodNode callGetAttr,
1057+
@TruffleBoundary
1058+
private static int length(SequenceStorage storage, GetInternalObjectArrayNode getArrayNode) {
1059+
int result = 0;
1060+
int length = storage.length();
1061+
Object[] slots = getArrayNode.execute(storage);
1062+
for (int i = 0; i < length; i++) {
1063+
// omit __DICT__ and __WEAKREF__, they cause no class layout conflict
1064+
// see also test_slts.py#test_no_bases_have_class_layout_conflict
1065+
if (!(slots[i].equals(__DICT__) || slots[i].equals(__WEAKREF__))) {
1066+
result++;
1067+
}
1068+
}
1069+
return result;
1070+
}
1071+
1072+
private static Object getSlotsFromDict(Object type, LookupSpecialMethodNode.Dynamic lookupGetAttribute, CallBinaryMethodNode callGetAttr,
10541073
GetDictStorageNode getDictStorageNode, PythonObjectLibrary objectLibrary, HashingStorageLibrary lib) {
10551074
Object getAttr = lookupGetAttribute.execute(objectLibrary.getLazyPythonClass(type), __GETATTRIBUTE__, type, false);
10561075
Object dict = callGetAttr.executeObject(getAttr, type, __DICT__);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,8 @@ public abstract class ErrorMessages {
467467
public static final String SIZE_MUST_BE_D_OR_S = "size must be %d or %s";
468468
public static final String SLICE_INDICES_MUST_BE_INT_NONE_HAVE_INDEX = "slice indices must be integers or None or have an __index__ method";
469469
public static final String SLICE_STEP_CANNOT_BE_ZERO = "slice step cannot be zero";
470-
public static final String SLOT_DISALLOWED_WE_GOT_ONE = "%s slot disallowed: we already got one";
470+
public static final String DICT_SLOT_DISALLOWED_WE_GOT_ONE = "__dict__ slot disallowed: we already got one";
471+
public static final String WEAKREF_SLOT_DISALLOWED_WE_GOT_ONE = "__weakref__ slot disallowed: either we already got one, or __itemsize__ != 0";
471472
public static final String STAR_WANTS_INT = "* wants int";
472473
public static final String TOO_MANY_DECIMAL_DIGITS_IN_FORMAT_STRING = "Too many decimal digits in format string";
473474
public static final String STARRED_ASSIGMENT_MUST_BE_IN_LIST_OR_TUPLE = "starred assignment target must be in a list or tuple";

0 commit comments

Comments
 (0)