[tarantool-patches] [PATCH v3 4/4] sql: Region-based allocations.
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Apr 25 19:52:23 MSK 2018
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
More information about the Tarantool-patches
mailing list