Ensure that string field lengths are properly aligned

Integers should always be aligned. For some platforms (ARM, SPARC) this
is more important than for others. This changeset ensures that the
string field string lengths are aligned on *all* platforms, not just on
the SPARC for which there was a workaround. It also fixes that the
length integer can be resized to 32 bits without problems if needed.

(closes issue ASTERISK-17310)
Reported by: radael, S Adrian
Reviewed by: Tzafrir Cohen, Terry Wilson
Tested by: S Adrian

Review: https://reviewboard.asterisk.org/r/1549


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@343157 65c4cc65-6c06-0410-ace0-fbb531ad65f3

include/asterisk/stringfields.h

@@ -114,6 +114,10 @@
 */
 typedef const char * ast_string_field;
 
+/* the type of storage used to track how many bytes were allocated for a field */
+
+typedef uint16_t ast_string_field_allocation;
+
 /*!
   \internal
   \brief A constant empty string used for fields that have no other value
@@ -123,13 +127,15 @@ extern const char *__ast_string_field_empty;
 /*!
   \internal
   \brief Structure used to hold a pool of space for string fields
+  \note base is aligned so base+used can stay aligned by incrementing used with
+        aligned numbers only
 */
 struct ast_string_field_pool {
 	struct ast_string_field_pool *prev;	/*!< pointer to the previous pool, if any */
 	size_t size;				/*!< the total size of the pool */
 	size_t used;				/*!< the space used in the pool */
 	size_t active;				/*!< the amount of space actively in use by fields */
-	char base[0];				/*!< storage space for the fields */
+	char base[0] __attribute__((aligned(sizeof(ast_string_field_allocation)))); /*!< storage space for the fields */
 };
 
 /*!
@@ -293,10 +299,6 @@ void * attribute_malloc __ast_calloc_with_stringfields(unsigned int num_structs,
 void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
 				       const ast_string_field ptr);
 
-/* the type of storage used to track how many bytes were allocated for a field */
-
-typedef uint16_t ast_string_field_allocation;
-
 /*!
   \brief Macro to provide access to the allocation field that lives immediately in front of a string field
   \param x Pointer to the string field

include/asterisk/utils.h

@@ -725,6 +725,38 @@ static void force_inline _ast_assert(int condition, const char *condition_str,
 
 #include "asterisk/strings.h"
 
+/*!
+ * \brief Add space and let result be a multiple of space.
+ * \param initial A number to add space to.
+ * \param space The space to add, this would typically be sizeof(sometype).
+ * \return The sum of initial plus space plus at most space-1.
+ *
+ * Many systems prefer integers to be stored on aligned on memory locations.
+ * A use case for this is when prepending length fields of type int to a buffer.
+ * If you keep the total used bytes a multiple of the size of the integer type,
+ * a next block of length+buffer will have the length field automatically
+ * aligned.
+ *
+ * It looks kind of ugly, but the compiler will optimize this down to 4 or 5
+ * inexpensive instructions (on x86_64).
+ *
+ * Examples:
+ * ast_add_and_make_multiple_of(0x18, sizeof(int64_t)) ==> 0x20
+ * ast_add_and_make_multiple_of(0x19, sizeof(int64_t)) ==> 0x28
+ */
+#define ast_add_and_make_multiple_of(initial, space) (((initial + (2 * space - 1)) / space) * space)
+
+/*!
+ * \brief Add bytes so that result is a multiple of size.
+ * \param initial A number to enlarge.
+ * \param size The block size the number should be a multiple of.
+ * \return The sum of initial plus at most size-1.
+ *
+ * Similar ast_add_and_make_multiple_of, except that this doesn't add the room
+ * for the length object, it only ensures that the total is aligned.
+ */
+#define ast_make_multiple_of(initial, size) (((initial + size - 1) / size) * size)
+
 /*!
  * \brief An Entity ID is essentially a MAC address, brief and unique 
  */

main/utils.c

@@ -1646,10 +1646,13 @@ ast_string_field __ast_string_field_alloc_space(struct ast_string_field_mgr *mgr
 {
 	char *result = NULL;
 	size_t space = (*pool_head)->size - (*pool_head)->used;
-	size_t to_alloc = needed + sizeof(ast_string_field_allocation);
+	size_t to_alloc;
 
-	/* This +1 accounts for alignment on SPARC */
+	/* Make room for ast_string_field_allocation and make it a multiple of that. */
-	if (__builtin_expect(to_alloc + 1 > space, 0)) {
+	to_alloc = ast_add_and_make_multiple_of(needed, sizeof(ast_string_field_allocation));
+	ast_assert(to_alloc % sizeof(ast_string_field_allocation) == 0);
+
+	if (__builtin_expect(to_alloc > space, 0)) {
 		size_t new_size = (*pool_head)->size;
 
 		while (new_size < to_alloc) {
@@ -1665,14 +1668,11 @@ ast_string_field __ast_string_field_alloc_space(struct ast_string_field_mgr *mgr
 #endif
 	}
 
+	/* pool->base is always aligned (gcc aligned attribute). We ensure that
+	 * to_alloc is also a multiple of sizeof(ast_string_field_allocation)
+	 * causing result to always be aligned as well; which in turn fixes that
+	 * AST_STRING_FIELD_ALLOCATION(result) is aligned. */
 	result = (*pool_head)->base + (*pool_head)->used;
-#ifdef __sparc__
-	/* SPARC requires that the allocation field be aligned. */
-	if ((long) result % sizeof(ast_string_field_allocation)) {
-		result++;
-		(*pool_head)->used++;
-	}
-#endif
 	(*pool_head)->used += to_alloc;
 	(*pool_head)->active += needed;
 	result += sizeof(ast_string_field_allocation);
@@ -1746,13 +1746,10 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr,
 			available += space;
 		}
 	} else {
+		/* pool->used is always a multiple of sizeof(ast_string_field_allocation)
+		 * so we don't need to re-align anything here.
+		 */
 		target = (*pool_head)->base + (*pool_head)->used + sizeof(ast_string_field_allocation);
-#ifdef __sparc__
-		if ((long) target % sizeof(ast_string_field_allocation)) {
-			target++;
-			space--;
-		}
-#endif
 		available = space - sizeof(ast_string_field_allocation);
 	}
 
@@ -1778,15 +1775,15 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr,
 		__ast_string_field_release_active(*pool_head, *ptr);
 		mgr->last_alloc = *ptr = target;
 		AST_STRING_FIELD_ALLOCATION(target) = needed;
-		(*pool_head)->used += needed + sizeof(ast_string_field_allocation);
+		(*pool_head)->used += ast_add_and_make_multiple_of(needed, sizeof(ast_string_field_allocation));
 		(*pool_head)->active += needed;
 	} else if ((grow = (needed - AST_STRING_FIELD_ALLOCATION(*ptr))) > 0) {
 		/* the allocation was satisfied by using available space in the pool *and*
 		   the field was the last allocated field from the pool, so it grew
 		*/
-		(*pool_head)->used += grow;
-		(*pool_head)->active += grow;
 		AST_STRING_FIELD_ALLOCATION(*ptr) += grow;
+		(*pool_head)->used += ast_make_multiple_of(grow, sizeof(ast_string_field_allocation));
+		(*pool_head)->active += grow;
 	}
 }