Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, tsafin@tarantool.org,
	alyapunov@tarantool.org
Subject: [Tarantool-patches] [PATCH 05/11] xrow: don't cast double to float unconditionally
Date: Fri,  5 Jun 2020 01:43:12 +0200	[thread overview]
Message-ID: <7a8f49d2b31421f6672d5e4c6d47aff8e4d69a6a.1591313754.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1591313754.git.v.shpilevoy@tarantool.org>

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)

  parent reply	other threads:[~2020-06-04 23:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04 23:43 [Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 01/11] cmake: enable misc types of UB detection in clang Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 10/11] sql: fix usage of not initialized index_stat Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 11/11] sql: fix mem_apply_type double type truncation Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 02/11] util: introduce double_compare_nint64() Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 03/11] test: avoid usleep() usage for error injections Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 04/11] vinyl: fix 0 division in case of canceled dump Vladislav Shpilevoy
2020-06-04 23:43 ` Vladislav Shpilevoy [this message]
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 06/11] swim: fix zero division Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 07/11] test: fix signed integer overflow in vclock test Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 08/11] digest: eliminate UBs from guava() Vladislav Shpilevoy
2020-06-04 23:43 ` [Tarantool-patches] [PATCH 09/11] salad: fix UB pointer arithmetics in bps_tree Vladislav Shpilevoy
2020-06-05 22:09 ` [Tarantool-patches] [PATCH 00/11] Enable miscelaneous sanitations Timur Safin
2020-06-09  8:19 ` Cyrill Gorcunov
2020-06-09  8:28 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a8f49d2b31421f6672d5e4c6d47aff8e4d69a6a.1591313754.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alyapunov@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=tsafin@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 05/11] xrow: don'\''t cast double to float unconditionally' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox