strict aliasing of sized arrays (#1086)
common BUGFIX strict aliasing of sized arrays
The sized array is commonly used for storing an array of pointers or other
structures. Each sized array is preceded by a counter providing the
size of the array. Since we used uint32_t type for the counter, pointers
were misaligned on 64b architectures.
Currently, I don't have any performance measurement, but the sized
arrays are supposed to be used quite intensively in libyang2 and the
strict aliasing could have an impact on the performance. The cost is the
increased size of each array by 4 additional bytes.
The patch changes all the macros working with the counter to use a
generic LY_ARRAY_SIZE_TYPE. For now, it is set to be uint64_t. In the
future it is possible to simply change the type according to the detected
architecture in build time if such a mechanism will make sense.
Fixes #1044
diff --git a/src/common.h b/src/common.h
index e502fcf..d007f67 100644
--- a/src/common.h
+++ b/src/common.h
@@ -28,6 +28,7 @@
#include "hash_table.h"
#include "log.h"
#include "set.h"
+#include "tree.h"
struct ly_ctx;
@@ -503,15 +504,15 @@
*/
#define LY_ARRAY_NEW_RET(CTX, ARRAY, NEW_ITEM, RETVAL) \
if (!(ARRAY)) { \
- ARRAY = malloc(sizeof(uint32_t) + sizeof *(ARRAY)); \
- *((uint32_t*)(ARRAY)) = 1; \
+ ARRAY = malloc(sizeof(LY_ARRAY_SIZE_TYPE) + sizeof *(ARRAY)); \
+ *((LY_ARRAY_SIZE_TYPE*)(ARRAY)) = 1; \
} else { \
- ++(*((uint32_t*)(ARRAY) - 1)); \
- ARRAY = ly_realloc(((uint32_t*)(ARRAY) - 1), sizeof(uint32_t) + (*((uint32_t*)(ARRAY) - 1) * sizeof *(ARRAY))); \
+ ++(*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)); \
+ ARRAY = ly_realloc(((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1), sizeof(LY_ARRAY_SIZE_TYPE) + (*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) * sizeof *(ARRAY))); \
LY_CHECK_ERR_RET(!(ARRAY), LOGMEM(CTX), RETVAL); \
} \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
- (NEW_ITEM) = &(ARRAY)[*((uint32_t*)(ARRAY) - 1) - 1]; \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
+ (NEW_ITEM) = &(ARRAY)[*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) - 1]; \
memset(NEW_ITEM, 0, sizeof *(NEW_ITEM))
/**
@@ -528,20 +529,20 @@
*/
#define LY_ARRAY_NEW_GOTO(CTX, ARRAY, NEW_ITEM, RET, GOTO) \
if (!(ARRAY)) { \
- ARRAY = malloc(sizeof(uint32_t) + sizeof *(ARRAY)); \
- *((uint32_t*)(ARRAY)) = 1; \
+ ARRAY = malloc(sizeof(LY_ARRAY_SIZE_TYPE) + sizeof *(ARRAY)); \
+ *((LY_ARRAY_SIZE_TYPE*)(ARRAY)) = 1; \
} else { \
- ++(*((uint32_t*)(ARRAY) - 1)); \
- ARRAY = ly_realloc(((uint32_t*)(ARRAY) - 1), sizeof(uint32_t) + (*((uint32_t*)(ARRAY) - 1) * sizeof *(ARRAY))); \
+ ++(*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)); \
+ ARRAY = ly_realloc(((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1), sizeof(LY_ARRAY_SIZE_TYPE) + (*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) * sizeof *(ARRAY))); \
LY_CHECK_ERR_GOTO(!(ARRAY), LOGMEM(CTX); RET = LY_EMEM, GOTO); \
} \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
- (NEW_ITEM) = &(ARRAY)[*((uint32_t*)(ARRAY) - 1) - 1]; \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
+ (NEW_ITEM) = &(ARRAY)[*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) - 1]; \
memset(NEW_ITEM, 0, sizeof *(NEW_ITEM))
/**
* @brief Allocate a ([sized array](@ref sizedarrays)) for the specified number of items.
- * If the ARRAY already exists, it is resized (space for SIZE items is added).
+ * If the ARRAY already exists, it is resized (space for SIZE items is added and zeroed).
*
* Does not set the size information, it is supposed to be incremented via ::LY_ARRAY_INCREMENT
* when the items are filled.
@@ -554,14 +555,14 @@
*/
#define LY_ARRAY_CREATE_RET(CTX, ARRAY, SIZE, RETVAL) \
if (ARRAY) { \
- ARRAY = ly_realloc(((uint32_t*)(ARRAY) - 1), sizeof(uint32_t) + ((*((uint32_t*)(ARRAY) - 1) + SIZE) * sizeof *(ARRAY))); \
+ ARRAY = ly_realloc(((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1), sizeof(LY_ARRAY_SIZE_TYPE) + ((*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) + SIZE) * sizeof *(ARRAY))); \
LY_CHECK_ERR_RET(!(ARRAY), LOGMEM(CTX), RETVAL); \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
- memset(&(ARRAY)[*((uint32_t*)(ARRAY) - 1)], 0, SIZE * sizeof *(ARRAY)); \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
+ memset(&(ARRAY)[*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)], 0, SIZE * sizeof *(ARRAY)); \
} else { \
- ARRAY = calloc(1, sizeof(uint32_t) + SIZE * sizeof *(ARRAY)); \
+ ARRAY = calloc(1, sizeof(LY_ARRAY_SIZE_TYPE) + SIZE * sizeof *(ARRAY)); \
LY_CHECK_ERR_RET(!(ARRAY), LOGMEM(CTX), RETVAL); \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
}
/**
@@ -580,21 +581,55 @@
*/
#define LY_ARRAY_CREATE_GOTO(CTX, ARRAY, SIZE, RET, GOTO) \
if (ARRAY) { \
- ARRAY = ly_realloc(((uint32_t*)(ARRAY) - 1), sizeof(uint32_t) + ((*((uint32_t*)(ARRAY) - 1) + (SIZE)) * sizeof *(ARRAY))); \
+ ARRAY = ly_realloc(((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1), sizeof(LY_ARRAY_SIZE_TYPE) + ((*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1) + (SIZE)) * sizeof *(ARRAY))); \
LY_CHECK_ERR_GOTO(!(ARRAY), LOGMEM(CTX); RET = LY_EMEM, GOTO); \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
- memset(&(ARRAY)[*((uint32_t*)(ARRAY) - 1)], 0, (SIZE) * sizeof *(ARRAY)); \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
+ memset(&(ARRAY)[*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)], 0, (SIZE) * sizeof *(ARRAY)); \
} else { \
- ARRAY = calloc(1, sizeof(uint32_t) + (SIZE) * sizeof *(ARRAY)); \
+ ARRAY = calloc(1, sizeof(LY_ARRAY_SIZE_TYPE) + (SIZE) * sizeof *(ARRAY)); \
LY_CHECK_ERR_GOTO(!(ARRAY), LOGMEM(CTX); RET = LY_EMEM, GOTO); \
- ARRAY = (void*)((uint32_t*)(ARRAY) + 1); \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1); \
}
-#define LY_ARRAY_INCREMENT(ARRAY) \
- ++(*((uint32_t*)(ARRAY) - 1))
+/**
+ * @brief Resize a ([sized array](@ref sizedarrays)) to the the specified number of items.
+ *
+ * Does not change the size information, it is supposed to be incremented via ::LY_ARRAY_INCREMENT
+ * when the items are filled.
+ *
+ * @param[in] CTX libyang context for logging.
+ * @param[in,out] ARRAY Pointer to the array to create.
+ * @param[in] SIZE Number of items the array is supposed to hold. The size of the allocated
+ * space is then counted from the type of the ARRAY, so do not provide placeholder void pointers.
+ * @param[in] ERR Additional action(s) in case of error (passed to LY_CHECK_ERR_RET).
+ * @param[in] RETVAL Return value for the case of error (memory allocation failure).
+ */
+#define LY_ARRAY_RESIZE_ERR_RET(CTX, ARRAY, SIZE, ERR, RETVAL) \
+ ARRAY = ly_realloc(((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1), sizeof(LY_ARRAY_SIZE_TYPE) + ((SIZE) * sizeof *(ARRAY))); \
+ LY_CHECK_ERR_RET(!(ARRAY), LOGMEM(CTX); ERR, RETVAL); \
+ ARRAY = (void*)((LY_ARRAY_SIZE_TYPE*)(ARRAY) + 1);
+/**
+ * @brief Increment the items counter in a ([sized array](@ref sizedarrays)).
+ *
+ * Does not change the allocated memory used by the ARRAY. To do so, use LY_ARRAY_CREATE_RET,
+ * LY_ARRAY_CREATE_GOTO or LY_ARRAY_RESIZE_ERR_RET.
+ *
+ * @param[in] ARRAY Pointer to the array to affect.
+ */
+#define LY_ARRAY_INCREMENT(ARRAY) \
+ ++(*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1))
+
+/**
+ * @brief Decrement the items counter in a ([sized array](@ref sizedarrays)).
+ *
+ * Does not change the allocated memory used by the ARRAY. To do so, use LY_ARRAY_CREATE_RET,
+ * LY_ARRAY_CREATE_GOTO or LY_ARRAY_RESIZE_ERR_RET.
+ *
+ * @param[in] ARRAY Pointer to the array to affect.
+ */
#define LY_ARRAY_DECREMENT(ARRAY) \
- --(*((uint32_t*)(ARRAY) - 1))
+ --(*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1))
/**
* @brief Free the space allocated for the ([sized array](@ref sizedarrays)).
@@ -604,7 +639,7 @@
* @param[in] ARRAY A ([sized array](@ref sizedarrays)) to be freed.
*/
#define LY_ARRAY_FREE(ARRAY) \
- if (ARRAY){free((uint32_t*)(ARRAY) - 1);}
+ if (ARRAY){free((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1);}
/**
* @brief Insert item into linked list.
diff --git a/src/tree.h b/src/tree.h
index 32526f4..f2e46ea 100644
--- a/src/tree.h
+++ b/src/tree.h
@@ -22,6 +22,11 @@
#endif
/**
+ * @brief Type (i.e. size) of the [sized array](@ref sizedarrays)'s size counter.
+ */
+#define LY_ARRAY_SIZE_TYPE uint64_t
+
+/**
* @brief Macro selector for other LY_ARRAY_* macros, do not use directly!
*/
#define LY_ARRAY_SELECT(_1, _2, NAME, ...) NAME
@@ -37,7 +42,7 @@
*/
#define LY_ARRAY_FOR_ITER(ARRAY, TYPE, ITER) \
for (ITER = ARRAY; \
- (ARRAY) && ((void*)ITER - (void*)ARRAY)/(sizeof(TYPE)) < (*((uint32_t*)(ARRAY) - 1)); \
+ (ARRAY) && ((void*)ITER - (void*)ARRAY)/(sizeof(TYPE)) < (*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)); \
ITER = (void*)((TYPE*)ITER + 1))
/**
@@ -52,7 +57,7 @@
*/
#define LY_ARRAY_FOR_INDEX(ARRAY, INDEX) \
for (INDEX = 0; \
- ARRAY && INDEX < (*((uint32_t*)(ARRAY) - 1)); \
+ ARRAY && INDEX < (*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1)); \
++INDEX)
/**
@@ -67,7 +72,7 @@
*
* Does not check if array exists!
*/
-#define LY_ARRAY_SIZE(ARRAY) (*((uint32_t*)(ARRAY) - 1))
+#define LY_ARRAY_SIZE(ARRAY) (*((LY_ARRAY_SIZE_TYPE*)(ARRAY) - 1))
/**
* @brief Sized-array iterator (for-loop).
diff --git a/src/tree_schema_compile.c b/src/tree_schema_compile.c
index f440663..7a76a17 100644
--- a/src/tree_schema_compile.c
+++ b/src/tree_schema_compile.c
@@ -3098,9 +3098,8 @@
if (un->types[u + additional]->basetype == LY_TYPE_UNION) {
/* add space for additional types from the union subtype */
un_aux = (struct lysc_type_union *)un->types[u + additional];
- p = ly_realloc(((uint32_t*)(un->types) - 1), sizeof(uint32_t) + ((LY_ARRAY_SIZE(type_p->types) + additional + LY_ARRAY_SIZE(un_aux->types) - 1) * sizeof *(un->types)));
- LY_CHECK_ERR_RET(!p, LOGMEM(ctx->ctx);lysc_type_free(ctx->ctx, (struct lysc_type*)un_aux), LY_EMEM);
- un->types = (void*)((uint32_t*)(p) + 1);
+ LY_ARRAY_RESIZE_ERR_RET(ctx->ctx, un->types, (*((uint64_t*)(type_p->types) - 1)) + additional + LY_ARRAY_SIZE(un_aux->types) - 1,
+ lysc_type_free(ctx->ctx, (struct lysc_type*)un_aux), LY_EMEM);
/* copy subtypes of the subtype union */
for (v = 0; v < LY_ARRAY_SIZE(un_aux->types); ++v) {