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)
next prev 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