From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@dev.tarantool.org, alyapunov@tarantool.org, korablev@tarantool.org, tsafin@tarantool.org Subject: [Tarantool-patches] [PATCH v2 06/10] region: use aligned allocations where necessary Date: Thu, 28 May 2020 01:32:25 +0200 [thread overview] Message-ID: <31172635747a4f2d20c37525379285aa5ba69ab2.1590622225.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1590622225.git.v.shpilevoy@tarantool.org> Region is used for temporary allocations, usually bound to a transaction, but not always. Keyword - temporary. It is usually much faster to allocate something on the region than on the heap, and much much faster to free the region, since it frees data in whole slabs. Lots of objects at once. Region is used both for byte arrays (MessagePack, strings, blobs), and for objects like standard types, structs. Almost always for allocations was used region_alloc() function, which returns a not aligned address. It can be anything, even not multiple of 2. That led to alignment violation for standard types and structs. Usually it is harmless in terms of errors, but can be slower than aligned access, and on some platforms may even crash. Also the crash can be forced using clang undefined behaviour sanitizer, which revealed all the not aligned accesses fixed in this patch. Part of #4609 --- src/box/alter.cc | 39 ++++++++++++++---------- src/box/applier.cc | 22 +++++++------- src/box/bind.c | 7 +++-- src/box/ck_constraint.c | 11 +++---- src/box/field_map.c | 17 ++++++----- src/box/fk_constraint.h | 14 +++++---- src/box/index_def.c | 9 +++--- src/box/key_def.c | 9 +++--- src/box/lua/execute.c | 7 +++-- src/box/lua/key_def.c | 7 +++-- src/box/lua/misc.cc | 8 ++--- src/box/memtx_tree.c | 7 +++-- src/box/space_def.c | 7 +++-- src/box/sql.c | 20 ++++++++----- src/box/sql/build.c | 60 ++++++++++++++++++++++--------------- src/box/sql/func.c | 7 +++-- src/box/sql/select.c | 14 +++++---- src/box/sql/update.c | 6 ++-- src/box/sql/vdbe.c | 8 ++--- src/box/sql/wherecode.c | 9 ++++-- src/box/tuple_format.c | 3 +- src/box/txn.c | 12 ++++---- src/box/user.cc | 8 ++--- src/box/vinyl.c | 26 +++++++++------- src/box/vy_log.c | 33 ++++++++++---------- src/box/vy_point_lookup.c | 9 +++--- src/box/xrow_update_map.c | 7 +++-- src/box/xrow_update_route.c | 7 +++-- src/lib/core/backtrace.cc | 6 ++-- src/lua/popen.c | 20 +++++++------ 30 files changed, 240 insertions(+), 179 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index dbbbcbc44..bb4254878 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -537,11 +537,13 @@ space_format_decode(const char *data, uint32_t *out_count, *fields = NULL; return 0; } - size_t size = count * sizeof(struct field_def); + size_t size; struct field_def *region_defs = - (struct field_def *) region_alloc(region, size); + region_alloc_array(region, typeof(region_defs[0]), count, + &size); if (region_defs == NULL) { - diag_set(OutOfMemory, size, "region", "struct field_def"); + diag_set(OutOfMemory, size, "region_alloc_array", + "region_defs"); return -1; } /* @@ -2452,10 +2454,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) * comparators must be updated. */ struct key_def **keys; - size_t bsize = old_space->index_count * sizeof(keys[0]); - keys = (struct key_def **) region_alloc(&fiber()->gc, bsize); + size_t bsize; + keys = region_alloc_array(&fiber()->gc, typeof(keys[0]), + old_space->index_count, &bsize); if (keys == NULL) { - diag_set(OutOfMemory, bsize, "region", "new slab"); + diag_set(OutOfMemory, bsize, "region_alloc_array", + "keys"); return -1; } for (uint32_t i = 0; i < old_space->index_count; ++i) @@ -2733,10 +2737,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * index and a space format, defined by a user. */ struct key_def **keys; - size_t bsize = old_space->index_count * sizeof(keys[0]); - keys = (struct key_def **) region_alloc(&fiber()->gc, bsize); + size_t bsize; + keys = region_alloc_array(&fiber()->gc, typeof(keys[0]), + old_space->index_count, &bsize); if (keys == NULL) { - diag_set(OutOfMemory, bsize, "region", "new slab"); + diag_set(OutOfMemory, bsize, "region_alloc_array", + "keys"); return -1; } for (uint32_t i = 0, j = 0; i < old_space->index_count; ++i) { @@ -5009,11 +5015,13 @@ decode_fk_links(struct tuple *tuple, uint32_t *out_count, return NULL; } *out_count = count; - size_t size = count * sizeof(struct field_link); + size_t size; struct field_link *region_links = - (struct field_link *)region_alloc(&fiber()->gc, size); + region_alloc_array(&fiber()->gc, typeof(region_links[0]), count, + &size); if (region_links == NULL) { - diag_set(OutOfMemory, size, "region", "struct field_link"); + diag_set(OutOfMemory, size, "region_alloc_array", + "region_links"); return NULL; } memset(region_links, 0, size); @@ -5054,7 +5062,9 @@ fk_constraint_def_new_from_tuple(struct tuple *tuple, uint32_t errcode) name_len, errcode); if (links == NULL) return NULL; - size_t fk_def_sz = fk_constraint_def_sizeof(link_count, name_len); + uint32_t links_offset; + size_t fk_def_sz = fk_constraint_def_sizeof(link_count, name_len, + &links_offset); struct fk_constraint_def *fk_def = (struct fk_constraint_def *) malloc(fk_def_sz); if (fk_def == NULL) { @@ -5065,8 +5075,7 @@ fk_constraint_def_new_from_tuple(struct tuple *tuple, uint32_t errcode) auto def_guard = make_scoped_guard([=] { free(fk_def); }); memcpy(fk_def->name, name, name_len); fk_def->name[name_len] = '\0'; - fk_def->links = (struct field_link *)((char *)&fk_def->name + - name_len + 1); + fk_def->links = (struct field_link *)((char *)fk_def + links_offset); memcpy(fk_def->links, links, link_count * sizeof(struct field_link)); fk_def->field_count = link_count; if (tuple_field_u32(tuple, BOX_FK_CONSTRAINT_FIELD_CHILD_ID, diff --git a/src/box/applier.cc b/src/box/applier.cc index c6deeff1b..df48b4796 100644 --- a/src/box/applier.cc +++ b/src/box/applier.cc @@ -588,13 +588,12 @@ applier_read_tx_row(struct applier *applier) { struct ev_io *coio = &applier->io; struct ibuf *ibuf = &applier->ibuf; - - struct applier_tx_row *tx_row = (struct applier_tx_row *) - region_alloc(&fiber()->gc, sizeof(struct applier_tx_row)); + size_t size; + struct applier_tx_row *tx_row = + region_alloc_object(&fiber()->gc, typeof(*tx_row), &size); if (tx_row == NULL) - tnt_raise(OutOfMemory, sizeof(struct applier_tx_row), - "region", "struct applier_tx_row"); + tnt_raise(OutOfMemory, size, "region_alloc_object", "tx_row"); struct xrow_header *row = &tx_row->row; @@ -809,13 +808,14 @@ applier_apply_tx(struct stailq *rows) /* We are ready to submit txn to wal. */ struct trigger *on_rollback, *on_commit; - on_rollback = (struct trigger *)region_alloc(&txn->region, - sizeof(struct trigger)); - on_commit = (struct trigger *)region_alloc(&txn->region, - sizeof(struct trigger)); + size_t size; + on_rollback = region_alloc_object(&txn->region, typeof(*on_rollback), + &size); + on_commit = region_alloc_object(&txn->region, typeof(*on_commit), + &size); if (on_rollback == NULL || on_commit == NULL) { - diag_set(OutOfMemory, sizeof(struct trigger), - "region_alloc", "on_rollback/on_commit"); + diag_set(OutOfMemory, size, "region_alloc_object", + "on_rollback/on_commit"); goto rollback; } diff --git a/src/box/bind.c b/src/box/bind.c index bbc1f56df..d45a0f9a7 100644 --- a/src/box/bind.c +++ b/src/box/bind.c @@ -137,10 +137,11 @@ sql_bind_list_decode(const char *data, struct sql_bind **out_bind) } struct region *region = &fiber()->gc; uint32_t used = region_used(region); - size_t size = sizeof(struct sql_bind) * bind_count; - struct sql_bind *bind = (struct sql_bind *) region_alloc(region, size); + size_t size; + struct sql_bind *bind = region_alloc_array(region, typeof(bind[0]), + bind_count, &size); if (bind == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "struct sql_bind"); + diag_set(OutOfMemory, size, "region_alloc_array", "bind"); return -1; } for (uint32_t i = 0; i < bind_count; ++i) { diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c index ff3f05587..b629a73eb 100644 --- a/src/box/ck_constraint.c +++ b/src/box/ck_constraint.c @@ -190,12 +190,13 @@ ck_constraint_on_replace_trigger(struct trigger *trigger, void *event) struct space *space = stmt->space; assert(space != NULL); - uint32_t field_ref_sz = sizeof(struct vdbe_field_ref) + - sizeof(uint32_t) * space->def->field_count; - struct vdbe_field_ref *field_ref = - region_alloc(&fiber()->gc, field_ref_sz); + struct vdbe_field_ref *field_ref; + size_t size = sizeof(field_ref->slots[0]) * space->def->field_count + + sizeof(*field_ref); + field_ref = (struct vdbe_field_ref *) + region_aligned_alloc(&fiber()->gc, size, alignof(*field_ref)); if (field_ref == NULL) { - diag_set(OutOfMemory, field_ref_sz, "region_alloc", + diag_set(OutOfMemory, size, "region_aligned_alloc", "field_ref"); return -1; } diff --git a/src/box/field_map.c b/src/box/field_map.c index 1876bdd95..5f4661941 100644 --- a/src/box/field_map.c +++ b/src/box/field_map.c @@ -43,10 +43,12 @@ field_map_builder_create(struct field_map_builder *builder, builder->slots = NULL; return 0; } - uint32_t sz = builder->slot_count * sizeof(builder->slots[0]); - builder->slots = region_alloc(region, sz); + uint32_t sz; + builder->slots = region_alloc_array(region, typeof(builder->slots[0]), + builder->slot_count, &sz); if (builder->slots == NULL) { - diag_set(OutOfMemory, sz, "region_alloc", "field_map"); + diag_set(OutOfMemory, sz, "region_alloc_array", + "builder->slots"); return -1; } memset((char *)builder->slots, 0, sz); @@ -61,11 +63,12 @@ field_map_builder_slot_extent_new(struct field_map_builder *builder, { struct field_map_builder_slot_extent *extent; assert(!builder->slots[offset_slot].has_extent); - uint32_t sz = sizeof(struct field_map_builder_slot_extent) + - multikey_count * sizeof(uint32_t); - extent = region_alloc(region, sz); + uint32_t sz = sizeof(*extent) + + multikey_count * sizeof(extent->offset[0]); + extent = (struct field_map_builder_slot_extent *) + region_aligned_alloc(region, sz, alignof(*extent)); if (extent == NULL) { - diag_set(OutOfMemory, sz, "region", "extent"); + diag_set(OutOfMemory, sz, "region_aligned_alloc", "extent"); return NULL; } memset(extent, 0, sz); diff --git a/src/box/fk_constraint.h b/src/box/fk_constraint.h index b1e0cfb84..dcc5363c0 100644 --- a/src/box/fk_constraint.h +++ b/src/box/fk_constraint.h @@ -32,7 +32,8 @@ */ #include <stdbool.h> #include <stdint.h> - +#include "trivia/util.h" +#include "small/slab_arena.h" #include "small/rlist.h" #if defined(__cplusplus) @@ -125,15 +126,18 @@ struct fk_constraint { * |----------------------------------| * | name + \0 | * |----------------------------------| + * | memory align padding | + * |----------------------------------| * | links | * +----------------------------------+ */ static inline size_t -fk_constraint_def_sizeof(uint32_t link_count, uint32_t name_len) +fk_constraint_def_sizeof(uint32_t link_count, uint32_t name_len, + uint32_t *links_offset) { - return sizeof(struct fk_constraint_def) + - link_count * sizeof(struct field_link) + - name_len + 1; + *links_offset = small_align(sizeof(struct fk_constraint_def) + + name_len + 1, alignof(struct field_link)); + return *links_offset + link_count * sizeof(struct field_link); } static inline bool diff --git a/src/box/index_def.c b/src/box/index_def.c index 85128b1a5..98029612c 100644 --- a/src/box/index_def.c +++ b/src/box/index_def.c @@ -255,11 +255,12 @@ index_def_to_key_def(struct rlist *index_defs, int *size) struct index_def *index_def; rlist_foreach_entry(index_def, index_defs, link) key_count++; - size_t sz = sizeof(struct key_def *) * key_count; - struct key_def **keys = (struct key_def **) region_alloc(&fiber()->gc, - sz); + size_t bsize; + struct key_def **keys = + region_alloc_array(&fiber()->gc, typeof(keys[0]), key_count, + &bsize); if (keys == NULL) { - diag_set(OutOfMemory, sz, "region_alloc", "keys"); + diag_set(OutOfMemory, bsize, "region_alloc_array", "keys"); return NULL; } *size = key_count; diff --git a/src/box/key_def.c b/src/box/key_def.c index 3e3782163..18af44961 100644 --- a/src/box/key_def.c +++ b/src/box/key_def.c @@ -847,11 +847,12 @@ key_def_find_pk_in_cmp_def(const struct key_def *cmp_def, size_t region_svp = region_used(region); /* First, dump primary key parts as is. */ - struct key_part_def *parts = region_alloc(region, - pk_def->part_count * sizeof(*parts)); + size_t size; + struct key_part_def *parts = + region_alloc_array(region, typeof(parts[0]), pk_def->part_count, + &size); if (parts == NULL) { - diag_set(OutOfMemory, pk_def->part_count * sizeof(*parts), - "region", "key def parts"); + diag_set(OutOfMemory, size, "region_alloc_array", "parts"); goto out; } if (key_def_dump_parts(pk_def, parts, region) != 0) diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c index b4c464af7..926a0a61c 100644 --- a/src/box/lua/execute.c +++ b/src/box/lua/execute.c @@ -404,15 +404,16 @@ lua_sql_bind_list_decode(struct lua_State *L, struct sql_bind **out_bind, } struct region *region = &fiber()->gc; uint32_t used = region_used(region); - size_t size = sizeof(struct sql_bind) * bind_count; + size_t size; /* * Memory allocated here will be freed in * sql_stmt_finalize() or in txn_commit()/txn_rollback() if * there is an active transaction. */ - struct sql_bind *bind = (struct sql_bind *) region_alloc(region, size); + struct sql_bind *bind = region_alloc_array(region, typeof(bind[0]), + bind_count, &size); if (bind == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "bind"); + diag_set(OutOfMemory, size, "region_alloc_array", "bind"); return -1; } for (uint32_t i = 0; i < bind_count; ++i) { diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index d8f96162d..1a99fab63 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -438,13 +438,14 @@ lbox_key_def_new(struct lua_State *L) "[, collation = <string>]}, ...}"); uint32_t part_count = lua_objlen(L, 1); - const ssize_t parts_size = sizeof(struct key_part_def) * part_count; struct region *region = &fiber()->gc; size_t region_svp = region_used(region); - struct key_part_def *parts = region_alloc(region, parts_size); + size_t size; + struct key_part_def *parts = + region_alloc_array(region, typeof(parts[0]), part_count, &size); if (parts == NULL) { - diag_set(OutOfMemory, parts_size, "region", "parts"); + diag_set(OutOfMemory, size, "region_alloc_array", "parts"); return luaT_error(L); } if (part_count == 0) { diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc index ae8fbb682..5da84b35a 100644 --- a/src/box/lua/misc.cc +++ b/src/box/lua/misc.cc @@ -184,13 +184,13 @@ lbox_tuple_format_new(struct lua_State *L) uint32_t count = lua_objlen(L, 1); if (count == 0) return lbox_push_tuple_format(L, tuple_format_runtime); - size_t size = count * sizeof(struct field_def); + size_t size; struct region *region = &fiber()->gc; size_t region_svp = region_used(region); - struct field_def *fields = - (struct field_def *)region_alloc(region, size); + struct field_def *fields = region_alloc_array(region, typeof(fields[0]), + count, &size); if (fields == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "fields"); + diag_set(OutOfMemory, size, "region_alloc_array", "fields"); return luaT_error(L); } for (uint32_t i = 0; i < count; ++i) { diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c index e155ecd65..76ff3fcd7 100644 --- a/src/box/memtx_tree.c +++ b/src/box/memtx_tree.c @@ -804,10 +804,11 @@ struct func_key_undo { struct func_key_undo * func_key_undo_new(struct region *region) { - struct func_key_undo *undo = - (struct func_key_undo *)region_alloc(region, sizeof(*undo)); + size_t size; + struct func_key_undo *undo = region_alloc_object(region, typeof(*undo), + &size); if (undo == NULL) { - diag_set(OutOfMemory, sizeof(*undo), "region", "undo"); + diag_set(OutOfMemory, size, "region_alloc_object", "undo"); return NULL; } return undo; diff --git a/src/box/space_def.c b/src/box/space_def.c index 0ff51b9a7..efb1c8ee9 100644 --- a/src/box/space_def.c +++ b/src/box/space_def.c @@ -71,10 +71,11 @@ space_def_sizeof(uint32_t name_len, const struct field_def *fields, } } } - - *fields_offset = sizeof(struct space_def) + name_len + 1; + *fields_offset = small_align(sizeof(struct space_def) + name_len + 1, + alignof(typeof(fields[0]))); *names_offset = *fields_offset + field_count * sizeof(struct field_def); - *def_expr_offset = *names_offset + field_strs_size; + *def_expr_offset = small_align(*names_offset + field_strs_size, + alignof(uint64_t)); return *def_expr_offset + def_exprs_size; } diff --git a/src/box/sql.c b/src/box/sql.c index fc1386f52..555fcfd1f 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -341,13 +341,17 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) uint32_t name_len = strlen("_COLUMN_") + 11; uint32_t size = field_count * (sizeof(struct field_def) + name_len) + part_count * sizeof(struct key_part_def); - struct field_def *fields = region_alloc(region, size); + struct field_def *fields = region_aligned_alloc(region, size, + alignof(fields[0])); if (fields == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "fields"); + diag_set(OutOfMemory, size, "region_aligned_alloc", "fields"); return NULL; } struct key_part_def *ephemer_key_parts = (void *)fields + field_count * sizeof(struct field_def); + static_assert(alignof(*fields) == alignof(*ephemer_key_parts), + "allocated in one block, and should have the same " + "alignment"); char *names = (char *)ephemer_key_parts + part_count * sizeof(struct key_part_def); for (uint32_t i = 0; i < field_count; ++i) { @@ -1234,9 +1238,10 @@ sql_ephemeral_space_def_new(struct Parse *parser, const char *name) uint32_t dummy; size_t size = space_def_sizeof(name_len, NULL, 0, &dummy, &dummy, &dummy); - def = (struct space_def *)region_alloc(&parser->region, size); + def = (struct space_def *)region_aligned_alloc(&parser->region, size, + alignof(*def)); if (def == NULL) { - diag_set(OutOfMemory, size, "region_alloc", + diag_set(OutOfMemory, size, "region_aligned_alloc", "sql_ephemeral_space_def_new"); parser->is_aborted = true; return NULL; @@ -1252,10 +1257,11 @@ sql_ephemeral_space_def_new(struct Parse *parser, const char *name) struct space * sql_ephemeral_space_new(Parse *parser, const char *name) { - size_t sz = sizeof(struct space); - struct space *space = (struct space *) region_alloc(&parser->region, sz); + size_t sz; + struct space *space = region_alloc_object(&parser->region, + typeof(*space), &sz); if (space == NULL) { - diag_set(OutOfMemory, sz, "region", "space"); + diag_set(OutOfMemory, sz, "region_alloc_object", "space"); parser->is_aborted = true; return NULL; } diff --git a/src/box/sql/build.c b/src/box/sql/build.c index b1f9fedb0..0b60d2ee7 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -260,12 +260,12 @@ sql_field_retrieve(Parse *parser, struct space_def *space_def, uint32_t id) uint32_t columns_new = space_def->exact_field_count; columns_new = (columns_new > 0) ? 2 * columns_new : 1; struct region *region = &parser->region; - field = region_alloc(region, columns_new * - sizeof(space_def->fields[0])); + size_t size; + field = region_alloc_array(region, typeof(field[0]), + columns_new, &size); if (field == NULL) { - diag_set(OutOfMemory, columns_new * - sizeof(space_def->fields[0]), - "region_alloc", "sql_field_retrieve"); + diag_set(OutOfMemory, size, "region_alloc_array", + "field"); parser->is_aborted = true; return NULL; } @@ -609,10 +609,12 @@ sql_create_check_contraint(struct Parse *parser) uint32_t expr_str_offset; uint32_t ck_def_sz = ck_constraint_def_sizeof(name_len, expr_str_len, &expr_str_offset); - struct ck_constraint_parse *ck_parse = - region_alloc(region, sizeof(*ck_parse) + ck_def_sz); + struct ck_constraint_parse *ck_parse; + size_t total = sizeof(*ck_parse) + ck_def_sz; + ck_parse = (struct ck_constraint_parse *) + region_aligned_alloc(region, total, alignof(*ck_parse)); if (ck_parse == NULL) { - diag_set(OutOfMemory, sizeof(*ck_parse) + ck_def_sz, "region", + diag_set(OutOfMemory, total, "region_aligned_alloc", "ck_parse"); parser->is_aborted = true; return; @@ -620,6 +622,9 @@ sql_create_check_contraint(struct Parse *parser) struct ck_constraint_def *ck_def = (struct ck_constraint_def *)((char *)ck_parse + sizeof(*ck_parse)); + static_assert(alignof(*ck_def) == alignof(*ck_parse), + "allocated in one block and should have the same " + "alignment"); ck_parse->ck_def = ck_def; rlist_create(&ck_parse->link); @@ -1869,11 +1874,13 @@ sql_create_foreign_key(struct Parse *parse_context) goto tnt_error; } } else { + size_t size; struct fk_constraint_parse *fk_parse = - region_alloc(&parse_context->region, sizeof(*fk_parse)); + region_alloc_object(&parse_context->region, + typeof(*fk_parse), &size); if (fk_parse == NULL) { - diag_set(OutOfMemory, sizeof(*fk_parse), "region_alloc", - "struct fk_constraint_parse"); + diag_set(OutOfMemory, size, "region_alloc_object", + "fk_parse"); goto tnt_error; } memset(fk_parse, 0, sizeof(*fk_parse)); @@ -1957,12 +1964,15 @@ sql_create_foreign_key(struct Parse *parse_context) } } int name_len = strlen(constraint_name); - size_t fk_def_sz = fk_constraint_def_sizeof(child_cols_count, name_len); - struct fk_constraint_def *fk_def = region_alloc(&parse_context->region, - fk_def_sz); + uint32_t links_offset; + size_t fk_def_sz = fk_constraint_def_sizeof(child_cols_count, name_len, + &links_offset); + struct fk_constraint_def *fk_def = (struct fk_constraint_def *) + region_aligned_alloc(&parse_context->region, fk_def_sz, + alignof(*fk_def)); if (fk_def == NULL) { - diag_set(OutOfMemory, fk_def_sz, "region", - "struct fk_constraint_def"); + diag_set(OutOfMemory, fk_def_sz, "region_aligned_alloc", + "fk_def"); goto tnt_error; } int actions = create_fk_def->actions; @@ -1973,7 +1983,7 @@ sql_create_foreign_key(struct Parse *parse_context) fk_def->match = (enum fk_constraint_match) (create_fk_def->match); fk_def->on_update = (enum fk_constraint_action) ((actions >> 8) & 0xff); fk_def->on_delete = (enum fk_constraint_action) (actions & 0xff); - fk_def->links = (struct field_link *) ((char *) fk_def->name + name_len + 1); + fk_def->links = (struct field_link *)((char *)fk_def + links_offset); /* Fill links map. */ for (uint32_t i = 0; i < fk_def->field_count; ++i) { if (!is_self_referenced && parent_cols == NULL) { @@ -2260,11 +2270,13 @@ index_fill_def(struct Parse *parse, struct index *index, int rc = -1; struct key_def *key_def = NULL; - struct key_part_def *key_parts = region_alloc(&fiber()->gc, - sizeof(*key_parts) * expr_list->nExpr); + size_t size; + struct key_part_def *key_parts = + region_alloc_array(&fiber()->gc, typeof(key_parts[0]), + expr_list->nExpr, &size); if (key_parts == NULL) { - diag_set(OutOfMemory, sizeof(*key_parts) * expr_list->nExpr, - "region", "key parts"); + diag_set(OutOfMemory, size, "region_alloc_array", + "key_parts"); goto tnt_error; } for (int i = 0; i < expr_list->nExpr; i++) { @@ -2514,10 +2526,10 @@ sql_create_index(struct Parse *parse) { parse->is_aborted = true; } } - - index = (struct index *) region_alloc(&parse->region, sizeof(*index)); + size_t size; + index = region_alloc_object(&parse->region, typeof(*index), &size); if (index == NULL) { - diag_set(OutOfMemory, sizeof(*index), "region", "index"); + diag_set(OutOfMemory, size, "region_alloc_object", "index"); parse->is_aborted = true; goto exit_create_index; } diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 2c510940b..4715ffabb 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -85,10 +85,11 @@ static struct Mem * vdbemem_alloc_on_region(uint32_t count) { struct region *region = &fiber()->gc; - struct Mem *ret = region_alloc(region, count * sizeof(*ret)); + size_t size; + struct Mem *ret = region_alloc_array(region, typeof(*ret), count, + &size); if (ret == NULL) { - diag_set(OutOfMemory, count * sizeof(*ret), - "region_alloc", "ret"); + diag_set(OutOfMemory, size, "region_alloc_array", "ret"); return NULL; } memset(ret, 0, count * sizeof(*ret)); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index f39484013..0b7358af4 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1773,11 +1773,12 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, if (pParse->colNamesSet || db->mallocFailed) return; assert(v != 0); - size_t var_pos_sz = pParse->nVar * sizeof(uint32_t); - uint32_t *var_pos = (uint32_t *) region_alloc(&pParse->region, - var_pos_sz); + size_t size; + uint32_t *var_pos = + region_alloc_array(&pParse->region, typeof(var_pos[0]), + pParse->nVar, &size); if (var_pos == NULL) { - diag_set(OutOfMemory, var_pos_sz, "region", "var_pos"); + diag_set(OutOfMemory, size, "region_alloc_array", "var_pos"); return; } assert(pTabList != 0); @@ -1910,9 +1911,10 @@ sqlColumnsFromExprList(Parse * parse, ExprList * expr_list, */ assert(space_def->fields == NULL); struct region *region = &parse->region; + size_t size; space_def->fields = - region_alloc(region, - column_count * sizeof(space_def->fields[0])); + region_alloc_array(region, typeof(space_def->fields[0]), + column_count, &size); if (space_def->fields == NULL) { sqlOomFault(db); goto cleanup; diff --git a/src/box/sql/update.c b/src/box/sql/update.c index d25262c21..24c7cfa27 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -120,10 +120,10 @@ sqlUpdate(Parse * pParse, /* The parser context */ int pk_cursor = pParse->nTab++; pTabList->a[0].iCursor = pk_cursor; struct index *pPk = space_index(space, 0); - i = sizeof(int) * def->field_count; - aXRef = (int *) region_alloc(&pParse->region, i); + aXRef = region_alloc_array(&pParse->region, typeof(aXRef[0]), + def->field_count, &i); if (aXRef == NULL) { - diag_set(OutOfMemory, i, "region_alloc", "aXRef"); + diag_set(OutOfMemory, i, "region_alloc_array", "aXRef"); goto update_cleanup; } memset(aXRef, -1, i); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7a42602a2..5bc106b5d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -605,11 +605,11 @@ static int vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id) { assert(vdbe != NULL); - struct autoinc_id_entry *id_entry = region_alloc(&fiber()->gc, - sizeof(*id_entry)); + size_t size; + struct autoinc_id_entry *id_entry = + region_alloc_object(&fiber()->gc, typeof(*id_entry), &size); if (id_entry == NULL) { - diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc", - "id_entry"); + diag_set(OutOfMemory, size, "region_alloc_object", "id_entry"); return -1; } id_entry->id = id; diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 5bc27f134..6d8768865 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1084,10 +1084,13 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W * predicates, so we consider term as sequence * of AND'ed predicates. */ - size_t addrs_sz = sizeof(int) * nEq; - int *seek_addrs = region_alloc(&pParse->region, addrs_sz); + size_t addrs_sz; + int *seek_addrs = region_alloc_array(&pParse->region, + typeof(seek_addrs[0]), nEq, + &addrs_sz); if (seek_addrs == NULL) { - diag_set(OutOfMemory, addrs_sz, "region", "seek_addrs"); + diag_set(OutOfMemory, addrs_sz, "region_alloc_array", + "seek_addrs"); pParse->is_aborted = true; return 0; } diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index beaeb0fe7..68ec2a749 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -981,7 +981,8 @@ tuple_format_iterator_create(struct tuple_format_iterator *it, if (validate) it->required_fields_sz = bitmap_size(format->total_field_count); uint32_t total_sz = frames_sz + 2 * it->required_fields_sz; - struct mp_frame *frames = region_alloc(region, total_sz); + struct mp_frame *frames = region_aligned_alloc(region, total_sz, + alignof(frames[0])); if (frames == NULL) { diag_set(OutOfMemory, total_sz, "region", "tuple_format_iterator"); diff --git a/src/box/txn.c b/src/box/txn.c index b81693c0a..123520166 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -802,13 +802,15 @@ struct txn_savepoint * txn_savepoint_new(struct txn *txn, const char *name) { assert(txn == in_txn()); - size_t svp_sz = sizeof(struct txn_savepoint); int name_len = name != NULL ? strlen(name) : 0; - svp_sz += name_len; - struct txn_savepoint *svp = - (struct txn_savepoint *) region_alloc(&txn->region, svp_sz); + struct txn_savepoint *svp; + static_assert(sizeof(svp->name) == 1, + "name member already has 1 byte for 0 termination"); + size_t size = sizeof(*svp) + name_len; + svp = (struct txn_savepoint *)region_aligned_alloc(&txn->region, size, + alignof(*svp)); if (svp == NULL) { - diag_set(OutOfMemory, svp_sz, "region", "svp"); + diag_set(OutOfMemory, size, "region_aligned_alloc", "svp"); return NULL; } svp->stmt = stailq_last(&txn->stmts); diff --git a/src/box/user.cc b/src/box/user.cc index fe0555886..198bf828b 100644 --- a/src/box/user.cc +++ b/src/box/user.cc @@ -192,11 +192,11 @@ user_grant_priv(struct user *user, struct priv_def *def) { struct priv_def *old = privset_search(&user->privs, def); if (old == NULL) { - size_t size = sizeof(struct priv_def); - old = (struct priv_def *) - region_alloc(&user->pool, size); + size_t size; + old = region_alloc_object(&user->pool, typeof(*old), &size); if (old == NULL) { - diag_set(OutOfMemory, size, "region", "struct priv_def"); + diag_set(OutOfMemory, size, "region_alloc_object", + "old"); return -1; } *old = *def; diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 3a582b6fe..e7669452e 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -603,10 +603,11 @@ vinyl_engine_create_space(struct engine *engine, struct space_def *def, rlist_foreach_entry(index_def, key_list, link) key_count++; struct key_def **keys; - size_t size = sizeof(*keys) * key_count; - keys = region_alloc(&fiber()->gc, size); + size_t size; + keys = region_alloc_array(&fiber()->gc, typeof(keys[0]), key_count, + &size); if (keys == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "keys"); + diag_set(OutOfMemory, size, "region_alloc_array", "keys"); free(space); return NULL; } @@ -4439,19 +4440,22 @@ vy_deferred_delete_on_replace(struct trigger *trigger, void *event) * which will propagate the WAL row LSN to * the LSM tree. */ - struct trigger *on_commit = region_alloc(&txn->region, - sizeof(*on_commit)); + size_t size; + struct trigger *on_commit = + region_alloc_object(&txn->region, typeof(*on_commit), + &size); if (on_commit == NULL) { - diag_set(OutOfMemory, sizeof(*on_commit), - "region", "struct trigger"); + diag_set(OutOfMemory, size, "region_alloc_object", + "on_commit"); rc = -1; break; } - struct trigger *on_rollback = region_alloc(&txn->region, - sizeof(*on_commit)); + struct trigger *on_rollback = + region_alloc_object(&txn->region, typeof(*on_rollback), + &size); if (on_rollback == NULL) { - diag_set(OutOfMemory, sizeof(*on_commit), - "region", "struct trigger"); + diag_set(OutOfMemory, size, "region_alloc_object", + "on_rollback"); rc = -1; break; } diff --git a/src/box/vy_log.c b/src/box/vy_log.c index 9ead066af..311985c72 100644 --- a/src/box/vy_log.c +++ b/src/box/vy_log.c @@ -621,12 +621,13 @@ vy_log_record_decode(struct vy_log_record *record, case VY_LOG_KEY_DEF: { struct region *region = &fiber()->gc; uint32_t part_count = mp_decode_array(&pos); - struct key_part_def *parts = region_alloc(region, - sizeof(*parts) * part_count); + size_t size; + struct key_part_def *parts = + region_alloc_array(region, typeof(parts[0]), + part_count, &size); if (parts == NULL) { - diag_set(OutOfMemory, - sizeof(*parts) * part_count, - "region", "struct key_part_def"); + diag_set(OutOfMemory, size, + "region_alloc_array", "parts"); return -1; } if (key_def_decode_parts(parts, part_count, &pos, @@ -701,18 +702,18 @@ static struct vy_log_record * vy_log_record_dup(struct region *pool, const struct vy_log_record *src) { size_t used = region_used(pool); - - struct vy_log_record *dst = region_alloc(pool, sizeof(*dst)); + size_t size; + struct vy_log_record *dst = region_alloc_object(pool, typeof(*dst), + &size); if (dst == NULL) { - diag_set(OutOfMemory, sizeof(*dst), - "region", "struct vy_log_record"); + diag_set(OutOfMemory, size, "region_alloc_object", "dst"); goto err; } *dst = *src; if (src->begin != NULL) { const char *data = src->begin; mp_next(&data); - size_t size = data - src->begin; + size = data - src->begin; dst->begin = region_alloc(pool, size); if (dst->begin == NULL) { diag_set(OutOfMemory, size, "region", @@ -724,7 +725,7 @@ vy_log_record_dup(struct region *pool, const struct vy_log_record *src) if (src->end != NULL) { const char *data = src->end; mp_next(&data); - size_t size = data - src->end; + size = data - src->end; dst->end = region_alloc(pool, size); if (dst->end == NULL) { diag_set(OutOfMemory, size, "region", @@ -734,12 +735,12 @@ vy_log_record_dup(struct region *pool, const struct vy_log_record *src) memcpy((char *)dst->end, src->end, size); } if (src->key_def != NULL) { - size_t size = src->key_def->part_count * - sizeof(struct key_part_def); - dst->key_parts = region_alloc(pool, size); + dst->key_parts = + region_alloc_array(pool, typeof(dst->key_parts[0]), + src->key_def->part_count, &size); if (dst->key_parts == NULL) { - diag_set(OutOfMemory, size, "region", - "struct key_part_def"); + diag_set(OutOfMemory, size, "region_alloc_array", + "def->key_parts"); goto err; } if (key_def_dump_parts(src->key_def, dst->key_parts, pool) != 0) diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c index ecdcde7db..80b5c5933 100644 --- a/src/box/vy_point_lookup.c +++ b/src/box/vy_point_lookup.c @@ -167,11 +167,12 @@ vy_point_lookup_scan_slices(struct vy_lsm *lsm, const struct vy_read_view **rv, ITER_EQ, key); assert(range != NULL); int slice_count = range->slice_count; - struct vy_slice **slices = (struct vy_slice **) - region_alloc(&fiber()->gc, slice_count * sizeof(*slices)); + size_t size; + struct vy_slice **slices = + region_alloc_array(&fiber()->gc, typeof(slices[0]), slice_count, + &size); if (slices == NULL) { - diag_set(OutOfMemory, slice_count * sizeof(*slices), - "region", "slices array"); + diag_set(OutOfMemory, size, "region_alloc_array", "slices"); return -1; } int i = 0; diff --git a/src/box/xrow_update_map.c b/src/box/xrow_update_map.c index ff53a9ac4..57fb27f18 100644 --- a/src/box/xrow_update_map.c +++ b/src/box/xrow_update_map.c @@ -63,10 +63,11 @@ struct xrow_update_map_item { static inline struct xrow_update_map_item * xrow_update_map_item_alloc(void) { - struct xrow_update_map_item *item = (struct xrow_update_map_item *) - region_alloc(&fiber()->gc, sizeof(*item)); + size_t size; + struct xrow_update_map_item *item = + region_alloc_object(&fiber()->gc, typeof(*item), &size); if (item == NULL) - diag_set(OutOfMemory, sizeof(*item), "region_alloc", "item"); + diag_set(OutOfMemory, size, "region_alloc_object", "item"); return item; } diff --git a/src/box/xrow_update_route.c b/src/box/xrow_update_route.c index 122f0329e..0352aec63 100644 --- a/src/box/xrow_update_route.c +++ b/src/box/xrow_update_route.c @@ -244,10 +244,11 @@ xrow_update_route_branch(struct xrow_update_field *field, bool transform_root = (saved_old_offset == 0); struct xrow_update_field *next_hop; if (!transform_root) { - next_hop = (struct xrow_update_field *) - region_alloc(&fiber()->gc, sizeof(*next_hop)); + size_t size; + next_hop = region_alloc_object(&fiber()->gc, typeof(*next_hop), + &size); if (next_hop == NULL) { - diag_set(OutOfMemory, sizeof(*next_hop), "region_alloc", + diag_set(OutOfMemory, size, "region_alloc_object", "next_hop"); return NULL; } diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc index b5531a596..946420885 100644 --- a/src/lib/core/backtrace.cc +++ b/src/lib/core/backtrace.cc @@ -110,9 +110,9 @@ get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache) } else { unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name), offset); - - entry = (struct proc_cache_entry *) - region_alloc(&cache_region, sizeof(struct proc_cache_entry)); + size_t size; + entry = region_alloc_object(&cache_region, typeof(*entry), + &size); if (entry == NULL) goto error; node.key = ip; diff --git a/src/lua/popen.c b/src/lua/popen.c index 18c8282f1..354429f32 100644 --- a/src/lua/popen.c +++ b/src/lua/popen.c @@ -778,11 +778,12 @@ luaT_popen_parse_env(struct lua_State *L, int idx, struct region *region) idx = lua_gettop(L) + idx + 1; size_t capacity = POPEN_LUA_ENV_CAPACITY_DEFAULT; - size_t size = capacity * sizeof(char *); size_t region_svp = region_used(region); - char **env = region_alloc(region, size); + size_t size; + char **env = region_alloc_array(region, typeof(env[0]), capacity, + &size); if (env == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "env array"); + diag_set(OutOfMemory, size, "region_alloc_array", "env"); return NULL; } size_t nr_env = 0; @@ -829,10 +830,10 @@ luaT_popen_parse_env(struct lua_State *L, int idx, struct region *region) * the traverse again and fill `env` array. */ capacity = nr_env + 1; - size = capacity * sizeof(char *); - if ((env = region_alloc(region, size)) == NULL) { + env = region_alloc_array(region, typeof(env[0]), capacity, &size); + if (env == NULL) { region_truncate(region, region_svp); - diag_set(OutOfMemory, size, "region_alloc", "env array"); + diag_set(OutOfMemory, size, "region_alloc_array", "env"); return NULL; } nr_env = 0; @@ -1039,10 +1040,11 @@ luaT_popen_parse_argv(struct lua_State *L, int idx, struct popen_opts *opts, if (opts->flags & POPEN_FLAG_SHELL) opts->nr_argv += 2; - size_t size = sizeof(char *) * opts->nr_argv; - opts->argv = region_alloc(region, size); + size_t size; + opts->argv = region_alloc_array(region, typeof(opts->argv[0]), + opts->nr_argv, &size); if (opts->argv == NULL) { - diag_set(OutOfMemory, size, "region_alloc", "argv"); + diag_set(OutOfMemory, size, "region_alloc_array", "opts->argv"); return -1; } -- 2.21.1 (Apple Git-122.3)
next prev parent reply other threads:[~2020-05-27 23:32 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-27 23:32 [Tarantool-patches] [PATCH v2 00/10] Sanitize unaligned access Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 01/10] small: sanitized rlist and new region API Vladislav Shpilevoy 2020-05-28 20:41 ` Timur Safin 2020-05-28 22:56 ` Vladislav Shpilevoy 2020-06-08 23:01 ` Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 10/10] xrow: use unaligned store operation in xrow_to_iovec() Vladislav Shpilevoy 2020-05-28 20:20 ` Timur Safin 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 02/10] cmake: ignore warnings on alignof() and offsetof() Vladislav Shpilevoy 2020-05-28 20:18 ` Timur Safin 2020-05-29 6:24 ` Kirill Yukhin 2020-05-29 22:34 ` Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 03/10] cmake: add option ENABLE_UB_SANITIZER Vladislav Shpilevoy 2020-05-28 20:42 ` Timur Safin 2020-05-29 8:53 ` Sergey Bronnikov 2020-05-29 22:36 ` Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 04/10] crc32: align memory access Vladislav Shpilevoy 2020-05-28 20:11 ` Timur Safin 2020-05-28 23:23 ` Vladislav Shpilevoy 2020-05-28 23:32 ` Timur Safin 2020-06-08 22:33 ` Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 05/10] sql: make BtCursor's memory aligned Vladislav Shpilevoy 2020-05-28 20:20 ` Timur Safin 2020-05-27 23:32 ` Vladislav Shpilevoy [this message] 2020-05-28 20:35 ` [Tarantool-patches] [PATCH v2 06/10] region: use aligned allocations where necessary Timur Safin 2020-05-28 23:07 ` Vladislav Shpilevoy 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 07/10] vinyl: align statements and bps tree extents Vladislav Shpilevoy 2020-05-28 20:38 ` Timur Safin 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 08/10] tuple: use unaligned store-load for field map Vladislav Shpilevoy 2020-05-28 20:22 ` Timur Safin 2020-05-27 23:32 ` [Tarantool-patches] [PATCH v2 09/10] port: make port_c_entry not PACKED Vladislav Shpilevoy 2020-05-28 20:42 ` Timur Safin 2020-06-03 21:27 ` [Tarantool-patches] [PATCH v2 00/10] Sanitize unaligned access Vladislav Shpilevoy 2020-06-08 22:33 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=31172635747a4f2d20c37525379285aa5ba69ab2.1590622225.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=alyapunov@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tsafin@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 06/10] region: use aligned allocations where necessary' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox