[Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations

[Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations

[Vladislav Shpilevoy]

The patchset is a second part of the undefined behaviour fixes. It
is based on the UB alignment patchset, and enables all the other
UB checks except a few last ones, described in the first commit.

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

Aleksandr Lyapunov (1):
  salad: fix UB pointer arithmetics in bps_tree

Vladislav Shpilevoy (10):
  cmake: enable misc types of UB detection in clang
  util: introduce double_compare_nint64()
  test: avoid usleep() usage for error injections
  vinyl: fix 0 division in case of canceled dump
  xrow: don't cast double to float unconditionally
  swim: fix zero division
  test: fix signed integer overflow in vclock test
  digest: eliminate UBs from guava()
  sql: fix usage of not initialized index_stat
  sql: fix mem_apply_type double type truncation

 cmake/compiler.cmake        | 16 ++++++++++++++-
 src/box/sql/vdbe.c          | 18 +++++++++++------
 src/box/sql/vdbeaux.c       | 16 +++++++--------
 src/box/sql/where.c         |  4 ++--
 src/box/tuple_compare.cc    |  9 ++-------
 src/box/vy_regulator.c      | 10 ++++++----
 src/box/vy_run.c            |  2 +-
 src/box/vy_scheduler.c      |  2 +-
 src/box/xrow_update_field.c | 19 +++++++++---------
 src/lib/core/util.c         | 39 ++++++++++++++++++++++++++++++++++++-
 src/lib/salad/bps_tree.h    | 14 ++++++-------
 src/lib/salad/guava.c       | 18 ++++++++++-------
 src/lib/salad/guava.h       |  2 +-
 src/lib/swim/swim.c         | 10 +++++-----
 src/lua/digest.lua          |  2 +-
 src/trivia/util.h           | 29 +++++++++++++++++++++++++++
 test/app/digest.result      |  2 +-
 test/box/update.result      |  2 +-
 test/box/update.test.lua    |  2 +-
 test/unit/vclock.cc         | 28 +++++++++++++-------------
 test/unit/vclock.result     |  2 +-
 21 files changed, 167 insertions(+), 79 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 01/11] cmake: enable misc types of UB detection in clang

[Vladislav Shpilevoy]

Option ENABLE_UB_SANITIZER enables clang undefined behaviour
sanitizer. So far the only UB to detect was alignment violation.
This was the biggest problem found by the sanitizer. Now when it
is fixed, most of the other types of UB are also turned on to fix
them as well.

There is a few of exceptions - pointer type overflow, vptr check,
and all types of integer overflow and truncation.

Pointer type overflow detection is disabled because it is abused
in the source code a lot, by stailq data structure.

Vptr sanitation is a runtime check ensuring that a pointer at a
non-POD type really points at an object of this type, using RTTI.
The check false-positively fails in alter.cc when AlterSpaceOp
class objects are stored in an rlist, and the list is iterated
using rlist_foreach_entry(). In the cycle there is a condition:

    &item->member != head

In the end the 'item' points not at an AlterSpaceOp, but at the
rlist head - offsetof(typeof(item), member), at an rlist
structure. Despite 'item' is never dereferenced, clang anyway
generates vptr check here, which of course fails. Note,
'&item->member' does not dereference item. It is
item + offsetof(typeof(item), member). Just another address a few
bytes after item.

Integer overflow and truncation are disabled because SQL uses
int64_t variables as a container of values of range [INT64_MIN,
UINT64_MAX]. This works because there is a flag 'is_neg' near
each such value which tells how to interpret it - as negative
int64_t, or as positive uint64_t. As a result, some operations
lead to a false-positive overflow. For example, consider
expr_code_int() function. It essentially can do this:

    int64_t value;
    ((uint64_t *)&value) = 9223372036854775808;
    value = -value;

9223372036854775808 is -INT64_MIN. It can't be stored in int64_t.
But the thing is that (uint64_t)9223372036854775808 is stored
exactly like (int64_t)INT64_MIN, in binary form. So the expression
"value = -value" looks perfectly valid:
"value = -9223372036854775808", But in fact it is interpreted as
"value = -(-9223372036854775808)".

These integer overflow/truncation problems are going to be fixed
in a separate commit due to big amount of changes needed for that.

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

diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index 4b23d6631..6c0fa635c 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -269,7 +269,21 @@ macro(enable_tnt_compile_flags)
         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")
+            set(SANITIZE_FLAGS "-fsanitize=undefined -fno-sanitize-recover=undefined")
+            # Stailq data structure subtracts a positive value from NULL.
+            set(SANITIZE_FLAGS ${SANITIZE_FLAGS} -fno-sanitize=pointer-overflow)
+            # Intrusive data structures may abuse '&obj->member' on pointer
+            # 'obj' which is not really a pointer at an object of its type.
+            # For example, rlist uses '&item->member' expression in macro cycles
+            # to check end of cycle, but on the last iteration 'item' points at
+            # the list metadata head, not at an object of type stored in this
+            # list.
+            set(SANITIZE_FLAGS ${SANITIZE_FLAGS} -fno-sanitize=vptr)
+            # Integer overflow and truncation are disabled due to extensive
+            # usage of this UB in SQL code to 'implement' some kind of int65_t.
+            set(SANITIZE_FLAGS ${SANITIZE_FLAGS} -fno-sanitize=implicit-signed-integer-truncation -fno-sanitize=implicit-integer-sign-change -fno-sanitize=signed-integer-overflow)
+
+            add_compile_flags("C;CXX" "${SANITIZE_FLAGS}")
         endif()
     endif()
 
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 10/11] sql: fix usage of not initialized index_stat

[Vladislav Shpilevoy]

Query planner uses a temporary index definition object 'to
represent the primary key index'. Whatever real purpose of this
index_def is (query planner wasn't changed since SQLite merge,
and may be broken), its opts.stat field pointed at a partially
initialized index_stat structure. Which is supposed to be used by
the planner to make decisions such as search by which index would
be the optimal.

The patch initializes the statistics with 0.

