Early detection of database corruption in balance_deeper().

FossilOrigin-Name: 61c2233654158e65a3d3baeea947903a919a569fcc4a5b342b2e9a68cec1b6f3

diff --git a/manifest b/manifest
index e76f1203323a67d4a542ccedb6387b79283de47e..11932bc710abccdaa04d94d5ec5484a539804141 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Fix\sa\scase\sin\swhich\sSQLite\scould\sfail\sto\sidentify\s"x\sBETWEEN\s?\sAND\s?"\sbeing\strue\sas\simplying\sthat\sx\sis\snot\snull.\sTicket\s[dfd66334].
-D 2019-12-23T15:17:11.892
+C Early\sdetection\sof\sdatabase\scorruption\sin\sbalance_deeper().
+D 2019-12-23T18:02:15.079
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -468,7 +468,7 @@ F src/auth.c a3d5bfdba83d25abed1013a8c7a5f204e2e29b0c25242a56bc02bb0c07bf1e06
 F src/backup.c f70077d40c08b7787bfe934e4d1da8030cb0cc57d46b345fba2294b7d1be23ab
 F src/bitvec.c 17ea48eff8ba979f1f5b04cc484c7bb2be632f33
 F src/btmutex.c 8acc2f464ee76324bf13310df5692a262b801808984c1b79defb2503bbafadb6
-F src/btree.c 716fc9bd12eb7d35e3d66c5c2c81c37df3fdae49cd25bceaff4e7d702d513d80
+F src/btree.c 695bcbc62177cf70faaf6b34f89c22415e5581a13c57d67c862057636d3f09b9
 F src/btree.h f27a33c49280209a93385e218306c4ee5f46ba8d7649d2f81a7166b282232484
 F src/btreeInt.h 91806f01fd1145a9a86ba3042f25c38d8faf6002701bf5e780742cf88bcff437
 F src/build.c 1d999886fa656e6211e14d5402a6f92cadbdaa5d2f4f0597c797f7818d510e33
@@ -532,7 +532,7 @@ F src/shell.c.in 4a3a9e1c11847b1904f2b01d087af1c052f660902755abab457cab1756817de
 F src/sqlite.h.in 2a23e8161775253d9cf383c2c6aa559005dc787d350dcb0be67a6c4cc3bd1d19
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h 72af51aa4e912e14cd495fb6e7fac65f0940db80ed950d90911aff292cc47ce2
-F src/sqliteInt.h 60d92fad64da7c3e77bbc35ee306340814cdaa5df32892b0ec58d306d99b5733
+F src/sqliteInt.h eada1e78b6b950670eb4cc093ce6eb96a7df8bc548cabd7bc6127fa4fbce8ba5
 F src/sqliteLimit.h 1513bfb7b20378aa0041e7022d04acb73525de35b80b252f1b83fedb4de6a76b
 F src/status.c 46e7aec11f79dad50965a5ca5fa9de009f7d6bde08be2156f1538a0a296d4d0e
 F src/table.c b46ad567748f24a326d9de40e5b9659f96ffff34
@@ -1852,7 +1852,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 0b1dbd60f5db3abe2097dbc0b6de9671685ca5eaf7d3fc8e3f87ff5065a9d114
-R 3f13118066cf3e145065219b1e28432f
-U dan
-Z 5ab62146546bc685f4a7b032d9c74d0c
+P 2f17974912ec5e99089dc0da803e7ff1bf033377a49762d2689a812c005f2641
+R 8b7584d5dce9cabfa7788fda2b2b4c7c
+U drh
+Z 20f6b4d0a1e07f848f9073291aeee414

diff --git a/manifest.uuid b/manifest.uuid
index a458bcced67751133c58de4478cafcb015983f5c..f3b00eb01569febad816ac3311f76341a2a40cbc 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-2f17974912ec5e99089dc0da803e7ff1bf033377a49762d2689a812c005f2641
\ No newline at end of file
+61c2233654158e65a3d3baeea947903a919a569fcc4a5b342b2e9a68cec1b6f3
\ No newline at end of file

