[Tarantool-patches] [PATCH 00/10] Sanitize unaligned access

[Tarantool-patches] [PATCH 00/10] Sanitize unaligned access

[Vladislav Shpilevoy]

The patchset introduces a new cmake option ENABLE_UB_SANITIZER, to
enable clang undefined behaviour sanitizer.

The sanitizer revealed lots of unaligned memory accesses, and the
patchset fixed all of them (which were found).

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4609-sanitize-alignment
Issue: https://github.com/tarantool/tarantool/issues/4609

Vladislav Shpilevoy (10):
  small: sanitized rlist and new region API
  cmake: ignore warnings on alignof() and offsetof()
  cmake: add option ENABLE_UB_SANITIZER
  crc32: disable align sanitizer
  sql: make BtCursor's memory aligned
  region: use aligned allocations where necessary
  vinyl: align statements and bps tree extents
  tuple: use unaligned store-load for field map
  port: make port_c_entry not PACKED
  xrow: use unaligned store operation in xrow_to_iovec()

 cmake/compiler.cmake            | 13 ++++++-
 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             | 30 +++++++++++------
 src/box/field_map.h             | 21 +++++++-----
 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/lua/schema.lua          |  2 +-
 src/box/memtx_tree.c            |  7 ++--
 src/box/port.h                  | 10 +-----
 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              | 16 ++++-----
 src/box/sql/wherecode.c         |  9 +++--
 src/box/tuple_format.c          |  3 +-
 src/box/txn.c                   | 29 +++++++++-------
 src/box/user.cc                 |  8 ++---
 src/box/vinyl.c                 | 26 ++++++++------
 src/box/vy_log.c                | 33 +++++++++---------
 src/box/vy_mem.c                |  9 ++---
 src/box/vy_point_lookup.c       |  9 ++---
 src/box/vy_stmt.c               | 13 ++++---
 src/box/vy_write_iterator.c     |  7 ++--
 src/box/xrow.c                  |  2 +-
 src/box/xrow_update_map.c       |  7 ++--
 src/box/xrow_update_route.c     |  7 ++--
 src/cpu_feature.c               |  3 +-
 src/lib/core/backtrace.cc       |  6 ++--
 src/lib/core/port.h             |  2 +-
 src/lib/small                   |  2 +-
 src/lua/popen.c                 | 20 ++++++-----
 src/trivia/util.h               |  6 ++++
 test/vinyl/quota.result         | 10 +++---
 test/vinyl/quota_timeout.result |  4 +--
 test/vinyl/stat.result          |  4 +--
 45 files changed, 325 insertions(+), 240 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 01/10] small: sanitized rlist and new region API

[Vladislav Shpilevoy]

Rlist used a hack to implement offsetof() leading to crash under
undefined behaviour clang sanitizer. It was fixed in this update.

Additionally, region_alloc_object() is changed to return the used
size and a new macro region_alloc_array() is added. This small
API change is supposed to simplify switching lots of region
allocations to aligned versions in scope of #4609.

