[GR-38286] Avoid native pointer sharing for PrimitiveNativeWrappers.

PullRequest: graalpython/2234

Author: fangerer

graalpython/com.oracle.graal.python.cext/src/capi.c

@@ -876,6 +876,11 @@ int PyTruffle_Debug(void *arg) {
     return 0;
 }
 
+int PyTruffle_ToNative(void *arg) {
+    polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_ToNative", arg);
+    return 0;
+}
+
 int truffle_ptr_compare(void* x, void* y, int op) {
     switch (op) {
     case Py_LT:

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_misc.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -228,6 +228,53 @@ def compile_module(self, name):
         cmpfunc=unhandled_error_compare
     )
 
+    # Tests if wrapped Java primitive values do not share the same
+    # native pointer.
+    test_primitive_sharing = CPyExtFunction(
+        lambda args: True,
+        lambda: (
+            (123.0, ),
+        ),
+        code="""
+        #ifdef GRAALVM_PYTHON
+        // internal function defined in 'capi.c'
+        int PyTruffle_ToNative(void *);
+        #else
+        // nothing to do on CPython
+        static inline int PyTruffle_ToNative(void *arg) {
+            return 0;
+        }
+        #endif
+        
+        PyObject* primitive_sharing(PyObject* val) {
+            Py_ssize_t val_refcnt = Py_REFCNT(val);
+            // assume val's refcnt is X > 0
+            Py_INCREF(val);
+            // val's refcnt should now be X+1
+
+            double dval = PyFloat_AsDouble(val);
+
+            PyTruffle_ToNative(val);
+
+            // a fresh object with the same value
+            PyObject *val1 = PyFloat_FromDouble(dval);
+            PyTruffle_ToNative(val1);
+
+            // now, kill it
+            Py_DECREF(val1);
+
+            // reset val's refcnt to X
+            Py_DECREF(val);
+
+            return val_refcnt == Py_REFCNT(val) ? Py_True : Py_False;
+        }
+        """,
+        resultspec="O",
+        argspec="O",
+        arguments=["PyObject* val"],
+        cmpfunc=unhandled_error_compare
+    )
+
     test_PyOS_double_to_string = CPyExtFunction(
         _reference_format_float,
         lambda: (
@@ -256,7 +303,7 @@ def compile_module(self, name):
             tuple(),
         ),
         code="""
-        PyObject* wrap_PyEval_GetBuiltins() {
+        PyObject* wrap_PyEval_GetBuiltins(void) {
             return (PyObject *) Py_TYPE(PyEval_GetBuiltins());
         }
         """,

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java

@@ -212,6 +212,7 @@
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.RootCallTarget;
+import com.oracle.truffle.api.TruffleLanguage.Env;
 import com.oracle.truffle.api.TruffleLogger;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
@@ -232,6 +233,7 @@
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
 import com.oracle.truffle.api.interop.UnsupportedTypeException;
 import com.oracle.truffle.api.library.CachedLibrary;
+import com.oracle.truffle.api.nodes.LanguageInfo;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
@@ -241,6 +243,7 @@
 import com.oracle.truffle.api.profiles.LoopConditionProfile;
 import com.oracle.truffle.api.profiles.ValueProfile;
 import com.oracle.truffle.api.utilities.CyclicAssumption;
+import com.oracle.truffle.llvm.api.Toolchain;
 
 @CoreFunctions(defineModule = PythonCextBuiltins.PYTHON_CEXT)
 @GenerateNodeFactory
@@ -269,6 +272,15 @@ public void initialize(Python3Core core) {
         builtinConstants.put("PyGILState_Release", new PyGILStateRelease());
     }
 
+    @Override
+    public void postInitialize(Python3Core core) {
+        super.postInitialize(core);
+        if (!core.getContext().getOption(PythonOptions.EnableDebuggingBuiltins)) {
+            PythonModule mod = core.lookupBuiltinModule(PYTHON_CEXT);
+            mod.setAttribute("PyTruffle_ToNative", PNone.NO_VALUE);
+        }
+    }
+
     @FunctionalInterface
     public interface TernaryFunction<T1, T2, T3, R> {
         R apply(T1 arg0, T2 arg1, T3 arg2);
@@ -2375,15 +2387,33 @@ Object tssDelete(Object key,
         }
     }
 
+    // directly called without landing function
     @Builtin(name = "PyTruffle_Debug", takesVarArgs = true)
     @GenerateNodeFactory
     public abstract static class PyTruffleDebugNode extends PythonBuiltinNode {
         @Specialization
         @TruffleBoundary
-        public Object doIt(Object[] args,
+        static Object doIt(Object[] args,
                         @Cached DebugNode debugNode) {
             debugNode.execute(args);
             return PNone.NONE;
         }
     }
+
+    // directly called without landing function
+    @Builtin(name = "PyTruffle_ToNative", minNumOfPositionalArgs = 1)
+    @GenerateNodeFactory
+    public abstract static class PyTruffleToNativeNode extends PythonUnaryBuiltinNode {
+        @Specialization
+        @TruffleBoundary
+        Object doIt(Object object) {
+            Env env = getContext().getEnv();
+            LanguageInfo llvmInfo = env.getInternalLanguages().get(PythonLanguage.LLVM_LANGUAGE);
+            Toolchain toolchain = env.lookup(llvmInfo, Toolchain.class);
+            if ("native".equals(toolchain.getIdentifier())) {
+                InteropLibrary.getUncached().toNative(object);
+            }
+            return PNone.NONE;
+        }
+    }
 }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/DynamicObjectNativeWrapper.java

@@ -2137,14 +2137,22 @@ int identityHashCode() {
         @ExportMessage
         TriState isIdenticalOrUndefined(Object obj) {
             if (obj instanceof PrimitiveNativeWrapper) {
-                // This basically emulates singletons for boxed values. However, we need to do
-                // so to
-                // preserve the invariant that storing an object into a list and getting it out
-                // (in
-                // the same critical region) returns the same object.
+                /*
+                 * This basically emulates singletons for boxed values. However, we need to do so to
+                 * preserve the invariant that storing an object into a list and getting it out (in
+                 * the same critical region) returns the same object.
+                 */
                 PrimitiveNativeWrapper other = (PrimitiveNativeWrapper) obj;
-                return TriState.valueOf(other.state == state && other.value == value &&
-                                (other.dvalue == dvalue || Double.isNaN(dvalue) && Double.isNaN(other.dvalue)));
+                if (other.state == state && other.value == value && (other.dvalue == dvalue || Double.isNaN(dvalue) && Double.isNaN(other.dvalue))) {
+                    /*
+                     * n.b.: in the equals, we also require the native pointer to be the same. The
+                     * reason for this is to avoid native pointer sharing. Handles are shared if the
+                     * objects are equal but in this case we must not share because otherwise we
+                     * would mess up the reference counts.
+                     */
+                    return TriState.valueOf(GetNativePointer.getGenericPtr(this) == GetNativePointer.getGenericPtr(other));
+                }
+                return TriState.FALSE;
             } else {
                 return TriState.UNDEFINED;
             }