Part of #4609
---
 src/box/sql/where.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 7ec43e184..e9e936856 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -2794,9 +2794,9 @@ tnt_error:
 		fake_index->iid = UINT32_MAX;
 		int size = sizeof(struct index_stat) + sizeof(log_est_t) * 2;
 
-		struct index_stat *stat = (struct index_stat *) malloc(size);
+		struct index_stat *stat = (struct index_stat *) calloc(1, size);
 		if (stat == NULL) {
-			diag_set(OutOfMemory, size, "malloc", "stat");
+			diag_set(OutOfMemory, size, "calloc", "stat");
 			goto tnt_error;
 		}
 		stat->tuple_log_est = (log_est_t *) ((char *) (stat + 1));
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 11/11] sql: fix mem_apply_type double type truncation

[Vladislav Shpilevoy]

mem_apply_type(), when tried to cast a double value to an integer,
used the expressions:

    int64_t i = (int64_t) d;
    uint64_t u = (uint64_t) d;

To obtain integer versions of the double value, cast them back to
double, and see if they are equal. Assuming that if they are, the
double can be safely cast to one of them.

But this is undefined behaviour. Double can't be cast to int64_t,
if it is > INT64_MAX or < INT64_MIN. And can't be cast to
uint64_t, if it is < 0 or > UINT64_MAX.

The patch adds explicit checks for these borders before doing the
cast.

Part of #4609
---
 src/box/sql/vdbe.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 5bc106b5d..6b769805c 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -324,12 +324,18 @@ mem_apply_type(struct Mem *record, enum field_type type)
 			return 0;
 		if ((record->flags & MEM_Real) == MEM_Real) {
 			double d = record->u.r;
-			int64_t i = (int64_t) d;
-			uint64_t u = (uint64_t) d;
-			if (i == d)
-				mem_set_int(record, i, i <= -1);
-			else if (u == d)
-				mem_set_u64(record, u);
+			if (d >= 0) {
+				if (double_compare_uint64(d, UINT64_MAX,
+							  1) > 0)
+					return 0;
+				if ((double)(uint64_t)d == d)
+					mem_set_u64(record, (uint64_t)d);
+			} else {
+				if (double_compare_nint64(d, INT64_MIN, 1) < 0)
+					return 0;
+				if ((double)(int64_t)d == d)
+					mem_set_int(record, (int64_t)d, true);
+			}
 			return 0;
 		}
 		if ((record->flags & MEM_Str) != 0) {
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 02/11] util: introduce double_compare_nint64()

[Vladislav Shpilevoy]

Utility module (util.h and util.c) offers a function
double_compare_uint64() to compare double and uint64_t in a
reliable way, without undefined behaviour, without losses, even if
values are about 2^53 - 2^64.

There was no a similar function to compare double and int64_t,
which is needed when the right value is negative. To workaround it
the right value (int64_t) was multiplied by -1 when it was
negative to be able to use it in double_compare_uint64(). This led
to undefined behaviour, when right value was INT64_MIN, because
expression (uint64_t)-value did not help. Firstly -value was
calculated, and then it was cast to uint64_t. The first step is
detected by the sanitizer as undefined behaviour.

Not counting, that it was slower than straightforward comparison
of negative int64_t and a double value. Because involved
additional negation.

Besides, there was an expression:

    assert((uint64_t)(double)rhs == rhs || rhs > (uint64_t)EXP2_53);

Rhs is a uint64_t. And there was hidden an undefined behaviour,
when rhs is UINT64_MAX. The problem is that UINT64_MAX is
2^64 - 1. And it can't be stored in a double value without
precision loss. It is rounded up to 2^64 = UINT64_MAX + 1. This
is what happens in "(double)rhs" expression. And when it is
cast back to uint64_t: "(uint64_t)(double)rhs", it explodes.

The patch fixes it by simply changing check order. If rhs is
bigger than 2^53 (below this border all the integers can be
represented), then the cast is not done.

