Merge pull request #1200 from kmod/extra_refs

Some work on the numpy test suite

Makefile

@@ -722,6 +722,7 @@ PROFILE_TARGET := ./pyston $(SRC_DIR)/minibenchmarks/combined.py
 $(CMAKE_DIR_RELEASE_GCC_PGO)/.trained: pyston_release_gcc_pgo_instrumented
 	@echo "Training pgo"
 	mkdir -p $(CMAKE_DIR_RELEASE_GCC_PGO)
+	rm -rf $(CMAKE_DIR_RELEASE_GCC_PGO)/from_cpython/{Lib,Modules,Objects,Python,Parser} $(CMAKE_DIR_RELEASE_GCC_PGO)/src
 	(cd $(CMAKE_DIR_RELEASE_GCC_PGO_INSTRUMENTED) && $(PROFILE_TARGET) && $(PROFILE_TARGET) ) && touch $(CMAKE_DIR_RELEASE_GCC_PGO)/.trained
 
 pyston_pgo: pyston_release_gcc_pgo

src/asm_writing/rewriter.cpp

@@ -1248,7 +1248,7 @@ std::vector<Location> Rewriter::getDecrefLocations() {
         ASSERT(var->locations.size() == 1, "this code only looks at one location");
         Location l = *var->locations.begin();
 
-        assert(l.type == Location::Scratch);
+        assert(l.type == Location::Scratch || l.type == Location::Stack);
 
         int offset1 = indirectFor(l).offset;
         int offset2 = p.second;

src/capi/typeobject.cpp

@@ -872,7 +872,7 @@ static PyObject* call_attribute(PyObject* self, PyObject* attr, PyObject* name)
 
 template <ExceptionStyle S, Rewritable rewritable>
 Box* slotTpGetattrHookInternal(Box* self, BoxedString* name, GetattrRewriteArgs* rewrite_args, bool for_call,
-                               Box** bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI) {
+                               BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;

src/capi/typeobject.h

@@ -58,7 +58,7 @@ int compatible_for_assignment(PyTypeObject* oldto, PyTypeObject* newto, const ch
 class GetattrRewriteArgs;
 template <ExceptionStyle S, Rewritable rewritable>
 Box* slotTpGetattrHookInternal(Box* self, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool for_call,
-                               Box** bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI);
+                               BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI);
 }
 
 #endif

src/runtime/objmodel.cpp

@@ -43,6 +43,7 @@
 #include "runtime/generator.h"
 #include "runtime/hiddenclass.h"
 #include "runtime/ics.h"
+#include "runtime/import.h"
 #include "runtime/iterobject.h"
 #include "runtime/long.h"
 #include "runtime/rewrite_args.h"
@@ -1807,7 +1808,7 @@ bool isNondataDescriptorInstanceSpecialCase(Box* descr) {
 
 template <Rewritable rewritable>
 Box* nondataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, Box* obj, Box* descr, RewriterVar* r_descr,
-                                           bool for_call, Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
+                                           bool for_call, BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;
@@ -1888,7 +1889,7 @@ Box* nondataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, Box
             }
             return boxInstanceMethod(im_self, im_func, im_class);
         } else {
-            *bind_obj_out = incref(im_self);
+            *bind_obj_out = im_self;
             if (rewrite_args) {
                 rewrite_args->setReturn(r_im_func, ReturnConvention::HAS_RETURN);
                 *r_bind_obj_out = r_im_self;
@@ -1915,7 +1916,7 @@ Box* nondataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, Box
                 rewrite_args->setReturn(r_descr, ReturnConvention::HAS_RETURN);
                 *r_bind_obj_out = rewrite_args->obj;
             }
-            *bind_obj_out = incref(obj);
+            *bind_obj_out = obj;
             return incref(descr);
         } else {
             BoxedWrapperDescriptor* self = static_cast<BoxedWrapperDescriptor*>(descr);
@@ -1944,7 +1945,7 @@ Box* nondataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, Box
 // r_descr must represent a valid object.
 template <Rewritable rewritable>
 Box* descriptorClsSpecialCases(GetattrRewriteArgs* rewrite_args, BoxedClass* cls, Box* descr, RewriterVar* r_descr,
-                               bool for_call, Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
+                               bool for_call, BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;
@@ -1999,7 +2000,7 @@ Box* boxChar(char c) {
 // r_descr needs to represent a valid object
 template <Rewritable rewritable>
 Box* dataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, BoxedString* attr_name, Box* obj, Box* descr,
-                                        RewriterVar* r_descr, bool for_call, Box** bind_obj_out,
+                                        RewriterVar* r_descr, bool for_call, BORROWED(Box**) bind_obj_out,
                                         RewriterVar** r_bind_obj_out) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
@@ -2214,7 +2215,7 @@ static void ensureValidCapiReturn(Box* r) {
 
 template <ExceptionStyle S, Rewritable rewritable>
 Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
-                       Box** bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI) {
+                       BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) noexcept(S == CAPI) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;
@@ -2431,7 +2432,7 @@ Box* processDescriptor(Box* obj, Box* inst, Box* owner) {
 
 template <bool IsType, Rewritable rewritable>
 Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
-                            Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
+                            BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;
@@ -3745,8 +3746,6 @@ Box* callattrInternal(Box* obj, BoxedString* attr, LookupScope scope, CallattrRe
         return val;
     }
 
-    AUTO_XDECREF(bind_obj);
-
     if (bind_obj != NULL) {
         Box** new_args = NULL;
         if (npassed_args >= 3) {
@@ -4214,12 +4213,19 @@ Box* rearrangeArgumentsAndCallInternal(ParamReceiveSpec paramspec, const ParamNa
     }
 
     // Clear any increfs we did for when we throw an exception:
+    int positional_to_positional = std::min(argspec.num_args, paramspec.num_args);
     auto clear_refs = [&]() {
-        Py_XDECREF(oarg1);
-        Py_XDECREF(oarg2);
-        Py_XDECREF(oarg3);
-        for (int i = 0; i < num_output_args - 3; i++) {
-            Py_XDECREF(oargs[i]);
+        switch (positional_to_positional) {
+            case 0:
+                Py_XDECREF(oarg1);
+            case 1:
+                Py_XDECREF(oarg2);
+            case 2:
+                Py_XDECREF(oarg3);
+            default:
+                for (int i = std::max(positional_to_positional - 3, 0); i < num_output_args - 3; i++) {
+                    Py_XDECREF(oargs[i]);
+                }
         }
     };
     ExceptionCleanup<decltype(clear_refs)> cleanup(
@@ -4274,9 +4280,8 @@ Box* rearrangeArgumentsAndCallInternal(ParamReceiveSpec paramspec, const ParamNa
 
     ////
     // First, match up positional parameters to positional/varargs:
-    int positional_to_positional = std::min(argspec.num_args, paramspec.num_args);
     for (int i = 0; i < positional_to_positional; i++) {
-        getArg(i, oarg1, oarg2, oarg3, oargs) = incref(getArg(i, arg1, arg2, arg3, args));
+        getArg(i, oarg1, oarg2, oarg3, oargs) = getArg(i, arg1, arg2, arg3, args);
     }
 
     int varargs_to_positional = std::min((int)varargs_size, paramspec.num_args - positional_to_positional);
@@ -4960,7 +4965,7 @@ Box* callCLFunc(FunctionMetadata* md, CallRewriteArgs* rewrite_args, int num_out
 
     // We check for this assertion later too - by checking it twice, we know
     // if the error state was set before calling the chosen CF or after.
-    ASSERT(!PyErr_Occurred(), "");
+    ASSERT(imported_foreign_cextension || !PyErr_Occurred(), "");
 
     Box* r;
     // we duplicate the call to callChosenCf here so we can
@@ -4985,7 +4990,7 @@ Box* callCLFunc(FunctionMetadata* md, CallRewriteArgs* rewrite_args, int num_out
         ASSERT(chosen_cf->spec->rtn_type->isFitBy(r->cls), "%s (%p) was supposed to return %s, but gave a %s",
                g.func_addr_registry.getFuncNameAtAddress(chosen_cf->code, true, NULL).c_str(), chosen_cf->code,
                chosen_cf->spec->rtn_type->debugName().c_str(), r->cls->tp_name);
-        ASSERT(!PyErr_Occurred(), "%p", chosen_cf->code);
+        ASSERT(imported_foreign_cextension || !PyErr_Occurred(), "%p", chosen_cf->code);
     }
 
     return r;

src/runtime/objmodel.h

@@ -168,7 +168,7 @@ template <ExceptionStyle S> inline Box* getattrInternal(Box* obj, BoxedString* a
 // __getattribute__.
 template <bool IsType, Rewritable rewritable>
 Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
-                            Box** bind_obj_out, RewriterVar** r_bind_obj_out);
+                            BORROWED(Box**) bind_obj_out, RewriterVar** r_bind_obj_out);
 
 extern "C" PyObject* type_getattro(PyObject* o, PyObject* name) noexcept;
 

test/extra/numpy_fulltest.py

+import os
+import sys
+import subprocess
+
+sys.path.append(os.path.dirname(__file__) + "/../lib")
+import test_helper
+
+def print_progress_header(text):
+    print "\n>>>"
+    print ">>> " + text + "..."
+    print ">>>"
+
+ENV_NAME = "numpy_fulltest_env_" + os.path.basename(sys.executable)
+DEPENDENCIES = ["nose==1.3.7"]
+
+import platform
+USE_CUSTOM_PATCHES = (platform.python_implementation() == "Pyston")
+
+test_helper.create_virtenv(ENV_NAME, DEPENDENCIES)
+
+
+SRC_DIR = ENV_NAME
+ENV_DIR = os.path.abspath(ENV_NAME)
+PYTHON_EXE = os.path.abspath(ENV_NAME + "/bin/python")
+CYTHON_DIR = os.path.abspath(os.path.join(SRC_DIR, "Cython-0.22"))
+NUMPY_DIR = os.path.abspath(os.path.join(SRC_DIR, "numpy"))
+
+print_progress_header("Setting up Cython...")
+if not os.path.exists(CYTHON_DIR):
+
+    url = "http://cython.org/release/Cython-0.22.tar.gz"
+    subprocess.check_call(["wget", url], cwd=SRC_DIR)
+    subprocess.check_call(["tar", "-zxf", "Cython-0.22.tar.gz"], cwd=SRC_DIR)
+
+    if USE_CUSTOM_PATCHES:
+        PATCH_FILE = os.path.abspath(os.path.join(os.path.dirname(__file__), "../integration/Cython-0.22.patch"))
+        subprocess.check_call(["patch", "-p1", "--input=" + PATCH_FILE], cwd=CYTHON_DIR)
+        print ">>> Applied Cython patch"
+
+
+    try:
+        subprocess.check_call([PYTHON_EXE, "setup.py", "install"], cwd=CYTHON_DIR)
+        subprocess.check_call([PYTHON_EXE, "-c", "import Cython"], cwd=CYTHON_DIR)
+    except:
+        subprocess.check_call(["rm", "-rf", CYTHON_DIR])
+else:
+    print ">>> Cython already installed."
+
+NUMPY_PATCH_FILE = os.path.abspath(os.path.join(os.path.dirname(__file__), "../integration/numpy_patch.patch"))
+
+print_progress_header("Cloning up NumPy...")
+if not os.path.exists(NUMPY_DIR):
+    url = "https://github.com/numpy/numpy"
+    subprocess.check_call(["git", "clone", "--depth", "1", "--branch", "v1.11.0", url], cwd=SRC_DIR)
+else:
+    print ">>> NumPy already installed."
+
+PATCH_FILE = os.path.abspath(os.path.join(os.path.dirname(__file__), "../integration/numpy_patch.patch"))
+
+if USE_CUSTOM_PATCHES:
+    print_progress_header("Patching NumPy...")
+    subprocess.check_call(["patch", "-p1", "--input=" + PATCH_FILE], cwd=NUMPY_DIR)
+
+try:
+    env = os.environ
+    CYTHON_BIN_DIR = os.path.abspath(os.path.join(ENV_NAME + "/bin"))
+    env["PATH"] = CYTHON_BIN_DIR + ":" + env["PATH"]
+
+    # Should be able to do this:
+    # subprocess.check_call([os.path.join(SRC_DIR, "bin/pip"), "install", NUMPY_DIR])
+    # but they end up naming f2py "f2py_release"/"f2py_dbg"
+
+    print_progress_header("Setting up NumPy...")
+    subprocess.check_call([PYTHON_EXE, "setup.py", "build"], cwd=NUMPY_DIR, env=env)
+
+    print_progress_header("Installing NumPy...")
+    subprocess.check_call([PYTHON_EXE, "setup.py", "install"], cwd=NUMPY_DIR, env=env)
+except:
+    if USE_CUSTOM_PATCHES:
+        print_progress_header("Unpatching NumPy...")
+        cmd = ["patch", "-p1", "--forward", "-i", NUMPY_PATCH_FILE, "-R", "-d", NUMPY_DIR]
+        subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+
+    # TODO: I'm not sure we need to do this:
+    subprocess.check_call(["rm", "-rf", NUMPY_DIR + "/build"])
+    subprocess.check_call(["rm", "-rf", NUMPY_DIR + "/dist"])
+
+    raise
+
+try:
+    test_helper.run_test(['sh', '-c', '. %s/bin/activate && python %s/numpy/tools/test-installed-numpy.py' % (ENV_DIR, ENV_DIR)],
+            ENV_NAME, [dict(ran=5781, errors=2, failures=3)])
+finally:
+    if USE_CUSTOM_PATCHES:
+        print_progress_header("Unpatching NumPy...")
+        cmd = ["patch", "-p1", "--forward", "-i", NUMPY_PATCH_FILE, "-R", "-d", NUMPY_DIR]
+        subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+
+print
+print "PASSED"

test/integration/numpy_test.py

@@ -91,7 +91,7 @@ NUMPY_PATCH_FILE = os.path.abspath(os.path.join(os.path.dirname(__file__), "nump
 print_progress_header("Cloning up NumPy...")
 if not os.path.exists(NUMPY_DIR):
     url = "https://github.com/numpy/numpy"
-    subprocess.check_call(["git", "clone", "--branch", "v1.11.0", url], cwd=SRC_DIR)
+    subprocess.check_call(["git", "clone", "--depth", "1", "--branch", "v1.11.0", url], cwd=SRC_DIR)
 else:
     print ">>> NumPy already installed."
 

test/lib/test_helper.py

@@ -8,9 +8,9 @@ def create_virtenv(name, package_list = None, force_create = False):
     if force_create or not os.path.exists(name) or os.stat(sys.executable).st_mtime > os.stat(name + "/bin/python").st_mtime:
         if os.path.exists(name):
             subprocess.check_call(["rm", "-rf", name])
-        
+
         print "Creating virtualenv to install testing dependencies..."
-        
+
         VIRTUALENV_SCRIPT = os.path.dirname(__file__) + "/../lib/virtualenv/virtualenv.py"
 
         try:
@@ -58,11 +58,12 @@ def parse_output(output):
     return result
 
 def run_test(cmd, cwd, expected, env = None):
+    print "Running", cmd
     process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd, env=env)
     output, unused_err = process.communicate()
     errcode = process.poll()
     result = parse_output(output)
-    
+
     print
     print "Return code:", errcode
     if expected == result:

test/tests/arg_refs.py

+# skip-if: '-n' in EXTRA_JIT_ARGS or '-O' in EXTRA_JIT_ARGS
 # Tests to see if we add any extra refs to function arguments.
 
 import sys
@@ -7,4 +8,13 @@ def f(o):
     print sys.getrefcount(o)
 
 # This gives 3 for CPython and our interpreter, but 2 for the llvm tier:
-# f(object())
+f(object())
+
+import sys
+class C(object):
+    def foo(self, *args):
+        print sys.getrefcount(self)
+c = C()
+a = (c.foo)
+print sys.getrefcount(c)
+c.foo()

test/tests/cpython_oldstyle_getattr_crash.py

+# skip-if: True
+# - sadly, we have now inherited CPython's buggy behavior here
+
 # This segfaults under python-dbg
 # (In a release build it "works" since the use-after-free happens without penalty.)