Part of #4609
---
 src/box/txn.c               | 17 +++++++++--------
 src/box/vy_write_iterator.c |  7 +++++--
 src/lib/small               |  2 +-
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/box/txn.c b/src/box/txn.c
index 488aa4bdd..b81693c0a 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -54,11 +54,11 @@ static int
 txn_add_redo(struct txn *txn, struct txn_stmt *stmt, struct request *request)
 {
 	/* Create a redo log row. */
+	int size;
 	struct xrow_header *row;
-	row = region_alloc_object(&txn->region, struct xrow_header);
+	row = region_alloc_object(&txn->region, struct xrow_header, &size);
 	if (row == NULL) {
-		diag_set(OutOfMemory, sizeof(*row),
-			 "region", "struct xrow_header");
+		diag_set(OutOfMemory, size, "region_alloc_object", "row");
 		return -1;
 	}
 	if (request->header != NULL) {
@@ -91,11 +91,11 @@ txn_add_redo(struct txn *txn, struct txn_stmt *stmt, struct request *request)
 static struct txn_stmt *
 txn_stmt_new(struct region *region)
 {
+	int size;
 	struct txn_stmt *stmt;
-	stmt = region_alloc_object(region, struct txn_stmt);
+	stmt = region_alloc_object(region, struct txn_stmt, &size);
 	if (stmt == NULL) {
-		diag_set(OutOfMemory, sizeof(*stmt),
-			 "region", "struct txn_stmt");
+		diag_set(OutOfMemory, size, "region_alloc_object", "stmt");
 		return NULL;
 	}
 
@@ -178,9 +178,10 @@ txn_new(void)
 	region_create(&region, &cord()->slabc);
 
 	/* Place txn structure on the region. */
-	struct txn *txn = region_alloc_object(&region, struct txn);
+	int size;
+	struct txn *txn = region_alloc_object(&region, struct txn, &size);
 	if (txn == NULL) {
-		diag_set(OutOfMemory, sizeof(*txn), "region", "struct txn");
+		diag_set(OutOfMemory, size, "region_alloc_object", "txn");
 		return NULL;
 	}
 	assert(region_used(&region) == sizeof(*txn));
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index 01962faa3..78a52ae5c 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -102,9 +102,12 @@ static inline struct vy_write_history *
 vy_write_history_new(struct vy_entry entry, struct vy_write_history *next)
 {
 	struct vy_write_history *h;
-	h = region_alloc_object(&fiber()->gc, struct vy_write_history);
-	if (h == NULL)
+	int size;
+	h = region_alloc_object(&fiber()->gc, struct vy_write_history, &size);
+	if (h == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc_object", "h");
 		return NULL;
+	}
 	h->entry = entry;
 	assert(next == NULL || (next->entry.stmt != NULL &&
 	       vy_stmt_lsn(next->entry.stmt) > vy_stmt_lsn(entry.stmt)));
diff --git a/src/lib/small b/src/lib/small
index 118b53334..fc75e99f5 160000
--- a/src/lib/small
+++ b/src/lib/small
@@ -1 +1 @@
-Subproject commit 118b53334ad785ebe0534758832841dc2786fdc4
+Subproject commit fc75e99f5b2a695c9f52db78198ea7ada7ef6a82
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 10/10] xrow: use unaligned store operation in xrow_to_iovec()

[Vladislav Shpilevoy]

xrow_to_iovec() tried to save a uint32_t value by a not aligned
address. The patch makes it use a special operation for that
instead of regular assignment.

Part of #4609
---
 src/box/xrow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/box/xrow.c b/src/box/xrow.c
index 06473a8bc..bb64864b2 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -892,7 +892,7 @@ xrow_to_iovec(const struct xrow_header *row, struct iovec *out)
 	/* Encode length */
 	char *data = (char *) out[0].iov_base;
 	*(data++) = 0xce; /* MP_UINT32 */
-	*(uint32_t *) data = mp_bswap_u32(len);
+	store_u32(data, mp_bswap_u32(len));
 
 	assert(iovcnt <= XROW_IOVMAX);
 	return iovcnt;
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 02/10] cmake: ignore warnings on alignof() and offsetof()

[Vladislav Shpilevoy]

Warning about invalid offsetof() (used on non-POD types) was set
for g++, but wasn't for clang++.

Warning about invalid alignof() (when expression is passed to it
instead of a type) wasn't ignored, but is going to be very
useful in upcoming unaligned memory access patches. That allows
to write something like:

    struct some_long_type *object = region_aligned_alloc(
            region, size, alignof(*object));

This will work even if type of 'object' will change in future,
and so it is safer. And shorter.

Part of #4609
---
 cmake/compiler.cmake | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index 56429dc20..ce3e7e506 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -276,11 +276,12 @@ macro(enable_tnt_compile_flags)
         add_compile_flags("C;CXX" "-Wno-format-truncation")
     endif()
 
-    if (CMAKE_COMPILER_IS_GNUCXX)
+    if (CMAKE_COMPILER_IS_CLANG OR CMAKE_COMPILER_IS_GNUCXX)
         # G++ bug. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31488
         add_compile_flags("CXX"
             "-Wno-invalid-offsetof"
         )
+        add_compile_flags("C;CXX" "-Wno-gnu-alignof-expression")
     endif()
 
     if (CMAKE_COMPILER_IS_GNUCC)
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 03/10] cmake: add option ENABLE_UB_SANITIZER

[Vladislav Shpilevoy]

Clang has a built-in sanitizer for undefined behaviour. Such as
wrong memory alignment, array boundaries violation, 0 division,
bool values with non standard content, etc.

The sanitizer emits runtime checks which lead to either crash, or
a trap, or a warning print, depending on what is chosen.

The patch makes it possible to turn the sanitizer on and catch
UBs. The only supported UB so far is alignment check. Other types
can be added gradually, along with fixing bugs which they find.

Sometimes it happens that unaligned memory access is done
intentionally, or can't be simply fixed. To disable the sanitizer
for such places an attribute 'no_sanitize' can be used. It is
added inside a macro NOSANITIZE_ALIGN.

Part of #4609
---
 cmake/compiler.cmake | 10 ++++++++++
 src/trivia/util.h    |  6 ++++++
 2 files changed, 16 insertions(+)

diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index ce3e7e506..373bcd3b0 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -238,6 +238,8 @@ endif()
 
 option(ENABLE_WERROR "Make all compiler warnings into errors" OFF)
 
+option(ENABLE_UB_SANITIZER "Make the compiler generate runtime code to perform undefined behaviour checks" OFF)
+
 macro(enable_tnt_compile_flags)
     # Tarantool code is written in GNU C dialect.
     # Additionally, compile it with more strict flags than the rest
@@ -263,6 +265,14 @@ macro(enable_tnt_compile_flags)
         "-Wno-strict-aliasing"
     )
 
