[python-greenlet] fix a segfault on i686 (#746771)
Pádraig Brady
pbrady at fedoraproject.org
Tue Nov 1 00:56:43 UTC 2011
commit 12bf2042a6dfac0d771a5c6cc8fabd5be39d76f4
Author: Pádraig Brady <P at draigBrady.com>
Date: Mon Oct 24 18:27:57 2011 +0100
fix a segfault on i686 (#746771)
* i686-register-fixes.patch: The specific fix from upstream.
* get-rid-of-ts_origin.patch: This cleanup from upstream also
avoids the issue and is extensively tested elsewhere so included.
* python-greenlet.spec: Reference the two upstream patches and
exclude the tests on the ppc64 platform where the issue still remains.
get-rid-of-ts_origin.patch | 106 ++++++++++++++++++++++++++++++++++++++++++++
i686-register-fixes.patch | 62 +++++++++++++++++++++++++
python-greenlet.spec | 33 +++++++++++++-
3 files changed, 199 insertions(+), 2 deletions(-)
---
diff --git a/get-rid-of-ts_origin.patch b/get-rid-of-ts_origin.patch
new file mode 100644
index 0000000..0840b5f
--- /dev/null
+++ b/get-rid-of-ts_origin.patch
@@ -0,0 +1,106 @@
+diff -up greenlet-0.3.1/greenlet.c.get-rid-of-ts_origin greenlet-0.3.1/greenlet.c
+--- greenlet-0.3.1/greenlet.c.get-rid-of-ts_origin 2010-04-05 17:24:25.000000000 -0400
++++ greenlet-0.3.1/greenlet.c 2011-10-19 13:59:30.485035920 -0400
+@@ -116,10 +116,8 @@ extern PyTypeObject PyGreenlet_Type;
+
+ /* The current greenlet in this thread state (holds a reference) */
+ static PyGreenlet* ts_current = NULL;
+-/* Holds a reference to the switching-from stack during the slp switch */
+-static PyGreenlet* ts_origin = NULL;
+ /* Holds a reference to the switching-to stack during the slp switch */
+-static PyGreenlet* ts_target = NULL;
++static PyGreenlet* volatile ts_target = NULL;
+ /* NULL if error, otherwise args tuple to pass around during slp switch */
+ static PyObject* ts_passaround_args = NULL;
+ static PyObject* ts_passaround_kwargs = NULL;
+@@ -257,6 +255,7 @@ static int g_save(PyGreenlet* g, char* s
+ static void slp_restore_state(void)
+ {
+ PyGreenlet* g = ts_target;
++ PyGreenlet* owner = ts_current;
+
+ /* Restore the heap copy back into the C stack */
+ if (g->stack_saved != 0) {
+@@ -265,30 +264,32 @@ static void slp_restore_state(void)
+ g->stack_copy = NULL;
+ g->stack_saved = 0;
+ }
+- if (ts_current->stack_stop == g->stack_stop)
+- g->stack_prev = ts_current->stack_prev;
+- else
+- g->stack_prev = ts_current;
++ if (owner->stack_start == NULL)
++ owner = owner->stack_prev; /* greenlet is dying, skip it */
++ while (owner && owner->stack_stop <= g->stack_stop)
++ owner = owner->stack_prev; /* find greenlet with more stack */
++ g->stack_prev = owner;
+ }
+
+ static int slp_save_state(char* stackref)
+ {
+ /* must free all the C stack up to target_stop */
+ char* target_stop = ts_target->stack_stop;
+- assert(ts_current->stack_saved == 0);
+- if (ts_current->stack_start == NULL)
+- ts_current = ts_current->stack_prev; /* not saved if dying */
++ PyGreenlet* owner = ts_current;
++ assert(owner->stack_saved == 0);
++ if (owner->stack_start == NULL)
++ owner = owner->stack_prev; /* not saved if dying */
+ else
+- ts_current->stack_start = stackref;
++ owner->stack_start = stackref;
+
+- while (ts_current->stack_stop < target_stop) {
++ while (owner->stack_stop < target_stop) {
+ /* ts_current is entierely within the area to free */
+- if (g_save(ts_current, ts_current->stack_stop))
++ if (g_save(owner, owner->stack_stop))
+ return -1; /* XXX */
+- ts_current = ts_current->stack_prev;
++ owner = owner->stack_prev;
+ }
+- if (ts_current != ts_target) {
+- if (g_save(ts_current, target_stop))
++ if (owner != ts_target) {
++ if (g_save(owner, target_stop))
+ return -1; /* XXX */
+ }
+ return 0;
+@@ -337,11 +338,11 @@ static int g_switchstack(void)
+ */
+ int err;
+ { /* save state */
++ PyGreenlet* current = ts_current;
+ PyThreadState* tstate = PyThreadState_GET();
+- ts_current->recursion_depth = tstate->recursion_depth;
+- ts_current->top_frame = tstate->frame;
++ current->recursion_depth = tstate->recursion_depth;
++ current->top_frame = tstate->frame;
+ }
+- ts_origin = ts_current;
+ err = _PyGreenlet_slp_switch();
+ if (err < 0) { /* error */
+ Py_XDECREF(ts_passaround_args);
+@@ -351,13 +352,15 @@ static int g_switchstack(void)
+ ts_passaround_kwargs = NULL;
+ }
+ else {
++ PyGreenlet* target = ts_target;
++ PyGreenlet* origin = ts_current;
+ PyThreadState* tstate = PyThreadState_GET();
+- tstate->recursion_depth = ts_target->recursion_depth;
+- tstate->frame = ts_target->top_frame;
+- ts_target->top_frame = NULL;
+- ts_current = ts_target;
+- Py_INCREF(ts_target);
+- Py_DECREF(ts_origin);
++ tstate->recursion_depth = target->recursion_depth;
++ tstate->frame = target->top_frame;
++ target->top_frame = NULL;
++ ts_current = target;
++ Py_INCREF(target);
++ Py_DECREF(origin);
+ }
+ return err;
+ }
diff --git a/i686-register-fixes.patch b/i686-register-fixes.patch
new file mode 100644
index 0000000..b7a26d0
--- /dev/null
+++ b/i686-register-fixes.patch
@@ -0,0 +1,62 @@
+# HG changeset patch
+# User Alexey Borzenkov <snaury at gmail.com>
+# Date 1313701525 -14400
+# Node ID 25bf29f4d3b79b026c1c05787bb741a8e7ef2229
+# Parent c0bf397a723d4b61d7ef78cf575dea4c0fdb527e
+Fix compilation and register problems on some i386 configurations
+
+diff -r c0bf397a723d4b61d7ef78cf575dea4c0fdb527e -r 25bf29f4d3b79b026c1c05787bb741a8e7ef2229 platform/switch_x86_unix.h
+--- a/platform/switch_x86_unix.h Thu Aug 18 02:44:20 2011 +0400
++++ b/platform/switch_x86_unix.h Fri Aug 19 01:05:25 2011 +0400
+@@ -2,6 +2,8 @@
+ * this is the internal transfer function.
+ *
+ * HISTORY
++ * 19-Aug-11 Alexey Borzenkov <snaury at gmail.com>
++ * Correctly save ebp, ebx and cw
+ * 07-Sep-05 (py-dev mailing list discussion)
+ * removed 'ebx' from the register-saved. !!!! WARNING !!!!
+ * It means that this file can no longer be compiled statically!
+@@ -34,18 +36,13 @@
+ static int
+ slp_switch(void)
+ {
++ void *ebp, *ebx;
++ unsigned short cw;
+ register int *stackref, stsizediff;
+- /* !!!!WARNING!!!! need to add "ebx" in the next line, as well as in the
+- * last line of this function, if this header file is meant to be compiled
+- * non-dynamically!
+- */
+- __asm__ volatile ("" : : :
+- "esi",
+- "edi"
+-#ifdef __MINGW32__
+- , "ebx"
+-#endif
+- );
++ __asm__ volatile ("" : : : "esi", "edi");
++ __asm__ volatile ("fstcw %0" : "=m" (cw));
++ __asm__ volatile ("movl %%ebp, %0" : "=m" (ebp));
++ __asm__ volatile ("movl %%ebx, %0" : "=m" (ebx));
+ __asm__ ("movl %%esp, %0" : "=g" (stackref));
+ {
+ SLP_SAVE_STATE(stackref, stsizediff);
+@@ -57,13 +54,10 @@
+ );
+ SLP_RESTORE_STATE();
+ }
+- __asm__ volatile ("" : : :
+- "esi",
+- "edi"
+-#ifdef __MINGW32__
+- , "ebx"
+-#endif
+- );
++ __asm__ volatile ("movl %0, %%ebx" : : "m" (ebx));
++ __asm__ volatile ("movl %0, %%ebp" : : "m" (ebp));
++ __asm__ volatile ("fldcw %0" : : "m" (cw));
++ __asm__ volatile ("" : : : "esi", "edi");
+ return 0;
+ }
+
diff --git a/python-greenlet.spec b/python-greenlet.spec
index 478c6b6..45beccc 100644
--- a/python-greenlet.spec
+++ b/python-greenlet.spec
@@ -4,12 +4,20 @@
Name: python-greenlet
Version: 0.3.1
-Release: 4%{?dist}
+Release: 6%{?dist}
Summary: Lightweight in-process concurrent programming
Group: Development/Libraries
License: MIT
URL: http://pypi.python.org/pypi/greenlet
Source0: http://pypi.python.org/packages/source/g/greenlet/greenlet-%{version}.tar.gz
+
+# Based on https://bitbucket.org/ambroff/greenlet/changeset/2d5b17472757
+# slightly fixed up to apply cleanly. Avoid rhbz#746771
+Patch1: get-rid-of-ts_origin.patch
+# Apply https://bitbucket.org/ambroff/greenlet/changeset/25bf29f4d3b7
+# to fix the i686 crash in rhbz#746771
+Patch2: i686-register-fixes.patch
+
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
BuildRequires: python2-devel
@@ -31,7 +39,8 @@ This package contains header files required for C modules development.
%prep
%setup -q -n greenlet-%{version}
-
+%patch1 -p1 -b .get-rid-of-ts_origin
+%patch2 -p1 -b .i686_register_fixes
%build
CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
@@ -44,6 +53,18 @@ rm -rf %{buildroot}
%clean
rm -rf %{buildroot}
+# FIXME!!
+# The checks segfault on ppc64. So this arch
+# is essentially not supported until this is fixed
+%ifnarch ppc ppc64
+%check
+# Run the upstream test suite:
+%{__python} setup.py test
+
+# Run the upstream benchmarking suite to further exercise the code:
+PYTHONPATH=$(pwd) %{__python} benchmarks/chain.py
+PYTHONPATH=$(pwd) %{__python} benchmarks/switch.py
+%endif
%files
%defattr(-,root,root,-)
@@ -56,6 +77,14 @@ rm -rf %{buildroot}
%{_includedir}/python*/greenlet
%changelog
+* Mon Oct 24 2011 Pádraig Brady <P at draigBrady.com> - 0.3.1-6
+- cherrypick 25bf29f4d3b7 from upstream (rhbz#746771)
+- exclude the %check from ppc where the tests segfault
+
+* Wed Oct 19 2011 David Malcolm <dmalcolm at redhat.com> - 0.3.1-5
+- add a %%check section
+- cherrypick 2d5b17472757 from upstream (rhbz#746771)
+
* Tue Feb 08 2011 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 0.3.1-4
- Rebuilt for https://fedoraproject.org/wiki/Fedora_15_Mass_Rebuild
More information about the scm-commits
mailing list