Part of #4609
---
 src/box/sql/vdbeaux.c    | 16 ++++++++--------
 src/box/tuple_compare.cc |  9 ++-------
 src/lib/core/util.c      | 22 +++++++++++++++++++++-
 src/trivia/util.h        | 22 ++++++++++++++++++++++
 4 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 71f694b20..bae1a5352 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2876,8 +2876,8 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl)
 		}
 		if ((f1 & MEM_Int) != 0) {
 			if ((f2 & MEM_Real) != 0) {
-				return double_compare_uint64(-pMem2->u.r,
-							     -pMem1->u.i, 1);
+				return double_compare_nint64(pMem2->u.r,
+							     pMem1->u.i, -1);
 			} else {
 				return -1;
 			}
@@ -2894,8 +2894,8 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl)
 		}
 		if ((f1 & MEM_Real) != 0) {
 			if ((f2 & MEM_Int) != 0) {
-				return double_compare_uint64(-pMem1->u.r,
-							     -pMem2->u.i, -1);
+				return double_compare_nint64(pMem1->u.r,
+							     pMem2->u.i, 1);
 			} else if ((f2 & MEM_UInt) != 0) {
 				return double_compare_uint64(pMem1->u.r,
 							     pMem2->u.u, 1);
@@ -3073,8 +3073,8 @@ sqlVdbeCompareMsgpack(const char **key1,
 					rc = +1;
 				}
 			} else if (pKey2->flags & MEM_Real) {
-				rc = double_compare_uint64(-pKey2->u.r,
-							   -mem1.u.i, 1);
+				rc = double_compare_nint64(pKey2->u.r, mem1.u.i,
+							   -1);
 			} else if ((pKey2->flags & MEM_Null) != 0) {
 				rc = 1;
 			} else if ((pKey2->flags & MEM_Bool) != 0) {
@@ -3092,8 +3092,8 @@ sqlVdbeCompareMsgpack(const char **key1,
 			mem1.u.r = mp_decode_double(&aKey1);
  do_float:
 			if ((pKey2->flags & MEM_Int) != 0) {
-				rc = double_compare_uint64(-mem1.u.r,
-							   -pKey2->u.i, -1);
+				rc = double_compare_nint64(mem1.u.r, pKey2->u.i,
+							   1);
 			} else if (pKey2->flags & MEM_UInt) {
 				rc = double_compare_uint64(mem1.u.r,
 							   pKey2->u.u, 1);
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index bc01fe068..d059c709f 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -168,13 +168,8 @@ static int
 mp_compare_double_any_int(double lhs, const char *rhs, enum mp_type rhs_type,
 			  int k)
 {
-	if (rhs_type == MP_INT) {
-		int64_t v = mp_decode_int(&rhs);
-		if (v < 0) {
-			return double_compare_uint64(-lhs, (uint64_t)-v, -k);
-		}
-		return double_compare_uint64(lhs, (uint64_t)v, k);
-	}
+	if (rhs_type == MP_INT)
+		return double_compare_int64(lhs, mp_decode_int(&rhs), k);
 	assert(rhs_type == MP_UINT);
 	return double_compare_uint64(lhs, mp_decode_uint(&rhs), k);
 }
diff --git a/src/lib/core/util.c b/src/lib/core/util.c
index ffa1d2b51..a65bc496c 100644
--- a/src/lib/core/util.c
+++ b/src/lib/core/util.c
@@ -327,6 +327,7 @@ fpconv_check()
 }
 
 #define EXP2_53 9007199254740992.0      /* 2.0 ^ 53 */
+#define EXP2_63 9223372036854775808.0   /* 2.0 ^ 63 */
 #define EXP2_64 1.8446744073709552e+19  /* 2.0 ^ 64 */
 
 int
@@ -378,7 +379,7 @@ double_compare_uint64(double lhs, uint64_t rhs, int k)
 		 * rounding happens.
 		 */
 		assert(lhs < EXP2_53);
-		assert((uint64_t)(double)rhs == rhs || rhs > (uint64_t)EXP2_53);
+		assert(rhs > (uint64_t)EXP2_53 || (uint64_t)(double)rhs == rhs);
 		return k*COMPARE_RESULT(lhs, (double)rhs);
 	}
 	/*
@@ -387,3 +388,22 @@ double_compare_uint64(double lhs, uint64_t rhs, int k)
 	 */
 	return -k;
 }
+
+int
+double_compare_nint64(double lhs, int64_t rhs, int k)
+{
+	assert(rhs < 0);
+	assert(k==1 || k==-1);
+	if (lhs <= -EXP2_53) {
+		assert((int64_t)-EXP2_63 == INT64_MIN);
+		if (lhs < -EXP2_63)
+			return -k;
+		assert((double)(int64_t)lhs == lhs);
+		return k*COMPARE_RESULT((int64_t)lhs, rhs);
+	}
+	if (!isnan(lhs)) {
+		assert(rhs < (int64_t)-EXP2_53 || (int64_t)(double)rhs == rhs);
+		return k*COMPARE_RESULT(lhs, (double)rhs);
+	}
+	return -k;
+}
diff --git a/src/trivia/util.h b/src/trivia/util.h
index 8a3d22b38..973c9df33 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -512,6 +512,28 @@ json_escape(char *buf, int size, const char *data);
 int
 double_compare_uint64(double lhs, uint64_t rhs, int k);
 
+/**
+ * The same as double_compare_uint64(), but for negative int64_t
+ * value. To avoid unnecessary negation for cast to uint64_t to
+ * be able to use the other function, and to avoid the undefined
+ * behaviour in it, because "(uint64_t)-value" is UB, if value is
+ * INT64_MIN.
+ */
+int
+double_compare_nint64(double lhs, int64_t rhs, int k);
+
+/**
+ * A shortcut to automatically choose a needed double vs
+ * int64/uint64 function.
+ */
+static inline int
+double_compare_int64(double lhs, int64_t rhs, int k)
+{
+	if (rhs >= 0)
+		return double_compare_uint64(lhs, (uint64_t)rhs, k);
+	return double_compare_nint64(lhs, rhs, k);
+}
+
 #if !defined(__cplusplus) && !defined(static_assert)
 # define static_assert _Static_assert
 #endif
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 03/11] test: avoid usleep() usage for error injections

[Vladislav Shpilevoy]

Some error injections use usleep() to put the current thread in
sleep. For example, to simulate, that WAL thread is slow.

A few of them allowed to pass custom number of microseconds to
usleep, in form:

    usleep(injection->dvalue * 1000000);

Assuming, that dvalue is a number of seconds. But usleep argument
is uint32_t, at least on Mac (it is useconds_t, whose size is 4).
It means, that too big dvalue easily overflows it.

The patch makes it use nanosleep(), in a new wrapper:
thread_sleep(). It takes a double value, and does not truncate it
to 4 bytes.

The overflow was the case for ERRINJ_VY_READ_PAGE_TIMEOUT = 9000
in test/vinyl/errinj_vylog.test.lua. And
ERRINJ_VY_RUN_WRITE_STMT_TIMEOUT = 9000 in
test/vinyl/errinj.test.lua.

Part of #4609
---
 src/box/vy_run.c       |  2 +-
 src/box/vy_scheduler.c |  2 +-
 src/lib/core/util.c    | 17 +++++++++++++++++
 src/trivia/util.h      |  7 +++++++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index f8b096f6b..54cf028d0 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -914,7 +914,7 @@ vy_page_read(struct vy_page *page, const struct vy_page_info *page_info,
 
 	struct errinj *inj = errinj(ERRINJ_VY_READ_PAGE_TIMEOUT, ERRINJ_DOUBLE);
 	if (inj != NULL && inj->dparam > 0)
-		usleep(inj->dparam * 1000000);
+		thread_sleep(inj->dparam);
 
 	ERROR_INJECT_SLEEP(ERRINJ_VY_READ_PAGE_DELAY);
 
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index cf58d5f60..a0b7ad006 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -1079,7 +1079,7 @@ vy_task_write_run(struct vy_task *task, bool no_compression)
 		struct errinj *inj = errinj(ERRINJ_VY_RUN_WRITE_STMT_TIMEOUT,
 					    ERRINJ_DOUBLE);
 		if (inj != NULL && inj->dparam > 0)
-			usleep(inj->dparam * 1000000);
+			thread_sleep(inj->dparam);
 
 		rc = vy_run_writer_append_stmt(&writer, entry);
 		if (rc != 0)
diff --git a/src/lib/core/util.c b/src/lib/core/util.c
index a65bc496c..d7f2344ed 100644
--- a/src/lib/core/util.c
+++ b/src/lib/core/util.c
@@ -407,3 +407,20 @@ double_compare_nint64(double lhs, int64_t rhs, int k)
 	}
 	return -k;
 }
