Bug 11145

Could you print out the stack pointer where it crashes?

Some more things we figured out while debugging via IRC:
- make check segfaults in stack_align.
- make clean && make CFLAGS="" does not fix it.
- make maintainer-clean and rebuilding without my CFLAGS (-ggdb3 -march=athlon-xp) fixes it.

I'm currently suspecting either a Makefile problem (not rebuilding some files) or an issue with the above CFLAGS.

make clean ; make CFLAGS="" isn't supposed to change the CFLAGS.  Only running ./configure will do that.

NEEDINFO.  Does this still happen?

I think this has been fixed for ages.

I've just run into this building songbird against gstreamer. We use a local version of gstreamer and various dependencies, including liboil.

This is with liboil 0.3.13:

#0  0xad45c569 in add_f32_sse_unroll2 () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#1  0xad4426f3 in _oil_test_marshal_function () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#2  0xad445284 in oil_test_check_function () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#3  0xad44181d in oil_fault_check_try () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#4  0xad445500 in oil_test_check_impl () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#5  0xad441e66 in oil_class_optimize () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#6  0xad4419d0 in oil_optimize_all () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#7  0xad44190b in oil_init () from /home/michael/svn/songbird/compiled/dist/lib/liboil-0.3.so
#8  0xac7ca0f9 in resample_init () at resample.c:47
#9  0xac7c97c0 in plugin_init (plugin=0x96d0720) at gstaudioresample.c:829
#10 0xad2a33d8 in gst_plugin_register_func (plugin=0x96d0720, desc=0xac7cfce0) at gstplugin.c:337
#11 0xad2a3c8f in gst_plugin_load_file (filename=0x8f1b338 "/home/michael/svn/songbird/compiled/dist/gst-plugins/libgstaudioresample.so", 
    error=0xbfbbfcc4) at gstplugin.c:533
#12 0xad2a44d7 in gst_plugin_load_by_name (name=0x8f1ad20 "audioresample") at gstplugin.c:951


You previously asked for the stack pointer at the point it crashed, I guess this is ESP from 'info registers'?

(gdb) info registers 
eax            0x9ceb7d8        164542424
ecx            0x9cebb70        164543344
edx            0x64     100
ebx            0xad4921c0       -1387716160
esp            0xbfbbf75c       0xbfbbf75c
ebp            0xbfbbf804       0xbfbbf804
esi            0x9ceb7d8        164542424
edi            0xad45c518       -1387936488
eip            0xad45c569       0xad45c569 <add_f32_sse_unroll2+81>
eflags         0x10212  [ AF IF RF ]
cs             0x73     115
ss             0x7b     123
ds             0x7b     123
es             0x7b     123
fs             0x0      0
gs             0x33     51

I can try upgrading things to 0.3.14 if you want, but this doesn't sound like an issue known to be fixed in that version.

I've looked into this a bit deeper. There's some code further up the stack (mozilla stuff) which is a bit unusual, and misaligns the stack (i.e. doesn't retain 16 byte alignment).

This is compatible with the x86 ABI, but is different from what gcc defaults to. So, it seems like liboil assumes/requires the gcc default stack alignment, rather than aligning things properly.

So I think this is a liboil bug, though for performance reasons it'll be desirable to change the mozilla code which is changing the stack alignment.

Hrm... I thought all the unaligned stack problems had been solved.

MikeS:

It's probably mono code that is unaligning the stack.

It's actually gcc that is assuming that the stack is aligned.  All liboil asm code is written to handled unaligned stacks, it's only the SSE intrinsics code that doesn't.  The test 'stack_align' is supposed to recreate an unaligned stack and trigger these bugs, but it doesn't seem to work well enough on my system.  Could you check this?

How I'll fix this is to disable any function that doesn't work with an unaligned stack until gcc gets fixed.  Disabling a function is as simple as putting #if 0/#endif around the function and the OIL_IMPL_DEFINE...() line immediately following.

Created attachment 16113 [details] [review]
Enable SSE wrappers and use for more functions

Re-enable the SSE wrappers in configure.ac

Go through the rest of the SSE functions and use the wrapper functionality for the rest of them.

Works for me with this.

It's mozilla that's un-aligning the stack, in this code:

http://mxr.mozilla.org/mozilla/source/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp#130

but that's just going to make things slower, it's not _wrong_. As you say, it's gcc that's wrong for not generating stack alignment code when using the intrinsics that require it.

The patch I attached is sufficient to make it work for me. Seems better than just disabling the SSE code entirely. I didn't exhaustively check that this is needed for _all_ the SSE functions, but it's certainly needed for more than the ones that already had it.

The wrapper is buggy and can't be reenabled.  I'm pretty sure there is no possible way to implement the wrapper in a portable manner, since the correct implementation depends on gcc -f options.

Ok. Turns out gcc 4.2 and above support a new attribute which makes gcc itself emit an appropriate prologue/epilogue to the code to realign the stack.

Using this on the SSE code seems to work.

Attaching a preliminary patch. Needs work still: this attribute should only be defined if we're compiling with gcc (other compilers that support the sse intrinsics are presumably fine). Further, the SSE code should be disabled entirely if we're building with a gcc that doesn't support the attribute. So, this is just a proof of concept - but it seems sufficient to me.

Not sure how to do the autoconf stuff to detect if we have an appropriate gcc version; do you think you could handle that?

Created attachment 16162 [details] [review]
Use __attribute__((force_align_arg_pointer)) for SSE code

commit e1c2b6e84c82615200172d627d9e0214def3f141
Author: David Schleef <ds@ginger.bigkitten.com>
Date:   Mon May 12 17:08:50 2008 -0700

    Check for gcc-4.2 for intrinsics, since we need gcc to realign the
    stack when higher-level libraries (this means you, mozilla) unalign
    it.  Fixes #11145.

commit 99c2ef9c10b6735592a715a17919690d617c3a4f
Author: David Schleef <ds@ginger.bigkitten.com>
Date:   Mon May 12 16:47:37 2008 -0700

    Use __attribute__((force_align_arg_pointer)) for SSE code.
    Patch from Mike Smith.  Partial fix for #11145


Fixed.