+    if (ENABLE_UB_SANITIZER)
+        if (NOT CMAKE_COMPILER_IS_CLANG)
+            message(FATAL_ERROR "Undefined behaviour sanitizer only available for clang")
+        else()
+            add_compile_flags("C;CXX" "-fsanitize=alignment -fno-sanitize-recover=alignment")
+        endif()
+    endif()
+
     if (CMAKE_COMPILER_IS_CLANG AND CC_HAS_WNO_UNUSED_VALUE)
         # False-positive warnings for ({ xx = ...; x; }) macroses
         add_compile_flags("C;CXX" "-Wno-unused-value")
diff --git a/src/trivia/util.h b/src/trivia/util.h
index 8a3d22b38..466cb6e55 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -392,6 +392,12 @@ strnindex(const char **haystack, const char *needle, uint32_t len, uint32_t hmax
 
 /** \endcond public */
 
+#if __has_attribute(no_sanitize)
+#define NOSANITIZE_ALIGN __attribute__((no_sanitize("alignment")))
+#else
+#define NOSANITIZE_ALIGN
+#endif
+
 void close_all_xcpt(int fdc, ...);
 
 void __gcov_flush();
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 04/10] crc32: disable align sanitizer

[Vladislav Shpilevoy]

There is some assembly working with a byte array like with an
array of unsigned long values. Better allow it to continue
working like that with disabled sanitizer, than accidentally
break or slow down something here.

Part of #4609
---
I am sure there is a way to fix it instead of muting, but I don't
know how. Probably there is someone, who understands what is
written in this assembly code, and can help.

 src/cpu_feature.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/cpu_feature.c b/src/cpu_feature.c
index 98567ccb3..09b6c84ee 100644
--- a/src/cpu_feature.c
+++ b/src/cpu_feature.c
@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 #include "trivia/config.h"
+#include "trivia/util.h"
 #include <sys/types.h>
 #include <errno.h>
 #include <stdlib.h>
@@ -65,7 +66,7 @@ crc32c_hw_byte(uint32_t crc, unsigned char const *data, unsigned int length)
 }
 
 
-uint32_t
+NOSANITIZE_ALIGN uint32_t
 crc32c_hw(uint32_t crc, const char *buf, unsigned int len)
 {
 	unsigned int iquotient = len / SCALE_F;
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 05/10] sql: make BtCursor's memory aligned

[Vladislav Shpilevoy]

Vdbe at runtime allocates VdbeCursor structure using
allocateCursor() function. Inside there is a pointer at BtCursor
structure. To make the allocation faster and improve cache
locality, both cursors are allocated in one memory block + some
extra memory for uint32_t array, where BtCursor followed
VdbeCursor and the array without any padding:

   VdbeCursor + uint32_t * N + BtCursor

The problem is that BtCursor needs 8 byte alignment. When it
followed VdbeCursor (aligned by 8) + some uint32_t values, its
actual alignment could become 4 bytes. That led to a crash when
alignment sanitizer is enabled in clang.

The patch makes BtCursor offset aligned by 8 bytes.

Part of #4609
---
 src/box/sql/vdbe.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 724bc188b..7a42602a2 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -234,10 +234,9 @@ allocateCursor(
 	 */
 	Mem *pMem = iCur>0 ? &p->aMem[p->nMem-iCur] : p->aMem;
 
-	int nByte;
 	VdbeCursor *pCx = 0;
-	nByte =
-		ROUND8(sizeof(VdbeCursor)) + sizeof(u32)*nField +
+	int bt_offset = ROUND8(sizeof(VdbeCursor) + sizeof(uint32_t) * nField);
+	int nByte = bt_offset +
 		(eCurType==CURTYPE_TARANTOOL ? ROUND8(sizeof(BtCursor)) : 0);
 
 	assert(iCur>=0 && iCur<p->nCursor);
@@ -251,8 +250,7 @@ allocateCursor(
 		pCx->eCurType = eCurType;
 		pCx->nField = nField;
 		if (eCurType==CURTYPE_TARANTOOL) {
-			pCx->uc.pCursor = (BtCursor*)
-				&pMem->z[ROUND8(sizeof(VdbeCursor))+sizeof(u32)*nField];
+			pCx->uc.pCursor = (BtCursor*)&pMem->z[bt_offset];
 			sqlCursorZero(pCx->uc.pCursor);
 		}
 	}
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 06/10] region: use aligned allocations where necessary