+
+void
+thread_sleep(double sec)
+{
+	uint64_t ns = (uint64_t)(sec * 1000000000);
+	assert(ns > 0);
+	struct timespec req;
+	struct timespec next;
+	req.tv_sec = ns / 1000000000;
+	req.tv_nsec = ns % 1000000000;
+	assert(req.tv_nsec < 1000000000);
+	int rc;
+	while ((rc = nanosleep(&req, &next)) == -1 && errno == EINTR)
+		req = next;
+	assert(rc == 0);
+	(void)rc;
+}
diff --git a/src/trivia/util.h b/src/trivia/util.h
index 973c9df33..29c7f0194 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -534,6 +534,13 @@ double_compare_int64(double lhs, int64_t rhs, int k)
 	return double_compare_nint64(lhs, rhs, k);
 }
 
+/**
+ * Put the current thread in sleep for the given number of
+ * seconds.
+ */
+void
+thread_sleep(double sec);
+
 #if !defined(__cplusplus) && !defined(static_assert)
 # define static_assert _Static_assert
 #endif
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 04/11] vinyl: fix 0 division in case of canceled dump

[Vladislav Shpilevoy]

It can happen, that vinyl index dump is scheduled, but the index
is dropped before the dump starts. In this case the dump task is
finished with 0 dump time. The time then is used in a log message
to obtain dump rate as byte count / dump time. Since dump time is
0, this is undefined behaviour. Moreover, it was not even a dump,
so nothing to log here.

Also dump time is used to update throttling parameters in
vy_regulator. Here if the dump took 0 time, there was no a dump,
so it is just ignored.

Part of #4609
---
 src/box/vy_regulator.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/box/vy_regulator.c b/src/box/vy_regulator.c
index a6604c9f3..8ec7e25d6 100644
--- a/src/box/vy_regulator.c
+++ b/src/box/vy_regulator.c
@@ -291,9 +291,11 @@ vy_regulator_dump_complete(struct vy_regulator *regulator,
 	vy_quota_set_rate_limit(regulator->quota, VY_QUOTA_RESOURCE_MEMORY,
 				regulator->dump_bandwidth);
 
-	say_info("dumped %zu bytes in %.1f s, rate %.1f MB/s",
-		 mem_dumped, dump_duration,
-		 mem_dumped / dump_duration / 1024 / 1024);
+	if (dump_duration > 0) {
+		say_info("dumped %zu bytes in %.1f s, rate %.1f MB/s",
+			 mem_dumped, dump_duration,
+			 mem_dumped / dump_duration / 1024 / 1024);
+	}
 }
 
 void
@@ -420,7 +422,7 @@ vy_regulator_update_rate_limit(struct vy_regulator *regulator,
 	double compaction_time = stat->compaction_time - last->compaction_time;
 	*last = *stat;
 
-	if (dump_input < (ssize_t)VY_DUMP_SIZE_ACCT_MIN)
+	if (dump_input < (ssize_t)VY_DUMP_SIZE_ACCT_MIN || compaction_time == 0)
 		return;
 
 	recent->dump_count += dump_count;
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 05/11] xrow: don't cast double to float unconditionally

[Vladislav Shpilevoy]

Xrow update functions use function xrow_update_arith_make() to
execute arithmetic operation. In case any operand is a floating
point value, both are cast to double. Afterwards the result value
tries to shrink to float if possible, to save space.

The check whether the value fits float was done in a dumb but
simple way:

    (double)(float)value == value

This was done to see if the fractional part is not truncated. The
integral part was checked against FLT_MAX and -FLT_MAX. But the
latter check was done after the first check. That led to undefined
behaviour if the |value| is bigger than |FLT_MAX|. This patch
makes xrow update firstly check the integral bounds and then the
fractional part.

Alongside there is fixed a bug, when FLT_MAX value was considered
double.

Part of #4609
---
 src/box/xrow_update_field.c | 19 ++++++++++---------
 test/box/update.result      |  2 +-
 test/box/update.test.lua    |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
index cc64cf955..d635e82b9 100644
--- a/src/box/xrow_update_field.c
+++ b/src/box/xrow_update_field.c
@@ -404,16 +404,17 @@ xrow_update_arith_make(struct xrow_update_op *op,
 			unreachable();
 			break;
 		}
