Re-code the saving of the gp register for MIPS64 so it compiles

[madisongh]

In working on a project that wants to use greenlet on a MIPS64-based Linux system, I ran into the following compilation error:

In file included from slp_platformselect.h:36:0,
                  from greenlet.c:416:
platform/switch_mips_unix.h: In function 'slp_switch':
platform/switch_mips_unix.h:26:5: error: PIC register clobbered by '$28' in 'asm'
     __asm__ __volatile__ ("" : : : REGS_TO_SAVE);
     ^
platform/switch_mips_unix.h:41:5: error: PIC register clobbered by '$28' in 'asm'
     __asm__ __volatile__ ("" : : : REGS_TO_SAVE);
     ^

This was using gcc 4.7 and 4.9. To fix this, I came up with this alternative way of saving the gp register. The generated assembly looks correct, and all tests are passing.

[snaury]

Thanks for looking into this, but I don't understand how this can possibly work. Compiler would be free to optimize that save and restore away (as there are no side effects), and given that it's a PIC register in your case it would probably work by simply removing $28 equally well. Unfortunately working tests are not enough to prove correctness in this case.

If the situation is similar to %ebx and %rbx on x86, you could try an approach similar to those architectures, i.e. save/restore directly in assembly without telling C compiler about it.

[snaury]

Btw, how careful does one need to be when working with memory on mips? Would it be safe to use something like sw and lw?

[madisongh]

Thanks, and you're right, simply removing $28 altogether does work, as the prologue/epilogue code generated by the compiler for the slp_switch function already handles the save and restore, since slp_switch itself is making GP-relative references.

I double-checked on this by creating a C test program that invoked they Python interpreter. The program and companion shared library each exported methods that referenced global data. I had it run a Python script that created greenlets that invoked the exported methods and switched between them. Data references from both C and via the exported methods were correct, and examining the contents of the GP register in each context showed that it was indeed getting handled correctly.

[snaury]

Commit 10ff5cf actually looked very good, could you please remove last two commits (that removed sd/ld and sw/lw) and add a small comment at the top of the file detailing the change? (in case stackless wants to reimport the code later) Thanks.

P.S. The sad thing is that while ignoring $28 works for you there may be situations where it won't work (as evidenced by the current code, since it compiled for somebody, $28 wasn't a pic register for them). For example saving %ebx on x86 is wasteful too, modern linux distros compile pic code by default anyway, so it gives an illusion that it can just be skipped (since it would have the same value in all greenlets). Unfortunately, there's no guarantee that some build won't compile everything statically and without pic (freeing those registers to use by the compiler, but since they are not scratch registers they need to be saved). What's even worse is that compiler (during optimization) might recognize that some functions don't really need to access data via a pic register, so it becomes effectively free across internal calls, and then saved once somewhere high up the stack, but might not be saved in slp_switch. In that case pic register would not really be a pic register, thus on return from switch its value would be incorrect.

There seems to be no way to detect via ifdef's whether that happens on not, so it's better to just save it anyway. Yes, most of the time you would always restore the same value you stored, but as there's no way to detect "oops, we should really have saved that!" at compile time it would make a difference between a crash and no crash. :)

[madisongh]

Fair enough, although based on my re-reading of the MIPS ABI spec, I should just be using sd/ld for both the 64 ABI and the n32 ABI, as the registers themselves are 64 bits in both cases.

[snaury]

Are you sure? After some googling I found this: https://sourceware.org/ml/newlib/2012/msg00450.html

Which suggests that even _ABIO64 has 32-bit pointers. If true sd (64-bit) to a void* (32-bit) would cause a stack overrun. Not sure if that's true, would have to see gcc sources on that.

[snaury]

Ah, I see now that you explicitly use uint64_t, that would work.

[madisongh]

Yep. Documentation on N32 is pretty thin, but I used http://techpubs.sgi.com/library/manuals/2000/007-2816-004/pdf/007-2816-004.pdf as a reference, and examining the code generated by gcc and used in glibc for context switching, they both uniformly save/restore the full 64-bit registers. Thanks!

[snaury]

@madisongh would you please add a commit with an addition to HISTORY at the top of the file? Something like:

 * 20-Sep-14 Matt Madison  <email>
 *      Re-code the saving of the gp register for MIPS64.

This is for future reference so it would be easy to spot what changed and when. Thanks.