Aligned memory allocation fixes and enhancements

pjlib/include/pj/pool.h

@@ -405,9 +405,6 @@ PJ_IDECL(pj_pool_t*) pj_pool_create(pj_pool_factory *factory,
  *                          administrative info.
  * @param alignment         the default alignment of memory block allocated 
  *                          from this pool (must be power of 2).
- *                          The actual allocation alignment will be at least equal
- *                          to the alignment argument of the function, 
- *                          but not less than PJ_POOL_ALIGNMENT. 
  *                          Value of 0 means use PJ_POOL_ALIGNMENT.
  * @param callback          Callback to be called when error occurs in the pool.
  *                          If this value is NULL, then the callback from pool
@@ -516,10 +513,6 @@ PJ_IDECL(void*) pj_pool_alloc( pj_pool_t *pool, pj_size_t size);
  *
  * @param pool      the pool.
  * @param alignment the requested alignment of the allocation.
- *                  The actual alignment of the allocation will be at least 
- *                  equal to the alignment argument of the function, 
- *                  but not less than the default pool alignment specified 
- *                  when the pool was created.
  *                  Value of 0 means use the default alignment of this pool.
  * @param size      the requested size.
  *

pjlib/include/pj/pool_i.h

@@ -45,9 +45,11 @@ PJ_IDEF(void*) pj_pool_alloc_from_block( pj_pool_block *block, pj_size_t alignme
 {
     unsigned char *ptr;
 
-    pj_assert(PJ_IS_POWER_OF_TWO(alignment) && PJ_IS_ALIGNED(size, alignment));
+    pj_assert(PJ_IS_POWER_OF_TWO(alignment));
     // Size should be already aligned.
     // this code was moved up to pj_pool_aligned_alloc. 
+    // ...and then removed there
+    //
     ///* The operation below is valid for size==0. 
     // * When size==0, the function will return the pointer to the pool
     // * memory address, but no memory will be allocated.
@@ -57,8 +59,11 @@ PJ_IDEF(void*) pj_pool_alloc_from_block( pj_pool_block *block, pj_size_t alignme
     //}
     ptr = PJ_POOL_ALIGN_PTR(block->cur, alignment);
     if (block->cur <= ptr && /* check pointer overflow */
-        (pj_size_t)(block->end - ptr) >= size) /* check available size */
+        block->end - ptr >= (pj_ssize_t)size) /* check available size */
     {
+    //if (ptr + size <= block->end &&
+    //    /* here we check pointer overflow */
+    //    block->cur <= ptr && ptr <= ptr + size) {
         block->cur = ptr + size;
         return ptr;
     }
@@ -70,16 +75,19 @@ PJ_IDEF(void*) pj_pool_alloc( pj_pool_t *pool, pj_size_t size)
     return pj_pool_aligned_alloc(pool, 0, size);
 }
 
-PJ_IDECL(void *) pj_pool_aligned_alloc(pj_pool_t *pool, pj_size_t alignment,
-                                       pj_size_t size)
+PJ_IDEF(void*) pj_pool_aligned_alloc(pj_pool_t *pool, pj_size_t alignment,
+                                     pj_size_t size)
 {
     void *ptr;
 
     PJ_ASSERT_RETURN(!alignment || PJ_IS_POWER_OF_TWO(alignment), NULL);
 
-    if (alignment < pool->alignment)
+    if (!alignment)
         alignment = pool->alignment;
 
+#if 0
+    //Rounding up is the compiler's job, not the allocator's.
+
     /* The operation below is valid for size==0. 
      * When size==0, the function will return the pointer to the pool
      * memory address, but no memory will be allocated.
@@ -88,6 +96,7 @@ PJ_IDECL(void *) pj_pool_aligned_alloc(pj_pool_t *pool, pj_size_t alignment,
         size = (size + alignment) & ~(alignment -1);
     }
     pj_assert(PJ_IS_ALIGNED(size, alignment));
+#endif
 
     ptr = pj_pool_alloc_from_block(pool->block_list.next, 
                                    alignment, size);
@@ -119,12 +128,12 @@ PJ_IDEF(pj_pool_t*) pj_pool_create( pj_pool_factory *f,
     return pj_pool_aligned_create(f, name, initial_size, increment_size, 0, callback);
 }
 
-PJ_IDECL(pj_pool_t *) pj_pool_aligned_create(pj_pool_factory *f,
-                                             const char *name,
-                                             pj_size_t initial_size,
-                                             pj_size_t increment_size,
-                                             pj_size_t alignment,
-                                             pj_pool_callback *callback)
+PJ_IDEF(pj_pool_t*) pj_pool_aligned_create(pj_pool_factory *f,
+                                           const char *name,
+                                           pj_size_t initial_size,
+                                           pj_size_t increment_size,
+                                           pj_size_t alignment,
+                                           pj_pool_callback *callback)
 {
     return (*f->create_pool)(f, name, initial_size, increment_size, alignment, callback);
 }

pjlib/src/pj/pool.c

@@ -70,8 +70,8 @@ static pj_pool_block *pj_pool_create_block( pj_pool_t *pool, pj_size_t size)
     block->buf = ((unsigned char*)block) + sizeof(pj_pool_block);
     block->end = ((unsigned char*)block) + size;
 
-    /* Set the start pointer, aligning it as needed */
-    block->cur = PJ_POOL_ALIGN_PTR(block->buf, pool->alignment);
+    /* Set the start pointer, unaligned! */
+    block->cur = block->buf;
 
     /* Insert in the front of the list. */
     pj_list_insert_after(&pool->block_list, block);
@@ -98,7 +98,7 @@ PJ_DEF(void*) pj_pool_allocate_find(pj_pool_t *pool, pj_size_t alignment,
     unsigned i = 0;
 
     PJ_CHECK_STACK();
-    pj_assert(PJ_IS_POWER_OF_TWO(alignment) && PJ_IS_ALIGNED(size, alignment));
+    pj_assert(PJ_IS_POWER_OF_TWO(alignment));
 
     while (block != &pool->block_list) {
         p = pj_pool_alloc_from_block(block, alignment, size);
@@ -134,7 +134,7 @@ PJ_DEF(void*) pj_pool_allocate_find(pj_pool_t *pool, pj_size_t alignment,
     if (pool->increment_size < 
             sizeof(pj_pool_block)+ /*block header, itself may be unaligned*/
             alignment-1 + /* gap [0:alignment-1] to align first allocation*/
-            size)                  /* allocation size, already aligned    */
+            size)                  /* allocation size (NOT aligned!)      */
     {
         pj_size_t count;
         count = (pool->increment_size + 
@@ -180,7 +180,7 @@ PJ_DEF(void) pj_pool_init_int(pj_pool_t *pool,
 
     pool->increment_size = increment_size;
     pool->callback = callback;
-    pool->alignment = (alignment < PJ_POOL_ALIGNMENT) ? PJ_POOL_ALIGNMENT : alignment;
+    pool->alignment = !alignment ? PJ_POOL_ALIGNMENT : alignment;
 
     if (name) {
         char *p = pj_ansi_strchr(name, '%');
@@ -216,9 +216,6 @@ PJ_DEF(pj_pool_t*) pj_pool_create_int( pj_pool_factory *f, const char *name,
                      NULL);
     PJ_ASSERT_RETURN(!alignment || PJ_IS_POWER_OF_TWO(alignment), NULL);
 
-    if (alignment < PJ_POOL_ALIGNMENT)
-        alignment = PJ_POOL_ALIGNMENT;
-
     /* If callback is NULL, set calback from the policy */
     if (callback == NULL)
         callback = f->policy.callback;
@@ -240,8 +237,8 @@ PJ_DEF(pj_pool_t*) pj_pool_create_int( pj_pool_factory *f, const char *name,
     block->buf = ((unsigned char*)block) + sizeof(pj_pool_block);
     block->end = buffer + initial_size;
 
-    /* Set the start pointer, aligning it as needed */
-    block->cur = PJ_POOL_ALIGN_PTR(block->buf, alignment);
+    /* Set the start pointer, unaligned! */
+    block->cur = block->buf;
 
     pj_list_insert_after(&pool->block_list, block);
 
@@ -285,8 +282,8 @@ static void reset_pool(pj_pool_t *pool)
 
     block = pool->block_list.next;
 
-    /* Set the start pointer, aligning it as needed */
-    block->cur = PJ_POOL_ALIGN_PTR(block->buf, pool->alignment);
+    /* Set the start pointer, unaligned! */
+    block->cur = block->buf;
 
     pool->capacity = block->end - (unsigned char*)pool;
 }

pjlib/src/pj/pool_dbg.c

@@ -69,7 +69,7 @@ PJ_DEF(pj_pool_t*) pj_pool_create_imp( const char *file, int line,
     PJ_UNUSED_ARG(initial_size);
     PJ_UNUSED_ARG(increment_size);
 
-    if (alignment < PJ_POOL_ALIGNMENT)
+    if (!alignment)
         alignment = PJ_POOL_ALIGNMENT;
 
     PJ_ASSERT_RETURN(IS_POWER_OF_TWO(alignment), NULL);
@@ -174,7 +174,7 @@ PJ_DEF(void*) pj_pool_alloc_imp( const char *file, int line,
     PJ_UNUSED_ARG(file);
     PJ_UNUSED_ARG(line);
 
-    if (alignment < pool->alignment)
+    if (!alignment)
         alignment = pool->alignment;
 
     PJ_ASSERT_RETURN(IS_POWER_OF_TWO(alignment), NULL);

pjlib/src/pjlib-test/pool.c

@@ -42,6 +42,10 @@
 #define THIS_FILE   "pool.c"
 #define SIZE        4096
 
+#ifndef PJ_IS_ALIGNED
+#   define PJ_IS_ALIGNED(PTR, ALIGNMENT)       (!((pj_ssize_t)(PTR) & ((ALIGNMENT)-1)))
+#endif
+
 /* Normally we should throw exception when memory alloc fails.
  * Here we do nothing so that the flow will go back to original caller,
  * which will test the result using NULL comparison. Normally caller will
@@ -78,11 +82,14 @@ static int capacity_test(void)
 /* Test that the alignment works. */
 static int pool_alignment_test(void)
 {
-    pj_pool_t *pool, *pool2;
-    void *ptr;
+    pj_pool_t *pool = NULL, *pool2 = NULL;
+    unsigned char *ptr;
     enum { MEMSIZE = 64, LOOP = 100, POOL_ALIGNMENT_TEST = 4*PJ_POOL_ALIGNMENT };
     unsigned i;
     int rc = 0;
+    char msg[1024];
+    pj_size_t capacity;
+    pj_ssize_t alignment;
 
     PJ_LOG(3,("test", "...alignment test"));
 
@@ -95,8 +102,42 @@ static int pool_alignment_test(void)
     pj_pool_release(pool);
     PJ_TEST_NOT_NULL(ptr, NULL, return -300);
 
+    /* Test to reproduce the situation when PJ_POOL_ALIGN_PTR(block->cur, alignment) > block->end */
+    /* Create a non-expandable pool of small capacity */
+    pool = pj_pool_aligned_create(mem, NULL,
+                                  50 + sizeof(pj_pool_t) + sizeof(pj_pool_block),
+                                  0, 4,
+                                  &null_callback);
+    PJ_TEST_NOT_NULL(pool, NULL, return -301);
+
+    ptr = pj_pool_alloc(pool, 0);           /* ptr == block->cur */
+    PJ_TEST_NOT_NULL(ptr, NULL, { rc=-302; goto on_return; });
+
+    /* To guarantee PJ_POOL_ALIGN_PTR(block->cur, alignment) > block->end, 
+     * our alignment must be large enough that block does not contain 
+     * an aligned pointer. We find the alignment to be large enough that 
+     * the largest number divisible by the alignment and smaller 
+     * than block->end is also smaller than block->cur.
+     */
+    capacity = pj_pool_get_capacity(pool);  /* ptr + capacity >= block->end */
+    alignment = (pj_ssize_t)pool->alignment;
+    while (alignment > 0 && (((pj_size_t)(ptr+capacity)) & ~(alignment-1)) >= (pj_size_t)ptr)
+        alignment <<= 1;
+    PJ_TEST_GT(alignment, 0, NULL, { rc=-303; goto on_return; });
+
+    /* Now PJ_POOL_ALIGN_PTR(block->cur, alignment) should be > block->end.
+     * We should not be able to allocate anything with this alignment.
+     */
+    ptr = pj_pool_aligned_alloc(pool, alignment, 0);
+    pj_ansi_snprintf(msg, sizeof(msg), 
+        "alignment=%ld, capacity=%lu, block->buf=%p, ptr==block->cur=%p, block->end=%p", 
+        alignment, capacity, pool->block_list.next->buf,pool->block_list.next->cur,pool->block_list.next->end);
+    PJ_TEST_EQ(ptr, NULL, msg, { rc=-304; goto on_return; });
+
+    pj_pool_release(pool);
+
     pool = pj_pool_create(mem, NULL, PJ_POOL_SIZE+MEMSIZE, MEMSIZE, NULL);
-    PJ_TEST_NOT_NULL(pool, NULL, return -304);
+    PJ_TEST_NOT_NULL(pool, NULL, return -305);
 
     pool2 = pj_pool_aligned_create(mem, NULL, PJ_POOL_SIZE + MEMSIZE, MEMSIZE,
                                    POOL_ALIGNMENT_TEST, NULL);
@@ -111,17 +152,21 @@ static int pool_alignment_test(void)
         /* Test first allocation */
         if (i % 2)
         {
+            /* alignment > pool->alignment */
             ptr = pj_pool_aligned_alloc(pool, POOL_ALIGNMENT_TEST, 1);
             PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-310; goto on_return; });
 
+            /* alignment < pool->alignment */
             ptr = pj_pool_aligned_alloc(pool2, PJ_POOL_ALIGNMENT, 1);
-            PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-315; goto on_return; });
+            PJ_TEST_TRUE(IS_ALIGNED(ptr), NULL, { rc=-315; goto on_return; });
         }
         else
         {
+            /* alignment == pool->alignment */
             ptr = pj_pool_alloc(pool, 1);
             PJ_TEST_TRUE(IS_ALIGNED(ptr), NULL, { rc=-320; goto on_return; });
 
+            /* alignment == pool->alignment */
             ptr = pj_pool_alloc(pool2, 1);
             PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-325; goto on_return; });
         }
@@ -130,14 +175,17 @@ static int pool_alignment_test(void)
         ptr = pj_pool_alloc(pool, 1);
         PJ_TEST_TRUE(IS_ALIGNED(ptr), NULL, { rc=-330; goto on_return; });
 
+        /* alignment > pool->alignment */
         ptr = pj_pool_aligned_alloc(pool, POOL_ALIGNMENT_TEST, 1);
         PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-335; goto on_return; });
 
+        /* alignment == pool->alignment */
         ptr = pj_pool_alloc(pool2, 1);
         PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-340; goto on_return; });
 
+        /* alignment < pool->alignment */
         ptr = pj_pool_aligned_alloc(pool2, PJ_POOL_ALIGNMENT, 1);
-        PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-345; goto on_return; });
+        PJ_TEST_TRUE(IS_ALIGNED(ptr), NULL, { rc=-345; goto on_return; });
 
 
         /* Test allocation after new block is created */
@@ -151,7 +199,7 @@ static int pool_alignment_test(void)
         PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-360; goto on_return; });
 
         ptr = pj_pool_aligned_alloc(pool2, PJ_POOL_ALIGNMENT, MEMSIZE*2+1);
-        PJ_TEST_TRUE(IS_ALIGNED2(ptr), NULL, { rc=-365; goto on_return; });
+        PJ_TEST_TRUE(IS_ALIGNED(ptr), NULL, { rc=-365; goto on_return; });
 
         /* Reset the pool */
         pj_pool_reset(pool);
@@ -161,8 +209,10 @@ static int pool_alignment_test(void)
 
     /* Done */
 on_return:
-    pj_pool_release(pool);
-    pj_pool_release(pool2);
+    if (pool)
+        pj_pool_release(pool);
+    if (pool2)
+        pj_pool_release(pool2);
 
     return rc;
 }