Tcl Source Code

View Ticket
Login
Ticket UUID: 1851832
Title: badly aligned mem allocations?
Type: Bug Version: obsolete: 8.5b3
Submitter: msofer Created on: 2007-12-16 12:53:10
Subsystem: 41. Memory Allocation Assigned To: msofer
Priority: 8 Severity:
Status: Closed Last Modified: 2007-12-17 23:08:51
Resolution: Fixed Closed By: msofer
    Closed on: 2007-12-17 16:08:51
Description:
While investigating [Bug 1851524] I noticed that Tcl's memory allocators may not be doing the right thing wrt alignment of the allocated memory:

- TclStackAlloc returns a pointer to a memory address that is aligned to a multiple of sizeof(void *): 4-byte alignment on 32b platforms, 8-byte on 64b

- Tcl_Alloc returns 16-aligned memory on apple, 8-aligned otherwise (ALLOCALIGN macros defined both in tclAlloc.c and tclThreadAlloc.c)

- other platforms than apple (especially 64b) require 16-byte alignment (see eg http://msdn2.microsoft.com/en-us/library/ycsb6wwf(VS.80).aspx)
User Comments: msofer added on 2007-12-17 23:08:51:
Logged In: YES 
user_id=148712
Originator: YES

Patch committed to HEAD

msofer added on 2007-12-17 20:44:57:

File Added - 258880: alloc3.patch

Logged In: YES 
user_id=148712
Originator: YES

New patch alloc3.patch - previous patches removed. It sets TCL_ALLOCALIGN to 2*sizeof(void *) (or 16 for apple, as before), mimicking what gnu's malloc does. Note that the value is still 8 on 32b, but is now 16 on 64b. Both Tcl_Alloc and TclStackAlloc respect now the alignment reqs.
File Added: alloc3.patch

msofer added on 2007-12-17 19:21:06:

File Added - 258873: alloc2.patch

Logged In: YES 
user_id=148712
Originator: YES

An alternative fix for TclStackAlloc to always return 2-word aligned memory independently of whatever Tcl_Alloc does (ie, without looking at ALLOCALIGN or touching Tcl_Alloc) is attached as alloc2.patch.

Looking at TclpAlloc in more detail, I now see that the bug is "there but not there": struct Block contains a (Block *) and a size_t - on usual platforms, it is 2-word wide. The ALLOCALIGN=8 padding is only really functional on platforms where 2 words are less than 8 bytes (ie, on 16b platforms?); otherwise sizeof(Block) is 8 on 32b and 16 on 64b. The blocks themselves start at multiples of MINALLOC (16 resp 32) away from something returned by malloc, and the Tcl_Alloc'ed memory is computed in Block2Ptr as '(char *)(blockPtr+1)'.

All of this means that the alignment of Tcl_Alloc is not actually ALLOCALIGN=8 on 64b platforms, but rather 16. Setting it to 16 will not change anything in Tcl_Alloc, but "fix" the (patched with alloc.patch) TclStackAlloc and make the actual alignment of Tcl_Alloc clearer.

File Added: alloc2.patch

hobbs added on 2007-12-17 06:14:21:
Logged In: YES 
user_id=72656
Originator: NO

Passing to dgp as the primary author (?) of the TclStackAlloc changes.  Using the 8.5.0 RC, I forced a USE_TCLALLOC Linux-x64 build and built standard opts on HP-UX PA-RISC with 64-bit enabled and was not able to trigger this bug.  For that reason I moved it down to prio 8 - odd platform/compiler combo.(?)

msofer added on 2007-12-17 04:27:11:

File Deleted - 258792: 



File Added - 258809: alloc.patch

Logged In: YES 
user_id=148712
Originator: YES

File Added: alloc.patch

msofer added on 2007-12-17 02:28:13:

File Deleted - 258791:

msofer added on 2007-12-17 02:28:12:

File Added - 258792: alloc.patch

Logged In: YES 
user_id=148712
Originator: YES

File Added: alloc.patch

msofer added on 2007-12-17 02:01:23:

File Added - 258791: alloc.patch

Logged In: YES 
user_id=148712
Originator: YES

Attaching a patch that insures that TclStackAlloc uses the same alignment as Tcl_Alloc. The ALLOCALIGN macros are unified in tclInt.h as TCL_ALLOCALIGN. 

Still to do:
(a) test the patch on 64b platforms (did I do something stupid?)
(b) fix the TCL_ALLOCALIGN macros for the different platforms
File Added: alloc.patch

msofer added on 2007-12-16 20:15:33:
Logged In: YES 
user_id=148712
Originator: YES

To be determined: the macro TCL_ALIGN (used in the bytecode compiler and tbcload) currently insures 8-byte alignment; shouldn't it also be platform specific?

msofer added on 2007-12-16 19:59:18:
Logged In: YES 
user_id=148712
Originator: YES

"The address of a block returned by malloc or realloc in the GNU system is always a multiple of eight (or sixteen on 64-bit systems)"
(from http://www.delorie.com/gnu/docs/glibc/libc_31.html)

Attachments: