Re: PowerPC malloc alignment

[Daniel Jacobowitz <drow at false dot org>]

On Fri, Nov 02, 2007 at 03:40:11PM -0400, Daniel Jacobowitz wrote:
> I see two ways to increase the alignment.
> 
>   - Build a second set of malloc routines with the old alignment.  At
>     malloc_set_state time, install them as hooks if necessary.
> 
>   - Tell people to rebuild emacs.  Improve the crash by adding a
>     debugging message out of malloc_set_state when it detects the
>     problem.
> 
> You can guess which one of these I like better.  I doubt that anything
> besides Emacs uses malloc_set_state.  Some older librep may have, but
> nowadays it has only the vestigial configure check.  The only other
> references I could find were programs using malloc_get_state to peek
> at statistics, and they won't be affected anyway.
> 
> Let me know if you think we need the hooks approach.

Here is a patch which does neither of those extras.  Both extras are
for the sole benefit of existing emacs binaries; probably one of them
should accompany this patch, but at least this is a starting point.

-- 
Daniel Jacobowitz
CodeSourcery

2007-11-05  Daniel Jacobowitz  <dan@codesourcery.com>

	* malloc/malloc.c (MALLOC_ALIGNMENT): Use __alignof__ (long double).
	(SMALLBIN_CORRECTION): New.
	(MIN_LARGE_SIZE, smallbin_index): Use it to handle 16-byte alignment.
	(largebin_index_32_big): New.
	(largebin_index): Use it for 16-byte alignment.
	(sYSMALLOc): Handle MALLOC_ALIGNMENT > 2 * SIZE_SZ.

---
 malloc/malloc.c |   61 ++++++++++++++++++++++++++++++++------------------------
 1 files changed, 35 insertions(+), 26 deletions(-)

Index: glibc-2.5.90/malloc/malloc.c
===================================================================
--- glibc-2.5.90.orig/malloc/malloc.c	2007-11-02 12:43:15.000000000 -0700
+++ glibc-2.5.90/malloc/malloc.c	2007-11-02 17:23:03.000000000 -0700
@@ -378,16 +378,8 @@ extern "C" {
 
 
 #ifndef MALLOC_ALIGNMENT
-/* XXX This is the correct definition.  It differs from 2*SIZE_SZ only on
-   powerpc32.  For the time being, changing this is causing more
-   compatibility problems due to malloc_get_state/malloc_set_state than
-   will returning blocks not adequately aligned for long double objects
-   under -mlong-double-128.
-
 #define MALLOC_ALIGNMENT       (2 * SIZE_SZ < __alignof__ (long double) \
 				? __alignof__ (long double) : 2 * SIZE_SZ)
-*/
-#define MALLOC_ALIGNMENT       (2 * SIZE_SZ)
 #endif
 
 /* The corresponding bit mask value */
@@ -2121,18 +2113,23 @@ typedef struct malloc_chunk* mbinptr;
 
     The bins top out around 1MB because we expect to service large
     requests via mmap.
+
+    Bin 0 does not exist.  Bin 1 is the unordered list; if that would be
+    a valid chunk size the small bins are bumped up one.
 */
 
 #define NBINS             128
 #define NSMALLBINS         64
 #define SMALLBIN_WIDTH    MALLOC_ALIGNMENT
-#define MIN_LARGE_SIZE    (NSMALLBINS * SMALLBIN_WIDTH)
+#define SMALLBIN_CORRECTION (MALLOC_ALIGNMENT > 2 * SIZE_SZ)
+#define MIN_LARGE_SIZE    ((NSMALLBINS - SMALLBIN_CORRECTION) * SMALLBIN_WIDTH)
 
 #define in_smallbin_range(sz)  \
   ((unsigned long)(sz) < (unsigned long)MIN_LARGE_SIZE)
 
 #define smallbin_index(sz) \
-  (SMALLBIN_WIDTH == 16 ? (((unsigned)(sz)) >> 4) : (((unsigned)(sz)) >> 3))
+  ((SMALLBIN_WIDTH == 16 ? (((unsigned)(sz)) >> 4) : (((unsigned)(sz)) >> 3)) \
+   + SMALLBIN_CORRECTION)
 
 #define largebin_index_32(sz)                                                \
 (((((unsigned long)(sz)) >>  6) <= 38)?  56 + (((unsigned long)(sz)) >>  6): \
@@ -2142,6 +2139,14 @@ typedef struct malloc_chunk* mbinptr;
  ((((unsigned long)(sz)) >> 18) <=  2)? 124 + (((unsigned long)(sz)) >> 18): \
                                         126)
 
+#define largebin_index_32_big(sz)                                            \
+(((((unsigned long)(sz)) >>  6) <= 45)?  49 + (((unsigned long)(sz)) >>  6): \
+ ((((unsigned long)(sz)) >>  9) <= 20)?  91 + (((unsigned long)(sz)) >>  9): \
+ ((((unsigned long)(sz)) >> 12) <= 10)? 110 + (((unsigned long)(sz)) >> 12): \
+ ((((unsigned long)(sz)) >> 15) <=  4)? 119 + (((unsigned long)(sz)) >> 15): \
+ ((((unsigned long)(sz)) >> 18) <=  2)? 124 + (((unsigned long)(sz)) >> 18): \
+                                        126)
+
 // XXX It remains to be seen whether it is good to keep the widths of
 // XXX the buckets the same or whether it should be scaled by a factor
 // XXX of two as well.
@@ -2154,7 +2159,9 @@ typedef struct malloc_chunk* mbinptr;
                                         126)
 
 #define largebin_index(sz) \