-		float fc = (float) c;
-		if ((lowest_type == XUPDATE_TYPE_DOUBLE && c != (double) fc) ||
-		    (lowest_type == XUPDATE_TYPE_FLOAT &&
-		     (c >= FLT_MAX || c <= -FLT_MAX))) {
-			ret->type = XUPDATE_TYPE_DOUBLE;
-			ret->dbl = c;
-		} else {
-			ret->type = XUPDATE_TYPE_FLOAT;
-			ret->flt = fc;
+		if (c <= FLT_MAX && c >= -FLT_MAX) {
+			float fc = (float)c;
+			if (c == (double)fc) {
+				ret->type = XUPDATE_TYPE_FLOAT;
+				ret->flt = fc;
+				return 0;
+			}
 		}
+		ret->type = XUPDATE_TYPE_DOUBLE;
+		ret->dbl = c;
+		return 0;
 	} else {
 		decimal_t a, b, c;
 		if (! xrow_update_arg_arith_to_decimal(arg1, &a) ||
diff --git a/test/box/update.result b/test/box/update.result
index 04866006a..6ab1a5bc5 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -1687,7 +1687,7 @@ tests = {
     {{'float', flt_max}, {'float', flt_max}, mp_double},                        \
     {{'float', -flt_max}, {'float', -flt_max}, mp_double},                      \
     {{'float', 1}, {'int', 1}, mp_float},                                       \
-    {{'float', flt_max}, {'uint64_t', uint_max}, mp_double},                    \
+    {{'float', flt_max}, {'uint64_t', uint_max}, mp_float},                     \
     {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double},                \
 }
 ---
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index d52be4f3b..1538cae9c 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -624,7 +624,7 @@ tests = {
     {{'float', -flt_max}, {'float', -flt_max}, mp_double},                      \
 -- Float + int is float when fits the float range.                              \
     {{'float', 1}, {'int', 1}, mp_float},                                       \
-    {{'float', flt_max}, {'uint64_t', uint_max}, mp_double},                    \
+    {{'float', flt_max}, {'uint64_t', uint_max}, mp_float},                     \
 -- Precision matters too. Double is used when need to avoid                     \
 -- precision loss.                                                              \
     {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double},                \
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 06/11] swim: fix zero division

[Vladislav Shpilevoy]

If swim_cfg() fails during first configuration after the self
member is created, it is deleted before return. Deletion of the
member causes member update event addition to the dissemination
queue. The event TTL depends on log2(member count), where the
count is zero in the described situation. This leads to zero
division according to clang sanitizer.

The patch makes member deletion from the member table happen a
little later, so the member event registration can't ever see the
table empty.

Part of #4609
---
 src/lib/swim/swim.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 9a7b4ce85..396bd7c45 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -874,11 +874,6 @@ swim_delete_member(struct swim *swim, struct swim_member *member)
 {
 	say_verbose("SWIM %d: member %s is deleted", swim_fd(swim),
 		    tt_uuid_str(&member->uuid));
-	struct mh_swim_table_key key = {member->hash, &member->uuid};
-	mh_int_t rc = mh_swim_table_find(swim->members, key, NULL);
-	assert(rc != mh_end(swim->members));
-	mh_swim_table_del(swim->members, rc, NULL);
-	rlist_del_entry(member, in_round_queue);
 
 	/* Failure detection component. */
 	if (! heap_node_is_stray(&member->in_wait_ack_heap))
@@ -888,6 +883,11 @@ swim_delete_member(struct swim *swim, struct swim_member *member)
 	swim_on_member_update(swim, member, SWIM_EV_DROP);
 	rlist_del_entry(member, in_dissemination_queue);
 
+	struct mh_swim_table_key key = {member->hash, &member->uuid};
+	mh_int_t rc = mh_swim_table_find(swim->members, key, NULL);
+	assert(rc != mh_end(swim->members));
+	mh_swim_table_del(swim->members, rc, NULL);
+	rlist_del_entry(member, in_round_queue);
 	swim_member_delete(member);
 }
 
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 07/11] test: fix signed integer overflow in vclock test

[Vladislav Shpilevoy]

Vclock unit test had a test where all vclock components were close
to INT64_MAX. The sum was bigger than INT64_MAX, and it caused
overflow in vclock's signature.

The patch reduces order of the LSNs in this test so their sum does
not overflow anymore.

Part of #4609
---
 test/unit/vclock.cc     | 28 ++++++++++++++--------------
 test/unit/vclock.result |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/test/unit/vclock.cc b/test/unit/vclock.cc
index 15e9f9379..cbda7f4fc 100644
--- a/test/unit/vclock.cc
+++ b/test/unit/vclock.cc
@@ -275,20 +275,20 @@ test_tostring()
 	test(arg(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15),
 	     "{1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8, "
 	      "9: 9, 10: 10, 11: 11, 12: 12, 13: 13, 14: 14, 15: 15}");
-	test(arg(9223372036854775000, 9223372036854775001, 9223372036854775002,
-		 9223372036854775003, 9223372036854775004, 9223372036854775005,
-		 9223372036854775006, 9223372036854775007, 9223372036854775008,
-		 9223372036854775009, 9223372036854775010, 9223372036854775011,
-		 9223372036854775012, 9223372036854775013, 9223372036854775014,
-		 9223372036854775015),
-	     "{0: 9223372036854775000, 1: 9223372036854775001, "
-	      "2: 9223372036854775002, 3: 9223372036854775003, "
-	      "4: 9223372036854775004, 5: 9223372036854775005, "
-	      "6: 9223372036854775006, 7: 9223372036854775007, "
-	      "8: 9223372036854775008, 9: 9223372036854775009, "
-	     "10: 9223372036854775010, 11: 9223372036854775011, "
-	     "12: 9223372036854775012, 13: 9223372036854775013, "
-	     "14: 9223372036854775014, 15: 9223372036854775015}");
+	test(arg(9223372054775000, 9223372054775001, 9223372054775002,
+		 9223372054775003, 9223372054775004, 9223372054775005,
+		 9223372054775006, 9223372054775007, 9223372054775008,
+		 9223372054775009, 9223372054775010, 9223372054775011,
+		 9223372054775012, 9223372054775013, 9223372054775014,
+		 9223372054775015),
+	     "{0: 9223372054775000, 1: 9223372054775001, "
+	      "2: 9223372054775002, 3: 9223372054775003, "
+	      "4: 9223372054775004, 5: 9223372054775005, "
+	      "6: 9223372054775006, 7: 9223372054775007, "
+	      "8: 9223372054775008, 9: 9223372054775009, "
+	     "10: 9223372054775010, 11: 9223372054775011, "
+	     "12: 9223372054775012, 13: 9223372054775013, "
+	     "14: 9223372054775014, 15: 9223372054775015}");
 
 	footer();
 	return check_plan();
diff --git a/test/unit/vclock.result b/test/unit/vclock.result
index 2ca741bbd..610ecbe78 100644
--- a/test/unit/vclock.result
+++ b/test/unit/vclock.result
@@ -92,7 +92,7 @@ ok 2 - subtests
     ok 5 - tostring (10, 15, 20) => {0: 10, 1: 15, 2: 20}
     ok 6 - tostring (10, -1, 15, -1, 20) => {0: 10, 2: 15, 4: 20}
     ok 7 - tostring (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) => {1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8, 9: 9, 10: 10, 11: 11, 12: 12, 13: 13, 14: 14, 15: 15}
-    ok 8 - tostring (9223372036854775000, 9223372036854775001, 9223372036854775002, 9223372036854775003, 9223372036854775004, 9223372036854775005, 9223372036854775006, 9223372036854775007, 9223372036854775008, 9223372036854775009, 9223372036854775010, 9223372036854775011, 9223372036854775012, 9223372036854775013, 9223372036854775014, 9223372036854775015) => {0: 9223372036854775000, 1: 9223372036854775001, 2: 9223372036854775002, 3: 9223372036854775003, 4: 9223372036854775004, 5: 9223372036854775005, 6: 9223372036854775006, 7: 9223372036854775007, 8: 9223372036854775008, 9: 9223372036854775009, 10: 9223372036854775010, 11: 9223372036854775011, 12: 9223372036854775012, 13: 9223372036854775013, 14: 9223372036854775014, 15: 9223372036854775015}
+    ok 8 - tostring (9223372054775000, 9223372054775001, 9223372054775002, 9223372054775003, 9223372054775004, 9223372054775005, 9223372054775006, 9223372054775007, 9223372054775008, 9223372054775009, 9223372054775010, 9223372054775011, 9223372054775012, 9223372054775013, 9223372054775014, 9223372054775015) => {0: 9223372054775000, 1: 9223372054775001, 2: 9223372054775002, 3: 9223372054775003, 4: 9223372054775004, 5: 9223372054775005, 6: 9223372054775006, 7: 9223372054775007, 8: 9223372054775008, 9: 9223372054775009, 10: 9223372054775010, 11: 9223372054775011, 12: 9223372054775012, 13: 9223372054775013, 14: 9223372054775014, 15: 9223372054775015}
 	*** test_tostring: done ***
 ok 3 - subtests
     1..12
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 08/11] digest: eliminate UBs from guava()

