Skip to content

Commit fc71def

Browse files
steve-ssteve-s
committed
[GR-50439] [GR-50431] Fix boot issue with __eq__ special method slot.
PullRequest: graalpython/3084
2 parents 951a995 + c18a80e commit fc71def

File tree

11 files changed

+101
-24
lines changed

11 files changed

+101
-24
lines changed

‎graalpython/com.oracle.graal.python.test.integration/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ Additionally, one can change the polyglot artifacts version with
8787
</dependency>
8888
</dependencies>
8989
<configuration>
90+
<trimStackTrace>false</trimStackTrace>
9091
<testSourceDirectory>src</testSourceDirectory>
9192
<testClassesDirectory>${project.build.outputDirectory}</testClassesDirectory>
9293
<includes>

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@
4848
import com.oracle.graal.python.builtins.CoreFunctions;
4949
import com.oracle.graal.python.builtins.Python3Core;
5050
import com.oracle.graal.python.builtins.PythonBuiltins;
51+
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
5152
import com.oracle.graal.python.builtins.objects.dict.PDict;
5253
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;
54+
import com.oracle.graal.python.lib.PyObjectHashNode;
5355
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5456
import com.oracle.truffle.api.dsl.NodeFactory;
5557
import com.oracle.truffle.api.strings.TruffleString;
@@ -64,19 +66,21 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
6466
@Override
6567
public void initialize(Python3Core core) {
6668
super.initialize(core);
67-
PDict errorCode = core.factory().createDict();
69+
OSErrorEnum[] enumValues = OSErrorEnum.values();
70+
EconomicMapStorage storage = EconomicMapStorage.create(enumValues.length + 1);
6871

69-
for (OSErrorEnum value : OSErrorEnum.values()) {
72+
for (OSErrorEnum value : enumValues) {
7073
// if more OSError have the same number -> the last one wins
71-
addConstant(value.getNumber(), toTruffleStringUncached(value.name()), errorCode);
74+
addConstant(value.getNumber(), toTruffleStringUncached(value.name()), storage);
7275
}
7376

7477
// publish the dictionary with mapping code -> string name
78+
PDict errorCode = core.factory().createDict(storage);
7579
addBuiltinConstant("errorcode", errorCode);
7680
}
7781

78-
private void addConstant(int number, TruffleString name, PDict dict) {
82+
private void addConstant(int number, TruffleString name, EconomicMapStorage storage) {
7983
addBuiltinConstant(name, number);
80-
dict.setItem(number, name);
84+
storage.putUncachedWithJavaEq(number, PyObjectHashNode.hash(number), name);
8185
}
8286
}

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

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

4343
import static com.oracle.graal.python.util.PythonUtils.toTruffleStringUncached;
4444

45-
import java.util.LinkedHashMap;
4645
import java.util.List;
4746

4847
import com.oracle.graal.python.builtins.Builtin;
@@ -51,14 +50,18 @@
5150
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
5251
import com.oracle.graal.python.builtins.PythonBuiltins;
5352
import com.oracle.graal.python.builtins.PythonOS;
53+
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
5454
import com.oracle.graal.python.builtins.objects.module.PythonModule;
55+
import com.oracle.graal.python.lib.PyObjectHashNode;
5556
import com.oracle.graal.python.nodes.PRaiseNode;
5657
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5758
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
5859
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5960
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
6061
import com.oracle.truffle.api.dsl.NodeFactory;
6162
import com.oracle.truffle.api.dsl.Specialization;
63+
import com.oracle.truffle.api.strings.TruffleString;
64+
import com.oracle.truffle.api.strings.TruffleString.HashCodeNode;
6265

6366
@CoreFunctions(defineModule = "pyexpat", os = PythonOS.PLATFORM_WIN32)
6467
public final class PyExpatModuleBuiltins extends PythonBuiltins {
@@ -142,15 +145,17 @@ public void initialize(Python3Core core) {
142145
addBuiltinConstant("model", model);
143146

144147
PythonModule errors = core.factory().createPythonModule(toTruffleStringUncached("pyexpat.errors"));
145-
LinkedHashMap<String, Object> codes = new LinkedHashMap<>(ErrorConstant.values().length);
146-
LinkedHashMap<Object, Object> messages = new LinkedHashMap<>(ErrorConstant.values().length);
148+
EconomicMapStorage codes = EconomicMapStorage.create(ErrorConstant.values().length);
149+
EconomicMapStorage messages = EconomicMapStorage.create(ErrorConstant.values().length);
147150
for (ErrorConstant c : ErrorConstant.values()) {
148-
errors.setAttribute(toTruffleStringUncached(c.name()), toTruffleStringUncached(c.message));
149-
codes.put(c.message, c.ordinal() + 1);
150-
messages.put(c.ordinal() + 1, c.message);
151+
TruffleString messageTs = toTruffleStringUncached(c.message);
152+
errors.setAttribute(toTruffleStringUncached(c.name()), messageTs);
153+
int id = c.ordinal() + 1;
154+
codes.putUncachedWithJavaEq(messageTs, PyObjectHashNode.hash(messageTs, HashCodeNode.getUncached()), id);
155+
messages.putUncachedWithJavaEq(id, PyObjectHashNode.hash(id), c.message);
151156
}
152-
errors.setAttribute(toTruffleStringUncached("messages"), core.factory().createDictFromMapGeneric(messages));
153-
errors.setAttribute(toTruffleStringUncached("codes"), core.factory().createDictFromMap(codes));
157+
errors.setAttribute(toTruffleStringUncached("messages"), core.factory().createDict(messages));
158+
errors.setAttribute(toTruffleStringUncached("codes"), core.factory().createDict(codes));
154159
addBuiltinConstant("errors", errors);
155160
}
156161

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -673,8 +673,7 @@ public void postInitialize0(Python3Core core) {
673673
private static PFrozenSet createStdLibModulesSet(PythonObjectFactory factory) {
674674
EconomicMapStorage storage = EconomicMapStorage.create(STDLIB_MODULE_NAMES.length);
675675
for (String s : STDLIB_MODULE_NAMES) {
676-
TruffleString ts = toTruffleStringUncached(s);
677-
storage.putUncached(ts, PNone.NONE);
676+
storage.putUncachedWithJavaEq(s, PNone.NONE);
678677
}
679678
return factory.createFrozenSet(storage);
680679
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import java.security.NoSuchAlgorithmException;
5050
import java.security.Security;
5151
import java.util.ArrayList;
52-
import java.util.LinkedHashMap;
5352
import java.util.List;
5453
import java.util.Map;
5554

@@ -153,11 +152,11 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
153152

154153
@Override
155154
public void initialize(Python3Core core) {
156-
LinkedHashMap<String, Object> algos = new LinkedHashMap<>();
155+
EconomicMapStorage algos = EconomicMapStorage.create(DIGEST_ALGORITHMS.length);
157156
for (var digest : DIGEST_ALGORITHMS) {
158-
algos.put(digest, PNone.NONE);
157+
algos.putUncachedWithJavaEq(digest, PNone.NONE);
159158
}
160-
addBuiltinConstant("openssl_md_meth_names", core.factory().createFrozenSet(EconomicMapStorage.create(algos)));
159+
addBuiltinConstant("openssl_md_meth_names", core.factory().createFrozenSet(algos));
161160

162161
EconomicMapStorage storage = EconomicMapStorage.create();
163162
addBuiltinConstant(CONSTRUCTORS, core.factory().createMappingproxy(core.factory().createDict(storage)));

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/hpy/GraalHPyContext.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import static com.oracle.graal.python.util.PythonUtils.tsArray;
4848

4949
import java.io.IOException;
50+
import java.lang.invoke.VarHandle;
5051
import java.lang.ref.Reference;
5152
import java.lang.ref.ReferenceQueue;
5253
import java.lang.ref.WeakReference;
@@ -497,7 +498,9 @@ public void execute(PythonContext context) {
497498
private RootCallTarget getReferenceCleanerCallTarget() {
498499
if (referenceCleanerCallTarget == null) {
499500
CompilerDirectives.transferToInterpreterAndInvalidate();
500-
referenceCleanerCallTarget = PythonUtils.getOrCreateCallTarget(new HPyNativeSpaceCleanerRootNode(getContext()));
501+
RootCallTarget localTarget = PythonUtils.getOrCreateCallTarget(new HPyNativeSpaceCleanerRootNode(getContext()));
502+
VarHandle.storeStoreFence();
503+
referenceCleanerCallTarget = localTarget;
501504
}
502505
return referenceCleanerCallTarget;
503506
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/code/PCode.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ public Signature getSignature(Node inliningTarget, InlinedConditionProfile signa
591591

592592
@TruffleBoundary
593593
synchronized Signature initializeSignature(RootCallTarget rootCallTarget) {
594+
assert PythonContext.get(null).ownsGil(); // otherwise this is racy
594595
if (signature == null) {
595596
if (rootCallTarget.getRootNode() instanceof PRootNode) {
596597
signature = ((PRootNode) rootCallTarget.getRootNode()).getSignature();
@@ -610,6 +611,7 @@ public RootCallTarget getRootCallTarget() {
610611

611612
@TruffleBoundary
612613
synchronized RootCallTarget initializeCallTarget() {
614+
assert PythonContext.get(null).ownsGil(); // otherwise this is racy
613615
if (callTarget == null) {
614616
callTarget = (RootCallTarget) callTargetSupplier.get();
615617
callTargetSupplier = null;

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
package com.oracle.graal.python.builtins.objects.common;
4242

4343
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
44+
import static com.oracle.graal.python.util.PythonUtils.toTruffleStringUncached;
4445

4546
import java.util.LinkedHashMap;
4647
import java.util.Map.Entry;
@@ -153,6 +154,17 @@ private void putUncached(Object key, Object value) {
153154
ObjectHashMap.PutNode.putUncached(this.map, key, PyObjectHashNode.executeUncached(key), value);
154155
}
155156

157+
// Solves boot-order problem, do not use in normal code or during startup when __eq__ of
158+
// builtins may not be properly set-up
159+
public void putUncachedWithJavaEq(Object key, long keyHash, Object value) {
160+
ObjectHashMap.PutNode.putUncachedWithJavaEq(this.map, key, keyHash, value);
161+
}
162+
163+
public void putUncachedWithJavaEq(String key, Object value) {
164+
TruffleString ts = toTruffleStringUncached(key);
165+
putUncachedWithJavaEq(ts, PyObjectHashNode.hash(ts, HashCodeNode.getUncached()), value);
166+
}
167+
156168
private static void putAllUncached(LinkedHashMap<String, Object> map, EconomicMapStorage result) {
157169
CompilerAsserts.neverPartOfCompilation();
158170
for (Entry<String, Object> entry : map.entrySet()) {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,14 @@
4444

4545
import java.util.Arrays;
4646

47+
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
48+
import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
49+
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
50+
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
4751
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
4852
import com.oracle.graal.python.lib.PyObjectRichCompareBool.EqNode;
4953
import com.oracle.graal.python.util.PythonUtils;
54+
import com.oracle.truffle.api.CompilerAsserts;
5055
import com.oracle.truffle.api.CompilerDirectives;
5156
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5257
import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
@@ -60,6 +65,7 @@
6065
import com.oracle.truffle.api.nodes.Node;
6166
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
6267
import com.oracle.truffle.api.profiles.InlinedCountingConditionProfile;
68+
import com.oracle.truffle.api.strings.TruffleString;
6369

6470
/**
6571
* Generic dictionary/set backing storage implementation.
@@ -347,6 +353,9 @@ public static Object doGetWithRestart(Frame frame, Node inliningTarget, ObjectHa
347353
@Cached InlinedCountingConditionProfile collisionFoundNoValue,
348354
@Cached InlinedCountingConditionProfile collisionFoundEqKey,
349355
@Cached PyObjectRichCompareBool.EqNode eqNode) {
356+
// Must not call generic __eq__ before builtins are initialized
357+
// If this assert fires: we'll need something like putUncachedWithJavaEq also for get
358+
assert map.size == 0 || SpecialMethodSlot.areBuiltinSlotsInitialized();
350359
while (true) {
351360
try {
352361
return doGet(frame, map, key, keyHash, inliningTarget, foundNullKey, foundSameHashKey,
@@ -449,6 +458,14 @@ public static void putUncached(ObjectHashMap map, Object key, long keyHash, Obje
449458

450459
abstract void execute(Frame frame, Node inliningTarget, ObjectHashMap map, Object key, long keyHash, Object value);
451460

461+
static void putUncachedWithJavaEq(ObjectHashMap map, Object key, long keyHash, Object value) {
462+
assert isJavaEqualsAllowed(key) : key;
463+
doPutWithRestart(null, null, map, key, keyHash, value,
464+
InlinedBranchProfile.getUncached(), InlinedCountingConditionProfile.getUncached(), InlinedCountingConditionProfile.getUncached(),
465+
InlinedCountingConditionProfile.getUncached(), InlinedCountingConditionProfile.getUncached(), InlinedBranchProfile.getUncached(), InlinedBranchProfile.getUncached(),
466+
null);
467+
}
468+
452469
// "public" for testing...
453470
@Specialization
454471
public static void doPutWithRestart(Frame frame, Node inliningTarget, ObjectHashMap map, Object key, long keyHash, Object value,
@@ -460,6 +477,9 @@ public static void doPutWithRestart(Frame frame, Node inliningTarget, ObjectHash
460477
@Cached InlinedBranchProfile rehash1Profile,
461478
@Cached InlinedBranchProfile rehash2Profile,
462479
@Cached PyObjectRichCompareBool.EqNode eqNode) {
480+
// Must not call generic __eq__ before builtins are initialized
481+
// If this assert fires: make sure to use putUncachedWithJavaEq during initialization
482+
assert map.size == 0 || (SpecialMethodSlot.areBuiltinSlotsInitialized() || eqNode == null);
463483
while (true) {
464484
try {
465485
doPut(frame, map, key, keyHash, value, inliningTarget, foundNullKey, foundEqKey,
@@ -728,6 +748,10 @@ private boolean keysEqual(int[] originalIndices, Frame frame, Node inliningTarge
728748
if (originalKey == key) {
729749
return true;
730750
}
751+
if (CompilerDirectives.inInterpreter() && eqNode == null) {
752+
// this is hack, see putUncachedWithJavaEq
753+
return javaEquals(originalKey, key);
754+
}
731755
boolean result = eqNode.compare(frame, inliningTarget, originalKey, key);
732756
if (getKey(index) != originalKey || indices != originalIndices) {
733757
// Either someone overridden the slot we are just examining, or rehasing reallocated the
@@ -746,6 +770,18 @@ private boolean keysEqual(int[] originalIndices, Frame frame, Node inliningTarge
746770
return result;
747771
}
748772

773+
private static boolean javaEquals(Object a, Object b) {
774+
CompilerAsserts.neverPartOfCompilation();
775+
assert isJavaEqualsAllowed(a) : a;
776+
assert isJavaEqualsAllowed(b) : b;
777+
return a.equals(b);
778+
}
779+
780+
private static boolean isJavaEqualsAllowed(Object o) {
781+
return o instanceof PythonManagedClass || o instanceof PythonBuiltinClassType || //
782+
o instanceof PythonNativeClass || o instanceof Number || o instanceof TruffleString;
783+
}
784+
749785
/**
750786
* Called when we need space for new entry. It determines the new size from the number of slots
751787
* occupied by real values (i.e., does not count dummy entries), so the new size may be actually

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,10 @@ private static void initializeBuiltinClassSpecialMethodSlots(Python3Core core, P
444444
private static final Object builtinSlotsInitializationLock = new Object();
445445
private static volatile boolean builtinSlotsInitialized;
446446

447+
public static boolean areBuiltinSlotsInitialized() {
448+
return builtinSlotsInitialized;
449+
}
450+
447451
/**
448452
* Initialized builtin type according to its respective builtin class. Only context independent
449453
* values are pushed from the class to the type, because types are shared across contexts. This

0 commit comments

Comments
 (0)