-  (SIZE_SZ == 8 ? largebin_index_64 (sz) : largebin_index_32 (sz))
+  (SIZE_SZ == 8 ? largebin_index_64 (sz)                                     \
+   : MALLOC_ALIGNMENT == 16 ? largebin_index_32_big (sz)                     \
+   : largebin_index_32 (sz))
 
 #define bin_index(sz) \
  ((in_smallbin_range(sz)) ? smallbin_index(sz) : largebin_index(sz))
@@ -2951,14 +2958,14 @@ static Void_t* sYSMALLOc(nb, av) INTERNA
       Round up size to nearest page.  For mmapped chunks, the overhead
       is one SIZE_SZ unit larger than for normal chunks, because there
       is no following chunk whose prev_size field could be used.
+
+      See the front_misalign handling below, for glibc there is no
+      need for further alignments unless we have have high alignment.
     */
-#if 1
-    /* See the front_misalign handling below, for glibc there is no
-       need for further alignments.  */
-    size = (nb + SIZE_SZ + pagemask) & ~pagemask;
-#else
-    size = (nb + SIZE_SZ + MALLOC_ALIGN_MASK + pagemask) & ~pagemask;
-#endif
+    if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
+      size = (nb + SIZE_SZ + pagemask) & ~pagemask;
+    else
+      size = (nb + SIZE_SZ + MALLOC_ALIGN_MASK + pagemask) & ~pagemask;
     tried_mmap = true;
 
     /* Don't try if size wraps around 0 */
@@ -2976,13 +2983,16 @@ static Void_t* sYSMALLOc(nb, av) INTERNA
           address argument for later munmap in free() and realloc().
         */
 
-#if 1
-	/* For glibc, chunk2mem increases the address by 2*SIZE_SZ and
-	   MALLOC_ALIGN_MASK is 2*SIZE_SZ-1.  Each mmap'ed area is page
-	   aligned and therefore definitely MALLOC_ALIGN_MASK-aligned.  */
-        assert (((INTERNAL_SIZE_T)chunk2mem(mm) & MALLOC_ALIGN_MASK) == 0);
-#else
-        front_misalign = (INTERNAL_SIZE_T)chunk2mem(mm) & MALLOC_ALIGN_MASK;
+	if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
+	  {
+	    /* For glibc, chunk2mem increases the address by 2*SIZE_SZ and
+	       MALLOC_ALIGN_MASK is 2*SIZE_SZ-1.  Each mmap'ed area is page
+	       aligned and therefore definitely MALLOC_ALIGN_MASK-aligned.  */
+	    assert (((INTERNAL_SIZE_T)chunk2mem(mm) & MALLOC_ALIGN_MASK) == 0);
+	    front_misalign = 0;
+	  }
+	else
+	  front_misalign = (INTERNAL_SIZE_T)chunk2mem(mm) & MALLOC_ALIGN_MASK;
         if (front_misalign > 0) {
           correction = MALLOC_ALIGNMENT - front_misalign;
           p = (mchunkptr)(mm + correction);
@@ -2990,7 +3000,6 @@ static Void_t* sYSMALLOc(nb, av) INTERNA
           set_head(p, (size - correction) |IS_MMAPPED);
         }
         else
-#endif
 	  {
 	    p = (mchunkptr)mm;
 	    set_head(p, size|IS_MMAPPED);