[tarantool-patches] [PATCH v3 4/4] sql: Region-based allocations.

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Function sql_field_retrieve could leak names
in error cases. Let allocate all memory with
region allocator.

Needed for #3272.
---
 src/box/sql.c           | 44 +++++++++++++++++++++++++++++++-------------
 src/box/sql.h           |  9 +++++++++
 src/box/sql/build.c     | 37 ++++++++++++++++++++++++-------------
 src/box/sql/prepare.c   | 26 ++++++++++++++++----------
 src/box/sql/select.c    | 24 +++++++++++++++++++++---
 src/box/sql/sqliteInt.h |  2 ++
 src/box/sql/tokenize.c  |  3 +++
 src/box/sql/trigger.c   |  2 ++
 8 files changed, 108 insertions(+), 39 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 6d4255e..2893d70 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1685,22 +1685,43 @@ space_column_default_expr(uint32_t space_id, uint32_t fieldno)
 	return space->def->fields[fieldno].default_value_expr;
 }
 
+struct space_def *
+sql_ephemeral_space_def_new(Parse *parser)
+{
+	struct space_def *def = NULL;
+	struct region *region = &fiber()->gc;
+	size_t size = sizeof(struct space_def) + 1;
+	def = (struct space_def *)region_alloc(region, size);
+	if (def != NULL) {
+		memset(def, 0, size);
+		def->dict =
+			(struct tuple_dictionary *)
+				region_alloc(region,
+					     sizeof(struct tuple_dictionary));
+	}
+	if (def == NULL || def->dict == NULL) {
+		parser->rc = SQLITE_NOMEM_BKPT;
+		parser->nErr++;
+		return NULL;
+	}
+	def->dict->refs = 1;
+	def->opts.temporary = true;
+	return def;
+}
+
 Table *
 sql_ephemeral_table_new(Parse *parser)
 {
 	sqlite3 *db = parser->db;
 	struct space_def *def = NULL;
 	Table *table = sqlite3DbMallocZero(db, sizeof(Table));
-	if (table != NULL) {
-		def = space_def_new(0, 0, 0, NULL, 0, NULL, 0,
-				    &space_opts_default, NULL, 0);
-	}
+	if (table != NULL)
+		def = sql_ephemeral_space_def_new(parser);
 	if (def == NULL) {
 		sqlite3DbFree(db, table);
-		parser->rc = SQLITE_NOMEM_BKPT;
-		parser->nErr++;
 		return NULL;
 	}
+
 	table->def = def;
 	return table;
 }
@@ -1708,6 +1729,9 @@ sql_ephemeral_table_new(Parse *parser)
 int
 sql_table_def_rebuild(struct sqlite3 *db, struct Table *pTable)
 {
+	assert(pTable->def->opts.temporary == true);
+
+	/* All allocations are on region. */
 	struct space_def *old_def = pTable->def;
 	struct space_def *new_def = NULL;
 	new_def = space_def_new(old_def->id, old_def->uid,
@@ -1719,13 +1743,7 @@ sql_table_def_rebuild(struct sqlite3 *db, struct Table *pTable)
 		sqlite3OomFault(db);
 		return -1;
 	}
-	struct field_def *fields = old_def->fields;
-	for (uint32_t i = 0; i < old_def->field_count; ++i) {
-		sqlite3DbFree(db, fields[i].default_value);
-		sqlite3DbFree(db, fields[i].name);
-	}
-	space_def_delete(old_def);
-	sqlite3DbFree(db, fields);
 	pTable->def = new_def;
+	pTable->def->opts.temporary = false;
 	return 0;
 }
diff --git a/src/box/sql.h b/src/box/sql.h
index 9fb3ad1..410653b 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -155,6 +155,15 @@ struct Table *
 sql_ephemeral_table_new(struct Parse *parser);
 
 /**
+ * Create and initialize a new ephemeric space_def object.
+ * @param pParse SQL Parser object.
+ * @retval NULL on memory allocation error, Parser state changed.
+ * @retval not NULL on success.
+ */
+struct space_def *
+sql_ephemeral_space_def_new(struct Parse *parser);
+
+/**
  * Rebuild struct def in Table with memory allocated on a single
  * malloc. Fields and strings are expected to be allocated with
  * sqlite3DbMalloc.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 584e6b1..4db4356 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -386,7 +386,9 @@ deleteTable(sqlite3 * db, Table * pTable)
 	sqlite3DbFree(db, pTable->zColAff);
 	sqlite3SelectDelete(db, pTable->pSelect);
 	sqlite3ExprListDelete(db, pTable->pCheck);
-	if (pTable->def != NULL)
+	/* Do not delete pTable->def allocated not on region. */
+	assert(pTable->def != NULL);
+	if (pTable->def->opts.temporary == false)
 		space_def_delete(pTable->def);
 	sqlite3DbFree(db, pTable);
 
