Fix ems allocator unaligned memory access on riscv64 (#2140)

Make `hmu_tree_node` struct packed and add 4 padding bytes before `kfc_tree_root_buf`
field in `gc_heap_struct` struct to ensure the `left/right/parent` fields in `hmu_tree_node`
are 8-byte aligned on the 64-bit target which doesn't support unaligned memory access.

Fix the issue reported in #2136.
This commit is contained in:
Wenyong Huang 2023-04-26 18:13:11 +08:00 committed by GitHub
parent 15139d2bb8
commit 5c497e5a14
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 20 deletions

View File

@ -25,7 +25,7 @@ static bool
remove_tree_node(gc_heap_t *heap, hmu_tree_node_t *p)
{
hmu_tree_node_t *q = NULL, **slot = NULL, *parent;
hmu_tree_node_t *root = &heap->kfc_tree_root;
hmu_tree_node_t *root = heap->kfc_tree_root;
gc_uint8 *base_addr = heap->base_addr;
gc_uint8 *end_addr = base_addr + heap->current_size;
@ -38,13 +38,17 @@ remove_tree_node(gc_heap_t *heap, hmu_tree_node_t *p)
goto fail;
}
/* get the slot which holds pointer to node p*/
/* get the slot which holds pointer to node p */
if (p == p->parent->right) {
slot = &p->parent->right;
/* Don't use `slot = &p->parent->right` to avoid compiler warning */
slot = (hmu_tree_node_t **)((uint8 *)p->parent
+ offsetof(hmu_tree_node_t, right));
}
else if (p == p->parent->left) {
/* p should be a child of its parent*/
slot = &p->parent->left;
/* p should be a child of its parent */
/* Don't use `slot = &p->parent->left` to avoid compiler warning */
slot = (hmu_tree_node_t **)((uint8 *)p->parent
+ offsetof(hmu_tree_node_t, left));
}
else {
goto fail;
@ -241,7 +245,7 @@ gci_add_fc(gc_heap_t *heap, hmu_t *hmu, gc_size_t size)
node->left = node->right = node->parent = NULL;
/* find proper node to link this new node to */
root = &heap->kfc_tree_root;
root = heap->kfc_tree_root;
tp = root;
bh_assert(tp->size < size);
while (1) {
@ -289,6 +293,7 @@ alloc_hmu(gc_heap_t *heap, gc_size_t size)
uint32 node_idx = 0, init_node_idx = 0;
hmu_tree_node_t *root = NULL, *tp = NULL, *last_tp = NULL;
hmu_t *next, *rest;
uintptr_t tp_ret;
bh_assert(gci_is_heap_valid(heap));
bh_assert(size > 0 && !(size & 7));
@ -354,7 +359,7 @@ alloc_hmu(gc_heap_t *heap, gc_size_t size)
}
/* need to find a node in tree*/
root = &heap->kfc_tree_root;
root = heap->kfc_tree_root;
/* find the best node*/
bh_assert(root);
@ -402,7 +407,8 @@ alloc_hmu(gc_heap_t *heap, gc_size_t size)
heap->highmark_size = heap->current_size - heap->total_free_size;
hmu_set_size((hmu_t *)last_tp, size);
return (hmu_t *)last_tp;
tp_ret = (uintptr_t)last_tp;
return (hmu_t *)tp_ret;
}
return NULL;

View File

@ -204,13 +204,47 @@ set_hmu_normal_node_next(hmu_normal_node_t *node, hmu_normal_node_t *next)
}
}
/**
* Define hmu_tree_node as a packed struct, since it is at the 4-byte
* aligned address and the size of hmu_head is 4, so in 64-bit target,
* the left/right/parent fields will be at 8-byte aligned address,
* we can access them directly.
*/
#if UINTPTR_MAX == UINT64_MAX
#if defined(_MSC_VER)
__pragma(pack(push, 1));
#define __attr_packed
#elif defined(__GNUC__) || defined(__clang__)
#define __attr_packed __attribute__((packed))
#else
#error "packed attribute isn't used to define struct hmu_tree_node"
#endif
#else /* else of UINTPTR_MAX == UINT64_MAX */
#define __attr_packed
#endif
typedef struct hmu_tree_node {
hmu_t hmu_header;
gc_size_t size;
struct hmu_tree_node *left;
struct hmu_tree_node *right;
struct hmu_tree_node *parent;
} hmu_tree_node_t;
gc_size_t size;
} __attr_packed hmu_tree_node_t;
#if UINTPTR_MAX == UINT64_MAX
#if defined(_MSC_VER)
__pragma(pack(pop));
#endif
#endif
bh_static_assert(sizeof(hmu_tree_node_t) == 8 + 3 * sizeof(void *));
bh_static_assert(offsetof(hmu_tree_node_t, left) == 4);
#define ASSERT_TREE_NODE_ALIGNED_ACCESS(tree_node) \
do { \
bh_assert((((uintptr_t)&tree_node->left) & (sizeof(uintptr_t) - 1)) \
== 0); \
} while (0)
typedef struct gc_heap_struct {
/* for double checking*/
@ -223,8 +257,16 @@ typedef struct gc_heap_struct {
hmu_normal_list_t kfc_normal_list[HMU_NORMAL_NODE_CNT];
/* order in kfc_tree is: size[left] <= size[cur] < size[right]*/
hmu_tree_node_t kfc_tree_root;
#if UINTPTR_MAX == UINT64_MAX
/* make kfc_tree_root_buf 4-byte aligned and not 8-byte aligned,
so kfc_tree_root's left/right/parent fields are 8-byte aligned
and we can access them directly */
uint32 __padding;
#endif
uint8 kfc_tree_root_buf[sizeof(hmu_tree_node_t)];
/* point to kfc_tree_root_buf, the order in kfc_tree is:
size[left] <= size[cur] < size[right] */
hmu_tree_node_t *kfc_tree_root;
/* whether heap is corrupted, e.g. the hmu nodes are modified
by user */

View File

@ -27,7 +27,7 @@ gc_init_internal(gc_heap_t *heap, char *base_addr, gc_size_t heap_max_size)
heap->total_free_size = heap->current_size;
heap->highmark_size = 0;
root = &heap->kfc_tree_root;
root = heap->kfc_tree_root = (hmu_tree_node_t *)heap->kfc_tree_root_buf;
memset(root, 0, sizeof *root);
root->size = sizeof *root;
hmu_set_ut(&root->hmu_header, HMU_FC);
@ -38,6 +38,9 @@ gc_init_internal(gc_heap_t *heap, char *base_addr, gc_size_t heap_max_size)
hmu_set_ut(&q->hmu_header, HMU_FC);
hmu_set_size(&q->hmu_header, heap->current_size);
ASSERT_TREE_NODE_ALIGNED_ACCESS(q);
ASSERT_TREE_NODE_ALIGNED_ACCESS(root);
hmu_mark_pinuse(&q->hmu_header);
root->right = q;
q->parent = root;
@ -165,6 +168,7 @@ gc_migrate(gc_handle_t handle, char *pool_buf_new, gc_size_t pool_buf_size)
intptr_t offset = (uint8 *)base_addr_new - (uint8 *)heap->base_addr;
hmu_t *cur = NULL, *end = NULL;
hmu_tree_node_t *tree_node;
uint8 **p_left, **p_right, **p_parent;
gc_size_t heap_max_size, size;
if ((((uintptr_t)pool_buf_new) & 7) != 0) {
@ -188,9 +192,18 @@ gc_migrate(gc_handle_t handle, char *pool_buf_new, gc_size_t pool_buf_size)
}
heap->base_addr = (uint8 *)base_addr_new;
adjust_ptr((uint8 **)&heap->kfc_tree_root.left, offset);
adjust_ptr((uint8 **)&heap->kfc_tree_root.right, offset);
adjust_ptr((uint8 **)&heap->kfc_tree_root.parent, offset);
ASSERT_TREE_NODE_ALIGNED_ACCESS(heap->kfc_tree_root);
p_left = (uint8 **)((uint8 *)heap->kfc_tree_root
+ offsetof(hmu_tree_node_t, left));
p_right = (uint8 **)((uint8 *)heap->kfc_tree_root
+ offsetof(hmu_tree_node_t, right));
p_parent = (uint8 **)((uint8 *)heap->kfc_tree_root
+ offsetof(hmu_tree_node_t, parent));
adjust_ptr(p_left, offset);
adjust_ptr(p_right, offset);
adjust_ptr(p_parent, offset);
cur = (hmu_t *)heap->base_addr;
end = (hmu_t *)((char *)heap->base_addr + heap->current_size);
@ -206,12 +219,21 @@ gc_migrate(gc_handle_t handle, char *pool_buf_new, gc_size_t pool_buf_size)
if (hmu_get_ut(cur) == HMU_FC && !HMU_IS_FC_NORMAL(size)) {
tree_node = (hmu_tree_node_t *)cur;
adjust_ptr((uint8 **)&tree_node->left, offset);
adjust_ptr((uint8 **)&tree_node->right, offset);
if (tree_node->parent != &heap->kfc_tree_root)
ASSERT_TREE_NODE_ALIGNED_ACCESS(tree_node);
p_left = (uint8 **)((uint8 *)tree_node
+ offsetof(hmu_tree_node_t, left));
p_right = (uint8 **)((uint8 *)tree_node
+ offsetof(hmu_tree_node_t, right));
p_parent = (uint8 **)((uint8 *)tree_node
+ offsetof(hmu_tree_node_t, parent));
adjust_ptr(p_left, offset);
adjust_ptr(p_right, offset);
if (tree_node->parent != heap->kfc_tree_root)
/* The root node belongs to heap structure,
it is fixed part and isn't changed. */
adjust_ptr((uint8 **)&tree_node->parent, offset);
adjust_ptr(p_parent, offset);
}
cur = (hmu_t *)((char *)cur + size);
}