[Vladislav Shpilevoy]

Guava hash function follows the algorithm described in the paper
"A Fast, Minimal Memory, Consistent Hash Algorithm", John Lamping,
Eric Veach. But the implementation somewhy is ported from Java
instead of taking a ready to use C function right from the paper.

Java version, ported to C as is, leads to undefined behaviour:
- signed integer overflow;
- double value outside of integer range assigned to the integer.

Here is the full old function:

    static const int64_t K = 2862933555777941757;
    static const double  D = 0x1.0p31;

    static inline double lcg(int64_t *state)
    {
        return (double)((int32_t)(((uint64_t)*state >> 33) + 1)) / D;
    }

    int32_t
    guava(int64_t state, int32_t buckets)
    {
        int32_t candidate = 0;
        int32_t next;
        while (1) {
            state = K * state + 1;
            next = (int32_t)((candidate + 1) / lcg(&state));
            if (next >= 0 && next < buckets)
                candidate = next;
            else
                return candidate;
        }
    }

Signed integer overflow happened in this line:

    state = K * state + 1;

This UB is fixed by changing state type to uint64_t. This is ok
and does not change behaviour, because overflow for signed
integers in reality behaves the same as for unsigned integers,
in terms of binary representation. Change to uint64_t didn't lead
to change of any single line of the result assembly code.

Double -> int32_t truncation overflow happened in the next line:

    next = (int32_t)((candidate + 1) / lcg(&state));

Right expression can become something out of int32_t range. This
is UB, but in reality the double value on the right is truncated
to either INT32_MIN, if it is < INT32_MIN, or INT32_MAX, if it is
> INT32_MAX.

This is fixed by making 'next' and 'candidate' variables int64_t.

This fixed the UB, because the right expression can't become
< INT64_MIN and > INT64_MAX. Indeed, max possible value of 'next'
from the expression above is:

    (candidate + 1) / lcg(&state) < ((2^31 - 1) + 1) / 2^-31
        = 2^62 < INT64_MAX

Min possible value of 'next' is:

    (-2^31 + 1) / 2^-31 > -2^62 > INT64_MAX

'Candidate' is assumed to be inside int32_t range, because it is
initialized from the previous 'next' value. If it would be out of
int32_t, it would become either bigger than 'buckets', which is
int32_t, or < 0, and the cycle would stop on the previous
iteration.

The patch fixes the UBs, but guava() function is still broken.
Because it returns values different than the function in the
original paper from John Lamping and Eric Veach. Here is the
correct function:

    int32_t
    correct_guava(uint64_t key, int32_t num_buckets)
    {
        int64_t b = -1, j = 0;
        while (j < num_buckets) {
            b=j;
            key = key * 2862933555777941757ULL + 1;
            j = (b + 1) * ((double)(1LL << 31) / (double)((key >> 33) + 1));
        }
        return b;
    }

correct_guava(6356101026326471242, 813154869) = 362866355,
                            guava(<the same>) = 2.

correct_guava(112571758688054605, 1355446793) = 907865430,
                            guava(<the same>) = 907865431.

And there are more examples. It means, guava() returns something
with an unknown distrubution, consistency, and not described in
the paper. That is a subject for a separate patch to deprecate
this function and add a correct one.

Part of #4609
---
 src/lib/salad/guava.c  | 18 +++++++++++-------
 src/lib/salad/guava.h  |  2 +-
 src/lua/digest.lua     |  2 +-
 test/app/digest.result |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/lib/salad/guava.c b/src/lib/salad/guava.c
index 46bf1ac15..16c1e8cc4 100644
--- a/src/lib/salad/guava.c
+++ b/src/lib/salad/guava.c
@@ -39,22 +39,26 @@
  * John Lamping, Eric Veach
  */
 
-static const int64_t K = 2862933555777941757;
+static const uint64_t K = 2862933555777941757ULL;
 static const double  D = 0x1.0p31;
 
-static inline double lcg(int64_t *state)
+static inline double lcg(uint64_t *state)
 {
-	return (double )((int32_t)(((uint64_t )*state >> 33) + 1)) / D;
+	return (double)((int32_t)((*state >> 33) + 1)) / D;
 }
 