@@ -483,6 +485,7 @@ sqlite3PrimaryKeyIndex(Table * pTab)
 
 /**
  * Create and initialize a new SQL Table object.
+ * All memory is allocated on region.
  * @param parser SQL Parser object.
  * @param name Table to create name.
  * @retval NULL on memory allocation error.
@@ -603,29 +606,33 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
 static struct field_def *
 sql_field_retrieve(Parse *parser, Table *table, uint32_t id)
 {
-	sqlite3 *db = parser->db;
 	struct field_def *field;
 	assert(table->def != NULL);
 	assert(id < SQLITE_MAX_COLUMN);
 
 	if (id >= table->def->exact_field_count) {
-		uint32_t columns = table->def->exact_field_count;
-		columns = (columns > 0) ? 2 * columns : 1;
-		field = sqlite3DbRealloc(db, table->def->fields,
-					 columns * sizeof(table->def->fields[0]));
+		uint32_t columns_new = table->def->exact_field_count;
+		columns_new = (columns_new > 0) ? 2 * columns_new : 1;
+		struct region *region = &fiber()->gc;
+		field = region_alloc(region,
+				     columns_new * sizeof(table->def->fields[0]));
 		if (field == NULL) {
 			parser->rc = SQLITE_NOMEM_BKPT;
 			parser->nErr++;
 			return NULL;
 		}
 
-		for (uint32_t i = columns / 2; i < columns; i++) {
+		for (uint32_t i = 0; i < table->def->exact_field_count; i++) {
+			memcpy(&field[i], &table->def->fields[i],
+			       sizeof(struct field_def));
+		}
+		for (uint32_t i = columns_new / 2; i < columns_new; i++) {
 			memcpy(&field[i], &field_def_default,
 			       sizeof(struct field_def));
 		}
 
 		table->def->fields = field;
-		table->def->exact_field_count = columns;
+		table->def->exact_field_count = columns_new;
 	}
 
 	field = &table->def->fields[id];
@@ -651,6 +658,7 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
 	sqlite3 *db = pParse->db;
 	if ((p = pParse->pNewTable) == 0)
 		return;
+	assert(p->def->opts.temporary == true);
 #if SQLITE_MAX_COLUMN
 	if ((int)p->def->field_count + 1 > db->aLimit[SQLITE_LIMIT_COLUMN]) {
 		sqlite3ErrorMsg(pParse, "too many columns on %s", p->zName);
@@ -659,7 +667,8 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
 #endif
 	if (sql_field_retrieve(pParse, p, (uint32_t) p->def->field_count) == NULL)
 		return;
-	z = sqlite3DbMallocRaw(db, pName->n + 1);
+	struct region *region = &fiber()->gc;
+	z = region_alloc(region, pName->n + 1);
 	if (z == 0)
 		return;
 	memcpy(z, pName->z, pName->n);
@@ -668,7 +677,6 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
 	for (i = 0; i < (int)p->def->field_count; i++) {
 		if (strcmp(z, p->def->fields[i].name) == 0) {
 			sqlite3ErrorMsg(pParse, "duplicate column name: %s", z);
-			sqlite3DbFree(db, z);
 			return;
 		}
 	}
@@ -677,10 +685,8 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType)
 		aNew =
 		    sqlite3DbRealloc(db, p->aCol,
 				     (p->def->field_count + 8) * sizeof(p->aCol[0]));
-		if (aNew == 0) {
-			sqlite3DbFree(db, z);
+		if (aNew == 0)
 			return;
-		}
 		p->aCol = aNew;
 	}
 	pCol = &p->aCol[p->def->field_count];
@@ -848,6 +854,7 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan)
 	Table *p;
 	sqlite3 *db = pParse->db;
 	p = pParse->pNewTable;
+	assert(p->def->opts.temporary == true);
 	if (p != 0) {
 		if (!sqlite3ExprIsConstantOrFunction
 		    (pSpan->pExpr, db->init.busy)) {
@@ -1956,6 +1963,7 @@ sqlite3EndTable(Parse * pParse,	/* Parse context */
 	if (db->init.busy) {
 		Table *pOld;
 		Schema *pSchema = p->pSchema;
+		assert(p->def->opts.temporary == false);
 		pOld = sqlite3HashInsert(&pSchema->tblHash, p->zName, p);
 		if (pOld) {
 			assert(p == pOld);	/* Malloc must have failed inside HashInsert() */
@@ -2127,6 +2135,7 @@ sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
 								       pSel);
 			}
 		} else if (pSelTab) {
+			assert(pTable->def->opts.temporary == false);
 			/* CREATE VIEW name AS...  without an argument list.  Construct
 			 * the column names from the SELECT statement that defines the view.
 			 */
@@ -2167,6 +2176,8 @@ sqliteViewResetAll(sqlite3 * db)
 		if (pTab->pSelect) {
 			sqlite3DeleteColumnNames(db, pTab);
 			struct space_def *old_def = pTab->def;
+			assert(old_def->opts.temporary == false);
+			/* Ignore fields allocated on region */
 			pTab->def = space_def_new(old_def->id, old_def->uid,
 						  0,
 						  old_def->name,
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 2ab8751..7dc14f6 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -211,17 +211,19 @@ sqlite3InitDatabase(sqlite3 * db)
 void
 sqlite3ParserReset(Parse * pParse)
 {
-	if (pParse) {
-		sqlite3 *db = pParse->db;
-		sqlite3DbFree(db, pParse->aLabel);
-		sqlite3ExprListDelete(db, pParse->pConstExpr);
-		if (db) {
-			assert(db->lookaside.bDisable >=
-			       pParse->disableLookaside);
-			db->lookaside.bDisable -= pParse->disableLookaside;
-		}
-		pParse->disableLookaside = 0;
+	if (pParse == NULL)
+		return;
+	sqlite3 *db = pParse->db;
+	sqlite3DbFree(db, pParse->aLabel);
+	sqlite3ExprListDelete(db, pParse->pConstExpr);
+	if (db) {
+		assert(db->lookaside.bDisable >=
+		       pParse->disableLookaside);
+		db->lookaside.bDisable -= pParse->disableLookaside;
 	}
+	pParse->disableLookaside = 0;
+	struct region *region = &fiber()->gc;
+	region_truncate(region, pParse->region_initial_size);
 }
 
 /*
@@ -265,6 +267,10 @@ sqlite3Prepare(sqlite3 * db,	/* Database handle. */
 	 * works even if READ_UNCOMMITTED is set.
 	 */
 	sParse.db = db;
+	/* Store region initial size to revert future allocations */
+	struct region *region = &fiber()->gc;
+	sParse.region_initial_size = region_used(region);
+
 	if (nBytes >= 0 && (nBytes == 0 || zSql[nBytes - 1] != 0)) {
 		char *zSqlCopy;
 		int mxLen = db->aLimit[SQLITE_LIMIT_SQL_LENGTH];
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 9eeff8e..03bfcf9 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1822,9 +1822,20 @@ sqlite3ColumnsFromExprList(Parse * pParse,	/* Parsing context */
 		aCol = 0;
 	}
 	assert(nCol == (i16) nCol);
+
+	struct region *region = &fiber()->gc;
 	assert(pTable->def->fields == NULL);
+	if (pTable->def->opts.temporary == false) {
+		/* CREATE VIEW name AS...  without an argument list.  Construct
+		 * the column names from the SELECT statement that defines the view.
+		 */
+		pTable->def->field_count = 0;
+		space_def_delete(pTable->def);
+		pTable->def = sql_ephemeral_space_def_new(pParse);
+	}
 	pTable->def->fields =
-		sqlite3DbMallocZero(db, nCol*sizeof(pTable->def->fields[0]));
+		region_alloc(region, nCol*sizeof(pTable->def->fields[0]));
+	memset(pTable->def->fields, 0, nCol*sizeof(pTable->def->fields[0]));
 	pTable->def->field_count = (uint32_t)nCol;
 	pTable->aCol = aCol;
 
@@ -1877,9 +1888,16 @@ sqlite3ColumnsFromExprList(Parse * pParse,	/* Parsing context */
 			if (cnt > 3)
 				sqlite3_randomness(sizeof(cnt), &cnt);
 		}
-		pTable->def->fields[i].name = zName;
-		if (zName && sqlite3HashInsert(&ht, zName, pCol) == pCol) {
+		uint32_t zNameLen = (uint32_t)strlen(zName);
+		if (zName && sqlite3HashInsert(&ht, zName, pCol) == pCol)
+			sqlite3OomFault(db);
+		pTable->def->fields[i].name =
+			region_alloc(region, zNameLen + 1);
+		if (pTable->def->fields[i].name == NULL) {
 			sqlite3OomFault(db);
+		} else {
+			memcpy(pTable->def->fields[i].name, zName, zNameLen);
+			pTable->def->fields[i].name[zNameLen] = '\0';
 		}
 	}
 	sqlite3HashClear(&ht);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 94d5c80..520b74c 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2933,6 +2933,8 @@ struct Parse {
 	u8 eTriggerOp;		/* TK_UPDATE, TK_INSERT or TK_DELETE */
 	u8 eOrconf;		/* Default ON CONFLICT policy for trigger steps */
 	u8 disableTriggers;	/* True to disable triggers */
+	/** Region size at the Parser launch */
+	size_t region_initial_size;
 
   /**************************************************************************
   * Fields above must be initialized to zero.  The fields that follow,
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index c77aa9b..9055bd0 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -664,6 +664,9 @@ sql_expr_compile(sqlite3 *db, const char *expr, struct Expr **result)
 	memset(&parser, 0, sizeof(parser));
 	parser.db = db;
 	parser.parse_only = true;
+	struct region *region = &fiber()->gc;
+	parser.region_initial_size = region_used(region);
+
 	char *unused;
 	if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK) {
 		diag_set(ClientError, ER_SQL_EXECUTE, expr);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 28c56db..9b03be1 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -873,6 +873,8 @@ codeRowTrigger(Parse * pParse,	/* Current parse context */
 	pSubParse->pToplevel = pTop;
 	pSubParse->eTriggerOp = pTrigger->op;
 	pSubParse->nQueryLoop = pParse->nQueryLoop;
+	struct region *region = &fiber()->gc;
+	pSubParse->region_initial_size = region_used(region);
 
 	v = sqlite3GetVdbe(pSubParse);
 	if (v) {
-- 
2.7.4