[PATCH] sql: fix error handling in sql_analysis_load()

[PATCH] sql: fix error handling in sql_analysis_load()

[Serge Petrenko]

Previously if an error occured in box_index_len() called from
sql_analysis_load(), the return code (-1 on error) was cast to uint32_t
and used later as size of memory to be allocated. This lead to assertion
failures in slab_order() since allocation size was too big. This was
discovered during investigation of #3779.
Fix error handling and add some error logging.

Follow-up #3779
---
https://github.com/tarantool/tarantool/tree/sp/slab-assert-fix
https://github.com/tarantool/tarantool/issues/3779

 src/box/sql.c         | 7 +++++--
 src/box/sql/analyze.c | 4 +++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index d7df84874..49e1237d7 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -90,9 +90,12 @@ sql_load_schema()
 	if (stat->def->field_count == 0)
 		return;
 	db->init.busy = 1;
-	sql_analysis_load(db);
-	if (db->errCode != SQLITE_OK)
+	if (sql_analysis_load(db) != SQLITE_OK) {
+		if(!diag_is_empty(&fiber()->diag)) {
+			diag_log();
+		}
 		panic("failed to initialize SQL subsystem");
+	}
 	db->init.busy = 0;
 }
 
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index e786c9019..3f492800c 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1682,7 +1682,9 @@ stat_copy(struct index_stat *dest, const struct index_stat *src)
 int
 sql_analysis_load(struct sqlite3 *db)
 {
-	uint32_t index_count = box_index_len(BOX_SQL_STAT1_ID, 0);
+	ssize_t index_count = box_index_len(BOX_SQL_STAT1_ID, 0);
+	if (index_count < 0)
+		return SQL_TARANTOOL_ERROR;
 	if (box_txn_begin() != 0)
 		goto fail;
 	size_t stats_size = index_count * sizeof(struct index_stat);
-- 
2.17.2 (Apple Git-113)

Re: [PATCH] sql: fix error handling in sql_analysis_load()

[Vladimir Davydov]

On Thu, Nov 22, 2018 at 07:59:03PM +0300, Serge Petrenko wrote:
> Previously if an error occured in box_index_len() called from
> sql_analysis_load(), the return code (-1 on error) was cast to uint32_t
> and used later as size of memory to be allocated. This lead to assertion
> failures in slab_order() since allocation size was too big. This was
> discovered during investigation of #3779.
> Fix error handling and add some error logging.
> 
> Follow-up #3779

Pushed to 2.1