Bug 819726

== Comment: #0 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-02 20:46:23 ==
Currently python-greenlet does not support the ppc64 platform. Add support for this as per pull request https://github.com/python-greenlet/greenlet/pull/7 upstream.

== Comment: #1 - Michael Ellerman <ellerman.com> - 2012-05-02 21:59:15 ==
Upstream are concerned we're not handling r30 properly. We think we probably just forgot to add it to REGS_TO_SAVE.

Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?

== Comment: #2 - Benjamin Herrenschmidt <bherren.com> - 2012-05-02 22:12:13 ==
Note that this method of doing clobbers in C is totally gross, extremely fragile and will eventually break. This code should be written in asm ... but then the entire premises of greenlet memcpy's the stack around is broken by design.

From what we saw, r30 was handled properly by the generated code, I think we even had a problem building if we tried to mark it as a clobber but by all means test and see.

== Comment: #3 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-02 23:09:22 ==
(In reply to comment #1)
> Upstream are concerned we're not handling r30 properly. We think we probably
> just forgot to add it to REGS_TO_SAVE.
> 
> Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?

Will running the tests included with python-greenlet be sufficient? Or was there a specific test you guys ran?

Thanks,
Nish

== Comment: #4 - Michael Ellerman <ellerman.com> - 2012-05-02 23:54:36 ==
(In reply to comment #3)
> (In reply to comment #1)
> > Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?
> 
> Will running the tests included with python-greenlet be sufficient? Or was
> there a specific test you guys ran?

Necessary, but perhaps not sufficient :)  Certainly in the beginning the tests were crashing obviously, but a subtle change like just adding r30 may not be so obvious.

If the python-greenlet test works Paul can test it on our Openstack setup here.

== Comment: #5 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-03 01:17:42 ==
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?
> > 
> > Will running the tests included with python-greenlet be sufficient? Or was
> > there a specific test you guys ran?
> 
> Necessary, but perhaps not sufficient :)  Certainly in the beginning the
> tests were crashing obviously, but a subtle change like just adding r30 may
> not be so obvious.
> 
> If the python-greenlet test works Paul can test it on our Openstack setup
> here.

Will test it first thing in the morning.

Thanks,
Nish

== Comment: #6 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-03 13:36:01 ==
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #1)
> > > > Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?
> > > 
> > > Will running the tests included with python-greenlet be sufficient? Or was
> > > there a specific test you guys ran?
> > 
> > Necessary, but perhaps not sufficient :)  Certainly in the beginning the
> > tests were crashing obviously, but a subtle change like just adding r30 may
> > not be so obvious.
> > 
> > If the python-greenlet test works Paul can test it on our Openstack setup
> > here.
> 
> Will test it first thing in the morning.

Test fails with r30 in REGS_TO_SAVE:

[root@ltcfbp55a-lp1 greenlet]# git diff
diff --git a/platform/switch_ppc64_linux.h b/platform/switch_ppc64_linux.h
index ded1fda..4963604 100644
--- a/platform/switch_ppc64_linux.h
+++ b/platform/switch_ppc64_linux.h
@@ -44,8 +44,8 @@
  * is meant to be compiled non-dynamically!
  */
 #define REGS_TO_SAVE "r2", "r14", "r15", "r16", "r17", "r18", "r19", "r20", \