diff --git a/src/btree.c b/src/btree.c
index 98f9ec2d6207c7c03b842d606df478f902f2b51b..52c2ceaffb895309def828da245a8929022d8003 100644 (file)
--- a/src/btree.c
+++ b/src/btree.c
@@ -5718,8 +5718,11 @@ static SQLITE_NOINLINE int btreeNext(BtCursor *pCur){
   ** to be invalid here. This can only occur if a second cursor modifies
   ** the page while cursor pCur is holding a reference to it. Which can
   ** only happen if the database is corrupt in such a way as to link the
-  ** page into more than one b-tree structure. */
-  testcase( idx>pPage->nCell );
+  ** page into more than one b-tree structure.
+  **
+  ** Update 2019-12-23: appears to long longer be possible after the
+  ** addition of anotherValidCursor() condition on balance_deeper().  */
+  harmless( idx>pPage->nCell );
 
   if( idx>=pPage->nCell ){
     if( !pPage->leaf ){
@@ -8308,6 +8311,30 @@ static int balance_deeper(MemPage *pRoot, MemPage **ppChild){
   return SQLITE_OK;
 }
 
+/*
+** Return SQLITE_CORRUPT if any cursor other than pCur is currently valid
+** on the same B-tree as pCur.
+**
+** This can if a database is corrupt with two or more SQL tables
+** pointing to the same b-tree.  If an insert occurs on one SQL table
+** and causes a BEFORE TRIGGER to do a secondary insert on the other SQL
+** table linked to the same b-tree.  If the secondary insert causes a
+** rebalance, that can change content out from under the cursor on the
+** first SQL table, violating invariants on the first insert.
+*/
+static int anotherValidCursor(BtCursor *pCur){
+  BtCursor *pOther;
+  for(pOther=pCur->pBt->pCursor; pOther; pOther=pOther->pNext){
+    if( pOther!=pCur
+     && pOther->eState==CURSOR_VALID
+     && pOther->pPage==pCur->pPage
+    ){
+      return SQLITE_CORRUPT_BKPT;
+    }
+  }
+  return SQLITE_OK;
+}
+
 /*
 ** The page that pCur currently points to has just been modified in
 ** some way. This function figures out if this modification means the
@@ -8335,7 +8362,7 @@ static int balance(BtCursor *pCur){
     if( pPage->nOverflow==0 && pPage->nFree<=nMin ){
       break;
     }else if( (iPage = pCur->iPage)==0 ){
-      if( pPage->nOverflow ){
+      if( pPage->nOverflow && (rc = anotherValidCursor(pCur))==SQLITE_OK ){
         /* The root page of the b-tree is overfull. In this case call the
         ** balance_deeper() function to create a new child for the root-page
         ** and copy the current contents of the root-page to it. The

diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 4b4a9068cca2854d97454ef4b15d009a897c1d2c..a8cdb9f6adffa67d95916cdd23cad0f632810703 100644 (file)
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -446,6 +446,26 @@
 # define NEVER(X)       (X)
 #endif
 
+/*
+** The harmless(X) macro indicates that expression X is usually false
+** but can be true without causing any problems, but we don't know of
+** any way to cause X to be true.
+**
+** In debugging and testing builds, this macro will abort if X is ever
+** true.  In this way, developers are alerted to a possible test case
+** that causes X to be true.  If a harmless macro ever fails, that is
+** an opportunity to change the macro into a testcase() and add a new
+** test case to the test suite.
+**
+** For normal production builds, harmless(X) is a no-op, since it does
+** not matter whether expression X is true or false.
+*/
+#ifdef SQLITE_DEBUG
+# define harmless(X)  assert(!(X));
+#else
+# define harmless(X)
+#endif
+
 /*
 ** Some conditionals are optimizations only.  In other words, if the
 ** conditionals are replaced with a constant 1 (true) or 0 (false) then