* [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.
@ 2023-08-24 10:02 Sergey Kaplun via Tarantool-patches
2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-08-24 10:02 UTC (permalink / raw)
To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches
From: Mike Pall <mike>
Reported by Egor Skriptunoff.
(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
Binary number with fractional part is invalid. Parsing of such binary
numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
part, because it gives non-zero power of the exponent (`ex2`) in
`strscan_bin()`. But binary numbers with a zero fractional part
considered as valid numbers. To avoid such inconsistency the check, that
the given base doesn't equal 2, is added, when parsing decimal point
part of the literal.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#8825
---
Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
Issue: https://github.com/tarantool/tarantool/issues/8825
ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
Tarantool's CI is red, because static build on aarch64 can't fetch the
LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
src/lj_strscan.c | 1 +
.../fix-binary-number-parsing.test.lua | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
diff --git a/src/lj_strscan.c b/src/lj_strscan.c
index 11d341ee..a2d92714 100644
--- a/src/lj_strscan.c
+++ b/src/lj_strscan.c
@@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
/* Handle decimal point. */
if (dp) {
+ if (base == 2) return STRSCAN_ERROR;
fmt = STRSCAN_NUM;
if (dig) {
ex = (int32_t)(dp-(p-1)); dp = p-1;
diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
new file mode 100644
index 00000000..df82bd0b
--- /dev/null
+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
@@ -0,0 +1,17 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect behaviour of binary number
+-- parsing with fractional dot.
+-- See also:
+-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
+local test = tap.test('fix-binary-number-parsing')
+test:plan(2)
+
+-- Test that incorrect literal with non-0 fractional part still
+-- can't be converted to number.
+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
+-- Test that incorrect literal with 0 fractional part can't be
+-- converted to number.
+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
+
+test:done(true)
--
2.41.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.
2023-08-24 10:02 [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing Sergey Kaplun via Tarantool-patches
@ 2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
2023-08-25 12:01 ` Sergey Kaplun via Tarantool-patches
2023-08-28 14:02 ` Sergey Bronnikov via Tarantool-patches
2023-08-31 15:19 ` Igor Munkin via Tarantool-patches
2 siblings, 1 reply; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-08-25 10:12 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3175 bytes --]
Hi, Sergey!
Thanks for the patch!
LGTM, except for a few comments below.
>
>>From: Mike Pall <mike>
>>
>>Reported by Egor Skriptunoff.
>>
>>(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
>>
>>Binary number with fractional part is invalid. Parsing of such binary
>Typo: s/Binary/ A binary/
>Typo: s/fractional/a fractional/
>>numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
>Typo: s/with/with a/
>>part, because it gives non-zero power of the exponent (`ex2`) in
>Typo: s/part,/part/
>Typo: s/non-zero/a non-zero/
>>`strscan_bin()`. But binary numbers with a zero fractional part
>>considered as valid numbers. To avoid such inconsistency the check, that
>Typo: s/considered as/are considered/
>>the given base doesn't equal 2, is added, when parsing decimal point
>Typo: s/parsing/parsing the/
>>part of the literal.
>>
>>Sergey Kaplun:
>>* added the description and the test for the problem
>>
>>Part of tarantool/tarantool#8825
>>---
>>
>>Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
>>Issue: https://github.com/tarantool/tarantool/issues/8825
>>ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
>>Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
>>
>>Tarantool's CI is red, because static build on aarch64 can't fetch the
>>LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
>>
>> src/lj_strscan.c | 1 +
>> .../fix-binary-number-parsing.test.lua | 17 +++++++++++++++++
>> 2 files changed, 18 insertions(+)
>> create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
>>
>>diff --git a/src/lj_strscan.c b/src/lj_strscan.c
>>index 11d341ee..a2d92714 100644
>>--- a/src/lj_strscan.c
>>+++ b/src/lj_strscan.c
>>@@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>>
>> /* Handle decimal point. */
>> if (dp) {
>>+ if (base == 2) return STRSCAN_ERROR;
>> fmt = STRSCAN_NUM;
>> if (dig) {
>> ex = (int32_t)(dp-(p-1)); dp = p-1;
>>diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
>>new file mode 100644
>>index 00000000..df82bd0b
>>--- /dev/null
>>+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
>>@@ -0,0 +1,17 @@
>>+local tap = require('tap')
>>+
>>+-- Test file to demonstrate incorrect behaviour of binary number
>>+-- parsing with fractional dot.
>>+-- See also:
>>+-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
>>+local test = tap.test('fix-binary-number-parsing')
>>+test:plan(2)
>>+
>>+-- Test that incorrect literal with non-0 fractional part still
>Typo: s/that/that an/
>Typo: s/with/with a/
>>+-- can't be converted to number.
>Typo: s/number/a number/
>>+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
>>+-- Test that incorrect literal with 0 fractional part can't be
>Typo: s/that/that an/
>>+-- converted to number.
>Typo: s/number/a number/
>>+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
>>+
>>+test:done(true)
>>--
>>2.41.0
>--
>Best regards,
>Maxim Kokryashkin
>
[-- Attachment #2: Type: text/html, Size: 5902 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.
2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-08-25 12:01 ` Sergey Kaplun via Tarantool-patches
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-08-25 12:01 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
Hi, Maxim!
Thanks for the review!
Fixed your comments inline, rebased branch on the current master and
force-pushed it.
On 25.08.23, Maxim Kokryashkin wrote:
>
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few comments below.
> >
> >>From: Mike Pall <mike>
> >>
> >>Reported by Egor Skriptunoff.
> >>
> >>(cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
> >>
> >>Binary number with fractional part is invalid. Parsing of such binary
> >Typo: s/Binary/ A binary/
> >Typo: s/fractional/a fractional/
Fixed.
> >>numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> >Typo: s/with/with a/
Fixed.
> >>part, because it gives non-zero power of the exponent (`ex2`) in
> >Typo: s/part,/part/
Fixed.
> >Typo: s/non-zero/a non-zero/
Fixed.
> >>`strscan_bin()`. But binary numbers with a zero fractional part
> >>considered as valid numbers. To avoid such inconsistency the check, that
> >Typo: s/considered as/are considered/
Fixed, thanks.
> >>the given base doesn't equal 2, is added, when parsing decimal point
> >Typo: s/parsing/parsing the/
Fixed.
> >>part of the literal.
> >>
> >>Sergey Kaplun:
> >>* added the description and the test for the problem
> >>
> >>Part of tarantool/tarantool#8825
> >>---
<snipped>
> >>+
> >>+-- Test that incorrect literal with non-0 fractional part still
> >Typo: s/that/that an/
> >Typo: s/with/with a/
Fixed.
> >>+-- can't be converted to number.
> >Typo: s/number/a number/
Fixed.
> >>+test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
> >>+-- Test that incorrect literal with 0 fractional part can't be
> >Typo: s/that/that an/
Fixed.
> >>+-- converted to number.
> >Typo: s/number/a number/
Fixed.
See the iterative diff below:
===================================================================
diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
index df82bd0b..9f149082 100644
--- a/test/tarantool-tests/fix-binary-number-parsing.test.lua
+++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
@@ -7,11 +7,11 @@ local tap = require('tap')
local test = tap.test('fix-binary-number-parsing')
test:plan(2)
--- Test that incorrect literal with non-0 fractional part still
--- can't be converted to number.
+-- Test that an incorrect literal with a non-0 fractional part
+-- still can't be converted to a number.
test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
--- Test that incorrect literal with 0 fractional part can't be
--- converted to number.
+-- Test that an incorrect literal with 0 fractional part can't be
+-- converted to a number.
test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
test:done(true)
===================================================================
> >>+test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
> >>+
> >>+test:done(true)
> >>--
> >>2.41.0
> >--
> >Best regards,
> >Maxim Kokryashkin
> >
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.
2023-08-24 10:02 [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing Sergey Kaplun via Tarantool-patches
2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-08-28 14:02 ` Sergey Bronnikov via Tarantool-patches
2023-08-31 15:19 ` Igor Munkin via Tarantool-patches
2 siblings, 0 replies; 5+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-08-28 14:02 UTC (permalink / raw)
To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches
Sergey, thanks for the patch! LGTM. Sergey
On 8/24/23 13:02, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Egor Skriptunoff.
>
> (cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
>
> Binary number with fractional part is invalid. Parsing of such binary
> numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> part, because it gives non-zero power of the exponent (`ex2`) in
> `strscan_bin()`. But binary numbers with a zero fractional part
> considered as valid numbers. To avoid such inconsistency the check, that
> the given base doesn't equal 2, is added, when parsing decimal point
> part of the literal.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#8825
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
> Issue: https://github.com/tarantool/tarantool/issues/8825
> ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
>
> Tarantool's CI is red, because static build on aarch64 can't fetch the
> LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
>
> src/lj_strscan.c | 1 +
> .../fix-binary-number-parsing.test.lua | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+)
> create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
>
> diff --git a/src/lj_strscan.c b/src/lj_strscan.c
> index 11d341ee..a2d92714 100644
> --- a/src/lj_strscan.c
> +++ b/src/lj_strscan.c
> @@ -444,6 +444,7 @@ StrScanFmt lj_strscan_scan(const uint8_t *p, MSize len, TValue *o,
>
> /* Handle decimal point. */
> if (dp) {
> + if (base == 2) return STRSCAN_ERROR;
> fmt = STRSCAN_NUM;
> if (dig) {
> ex = (int32_t)(dp-(p-1)); dp = p-1;
> diff --git a/test/tarantool-tests/fix-binary-number-parsing.test.lua b/test/tarantool-tests/fix-binary-number-parsing.test.lua
> new file mode 100644
> index 00000000..df82bd0b
> --- /dev/null
> +++ b/test/tarantool-tests/fix-binary-number-parsing.test.lua
> @@ -0,0 +1,17 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate incorrect behaviour of binary number
> +-- parsing with fractional dot.
> +-- See also:
> +-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> +local test = tap.test('fix-binary-number-parsing')
> +test:plan(2)
> +
> +-- Test that incorrect literal with non-0 fractional part still
> +-- can't be converted to number.
> +test:is(tonumber('0b.1'), nil, '0b.1 is not converted')
> +-- Test that incorrect literal with 0 fractional part can't be
> +-- converted to number.
> +test:is(tonumber('0b.0'), nil, '0b.0 is not converted')
> +
> +test:done(true)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing.
2023-08-24 10:02 [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing Sergey Kaplun via Tarantool-patches
2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
2023-08-28 14:02 ` Sergey Bronnikov via Tarantool-patches
@ 2023-08-31 15:19 ` Igor Munkin via Tarantool-patches
2 siblings, 0 replies; 5+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-08-31 15:19 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
Sergey,
I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.
On 24.08.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
>
> Reported by Egor Skriptunoff.
>
> (cherry-picked from commit 377a8488b62a9f1b589bb68875dd1288aa70e76e)
>
> Binary number with fractional part is invalid. Parsing of such binary
> numbers returns `STRSCAN_ERROR` for any numbers with non-zero fractional
> part, because it gives non-zero power of the exponent (`ex2`) in
> `strscan_bin()`. But binary numbers with a zero fractional part
> considered as valid numbers. To avoid such inconsistency the check, that
> the given base doesn't equal 2, is added, when parsing decimal point
> part of the literal.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#8825
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-binary-number-parsing
> Issue: https://github.com/tarantool/tarantool/issues/8825
> ML: https://www.freelists.org/post/luajit/Fractional-binary-number-literals
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9028
>
> Tarantool's CI is red, because static build on aarch64 can't fetch the
> LuaJIT's submodule commit somehow. Looks unrelated to the commit:).
>
> src/lj_strscan.c | 1 +
> .../fix-binary-number-parsing.test.lua | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+)
> create mode 100644 test/tarantool-tests/fix-binary-number-parsing.test.lua
>
<snipped>
> --
> 2.41.0
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-31 15:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-24 10:02 [Tarantool-patches] [PATCH luajit] Fix binary number literal parsing Sergey Kaplun via Tarantool-patches
2023-08-25 10:12 ` Maxim Kokryashkin via Tarantool-patches
2023-08-25 12:01 ` Sergey Kaplun via Tarantool-patches
2023-08-28 14:02 ` Sergey Bronnikov via Tarantool-patches
2023-08-31 15:19 ` Igor Munkin via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox