pbxt crashes on Windows 64 (misalignment on SSE instruciton)

This affects debug compilation.
Analysis shows that the problematic area is memory debug functionality in PBXT that returns 8 byte aligned pointers (due to 8 byte prefix). The critical place here is

self = (XTThreadPtr) xt_calloc_ns(sizeof(XTThreadRec));

this returns 8 byte aligned (read 16 byte unaligned) pointer. This structure contains jmp_buf array, for which alignment requirements is 16 byte. When setjmp issues instruction which operates on 16 byte aligned memory, it crashes.
the instruction in question is
movdqa xmmword ptr [rcx+60h],xmm6

=== modified file 'storage/pbxt/src/memory_xt.h'
--- storage/pbxt/src/memory_xt.h 2009-08-17 11:12:36 +0000
+++ storage/pbxt/src/memory_xt.h 2010-12-10 02:12:48 +0000
@@ -30,8 +30,10 @@
 struct XTThread;

 #ifdef DEBUG
+#ifndef _WIN32
 #define DEBUG_MEMORY
 #endif
+#endif

 #ifdef DEBUG_MEMORY

there is no reason to reinvent debug malloc on Windows with Microsoft compiler. the C runtime (debug) contains that functionality already (with bit patterns against overwrites etc), it just does not contain such bugs :)

On other platforms, where debug malloc functionality does not exist (are there any?) it could make sense to ensure 16 byte alignment too, just in case some SSE functionality is used :)

Vladislav,

thanks for the report.

Regarding _WIN32 - irc it's defined for 64 bit code as well. MS docs confirm this:

http://msdn.microsoft.com/en-us/library/b0084kay%28VS.80%29.aspx

so does the #ifdef fix really work?

BR,
Vladimir

I Vladimir,
unfortunately the #ifdef trick does not really work :(
malloc() does not always align on 16 byte, it just happened for me on one test and crashed again on another one. But the point about usefulness of PBXT debug malloc() on top of CRT debug malloc() is still valid :)

As I see it there are 2 approaches to fix it
a) either introduce _aligned_malloc() equivalents (_mm_malloc, posix_memalign).
b) ensure that jmp_buf used in setjmp/longjmp is always on stack. Compiler will take care of proper alignment then (Microsoft compiler has for example CRT_ALIGN(16) for _SETJMP_FLOAT128 members of jmp_buf)

a) looks more labor-intensive, b) looks simpler

I'll attach a patch demostrating b)

Vladislav,

thanks for your patch, however may I ask you to try something else (I don't have a 64-bit windows setup, so I cannot try it myself).

The idea is to force alignment of the jump buffer. Paul suggested that if we put jmp_buf into a union with an integer that should force the whole union to be aligned. Here;s the patch:

=== modified file 'src/thread_xt.h'
--- src/thread_xt.h 2010-02-02 10:06:03 +0000
+++ src/thread_xt.h 2010-12-13 09:01:57 +0000
@@ -153,7 +153,10 @@
 typedef struct XTJumpBuf {
        XTResourcePtr jb_res_top;
        int jb_call_top;
- jmp_buf jb_buffer;
+ union {
+ int jb_aligner;
+ jmp_buf jb_buffer;
+ };
 } XTJumpBufRec, *XTJumpBufPtr;

 typedef struct XTCallStack {

Can you try it please?

Vladimir,
this unfortunately will not work for following reasons
1) the alignment requirements are not 32 bit (4 byte, int in your patch), but 128bit (16 bytes)
2) jmp_buf is properly aligned inside the structure already, the structure is not packed, and CRT_ALIGN(16) directive (which is __declspec(align(16)) underneath is followed by compiler. The alignment directive is propagated up to XTThreadRec, i.e the array of XTJumpBuf is also aligned properly.

That is, on stack, or global variable static memory will work ok. The problem is only that if you malloc/calloc(or PBXT debug malloc) memory, it is not guaranteed to be 16 bytes aligned, and you patch does not address it at all.

Is there anybody in PBXT who has a working Win64 setup? Can you use trials (there is an iso of both windows and visual studio expensive editions freely downloadable, yet timebombed)

turns out the first proposed patch https://bugs.launchpad.net/pbxt/+bug/688404/comments/2
was already correct. 16 bytes malloc() alignment is guaranteed on Windows x64 by all Microsoft compilers : http://msdn.microsoft.com/en-us/library/ycsb6wwf.aspx
.
It is PBXT debug malloc does not have that guarantee.

s.a http://lists.askmonty.org/pipermail/commits/2011-February/001256.html

Fixed in MartiaDB (long ago, no idea which release)

Vladislav,

I added an aligner to PBXT memory debugging structure, so that now PBXT debug malloc now works fine in 86x64 mode (see the linked lp:pbxt branch). If you're going to merge it to Maria I can push Maria branch with the fix.

Windows has debug malloc, it is malloc in debug C runtime. Besides, it has pageheap and application verifier as debug mallocs for release compilation. Personally I miss the point of adding yet another layer of debug mallocs to already existing ones.