SIGSEGV on armhf

[ahs3]

The patch I have attached is not my best work, unfortunately. However, I was unable to get 0.3.3 to compile for armhf on Linux properly, nor could I get it to run the test suite without a segfault. So, the patch does two things:

-- Add -fomit-frame-pointer for armv7l/Linux builds so that r7 can be saved, and

-- Bring platform/switch_arm32_gcc.h up-to-date with 0.3.4 (the messy part if you try to apply the patch) and -- the important bit -- makes it more conservative about what gets saved on switch. It may be saving too much.

With the patch, I can get the Debian unstable version of 0.3.3 to build and run the test suite. I have not tried this with top of tree but hopefully it isn't too difficult to figure out how to make the same changes.

Thanks.

[ahs3]

Index: python-greenlet-0.3.3/setup.py

--- python-greenlet-0.3.3.orig/setup.py 2012-02-08 18:34:38.000000000 +0000
+++ python-greenlet-0.3.3/setup.py 2012-04-27 21:34:57.000000000 +0000
@@ -7,6 +7,10 @@
if sys.platform == "openbsd4" and os.uname()[-1] == "i386":
os.environ["CFLAGS"] = ("%s %s" % (os.environ.get("CFLAGS", ""), "-Os")).lstrip()

+# workaround segfaults on ARMv7
+if sys.platform == "linux2" and os.uname()[-1] == "armv7l":

• os.environ["CFLAGS"] = ("%s %s" % (os.environ.get("CFLAGS", ""), "-fomit-frame-pointer")).lstrip()

try:
from setuptools import setup, Extension
setuptools_args = dict(test_suite='tests.test_collector', zip_safe=False)
Index: python-greenlet-0.3.3/platform/switch_arm32_gcc.h

--- python-greenlet-0.3.3.orig/platform/switch_arm32_gcc.h 2012-02-08 18:34:38.000000000 +0000
+++ python-greenlet-0.3.3/platform/switch_arm32_gcc.h 2012-04-27 21:40:55.000000000 +0000

@@ -25,7 +25,20 @@

#ifdef SLP_EVAL
#define STACK_MAGIC 0
-#define REGS_TO_SAVE "r4", "r5", "r6", "r7", "r8", "r9", "lr"
+#define REGS_TO_SAVE_GENERAL "r4", "r5", "r6", "r7", "r8", "r9", \

•                        "r10", "r11", "ip", "sp", "lr", "pc"
  

  +#if defined(SOFTFP)
  +#define REGS_TO_SAVE REGS_TO_SAVE_GENERAL
  +#elif defined(VFP_FP)
  +#define REGS_TO_SAVE REGS_TO_SAVE_GENERAL, "d8", "d9", "d10", "d11", \

•                                       "d12", "d13", "d14", "d15"
  

  +#elif defined(MAVERICK)
  +#define REGS_TO_SAVE REGS_TO_SAVE_GENERAL, "mvf4", "mvf5", "mvf6", "mvf7", \

•                                       "mvf8", "mvf9", "mvf10", "mvf11", \
  

•                                       "mvf12", "mvf13", "mvf14", "mvf15"
  

  +#else
  +#define REGS_TO_SAVE REGS_TO_SAVE_GENERAL, "f4", "f5", "f6", "f7"
  +#endif

  static int
  slp_switch(void)

@@ -33,8 +46,8 @@
void *fp;
register int *stackref, stsizediff;
asm volatile ("" : : : REGS_TO_SAVE);

•    **asm** volatile ("str fp,%0" : "=m" (fp));
  

•    **asm** ("mov %0,sp" : "=g" (stackref));
  

•    **asm** volatile ("mov r0, fp\n\tstr r0, %0" : "=m" (fp) : : "r0");
  

•    **asm** ("mov %0,sp" : "=r" (stackref));
   {
           SLP_SAVE_STATE(stackref, stsizediff);
           **asm** volatile (
  

  @@ -45,9 +58,9 @@
  );
  SLP_RESTORE_STATE();
  }

•    **asm** volatile ("ldr fp,%0" : : "m" (fp));
  

•    **asm** volatile ("ldr r0, %0\n\tmov fp, r0" : : "m" (fp) : "r0");
   **asm** volatile ("" : : : REGS_TO_SAVE);
   return 0;
  

  }

-#endif
\ No newline at end of file
+#endif

[schmir]

please open a proper pull request. The patch is unusable in it's current form. Please base your work on the last release (i.e. currently 0.3.4) or on the master branch.

[snaury]

I don't understand why you talk about omit-frame-pointer and r7, which should have nothing to do with frame pointer on arm. What is your hardware? What is you gcc version? If, by any chance, you are running on iPhone, I was working on a port some time ago, where indeed r7 is used as a frame pointer:

https://github.com/snaury/greenlet/blob/wip-iphone/platform/switch_arm32_ios.h

But I'm not aware yet of anyone else beside iOS using something like that. :-/

[snaury]

Ah, I think I understand it now, thumb uses different conventions...

[snaury]

Please test if this branch works for you:

https://github.com/snaury/greenlet/tree/wip-armhf

You shouldn't need -fomit-frame-pointer, it should just work.

[ahs3]

On 05/09/2012 09:52 AM, Alexey Borzenkov wrote:

Please test if this branch works for you:

https://github.com/snaury/greenlet/tree/wip-armhf

You shouldn't need -fomit-frame-pointer, it should just work.

---

Reply to this email directly or view it on GitHub:
#11 (comment)

I'll try to test this today and let you know. Off hand, it looks
like it should work.

Ciao,

al

Al Stone Debian Developer
E-mail: ahs3@ahs3.net http://www.debian.org

ahs3@debian.org

[ahs3]

Sorry for the delay -- this patch works correctly, and all test cases pass. Thanks!

[snaury]

Thank you very much for confirmation. I've also done some dances with android-ndk and various compiler switches and finally confirmed that gcc indeed uses r7 as frame pointer on thumb. Just pushed it to master.