Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: v.shpilevoy@tarantool.org, Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] [PATCH 1/4] sql: raise an error if int is overflowed during math operations
Date: Wed, 20 Feb 2019 14:57:37 +0300	[thread overview]
Message-ID: <df66ac8c51103722368dc17949c653b0b154bef9.1550663540.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1550663540.git.korablev@tarantool.org>
In-Reply-To: <cover.1550663540.git.korablev@tarantool.org>

Before this patch, if integer was overflowed during math operations
(OP_Add, OP_Subtract, OP_Multiply, OP_Divide), it would be implicitly
converted and stored as floating point number. This is obviously wrong
way to handle integer overflow errors. Instead, let's raise
corresponding error.

Part of #3735
---
 src/box/sql/vdbe.c                 | 13 +++++++-----
 test/sql/integer-overflow.result   | 42 ++++++++++++++++++++++++++++++++++++++
 test/sql/integer-overflow.test.lua | 16 +++++++++++++++
 3 files changed, 66 insertions(+), 5 deletions(-)
 create mode 100644 test/sql/integer-overflow.result
 create mode 100644 test/sql/integer-overflow.test.lua

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c370b81ec..d9797b22b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1614,13 +1614,13 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		iB = pIn2->u.i;
 		bIntint = 1;
 		switch( pOp->opcode) {
-		case OP_Add:       if (sqlAddInt64(&iB,iA)) goto fp_math;  break;
-		case OP_Subtract:  if (sqlSubInt64(&iB,iA)) goto fp_math;  break;
-		case OP_Multiply:  if (sqlMulInt64(&iB,iA)) goto fp_math;  break;
+		case OP_Add:       if (sqlAddInt64(&iB,iA)) goto integer_overflow; break;
+		case OP_Subtract:  if (sqlSubInt64(&iB,iA)) goto integer_overflow; break;
+		case OP_Multiply:  if (sqlMulInt64(&iB,iA)) goto integer_overflow; break;
 		case OP_Divide: {
 			if (iA == 0)
 				goto division_by_zero;
-			if (iA==-1 && iB==SMALLEST_INT64) goto fp_math;
+			if (iA==-1 && iB==SMALLEST_INT64) goto integer_overflow;
 			iB /= iA;
 			break;
 		}
@@ -1636,7 +1636,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		MemSetTypeFlag(pOut, MEM_Int);
 	} else {
 		bIntint = 0;
-	fp_math:
 		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "numeric");
@@ -1694,6 +1693,10 @@ division_by_zero:
 	diag_set(ClientError, ER_SQL_EXECUTE, "division by zero");
 	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
+integer_overflow:
+	diag_set(ClientError, ER_SQL_EXECUTE, "integer is overflowed");
+	rc = SQL_TARANTOOL_ERROR;
+	goto abort_due_to_error;
 }
 
 /* Opcode: CollSeq P1 * * P4
diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result
new file mode 100644
index 000000000..fedcd4290
--- /dev/null
+++ b/test/sql/integer-overflow.result
@@ -0,0 +1,42 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+-- gh-3735: make sure that integer overflows errors are
+-- handled during VDBE execution.
+--
+box.sql.execute('SELECT (2147483647 * 2147483647 * 2147483647);')
+---
+- error: 'Failed to execute SQL statement: integer is overflowed'
+...
+box.sql.execute('SELECT (-9223372036854775808 / -1);')
+---
+- error: 'Failed to execute SQL statement: integer is overflowed'
+...
+box.sql.execute('SELECT (-9223372036854775808 - 1);')
+---
+- error: 'Failed to execute SQL statement: integer is overflowed'
+...
+box.sql.execute('SELECT (9223372036854775807 + 1);')
+---
+- error: 'Failed to execute SQL statement: integer is overflowed'
+...
+-- Literals are checked right after parsing.
+--
+box.sql.execute('SELECT 9223372036854775808;')
+---
+- error: 'oversized integer: 9223372036854775808'
+...
+box.sql.execute('SELECT -9223372036854775809;')
+---
+- error: 'oversized integer: -9223372036854775809'
+...
+box.sql.execute('SELECT 9223372036854775808 - 1;')
+---
+- error: 'oversized integer: 9223372036854775808'
+...
diff --git a/test/sql/integer-overflow.test.lua b/test/sql/integer-overflow.test.lua
new file mode 100644
index 000000000..49ba5cd8f
--- /dev/null
+++ b/test/sql/integer-overflow.test.lua
@@ -0,0 +1,16 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+-- gh-3735: make sure that integer overflows errors are
+-- handled during VDBE execution.
+--
+box.sql.execute('SELECT (2147483647 * 2147483647 * 2147483647);')
+box.sql.execute('SELECT (-9223372036854775808 / -1);')
+box.sql.execute('SELECT (-9223372036854775808 - 1);')
+box.sql.execute('SELECT (9223372036854775807 + 1);')
+-- Literals are checked right after parsing.
+--
+box.sql.execute('SELECT 9223372036854775808;')
+box.sql.execute('SELECT -9223372036854775809;')
+box.sql.execute('SELECT 9223372036854775808 - 1;')
-- 
2.15.1

  reply	other threads:[~2019-02-20 11:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-20 11:57 [tarantool-patches] [PATCH 0/4] Fix integer overflow behaviour during VDBE execution Nikita Pettik
2019-02-20 11:57 ` Nikita Pettik [this message]
2019-02-20 11:57 ` [tarantool-patches] [PATCH 2/4] sql: raise an integer overflow error during CAST Nikita Pettik
2019-02-20 11:57 ` [tarantool-patches] [PATCH 3/4] sql: refactor sqlVdbeMsgpackGet() Nikita Pettik
2019-02-20 11:57 ` [tarantool-patches] [PATCH 4/4] sql: raise integer overflow error during msgpack decode Nikita Pettik
2019-02-20 18:25   ` [tarantool-patches] " Konstantin Osipov
2019-02-20 18:39     ` n.pettik
2019-02-20 18:46       ` Konstantin Osipov
2019-02-22 18:30 ` [tarantool-patches] Re: [PATCH 0/4] Fix integer overflow behaviour during VDBE execution Vladislav Shpilevoy
2019-02-25 11:58 ` 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=df66ac8c51103722368dc17949c653b0b154bef9.1550663540.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 1/4] sql: raise an error if int is overflowed during math operations' \
    /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