From 692c160360c688c4bee3dd13d5293f2c416ad92f Mon Sep 17 00:00:00 2001 From: drh <> Date: Tue, 20 Aug 2024 19:09:59 +0000 Subject: [PATCH] Tighter checking of access constraints on union members in SrcItem. Improved invariant checking. FossilOrigin-Name: fd72d3400a8fe5747f494eee81654698acee350bb95b9db269e87d857af03492 --- manifest | 24 ++++++++++++------------ manifest.uuid | 2 +- src/build.c | 11 ++++++++++- src/delete.c | 2 +- src/fkey.c | 2 +- src/insert.c | 1 + src/select.c | 4 ++++ src/sqliteInt.h | 30 +++++++++++++++++------------- src/trigger.c | 6 ++++-- 9 files changed, 51 insertions(+), 31 deletions(-) diff --git a/manifest b/manifest index 614e098984..298d9f0ab3 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\ssome\sstray\sSrcItem\sfield\sname\sfixes. -D 2024-08-20T17:41:44.355 +C Tighter\schecking\sof\saccess\sconstraints\son\sunion\smembers\sin\sSrcItem.\nImproved\sinvariant\schecking. +D 2024-08-20T19:09:59.313 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -702,24 +702,24 @@ F src/btmutex.c 79a43670447eacc651519a429f6ece9fd638563cf95b469d6891185ddae2b522 F src/btree.c 8b42fc7d9efdb2df05c30e8f91ff6cfbd979724ae24bf90269028468b7a13333 F src/btree.h 55066f513eb095db935169dab1dc2f7c7a747ef223c533f5d4ad4dfed346cbd0 F src/btreeInt.h 98aadb6dcb77b012cab2574d6a728fad56b337fc946839b9898c4b4c969e30b6 -F src/build.c 29ebc26d90a9d6e860a3018a6d61fb667c80ec8e80e3798c4164d2349ba28650 +F src/build.c 69d79a255a6a0547f5e99a1726c3cbd363fa221c4f0458425abc7ba59efb9f83 F src/callback.c db3a45e376deff6a16c0058163fe0ae2b73a2945f3f408ca32cf74960b28d490 F src/complete.c a3634ab1e687055cd002e11b8f43eb75c17da23e F src/ctime.c 64e4b1227b4ed123146f0aa2989131d1fbd9b927b11e80c9d58c6a68f9cd5ce3 F src/date.c 13dd752847afb32ed70510ad7345a5b9c841f51ad904dba5d010f1fa3a6a324e F src/dbpage.c 80e46e1df623ec40486da7a5086cb723b0275a6e2a7b01d9f9b5da0f04ba2782 F src/dbstat.c 3b677254d512fcafd4d0b341bf267b38b235ccfddbef24f9154e19360fa22e43 -F src/delete.c 3638367ea4ed5d422ee1728a74f0b7a2973af9d7eef9b8b959c9c1f6e761ab8d +F src/delete.c cfe1a09cd67e3c8aef51ef928c69ba2824e6e2439f451e990afbb89e774add01 F src/expr.c 6d5f2c38fe3ec06a7eac599dac822788b36064124e20112a844e9cd5156cb239 F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007 -F src/fkey.c 849049c74a4c68961154c124087b5c67d277217a2e642570d0c1bd6336859940 +F src/fkey.c 928ed2517e8732113d2b9821aa37af639688d752f4ea9ac6e0e393d713eeb76f F src/func.c 1f61e32e7a357e615b5d2e774bee563761fce4f2fd97ecb0f72c33e62a2ada5f F src/global.c 61a419dd9e993b9be0f91de4c4ccf322b053eb829868e089f0321dd669be3b90 F src/hash.c 9ee4269fb1d6632a6fecfb9479c93a1f29271bddbbaf215dd60420bcb80c7220 F src/hash.h 3340ab6e1d13e725571d7cee6d3e3135f0779a7d8e76a9ce0a85971fa3953c51 F src/hwtime.h f9c2dfb84dce7acf95ce6d289e46f5f9d3d1afd328e53da8f8e9008e3b3caae6 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 -F src/insert.c 37c0c12c6eea72c4dad77699b963be370ca6cbd7d18270ff04f5fe9d6031622d +F src/insert.c f8d1a0f8ee258411009c6b7f2d93170e351bd19f5ad89d57e1180644297cbe70 F src/json.c 5b6a1d6015997b9ee848a32948720bdb26a0ef2de5a2127ebf7355ce66dbdc0d F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c 7432c944ff197046d67a1207790a1b13eec4548c85a9457eb0896bb3641dfb36 @@ -760,12 +760,12 @@ F src/printf.c 17054fb94ffcf7a28362e9a5af4a0f813bd0c52200ae408eeebddc81feed9274 F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c F src/resolve.c 9afed5fd7b9111633bdb74a73cdc47324e28e4dc6c27113e3e9aee38fb9422ab F src/rowset.c 8432130e6c344b3401a8874c3cb49fefe6873fec593294de077afea2dce5ec97 -F src/select.c 9aa44e562f2fbfa33e600e3f935369dd9a07e4beb89b344dfc7187dc2adc19ce +F src/select.c 7addcd53d8f9bef2d47a7e47faeb1f6e401dba02f1f338f532298a24657ea4e6 F src/shell.c.in 94571558b0fb28c37a5cf6dbd6ea27285341023a28a8cb5795cd2768fab67704 F src/sqlite.h.in 1ad9110150773c38ebababbad11b5cb361bcd3997676dec1c91ac5e0416a7b86 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8 F src/sqlite3ext.h 3f046c04ea3595d6bfda99b781926b17e672fd6d27da2ba6d8d8fc39981dcb54 -F src/sqliteInt.h 630e356b7d94df1dcbc32d4f7d4569725a12c6de014fe064311219c22d9cbd54 +F src/sqliteInt.h 521a9abd808e2b6c2e377cfc85b5ad5508202c33f878f929dccf60d6aca13034 F src/sqliteLimit.h 6878ab64bdeb8c24a1d762d45635e34b96da21132179023338c93f820eee6728 F src/status.c cb11f8589a6912af2da3bb1ec509a94dd8ef27df4d4c1a97e0bcf2309ece972b F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1 @@ -825,7 +825,7 @@ F src/test_wsd.c 41cadfd9d97fe8e3e4e44f61a4a8ccd6f7ca8fe9 F src/threads.c 4ae07fa022a3dc7c5beb373cf744a85d3c5c6c3c F src/tokenize.c 3f703cacdab728d7741e5a6ac242006d74fe1c2754d4f03ed889d7253259bd68 F src/treeview.c e98dbc8068d27f3d0bd2a4cebfce6a7c21c776edc23d80e12c67a70cd2ba3fe5 -F src/trigger.c 68d849ea6ccab67beb450d1a0d065c6118f0884a779388b07f0d9a27dbda38c9 +F src/trigger.c 0bb986a5b96047fd597c6aac28588853df56064e576e6b81ba777ef2ccaac461 F src/update.c 0e01aa6a3edf9ec112b33eb714b9016a81241497b1fb7c3e74332f4f71756508 F src/upsert.c 215328c3f91623c520ec8672c44323553f12caeb4f01b1090ebdca99fdf7b4f1 F src/utf.c f23165685a67b4caf8ec08fb274cb3f319103decfb2a980b7cfd55d18dfa855e @@ -2205,8 +2205,8 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 3995c45fff0e4bd10bcf49cc16eb1050216426723c63ba56d3ae5ba738c34019 -R 652ce342076e266e8f982c0969e3c6bc +P bc5f5ce59e1e4323422bda7d002310923f927a03b4fd42749bf04f6c9853956b +R ee08ce1ecc132f8a5f905e65ae160cc1 U drh -Z ae5f81dac74fb395c307223c199b02f0 +Z 01685802d904a82a17f4e5855c564d7e # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index fa4bad2218..06a9d2670a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -bc5f5ce59e1e4323422bda7d002310923f927a03b4fd42749bf04f6c9853956b +fd72d3400a8fe5747f494eee81654698acee350bb95b9db269e87d857af03492 diff --git a/src/build.c b/src/build.c index 9cbedf1ec4..b2386035d8 100644 --- a/src/build.c +++ b/src/build.c @@ -501,6 +501,7 @@ Table *sqlite3LocateTableItem( int iDb = sqlite3SchemaToIndex(pParse->db, p->u4.pSchema); zDb = pParse->db->aDb[iDb].zDbSName; }else{ + assert( !p->fg.isSubquery ); zDb = p->u4.zDatabase; } return sqlite3LocateTable(pParse, flags, p->zName, zDb); @@ -3487,6 +3488,7 @@ void sqlite3DropTable(Parse *pParse, SrcList *pName, int isView, int noErr){ assert( pParse->nErr==0 ); assert( pName->nSrc==1 ); assert( pName->a[0].fg.fixedSchema==0 ); + assert( pName->a[0].fg.isSubquery==0 ); if( sqlite3ReadSchema(pParse) ) goto exit_drop_table; if( noErr ) db->suppressErr++; assert( isView==0 || isView==LOCATE_VIEW ); @@ -4587,6 +4589,7 @@ void sqlite3DropIndex(Parse *pParse, SrcList *pName, int ifExists){ assert( pParse->nErr==0 ); /* Never called with prior non-OOM errors */ assert( pName->nSrc==1 ); assert( pName->a[0].fg.fixedSchema==0 ); + assert( pName->a[0].fg.isSubquery==0 ); if( SQLITE_OK!=sqlite3ReadSchema(pParse) ){ goto exit_drop_index; } @@ -4893,6 +4896,7 @@ SrcList *sqlite3SrcListAppend( pDatabase = 0; } assert( pItem->fg.fixedSchema==0 ); + assert( pItem->fg.isSubquery==0 ); if( pDatabase ){ pItem->zName = sqlite3NameFromToken(db, pDatabase); pItem->u4.zDatabase = sqlite3NameFromToken(db, pTable); @@ -4959,8 +4963,13 @@ void sqlite3SrcListDelete(sqlite3 *db, SrcList *pList){ for(pItem=pList->a, i=0; inSrc; i++, pItem++){ /* Check invariants on SrcItem */ + assert( !pItem->fg.isIndexedBy || !pItem->fg.isTabFunc ); + assert( !pItem->fg.isCte || !pItem->fg.isIndexedBy ); assert( !pItem->fg.hadSchema || !pItem->fg.isSubquery ); - assert( pItem->fg.hadSchema==0 || pItem->fg.fixedSchema==1 ); + assert( !pItem->fg.hadSchema || pItem->fg.fixedSchema ); + assert( !pItem->fg.fixedSchema || !pItem->fg.isSubquery ); + assert( !pItem->fg.isSubquery || (pItem->u4.pSubq!=0 && + pItem->u4.pSubq->pSelect!=0) ); if( pItem->zName ) sqlite3DbNNFreeNN(db, pItem->zName); if( pItem->zAlias ) sqlite3DbNNFreeNN(db, pItem->zAlias); diff --git a/src/delete.c b/src/delete.c index 1aa67c6a62..1d7ba66a22 100644 --- a/src/delete.c +++ b/src/delete.c @@ -156,7 +156,7 @@ void sqlite3MaterializeView( if( pFrom ){ assert( pFrom->nSrc==1 ); pFrom->a[0].zName = sqlite3DbStrDup(db, pView->zName); - assert( pFrom->a[0].fg.fixedSchema==0 ); + assert( pFrom->a[0].fg.fixedSchema==0 && pFrom->a[0].fg.isSubquery==0 ); pFrom->a[0].u4.zDatabase = sqlite3DbStrDup(db, db->aDb[iDb].zDbSName); assert( pFrom->a[0].fg.isUsing==0 ); assert( pFrom->a[0].u3.pOn==0 ); diff --git a/src/fkey.c b/src/fkey.c index cd1f88502e..f1117a8845 100644 --- a/src/fkey.c +++ b/src/fkey.c @@ -1337,7 +1337,7 @@ static Trigger *fkActionTrigger( if( pSrc ){ assert( pSrc->nSrc==1 ); pSrc->a[0].zName = sqlite3DbStrDup(db, zFrom); - assert( pSrc->a[0].fg.fixedSchema==0 ); + assert( pSrc->a[0].fg.fixedSchema==0 && pSrc->a[0].fg.isSubquery==0 ); pSrc->a[0].u4.zDatabase = sqlite3DbStrDup(db, db->aDb[iDb].zDbSName); } pSelect = sqlite3SelectNew(pParse, diff --git a/src/insert.c b/src/insert.c index 8214cd9c5c..d380281bed 100644 --- a/src/insert.c +++ b/src/insert.c @@ -729,6 +729,7 @@ Select *sqlite3MultiValues(Parse *pParse, Select *pLeft, ExprList *pRow){ p = &pRet->pSrc->a[0]; p->fg.viaCoroutine = 1; p->iCursor = -1; + assert( !p->fg.isIndexedBy && !p->fg.isTabFunc ); p->u1.nRow = 2; if( sqlite3SrcItemAttachSubquery(pParse, p, pLeft, 0) ){ pSubq = p->u4.pSubq; diff --git a/src/select.c b/src/select.c index 0c10550fac..39955dd60a 100644 --- a/src/select.c +++ b/src/select.c @@ -4522,6 +4522,7 @@ static int flattenSubquery( }else{ pSub1 = 0; } + assert( pSubitem->fg.isSubquery==0 ); if( pSubitem->fg.fixedSchema==0 ){ sqlite3DbFree(db, pSubitem->u4.zDatabase); pSubitem->u4.zDatabase = 0; @@ -5812,6 +5813,7 @@ static int resolveFromTermToCte( sqlite3ErrorMsg(pParse, "no such index: \"%s\"", pFrom->u1.zIndexedBy); return 2; } + assert( !pFrom->fg.isIndexedBy ); pFrom->fg.isCte = 1; pFrom->u2.pCteUse = pCteUse; pCteUse->nUse++; @@ -7728,6 +7730,8 @@ int sqlite3Select( if( pItem->fg.fixedSchema ){ int iDb = sqlite3SchemaToIndex(pParse->db, pItem->u4.pSchema); zDb = db->aDb[iDb].zDbSName; + }else if( pItem->fg.isSubquery ){ + zDb = 0; }else{ zDb = pItem->u4.zDatabase; } diff --git a/src/sqliteInt.h b/src/sqliteInt.h index c282023de4..9e0faf9cc1 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -3298,26 +3298,30 @@ struct Subquery { ** In the colUsed field, the high-order bit (bit 63) is set if the table ** contains more than 63 columns and the 64-th or later column is used. ** -** Intenstive use of "union" helps keep the size of the object small. This -** has been shown to boost performance due to less time spend initializing -** fields to zero when a new instance of this object is allocated. The unions -** also help SrcItem, and hence SrcList and Select, use less memory. -** -** Union member validity: +** Aggressive use of "union" helps keep the size of the object small. This +** has been shown to boost performance, in addition to saving memory. +** Access to union elements is gated by the following rules which should +** always be checked, either by an if-statement or by an assert(). ** +** Field Only access if this is true +** --------------- ----------------------------------- ** u1.zIndexedBy fg.isIndexedBy ** u1.pFuncArg fg.isTabFunc ** u1.nRow !fg.isTabFunc && !fg.isIndexedBy ** -** u2.pIBIndex fg.isIndexedBy && !fg.isCte -** u2.pCteUse fg.isCte && !fg.isIndexedBy +** u2.pIBIndex fg.isIndexedBy +** u2.pCteUse fg.isCte ** -** u3.pOn fg.isUsing==0 -** u3.pUsing fg.isUsing==1 +** u3.pOn !fg.isUsing +** u3.pUsing fg.isUsing ** -** u4.zDatabase fg.fixedSchema==0 && !fg.isSubquery -** u4.pSchema fg.fixedSchema==1 +** u4.zDatabase !fg.fixedSchema && !fg.isSubquery +** u4.pSchema fg.fixedSchema ** u4.pSubq fg.isSubquery +** +** See also the sqlite3SrcListDelete() routine for assert() statements that +** check invariants on the fields of this object, especially the flags +** inside the fg struct. */ struct SrcItem { char *zName; /* Name of the table */ @@ -3342,7 +3346,7 @@ struct SrcItem { unsigned isNestedFrom :1; /* pSelect is a SF_NestedFrom subquery */ unsigned rowidUsed :1; /* The ROWID of this table is referenced */ unsigned fixedSchema :1; /* Uses u4.pSchema, not u4.zDatabase */ - unsigned hadSchema :1; /* Has u4.zDatabase before u4.pSchema */ + unsigned hadSchema :1; /* Had u4.zDatabase before u4.pSchema */ } fg; int iCursor; /* The VDBE cursor number used to access this table */ Bitmask colUsed; /* Bit N set if column N used. Details above for N>62 */ diff --git a/src/trigger.c b/src/trigger.c index 170f094eab..ff2df82cbc 100644 --- a/src/trigger.c +++ b/src/trigger.c @@ -151,7 +151,9 @@ void sqlite3BeginTrigger( ** To maintain backwards compatibility, ignore the database ** name on pTableName if we are reparsing out of the schema table */ - if( db->init.busy && iDb!=1 && ALWAYS(pTableName->a[0].fg.fixedSchema==0) ){ + if( db->init.busy && iDb!=1 ){ + assert( pTableName->a[0].fg.fixedSchema==0 ); + assert( pTableName->a[0].fg.isSubquery==0 ); sqlite3DbFree(db, pTableName->a[0].u4.zDatabase); pTableName->a[0].u4.zDatabase = 0; } @@ -631,7 +633,7 @@ void sqlite3DropTrigger(Parse *pParse, SrcList *pName, int noErr){ } assert( pName->nSrc==1 ); - assert( pName->a[0].fg.fixedSchema==0 ); + assert( pName->a[0].fg.fixedSchema==0 && pName->a[0].fg.isSubquery==0 ); zDb = pName->a[0].u4.zDatabase; zName = pName->a[0].zName; assert( zDb!=0 || sqlite3BtreeHoldsAllMutexes(db) );