Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>,
	Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit 2/2] Fix tonumber("-0") in dual-number mode.
Date: Wed, 26 Jan 2022 15:19:34 +0300	[thread overview]
Message-ID: <6ab9d085d486a481362259063598825a14f6876b.1643199076.git.skaplun@tarantool.org> (raw)
In-Reply-To: <cover.1643199076.git.skaplun@tarantool.org>

From: Mike Pall <mike>

Reported by Sergey Kaplun.

For DUALNUM build `tonumber("-0x0")` or `tonumber("-0")` returns 0
instead -0 due to the STRSCAN_OPT_TOINT option of `lj_strscan_scan()`
and cast value to integer with losing information about its sign.

This patch adds special checks for these corner cases during strscan.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#6548
---
 src/lj_strscan.c                                |  8 ++++++--
 test/tarantool-tests/lj-528-tonumber-0.test.lua | 10 ++--------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/lj_strscan.c b/src/lj_strscan.c
index 4e4ef6d3..11d341ee 100644
--- a/src/lj_strscan.c
+++ b/src/lj_strscan.c
@@ -121,7 +121,8 @@ static StrScanFmt strscan_hex(const uint8_t *p, TValue *o,
   /* Format-specific handling. */
   switch (fmt) {
   case STRSCAN_INT:
-    if (!(opt & STRSCAN_OPT_TONUM) && x < 0x80000000u+neg) {
+    if (!(opt & STRSCAN_OPT_TONUM) && x < 0x80000000u+neg &&
+	!(x == 0 && neg)) {
       o->i = neg ? -(int32_t)x : (int32_t)x;
       return STRSCAN_INT;  /* Fast path for 32 bit integers. */
     }
@@ -498,6 +499,9 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       if ((opt & STRSCAN_OPT_TONUM)) {
 	o->n = neg ? -(double)x : (double)x;
 	return STRSCAN_NUM;
+      } else if (x == 0 && neg) {
+	o->n = -0.0;
+	return STRSCAN_NUM;
       } else {
 	o->i = neg ? -(int32_t)x : (int32_t)x;
 	return STRSCAN_INT;
@@ -515,7 +519,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
       fmt = strscan_dec(sp, o, fmt, opt, ex, neg, dig);
 
     /* Try to convert number to integer, if requested. */
-    if (fmt == STRSCAN_NUM && (opt & STRSCAN_OPT_TOINT)) {
+    if (fmt == STRSCAN_NUM && (opt & STRSCAN_OPT_TOINT) && !tvismzero(o)) {
       double n = o->n;
       int32_t i = lj_num2int(n);
       if (n == (lua_Number)i) { o->i = i; return STRSCAN_INT; }
diff --git a/test/tarantool-tests/lj-528-tonumber-0.test.lua b/test/tarantool-tests/lj-528-tonumber-0.test.lua
index 03ba2aff..f3ad53d3 100644
--- a/test/tarantool-tests/lj-528-tonumber-0.test.lua
+++ b/test/tarantool-tests/lj-528-tonumber-0.test.lua
@@ -1,15 +1,9 @@
 local tap = require('tap')
 
--- Test disabled for DUALNUM mode default for some arches.
--- See also https://github.com/LuaJIT/LuaJIT/pull/787.
-require("utils").skipcond(
-  jit.arch ~= "x86" and jit.arch ~= "x64",
-  jit.arch.." in DUALNUM mode is clumsy for now"
-)
-
 -- Test file to demonstrate LuaJIT `tonumber('-0')` incorrect
 -- behaviour.
--- See also https://github.com/LuaJIT/LuaJIT/issues/528.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/528,
+-- https://github.com/LuaJIT/LuaJIT/pull/787.
 local test = tap.test('lj-528-tonumber-0')
 test:plan(1)
 
-- 
2.34.1


  parent reply	other threads:[~2022-01-26 12:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 12:19 [Tarantool-patches] [PATCH luajit 0/2] Fix tonumber("-0") inconsistencies Sergey Kaplun via Tarantool-patches
2022-01-26 12:19 ` [Tarantool-patches] [PATCH luajit 1/2] Fix tonumber("-0") Sergey Kaplun via Tarantool-patches
2022-06-27 13:47   ` Igor Munkin via Tarantool-patches
2022-06-27 20:29   ` sergos via Tarantool-patches
2022-01-26 12:19 ` Sergey Kaplun via Tarantool-patches [this message]
2022-01-27  9:50   ` [Tarantool-patches] [PATCH luajit 2/2] Fix tonumber("-0") in dual-number mode Sergey Kaplun via Tarantool-patches
2022-06-27 13:48   ` Igor Munkin via Tarantool-patches
2022-06-27 15:51   ` sergos via Tarantool-patches
2022-06-30 12:09 ` [Tarantool-patches] [PATCH luajit 0/2] Fix tonumber("-0") inconsistencies Igor Munkin via Tarantool-patches

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=6ab9d085d486a481362259063598825a14f6876b.1643199076.git.skaplun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 2/2] Fix tonumber("-0") in dual-number mode.' \
    /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