* generic/tclAlloc.c:

	* generic/tclExecute.c:
	* generic/tclInt.h:
	* generic/tclThreadAlloc.c: Fix alignment for memory returned by
	TclStackAlloc; insure that all memory allocators align to 16-byte
	boundaries on 64 bit platforms [Bug 1851832, 1851524]

4 files changed, 79 insertions, 48 deletions

diff --git a/generic/tclAlloc.c b/generic/tclAlloc.c
index 35524fd..de21c7c 100644
--- a/generic/tclAlloc.c
+++ b/generic/tclAlloc.c
@@ -15,7 +15,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclAlloc.c,v 1.26 2007/12/13 15:23:14 dgp Exp $
+ * RCS: @(#) $Id: tclAlloc.c,v 1.27 2007/12/17 15:28:27 msofer Exp $
  */
 
 /*
@@ -44,16 +44,6 @@ typedef unsigned long caddr_t;
 #endif
 
 /*
- * Alignment for allocated memory.
- */
-
-#if defined(__APPLE__)
-#define ALLOCALIGN	16
-#else
-#define ALLOCALIGN	8
-#endif
-
-/*
  * The overhead on a block is at least 8 bytes. When free, this space contains
  * a pointer to the next free block, and the bottom two bits must be zero.
  * When in use, the first byte is set to MAGIC, and the second byte is the
@@ -66,7 +56,7 @@ typedef unsigned long caddr_t;
 
 union overhead {
     union overhead *next;		/* when free */
-    unsigned char padding[ALLOCALIGN];	/* align struct to ALLOCALIGN bytes */
+    unsigned char padding[TCL_ALLOCALIGN];	/* align struct to TCL_ALLOCALIGN bytes */
     struct {
 	unsigned char magic0;		/* magic number */
 	unsigned char index;		/* bucket # */
@@ -110,7 +100,7 @@ union overhead {
  * precedes the data area returned to the user.
  */
 
-#define MINBLOCK	((sizeof(union overhead) + (ALLOCALIGN-1)) & ~(ALLOCALIGN-1))
+#define MINBLOCK	((sizeof(union overhead) + (TCL_ALLOCALIGN-1)) & ~(TCL_ALLOCALIGN-1))
 #define NBUCKETS	(13 - (MINBLOCK >> 4))
 #define MAXMALLOC	(1<<(NBUCKETS+2))
 static union overhead *nextf[NBUCKETS];
diff --git a/generic/tclExecute.c b/generic/tclExecute.c
index 1de984d..8793312 100644
--- a/generic/tclExecute.c
+++ b/generic/tclExecute.c
@@ -13,7 +13,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclExecute.c,v 1.357 2007/12/13 15:23:16 dgp Exp $
+ * RCS: @(#) $Id: tclExecute.c,v 1.358 2007/12/17 15:28:27 msofer Exp $
  */
 
 #include "tclInt.h"
@@ -836,6 +836,36 @@ TclFinalizeExecution(void)
 }
 
 /*
+ * Auxiliary code to insure that GrowEvaluationStack always returns correctly 
+ * aligned memory. This assumes that TCL_ALLOCALIGN is a multiple of the
+ * wordsize 'sizeof(Tcl_Obj *)'. 
+ */
+
+#define WALLOCALIGN \
+    (TCL_ALLOCALIGN/sizeof(Tcl_Obj *))
+
+static inline int
+OFFSET(
+    Tcl_Obj **markerPtr)
+{
+    /*
+     * Note that we are only interested in the low bits of the address, so
+     * that the fact that PTR2INT may lose the high bits is irrelevant.
+     */
+
+    int mask, base, new;
+
+    mask = WALLOCALIGN-1;
+    base = (PTR2INT(markerPtr) & mask);
+    new  = ((base + 1) + mask) & ~mask;
+    return (new - base);
+}
+
+#define MEMSTART(markerPtr) \
+    ((markerPtr) + OFFSET(markerPtr))
+
+
+/*
  *----------------------------------------------------------------------
  *
  * GrowEvaluationStack --
@@ -865,36 +895,44 @@ GrowEvaluationStack(
     ExecStack *esPtr = eePtr->execStackPtr, *oldPtr = NULL;
     int newBytes, newElems, currElems;
     int needed = growth - (esPtr->endPtr - esPtr->tosPtr);
-    Tcl_Obj **markerPtr = esPtr->markerPtr;
+    Tcl_Obj **markerPtr = esPtr->markerPtr, **memStart;
 
     if (move) {
 	if (!markerPtr) {
 	    Tcl_Panic("STACK: Reallocating with no previous alloc");
 	}
 	if (needed <= 0) {
-	    return markerPtr + 1;
+	    return MEMSTART(markerPtr);
 	}
-    } else if (needed < 0) {
-	/*
-	 * Put a marker pointing to the previous marker in this stack, and
-	 * store it in esPtr as the current marker. Return a pointer to one
-	 * word past the marker.
-	 */
+    } else {
+	Tcl_Obj **tmpMarkerPtr = esPtr->tosPtr + 1;
+	int offset = OFFSET(tmpMarkerPtr);
 
-	esPtr->markerPtr = ++esPtr->tosPtr;
-	*esPtr->markerPtr = (Tcl_Obj *) markerPtr;
-	return esPtr->markerPtr + 1;
+	if (needed + offset < 0) {
+	    /*
+	     * Put a marker pointing to the previous marker in this stack, and 
+	     * store it in esPtr as the current marker. Return a pointer to
+	     * the start of aligned memory.
+	     */
+
+	    esPtr->markerPtr = tmpMarkerPtr;
+	    memStart = tmpMarkerPtr + offset; 
+	    esPtr->tosPtr = memStart - 1;
+	    *esPtr->markerPtr = (Tcl_Obj *) markerPtr;
+	    return memStart;
+	}
     }
 
     /*
      * Reset move to hold the number of words to be moved to new stack (if
-     * any) and growth to hold the complete stack requirements.
+     * any) and growth to hold the complete stack requirements: add the marker
+     * and maximal possible offset. 
      */
 
     if (move) {
-	move = esPtr->tosPtr - markerPtr;
+	move = esPtr->tosPtr - MEMSTART(markerPtr) + 1;
     }
-    needed = growth + move + 1; /* Add the marker. */
+    needed = growth + move + WALLOCALIGN - 1;
 
     /*
      * Check if there is enough room in the next stack (if there is one, it
@@ -949,10 +987,12 @@ GrowEvaluationStack(
      */
 
     esPtr->stackWords[0] = NULL;
-    esPtr->markerPtr = esPtr->tosPtr = &esPtr->stackWords[0];
-
+    esPtr->markerPtr = &esPtr->stackWords[0];
+    memStart = MEMSTART(esPtr->markerPtr);
+    esPtr->tosPtr = memStart - 1;
+    
     if (move) {
-	memcpy(&esPtr->stackWords[1], (markerPtr+1), move*sizeof(Tcl_Obj *));
+	memcpy(memStart, MEMSTART(markerPtr), move*sizeof(Tcl_Obj *));
 	esPtr->tosPtr += move;
 	oldPtr->markerPtr = (Tcl_Obj **) *markerPtr;
 	oldPtr->tosPtr = markerPtr-1;
@@ -966,7 +1006,7 @@ GrowEvaluationStack(
 	DeleteExecStack(oldPtr);
     }
 
-    return &esPtr->stackWords[1];
+    return memStart;
 }
 
 /*
@@ -1043,7 +1083,7 @@ TclStackFree(
     esPtr = eePtr->execStackPtr;
     markerPtr = esPtr->markerPtr;
 
-    if ((markerPtr+1) != (Tcl_Obj **)freePtr) {
+    if (MEMSTART(markerPtr) != (Tcl_Obj **)freePtr) {
 	Tcl_Panic("TclStackFree: incorrect freePtr. Call out of sequence?");
     }
 
@@ -1104,7 +1144,7 @@ TclStackRealloc(
     esPtr = eePtr->execStackPtr;
     markerPtr = esPtr->markerPtr;
 
-    if ((markerPtr+1) != (Tcl_Obj **)ptr) {
+    if (MEMSTART(markerPtr) != (Tcl_Obj **)ptr) {
 	Tcl_Panic("TclStackRealloc: incorrect ptr. Call out of sequence?");
     }
 
diff --git a/generic/tclInt.h b/generic/tclInt.h
index 54a156f..b89b7b0 100644
--- a/generic/tclInt.h
+++ b/generic/tclInt.h
@@ -13,7 +13,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclInt.h,v 1.358 2007/12/13 15:23:18 dgp Exp $
+ * RCS: @(#) $Id: tclInt.h,v 1.359 2007/12/17 15:28:27 msofer Exp $
  */
 
 #ifndef _TCLINT
@@ -2034,6 +2034,17 @@ struct LimitHandler {
 #define UCHAR(c) ((unsigned char) (c))
 
 /*
+ * This macro is used to properly align the memory allocated by Tcl, giving
+ * the same alignment as the native malloc
+ */
+
+#if defined(__APPLE__)
+#define TCL_ALLOCALIGN	16
+#else
+#define TCL_ALLOCALIGN	(2*sizeof(void *))
+#endif
+
+/*
  * This macro is used to determine the offset needed to safely allocate any
  * data structure in memory. Given a starting offset or size, it "rounds up"
  * or "aligns" the offset to the next 8-byte boundary so that any data
diff --git a/generic/tclThreadAlloc.c b/generic/tclThreadAlloc.c
index 911ba07..3309058 100755
--- a/generic/tclThreadAlloc.c
+++ b/generic/tclThreadAlloc.c
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclThreadAlloc.c,v 1.24 2007/12/13 15:23:20 dgp Exp $
+ * RCS: @(#) $Id: tclThreadAlloc.c,v 1.25 2007/12/17 15:28:28 msofer Exp $
  */
 
 #include "tclInt.h"
@@ -40,16 +40,6 @@
 #define NOBJHIGH	1200
 
 /*
- * Alignment for allocated memory.
- */
-
-#if defined(__APPLE__)
-#define ALLOCALIGN	16
-#else
-#define ALLOCALIGN	8
-#endif
-
-/*
  * The following union stores accounting information for each block including
  * two small magic numbers and a bucket number when in use or a next pointer
  * when free. The original requested size (not including the Block overhead)
@@ -69,7 +59,7 @@ typedef union Block {
 	} u;
 	size_t reqSize;			/* Requested allocation size. */
     } b;
-    unsigned char padding[ALLOCALIGN];
+    unsigned char padding[TCL_ALLOCALIGN];
 } Block;
 #define nextBlock	b.u.next
 #define sourceBucket	b.u.s.bucket
@@ -83,7 +73,7 @@ typedef union Block {
  * of buckets in the bucket cache.
  */
 
-#define MINALLOC	((sizeof(Block) + 8 + (ALLOCALIGN-1)) & ~(ALLOCALIGN-1))
+#define MINALLOC	((sizeof(Block) + 8 + (TCL_ALLOCALIGN-1)) & ~(TCL_ALLOCALIGN-1))
 #define NBUCKETS	(11 - (MINALLOC >> 5))
 #define MAXALLOC	(MINALLOC << (NBUCKETS - 1))