-       "r21", "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r31", \
-       "cr2", "cr3", "cr4"
+       "r21", "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", \
+       "r31", "cr2", "cr3", "cr4"
 static int
 slp_switch(void)
 {
[root@ltcfbp55a-lp1 greenlet]# ./run-tests.py 
running build_ext
building 'greenlet' extension
gcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/usr/include/python2.7 -c greenlet.c -o build/temp.linux-ppc64-2.7/greenlet.o
In file included from slp_platformselect.h:14:0,
                 from greenlet.c:390:
platform/switch_ppc64_linux.h: In function 'slp_switch':
platform/switch_ppc64_linux.h:53:5: error: PIC register clobbered by 'r30' in 'asm'
In file included from slp_platformselect.h:14:0,
                 from greenlet.c:390:
platform/switch_ppc64_linux.h:66:5: error: PIC register clobbered by 'r30' in 'asm'
error: command 'gcc' failed with exit status 1
Traceback (most recent call last):
  File "./run-tests.py", line 38, in <module>
    spawn(cmd, search_path=0)
  File "/usr/lib64/python2.7/distutils/spawn.py", line 34, in spawn
    _spawn_posix(cmd, search_path, dry_run=dry_run)
  File "/usr/lib64/python2.7/distutils/spawn.py", line 167, in _spawn_posix
    (cmd[0], exit_status)
distutils.errors.DistutilsExecError: command '/bin/python' failed with exit status 1

For reference, the tests do succeed with the base commit only.

[root@ltcfbp55a-lp1 greenlet]# git reset --hard HEAD
HEAD is now at b17773a Add ppc64 platform
[root@ltcfbp55a-lp1 greenlet]# ./run-tests.py 
running build_ext
building 'greenlet' extension
gcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/usr/include/python2.7 -c greenlet.c -o build/temp.linux-ppc64-2.7/greenlet.o
creating build/lib.linux-ppc64-2.7
gcc -pthread -shared -Wl,-z,relro build/temp.linux-ppc64-2.7/greenlet.o -L. -lpython2.7 -o build/lib.linux-ppc64-2.7/greenlet.so
Linking /root/greenlet/build/lib.linux-ppc64-2.7/greenlet.so to /root/greenlet/greenlet.so
python 2.7.3 (64 bit) using greenlet 0.3.4 from /root/greenlet/greenlet.so
test_class (tests.test_throw.ThrowTests) ... ok
test_kill (tests.test_throw.ThrowTests) ... ok
test_throw_goes_to_original_parent (tests.test_throw.ThrowTests) ... ok
test_val (tests.test_throw.ThrowTests) ... ok
test_exception_switch (tests.test_cpp.CPPTests) ... ok
test_getcurrent (tests.test_extension_interface.CAPITests) ... ok
test_new_greenlet (tests.test_extension_interface.CAPITests) ... ok
test_raise_greenlet_dead (tests.test_extension_interface.CAPITests) ... ok
test_raise_greenlet_error (tests.test_extension_interface.CAPITests) ... ok
test_setparent (tests.test_extension_interface.CAPITests) ... ok
test_switch (tests.test_extension_interface.CAPITests) ... ok
test_switch_kwargs (tests.test_extension_interface.CAPITests) ... ok
test_throw (tests.test_extension_interface.CAPITests) ... ok
test_dead_weakref (tests.test_weakref.WeakRefTests) ... ok
test_inactive_weakref (tests.test_weakref.WeakRefTests) ... ok
test_arg_refs (tests.test_leaks.ArgRefcountTests) ... ok
test_kwarg_refs (tests.test_leaks.ArgRefcountTests) ... ok
test_version (tests.test_version.VersionTests) ... ok
test_generator (tests.test_generator.GeneratorTests) ... ok
test_genlet_bad (tests.test_generator_nested.NestedGeneratorTests) ... ok
test_genlet_simple (tests.test_generator_nested.NestedGeneratorTests) ... ok
test_layered_genlets (tests.test_generator_nested.NestedGeneratorTests) ... ok
test_nested_genlets (tests.test_generator_nested.NestedGeneratorTests) ... ok
test_permutations (tests.test_generator_nested.NestedGeneratorTests) ... ok
test_circular_greenlet (tests.test_gc.GCTests) ... ok
test_dead_circular_ref (tests.test_gc.GCTests) ... ok
test_inactive_ref (tests.test_gc.GCTests) ... ok
test_dealloc (tests.test_greenlet.GreenletTests) ... ok
test_dealloc_other_thread (tests.test_greenlet.GreenletTests) ... ok
test_exc_state (tests.test_greenlet.GreenletTests) ... ok
test_exception (tests.test_greenlet.GreenletTests) ... ok
test_frame (tests.test_greenlet.GreenletTests) ... ok
test_parent_equals_None (tests.test_greenlet.GreenletTests) ... ok
test_run_equals_None (tests.test_greenlet.GreenletTests) ... ok
test_send_exception (tests.test_greenlet.GreenletTests) ... ok
test_simple (tests.test_greenlet.GreenletTests) ... ok
test_switch_kwargs (tests.test_greenlet.GreenletTests) ... ok
test_switch_kwargs_to_parent (tests.test_greenlet.GreenletTests) ... ok
test_switch_to_another_thread (tests.test_greenlet.GreenletTests) ... ok
test_thread_bug (tests.test_greenlet.GreenletTests) ... ok
test_threads (tests.test_greenlet.GreenletTests) ... ok
test_two_children (tests.test_greenlet.GreenletTests) ... ok
test_two_recursive_children (tests.test_greenlet.GreenletTests) ... ok

----------------------------------------------------------------------
Ran 43 tests in 0.484s

OK

== Comment: #7 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-05 12:34:14 ==
(In reply to comment #1)
> Upstream are concerned we're not handling r30 properly. We think we probably
> just forgot to add it to REGS_TO_SAVE.
> 
> Nish can you try adding r30 to REGS_TO_SAVE and see if it still works?

Just as an FYI, while we should determine the right answer here, upstream has merged the change.

Thanks,
Nish

== Comment: #9 - NISHANTH ARAVAMUDAN <aravam.com> - 2012-05-07 19:04:43 ==

Backport of pull request https://github.com/python-greenlet/greenlet/pull/7