#3881 badly aligned mem allocations?

obsolete: 8.5b3
closed-fixed
miguel sofer
8
2007-12-17
2007-12-16
miguel sofer
No

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)

Discussion

  • miguel sofer
    miguel sofer
    2007-12-16

    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\)

     
  • miguel sofer
    miguel sofer
    2007-12-16

    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?

     
  • miguel sofer
    miguel sofer
    2007-12-16

    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

     
  • miguel sofer
    miguel sofer
    2007-12-16

    Logged In: YES
    user_id=148712
    Originator: YES

    File Added: alloc.patch

     
  • miguel sofer
    miguel sofer
    2007-12-16

     
    Attachments
  • miguel sofer
    miguel sofer
    2007-12-16

    Logged In: YES
    user_id=148712
    Originator: YES

    File Added: alloc.patch

     
  • Jeffrey Hobbs
    Jeffrey Hobbs
    2007-12-16

    • priority: 9 --> 8
    • assigned_to: hobbs --> dgp
     
  • Jeffrey Hobbs
    Jeffrey Hobbs
    2007-12-16

    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.(?)

     
  • miguel sofer
    miguel sofer
    2007-12-17

    fix GES to return 2-word aligned memory

     
    Attachments
  • miguel sofer
    miguel sofer
    2007-12-17

    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

     
  • miguel sofer
    miguel sofer
    2007-12-17

    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

     
  • miguel sofer
    miguel sofer
    2007-12-17

    ignore previous patches

     
    Attachments
  • Don Porter
    Don Porter
    2007-12-17

    • assigned_to: dgp --> msofer
     
  • miguel sofer
    miguel sofer
    2007-12-17

    Logged In: YES
    user_id=148712
    Originator: YES

    Patch committed to HEAD

     
  • miguel sofer
    miguel sofer
    2007-12-17

    • status: open --> closed-fixed