+/**
+ * The function does not conform to the original paper, and
+ * therefore has incorrect behaviour. It should be deprecated.
+ */
 int32_t
-guava(int64_t state, int32_t buckets)
+guava(uint64_t state, int32_t buckets)
 {
-	int32_t candidate = 0;
-	int32_t next;
+	int64_t candidate = 0;
+	int64_t next;
 	while (1) {
 		state = K * state + 1;
-		next = (int32_t)((candidate + 1) / lcg(&state));
+		next = (candidate + 1) / lcg(&state);
 		if (next >= 0 && next < buckets)
 			candidate = next;
 		else
diff --git a/src/lib/salad/guava.h b/src/lib/salad/guava.h
index 9ed93d641..bf0181f1e 100644
--- a/src/lib/salad/guava.h
+++ b/src/lib/salad/guava.h
@@ -39,7 +39,7 @@ extern "C" {
 #endif
 
 int32_t
-guava(int64_t state, int32_t buckets);
+guava(uint64_t state, int32_t buckets);
 
 #if defined(__cplusplus)
 } /* extern C */
diff --git a/src/lua/digest.lua b/src/lua/digest.lua
index 6ed91cfa2..6beaf562a 100644
--- a/src/lua/digest.lua
+++ b/src/lua/digest.lua
@@ -14,7 +14,7 @@ ffi.cdef[[
 
     typedef uint32_t (*crc32_func)(uint32_t crc,
         const unsigned char *buf, unsigned int len);
-    extern int32_t guava(int64_t state, int32_t buckets);
+    extern int32_t guava(uint64_t state, int32_t buckets);
     extern crc32_func crc32_calc;
 
     /* base64 */
diff --git a/test/app/digest.result b/test/app/digest.result
index ecfa518c9..d946c6a3b 100644
--- a/test/app/digest.result
+++ b/test/app/digest.result
@@ -357,7 +357,7 @@ digest.base64_decode(123)
 ...
 digest.guava('hello', 0)
 ---
-- error: 'bad argument #1 to ''?'' (cannot convert ''string'' to ''int64_t'')'
+- error: 'bad argument #1 to ''?'' (cannot convert ''string'' to ''uint64_t'')'
 ...
 digest.guava(1, 'nope_')
 ---
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 09/11] salad: fix UB pointer arithmetics in bps_tree

[Vladislav Shpilevoy]

From: Aleksandr Lyapunov <alyapunov@tarantool.org>

There is some pointer arithmetics in bps_tree that calculates
intermediate pointers that points out of array bounds. Though they
are never dereferenced and only used for further caclulation of
correct pointers, it is still UB and must be fixed.

Part of #4609
---
 src/lib/salad/bps_tree.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/lib/salad/bps_tree.h b/src/lib/salad/bps_tree.h
index d28b53f53..ef5ae3d7e 100644
--- a/src/lib/salad/bps_tree.h
+++ b/src/lib/salad/bps_tree.h
@@ -2654,7 +2654,7 @@ bps_tree_move_elems_to_right_inner(struct bps_tree *tree,
 	if (!move_to_empty)
 		BPS_TREE_DATAMOVE(b->elems + num, b->elems,
 				  b->header.size - 1, b, b);
-	BPS_TREE_DATAMOVE(b->elems, a->elems + a->header.size - num,
+	BPS_TREE_DATAMOVE(b->elems, a->elems + (a->header.size - num),
 			  num - 1, b, a);
 	if (move_to_empty)
 		*b_inner_path_elem->max_elem_copy =
@@ -2866,7 +2866,7 @@ bps_tree_insert_and_move_elems_to_right_inner(struct bps_tree *tree,
 				  mid_part_size - num, a, a);
 		a->child_ids[pos] = block_id;
 
-		BPS_TREE_DATAMOVE(b->elems, a->elems + a->header.size - num,
+		BPS_TREE_DATAMOVE(b->elems, a->elems + (a->header.size - num),
 				  num - 1, b, a);
 		if (move_to_empty)
 			*b_inner_path_elem->max_elem_copy =
@@ -2888,7 +2888,7 @@ bps_tree_insert_and_move_elems_to_right_inner(struct bps_tree *tree,
 				  mid_part_size - num, a, a);
 		a->child_ids[pos] = block_id;
 
-		BPS_TREE_DATAMOVE(b->elems, a->elems + a->header.size - num,
+		BPS_TREE_DATAMOVE(b->elems, a->elems + (a->header.size - num),
 				  num - 1, b, a);
 		if (move_to_empty)
 			*b_inner_path_elem->max_elem_copy =
@@ -2916,8 +2916,8 @@ bps_tree_insert_and_move_elems_to_right_inner(struct bps_tree *tree,
 			if (num > 1) {
 				/* +(num - 2) */
 				BPS_TREE_DATAMOVE(b->elems,
-						  a->elems + a->header.size
-						   - num + 1, num - 2, b, a);
+						  a->elems + (a->header.size
+						   - num + 1), num - 2, b, a);
 				/* +1 */
 				b->elems[num - 2] =
 					*a_inner_path_elem->max_elem_copy;
@@ -2930,7 +2930,7 @@ bps_tree_insert_and_move_elems_to_right_inner(struct bps_tree *tree,
 			assert(num > 1);
 
 			BPS_TREE_DATAMOVE(b->elems,
-					  a->elems + a->header.size - num + 1,
+					  a->elems + (a->header.size - num + 1),
 					  num - mid_part_size - 1, b, a);
 			b->elems[new_pos] = max_elem;
 			BPS_TREE_DATAMOVE(b->elems + new_pos + 1,
@@ -3142,7 +3142,7 @@ bps_tree_insert_and_move_elems_to_left_inner(struct bps_tree *tree,
 					b->elems[num - 2];
 		}
 		if (!move_all)
-			BPS_TREE_DATAMOVE(b->elems, b->elems + num - 1,
+			BPS_TREE_DATAMOVE(b->elems, b->elems + (num - 1),
 					  b->header.size - num, b, b);
 	}
 
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations

[Timur Safin]

I've looked into all patches of this patchset, and they look 
very much ok (especially the Guava story).

Have no energy to send LGTM to each part, thus sending it for
the whole set:

LGTM

Timur

: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Subject: [PATCH 00/11] Enable miscelaneous sanitations
: 
: The patchset is a second part of the undefined behaviour fixes. It
: is based on the UB alignment patchset, and enables all the other
: UB checks except a few last ones, described in the first commit.
: 
: Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4609-
: sanitize-misc-full-ci
: Issue: https://github.com/tarantool/tarantool/issues/4609
: 
: Aleksandr Lyapunov (1):
:   salad: fix UB pointer arithmetics in bps_tree
: 
: Vladislav Shpilevoy (10):
:   cmake: enable misc types of UB detection in clang
:   util: introduce double_compare_nint64()
:   test: avoid usleep() usage for error injections
:   vinyl: fix 0 division in case of canceled dump
:   xrow: don't cast double to float unconditionally
:   swim: fix zero division
:   test: fix signed integer overflow in vclock test
:   digest: eliminate UBs from guava()
:   sql: fix usage of not initialized index_stat
:   sql: fix mem_apply_type double type truncation
: 
:  cmake/compiler.cmake        | 16 ++++++++++++++-
:  src/box/sql/vdbe.c          | 18 +++++++++++------
:  src/box/sql/vdbeaux.c       | 16 +++++++--------
:  src/box/sql/where.c         |  4 ++--
:  src/box/tuple_compare.cc    |  9 ++-------
:  src/box/vy_regulator.c      | 10 ++++++----
:  src/box/vy_run.c            |  2 +-
:  src/box/vy_scheduler.c      |  2 +-
:  src/box/xrow_update_field.c | 19 +++++++++---------
:  src/lib/core/util.c         | 39 ++++++++++++++++++++++++++++++++++++-
:  src/lib/salad/bps_tree.h    | 14 ++++++-------
:  src/lib/salad/guava.c       | 18 ++++++++++-------
:  src/lib/salad/guava.h       |  2 +-
:  src/lib/swim/swim.c         | 10 +++++-----
:  src/lua/digest.lua          |  2 +-
:  src/trivia/util.h           | 29 +++++++++++++++++++++++++++
:  test/app/digest.result      |  2 +-
:  test/box/update.result      |  2 +-
:  test/box/update.test.lua    |  2 +-
:  test/unit/vclock.cc         | 28 +++++++++++++-------------
:  test/unit/vclock.result     |  2 +-
:  21 files changed, 167 insertions(+), 79 deletions(-)
: 
: --
: 2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations

[Cyrill Gorcunov]

lgtm

On Fri, Jun 5, 2020 at 2:43 AM Vladislav Shpilevoy
<v.shpilevoy@tarantool.org> wrote:
>
> The patchset is a second part of the undefined behaviour fixes. It
> is based on the UB alignment patchset, and enables all the other
> UB checks except a few last ones, described in the first commit.
>
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4609-sanitize-misc-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4609
>
> Aleksandr Lyapunov (1):
>   salad: fix UB pointer arithmetics in bps_tree
>
> Vladislav Shpilevoy (10):
>   cmake: enable misc types of UB detection in clang
>   util: introduce double_compare_nint64()
>   test: avoid usleep() usage for error injections
>   vinyl: fix 0 division in case of canceled dump
>   xrow: don't cast double to float unconditionally
>   swim: fix zero division
>   test: fix signed integer overflow in vclock test
>   digest: eliminate UBs from guava()
>   sql: fix usage of not initialized index_stat
>   sql: fix mem_apply_type double type truncation
>
>  cmake/compiler.cmake        | 16 ++++++++++++++-
>  src/box/sql/vdbe.c          | 18 +++++++++++------
>  src/box/sql/vdbeaux.c       | 16 +++++++--------
>  src/box/sql/where.c         |  4 ++--
>  src/box/tuple_compare.cc    |  9 ++-------
>  src/box/vy_regulator.c      | 10 ++++++----
>  src/box/vy_run.c            |  2 +-
>  src/box/vy_scheduler.c      |  2 +-
>  src/box/xrow_update_field.c | 19 +++++++++---------
>  src/lib/core/util.c         | 39 ++++++++++++++++++++++++++++++++++++-
>  src/lib/salad/bps_tree.h    | 14 ++++++-------
>  src/lib/salad/guava.c       | 18 ++++++++++-------
>  src/lib/salad/guava.h       |  2 +-
>  src/lib/swim/swim.c         | 10 +++++-----
>  src/lua/digest.lua          |  2 +-
>  src/trivia/util.h           | 29 +++++++++++++++++++++++++++
>  test/app/digest.result      |  2 +-
>  test/box/update.result      |  2 +-
>  test/box/update.test.lua    |  2 +-
>  test/unit/vclock.cc         | 28 +++++++++++++-------------
>  test/unit/vclock.result     |  2 +-
>  21 files changed, 167 insertions(+), 79 deletions(-)
>
> --
> 2.21.1 (Apple Git-122.3)
>

Re: [Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations

[Kirill Yukhin]

Hello,

On 05 июн 01:43, Vladislav Shpilevoy wrote:
> The patchset is a second part of the undefined behaviour fixes. It
> is based on the UB alignment patchset, and enables all the other
> UB checks except a few last ones, described in the first commit.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4609-sanitize-misc-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4609
> 
> Aleksandr Lyapunov (1):
>   salad: fix UB pointer arithmetics in bps_tree
> 
> Vladislav Shpilevoy (10):
>   cmake: enable misc types of UB detection in clang
>   util: introduce double_compare_nint64()
>   test: avoid usleep() usage for error injections
>   vinyl: fix 0 division in case of canceled dump
>   xrow: don't cast double to float unconditionally
>   swim: fix zero division
>   test: fix signed integer overflow in vclock test
>   digest: eliminate UBs from guava()
>   sql: fix usage of not initialized index_stat
>   sql: fix mem_apply_type double type truncation

I've checked your patchset into master.

--
Regards, Kirill YUkhin