[Vladislav Shpilevoy]

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 fee82afb0..3e666b5fb 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) +
-		link_count * sizeof(struct field_link) +
-		name_len + 1;
+	*links_offset = small_align(sizeof(struct fk_constraint) + 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..595d03ef0 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), 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..504a21ffb 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));
 	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..97c6a60ac 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), 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..eb757986c 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),
+				   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..451e58149 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),
+				   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..98ed781e3 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),
+					   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 950a5508c..65d0fa7a5 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;
 	}
@@ -4438,19 +4439,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)

[Tarantool-patches] [PATCH 07/10] vinyl: align statements and bps tree extents

[Vladislav Shpilevoy]

Vinyl tuples (vy_stmt) in 0 level of LSM tree are stored in
lsregion. They were allocated using lsregion_alloc(), which does
not align its results, and is good only for byte arrays.

As a result, vy_stmt object addresses in 0 LSM level were not
aligned. Unaligned memory access is slower, and may even crash on
some platforms.

Besides, even aligned allocations couldn't help upserts in 0 level
of the LSM tree, because upsert vy_stmt objects had 1 byte prefix
to count merged upserts stored in this statement. This 1 byte
prefix ruined all the alignment. Now the upsert counter is also
aligned, the same as vy_stmt. Note, it does not consume
significantly more memory, since it used only for vinyl and only
for upserts, stored in 0 level of the LSM tree.

The same about BPS tree extents. LSM 0 level is a BPS tree, whose
blocks are allocated on lsregion. The extents are used as pointer
arrays inside the tree, so they need alignof(void *) alignment.

The mentioned unaligned accesses were revealed by clang undefined
behaviour sanitizer, and are fixed by this patch.

Part of #4609
---
 src/box/vy_mem.c                |  9 +++++----
 src/box/vy_stmt.c               | 13 ++++++++-----
 test/vinyl/quota.result         | 10 +++++-----
 test/vinyl/quota_timeout.result |  4 ++--
 test/vinyl/stat.result          |  4 ++--
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index b4d016a68..98027e784 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -75,11 +75,12 @@ vy_mem_tree_extent_alloc(void *ctx)
 {
 	struct vy_mem *mem = (struct vy_mem *) ctx;
 	struct vy_mem_env *env = mem->env;
-	void *ret = lsregion_alloc(&env->allocator, VY_MEM_TREE_EXTENT_SIZE,
-				   mem->generation);
+	void *ret = lsregion_aligned_alloc(&env->allocator,
+					   VY_MEM_TREE_EXTENT_SIZE,
+					   alignof(void *), mem->generation);
 	if (ret == NULL) {
-		diag_set(OutOfMemory, VY_MEM_TREE_EXTENT_SIZE, "lsregion_alloc",
-			 "ret");
+		diag_set(OutOfMemory, VY_MEM_TREE_EXTENT_SIZE,
+			 "lsregion_aligned_alloc", "ret");
 		return NULL;
 	}
 	mem->tree_extent_size += VY_MEM_TREE_EXTENT_SIZE;
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index adc3ba452..dc6960068 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -223,20 +223,23 @@ vy_stmt_dup_lsregion(struct tuple *stmt, struct lsregion *lsregion,
 	size_t size = tuple_size(stmt);
 	size_t alloc_size = size;
 	struct tuple *mem_stmt;
+	const size_t align = alignof(struct vy_stmt);
 
 	/* Reserve one byte for UPSERT counter. */
 	if (type == IPROTO_UPSERT)
-		alloc_size++;
+		alloc_size += align;
 
-	mem_stmt = lsregion_alloc(lsregion, alloc_size, alloc_id);
+	mem_stmt = lsregion_aligned_alloc(lsregion, alloc_size, align,
+					  alloc_id);
 	if (mem_stmt == NULL) {
-		diag_set(OutOfMemory, size, "lsregion_alloc", "mem_stmt");
+		diag_set(OutOfMemory, size, "lsregion_aligned_alloc",
+			 "mem_stmt");
 		return NULL;
 	}
 
 	if (type == IPROTO_UPSERT) {
-		*(uint8_t *)mem_stmt = 0;
-		mem_stmt = (struct tuple *)((uint8_t *)mem_stmt + 1);
+		memset(mem_stmt, 0, align);
+		mem_stmt = (struct tuple *)((uint8_t *)mem_stmt + align);
 	}
 
 	memcpy(mem_stmt, stmt, size);
diff --git a/test/vinyl/quota.result b/test/vinyl/quota.result
index d1b28ee51..940df4e49 100644
--- a/test/vinyl/quota.result
+++ b/test/vinyl/quota.result
@@ -31,7 +31,7 @@ space:insert({1, 1})
 ...
 box.stat.vinyl().memory.level0
 ---
-- 98343
+- 98344
 ...
 space:insert({1, 1})
 ---
@@ -39,7 +39,7 @@ space:insert({1, 1})
 ...
 box.stat.vinyl().memory.level0
 ---
-- 98343
+- 98344
 ...
 space:update({1}, {{'!', 1, 100}}) -- try to modify the primary key
 ---
@@ -47,7 +47,7 @@ space:update({1}, {{'!', 1, 100}}) -- try to modify the primary key
 ...
 box.stat.vinyl().memory.level0
 ---
-- 98343
+- 98344
 ...
 space:insert({2, 2})
 ---
@@ -63,7 +63,7 @@ space:insert({4, 4})
 ...
 box.stat.vinyl().memory.level0
 ---
-- 98460
+- 98463
 ...
 box.snapshot()
 ---
@@ -89,7 +89,7 @@ _ = space:replace{1, 1, string.rep('a', 1024 * 1024 * 5)}
 ...
 box.stat.vinyl().memory.level0
 ---
-- 5292076
+- 5292080
 ...
 space:drop()
 ---
diff --git a/test/vinyl/quota_timeout.result b/test/vinyl/quota_timeout.result
index 7a71b29c6..31ca23670 100644
--- a/test/vinyl/quota_timeout.result
+++ b/test/vinyl/quota_timeout.result
@@ -49,7 +49,7 @@ s:count()
 ...
 box.stat.vinyl().memory.level0
 ---
-- 748241
+- 748248
 ...
 -- Since the following operation requires more memory than configured
 -- and dump is disabled, it should fail with ER_VY_QUOTA_TIMEOUT.
@@ -63,7 +63,7 @@ s:count()
 ...
 box.stat.vinyl().memory.level0
 ---
-- 748241
+- 748248
 ...
 --
 -- Check that increasing box.cfg.vinyl_memory wakes up fibers
diff --git a/test/vinyl/stat.result b/test/vinyl/stat.result
index d35def13d..a895528b9 100644
--- a/test/vinyl/stat.result
+++ b/test/vinyl/stat.result
@@ -761,7 +761,7 @@ put(1)
 ...
 stat_diff(gstat(), st, 'memory.level0')
 ---
-- 1061
+- 1064
 ...
 -- use cache
 st = gstat()
@@ -1130,7 +1130,7 @@ gstat()
   memory:
     tuple_cache: 14417
     tx: 0
-    level0: 262583
+    level0: 263210
     page_index: 1250
     bloom_filter: 140
   disk:
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 08/10] tuple: use unaligned store-load for field map

[Vladislav Shpilevoy]

A tuple can have a field map. It is an array of uint32_t values,
stored right after 'struct tuple' object in the same memory block.

'struct tuple' is 10 byte size, and is aligned by 4 bytes (even
though it is 'PACKED', so does not need an alignment). It means,
that field map is stored by an address like this: 4*N + 10.

This address is never aligned by 4, needed for uint32_t field map
array. So the array members should be accessed using operations
aware of unaligned nature of the addresses.

Unfortunately, it is impossible to make the field map aligned,
because that requires + 2 bytes for each tuple. It is unaffordable
luxury, since tuple count can be millions and even billions. Every
new byte may result in gigabytes of memory in a cluster.

The patch makes field map accessed using unaligned store-load
operations.

Part of #4609
---
 src/box/field_map.c | 13 +++++++++----
 src/box/field_map.h | 21 ++++++++++++---------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/box/field_map.c b/src/box/field_map.c
index 5f4661941..dc903115e 100644
--- a/src/box/field_map.c
+++ b/src/box/field_map.c
@@ -101,16 +101,21 @@ field_map_build(struct field_map_builder *builder, char *buffer)
 		(uint32_t *)(buffer + field_map_build_size(builder));
 	char *extent_wptr = buffer;
 	for (int32_t i = -1; i >= -(int32_t)builder->slot_count; i--) {
+		/*
+		 * Can not access field_map as a normal uint32
+		 * array because its alignment may be < 4 bytes.
+		 * Need to use unaligned store-load operations
+		 * explicitly.
+		 */
 		if (!builder->slots[i].has_extent) {
-			field_map[i] = builder->slots[i].offset;
+			store_u32(&field_map[i], builder->slots[i].offset);
 			continue;
 		}
 		struct field_map_builder_slot_extent *extent =
 						builder->slots[i].extent;
 		/** Retrive memory for the extent. */
-		field_map[i] =
-			(uint32_t)((char *)extent_wptr - (char *)field_map);
-		*(uint32_t *)extent_wptr = extent->size;
+		store_u32(&field_map[i], extent_wptr - (char *)field_map);
+		store_u32(extent_wptr, extent->size);
 		uint32_t extent_offset_sz = extent->size * sizeof(uint32_t);
 		memcpy(&((uint32_t *) extent_wptr)[1], extent->offset,
 			extent_offset_sz);
diff --git a/src/box/field_map.h b/src/box/field_map.h
index a1a5a9dba..d8ef726a1 100644
--- a/src/box/field_map.h
+++ b/src/box/field_map.h
@@ -33,6 +33,7 @@
 #include <assert.h>
 #include <stdint.h>
 #include <stddef.h>
+#include "bit/bit.h"
 
 struct region;
 struct field_map_builder_slot;
@@ -151,20 +152,22 @@ static inline uint32_t
 field_map_get_offset(const uint32_t *field_map, int32_t offset_slot,
 		     int multikey_idx)
 {
-	uint32_t offset;
-	if (multikey_idx != MULTIKEY_NONE &&
-	    (int32_t) field_map[offset_slot] < 0) {
+	/*
+	 * Can not access field_map as a normal uint32 array
+	 * because its alignment may be < 4 bytes. Need to use
+	 * unaligned store-load operations explicitly.
+	 */
+	uint32_t offset = load_u32(&field_map[offset_slot]);
+	if (multikey_idx != MULTIKEY_NONE && (int32_t)offset < 0) {
 		/**
 		 * The field_map extent has the following
 		 * structure: [size=N|slot1|slot2|..|slotN]
 		 */
-		uint32_t *extent = (uint32_t *)((char *)field_map +
-					(int32_t)field_map[offset_slot]);
-		if ((uint32_t)multikey_idx >= extent[0])
+		const uint32_t *extent = (const uint32_t *)
+			((const char *)field_map + (int32_t)offset);
+		if ((uint32_t)multikey_idx >= load_u32(&extent[0]))
 			return 0;
-		offset = extent[multikey_idx + 1];
-	} else {
-		offset = field_map[offset_slot];
+		offset = load_u32(&extent[multikey_idx + 1]);
 	}
 	return offset;
 }
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 09/10] port: make port_c_entry not PACKED

[Vladislav Shpilevoy]

PACKED structures don't have padding between their members and
after the structure (needed to be able to store them in an array).

Port_c_entry was PACKED, since it does not have padding between
its members anyway, and the padding in the end was not needed,
because these objects are never stored in an array. As a result,
sizeof(port_c_entry) was not aligned too.

Appeared, that mempool, used to allocate port_c_entry objects,
can't work correctly, when object size is not aligned at least by
8 bytes. Because mempool does not do any alignment internally, and
uses the free objects as a temporary storage for some metadata,
requiring 8 byte alignment.

The patch removes PACKED attribute from port_c_entry, so now its
size is aligned by 8 bytes, and mempool works fine.

Part of #4609
---
 src/box/lua/schema.lua |  2 +-
 src/box/port.h         | 10 +---------
 src/lib/core/port.h    |  2 +-
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index cdfbb65f7..e6844b45f 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -79,7 +79,7 @@ ffi.cdef[[
     box_txn_savepoint_t *
     box_txn_savepoint();
 
-    struct __attribute__((packed)) port_c_entry {
+    struct port_c_entry {
         struct port_c_entry *next;
         union {
             struct tuple *tuple;
diff --git a/src/box/port.h b/src/box/port.h
index 76bb5ed1b..43d0f9deb 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -106,12 +106,7 @@ void
 port_vdbemem_create(struct port *base, struct sql_value *mem,
 		    uint32_t mem_count);
 
-/**
- * The struct is PACKED to avoid unnecessary 4 byte padding
- * after mp_size. These objects are never stored on stack, neither
- * allocated as an array, so the padding is not needed.
- */
-struct PACKED port_c_entry {
+struct port_c_entry {
 	struct port_c_entry *next;
 	union {
 		/** Valid if mp_size is 0. */
@@ -126,9 +121,6 @@ struct PACKED port_c_entry {
 	uint32_t mp_size;
 };
 
-static_assert(sizeof(struct port_c_entry) == 20,
-	      "port_c_entry is expected to be perfectly aligned and packed");
-
 /**
  * C port is used by C functions from the public API. They can
  * return tuples and arbitrary MessagePack.
diff --git a/src/lib/core/port.h b/src/lib/core/port.h
index bb7787679..5c51f76e1 100644
--- a/src/lib/core/port.h
+++ b/src/lib/core/port.h
@@ -122,7 +122,7 @@ struct port {
 	 * Implementation dependent content. Needed to declare
 	 * an abstract port instance on stack.
 	 */
-	char pad[52];
+	char pad[60];
 };
 
 /** Is not inlined just to be exported. */
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 00/10] Sanitize unaligned access

[Sergey Bronnikov]

Vladislav,

On 22:37 Thu 21 May , Vladislav Shpilevoy wrote:
> The patchset introduces a new cmake option ENABLE_UB_SANITIZER, to
> enable clang undefined behaviour sanitizer.

Intruduced option is disabled by default.
Do you plan to enable it on CI runs?

> The sanitizer revealed lots of unaligned memory accesses, and the
> patchset fixed all of them (which were found).
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4609-sanitize-alignment
> Issue: https://github.com/tarantool/tarantool/issues/4609
> 
> Vladislav Shpilevoy (10):
>   small: sanitized rlist and new region API
>   cmake: ignore warnings on alignof() and offsetof()
>   cmake: add option ENABLE_UB_SANITIZER
>   crc32: disable align sanitizer
>   sql: make BtCursor's memory aligned
>   region: use aligned allocations where necessary
>   vinyl: align statements and bps tree extents
>   tuple: use unaligned store-load for field map
>   port: make port_c_entry not PACKED
>   xrow: use unaligned store operation in xrow_to_iovec()

<snipped>

Re: [Tarantool-patches] [PATCH 00/10] Sanitize unaligned access

[Vladislav Shpilevoy]

Hi! Thanks for the review!

On 22/05/2020 00:25, Sergey Bronnikov wrote:
> Vladislav,
> 
> On 22:37 Thu 21 May , Vladislav Shpilevoy wrote:
>> The patchset introduces a new cmake option ENABLE_UB_SANITIZER, to
>> enable clang undefined behaviour sanitizer.
> 
> Intruduced option is disabled by default.
> Do you plan to enable it on CI runs?

I added the UB detector to ASAN builds in CI. See v2 in a new thread.

Re: [Tarantool-patches] [PATCH 01/10] small: sanitized rlist and new region API

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:23PM +0200, Vladislav Shpilevoy wrote:
> Rlist used a hack to implement offsetof() leading to crash under
> undefined behaviour clang sanitizer. It was fixed in this update.
> 
> Additionally, region_alloc_object() is changed to return the used
> size and a new macro region_alloc_array() is added. This small
> API change is supposed to simplify switching lots of region
> allocations to aligned versions in scope of #4609.
> 
> Part of #4609

So the size variable is only needed to write diag_set message.
A good candidate to be moved into small library itself. But I think
it should be addressed separately. Also sizeof() returns size_t
type, not int, but should not be a problem for us.

Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Btw, the branch to be reviewed is

gerold103/gh-4609-sanitize-alignment-full-ci

Re: [Tarantool-patches] [PATCH 10/10] xrow: use unaligned store operation in xrow_to_iovec()

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:24PM +0200, Vladislav Shpilevoy wrote:
> xrow_to_iovec() tried to save a uint32_t value by a not aligned
> address. The patch makes it use a special operation for that
> instead of regular assignment.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 02/10] cmake: ignore warnings on alignof() and offsetof()

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:25PM +0200, Vladislav Shpilevoy wrote:
> Warning about invalid offsetof() (used on non-POD types) was set
> for g++, but wasn't for clang++.
> 
> Warning about invalid alignof() (when expression is passed to it
> instead of a type) wasn't ignored, but is going to be very
> useful in upcoming unaligned memory access patches. That allows
> to write something like:
> 
>     struct some_long_type *object = region_aligned_alloc(
>             region, size, alignof(*object));
> 
> This will work even if type of 'object' will change in future,
> and so it is safer. And shorter.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 03/10] cmake: add option ENABLE_UB_SANITIZER

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:26PM +0200, Vladislav Shpilevoy wrote:
> Clang has a built-in sanitizer for undefined behaviour. Such as
> wrong memory alignment, array boundaries violation, 0 division,
> bool values with non standard content, etc.
> 
> The sanitizer emits runtime checks which lead to either crash, or
> a trap, or a warning print, depending on what is chosen.
> 
> The patch makes it possible to turn the sanitizer on and catch
> UBs. The only supported UB so far is alignment check. Other types
> can be added gradually, along with fixing bugs which they find.
> 
> Sometimes it happens that unaligned memory access is done
> intentionally, or can't be simply fixed. To disable the sanitizer
> for such places an attribute 'no_sanitize' can be used. It is
> added inside a macro NOSANITIZE_ALIGN.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 04/10] crc32: disable align sanitizer

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:27PM +0200, Vladislav Shpilevoy wrote:
> There is some assembly working with a byte array like with an
> array of unsigned long values. Better allow it to continue
> working like that with disabled sanitizer, than accidentally
> break or slow down something here.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 05/10] sql: make BtCursor's memory aligned

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:28PM +0200, Vladislav Shpilevoy wrote:
> Vdbe at runtime allocates VdbeCursor structure using
> allocateCursor() function. Inside there is a pointer at BtCursor
> structure. To make the allocation faster and improve cache
> locality, both cursors are allocated in one memory block + some
> extra memory for uint32_t array, where BtCursor followed
> VdbeCursor and the array without any padding:
> 
>    VdbeCursor + uint32_t * N + BtCursor
> 
> The problem is that BtCursor needs 8 byte alignment. When it
> followed VdbeCursor (aligned by 8) + some uint32_t values, its
> actual alignment could become 4 bytes. That led to a crash when
> alignment sanitizer is enabled in clang.
> 
> The patch makes BtCursor offset aligned by 8 bytes.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 06/10] region: use aligned allocations where necessary

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:29PM +0200, Vladislav Shpilevoy wrote:
> 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
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 07/10] vinyl: align statements and bps tree extents

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:30PM +0200, Vladislav Shpilevoy wrote:
> Vinyl tuples (vy_stmt) in 0 level of LSM tree are stored in
> lsregion. They were allocated using lsregion_alloc(), which does
> not align its results, and is good only for byte arrays.
> 
> As a result, vy_stmt object addresses in 0 LSM level were not
> aligned. Unaligned memory access is slower, and may even crash on
> some platforms.
> 
> Besides, even aligned allocations couldn't help upserts in 0 level
> of the LSM tree, because upsert vy_stmt objects had 1 byte prefix
> to count merged upserts stored in this statement. This 1 byte
> prefix ruined all the alignment. Now the upsert counter is also
> aligned, the same as vy_stmt. Note, it does not consume
> significantly more memory, since it used only for vinyl and only
> for upserts, stored in 0 level of the LSM tree.
> 
> The same about BPS tree extents. LSM 0 level is a BPS tree, whose
> blocks are allocated on lsregion. The extents are used as pointer
> arrays inside the tree, so they need alignof(void *) alignment.
> 
> The mentioned unaligned accesses were revealed by clang undefined
> behaviour sanitizer, and are fixed by this patch.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 08/10] tuple: use unaligned store-load for field map

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:31PM +0200, Vladislav Shpilevoy wrote:
> A tuple can have a field map. It is an array of uint32_t values,
> stored right after 'struct tuple' object in the same memory block.
> 
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH 09/10] port: make port_c_entry not PACKED

[Cyrill Gorcunov]

On Thu, May 21, 2020 at 10:37:32PM +0200, Vladislav Shpilevoy wrote:
> PACKED structures don't have padding between their members and
> after the structure (needed to be able to store them in an array).
> 
> Port_c_entry was PACKED, since it does not have padding between
> its members anyway, and the padding in the end was not needed,
> because these objects are never stored in an array. As a result,
> sizeof(port_c_entry) was not aligned too.
> 
> Appeared, that mempool, used to allocate port_c_entry objects,
> can't work correctly, when object size is not aligned at least by
> 8 bytes. Because mempool does not do any alignment internally, and
> uses the free objects as a temporary storage for some metadata,
> requiring 8 byte alignment.
> 
> The patch removes PACKED attribute from port_c_entry, so now its
> size is aligned by 8 bytes, and mempool works fine.
> 
> Part of #4609
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>