Further tests and changes to make the r-tree module more robust.

FossilOrigin-Name: 7ff3574b9c581b5e1f2b6f98028106c638e59bb7

5 files changed, 108 insertions, 59 deletions

diff --git a/ext/rtree/rtree.c b/ext/rtree/rtree.c
index e860e2db9..8f67c801b 100644
--- a/ext/rtree/rtree.c
+++ b/ext/rtree/rtree.c
@@ -462,6 +462,7 @@ nodeAcquire(
   RtreeNode **ppNode         /* OUT: Acquired node */
 ){
   int rc;
+  int rc2 = SQLITE_OK;
   RtreeNode *pNode;
 
   /* Check if the requested node is already in the hash table. If so,
@@ -485,7 +486,7 @@ nodeAcquire(
     if( pRtree->iNodeSize==sqlite3_column_bytes(pRtree->pReadNode, 0) ){
       pNode = (RtreeNode *)sqlite3_malloc(sizeof(RtreeNode)+pRtree->iNodeSize);
       if( !pNode ){
-        rc = SQLITE_NOMEM;
+        rc2 = SQLITE_NOMEM;
       }else{
         pNode->pParent = pParent;
         pNode->zData = (u8 *)&pNode[1];
@@ -499,6 +500,7 @@ nodeAcquire(
     }
   }
   rc = sqlite3_reset(pRtree->pReadNode);
+  if( rc==SQLITE_OK ) rc = rc2;
 
   /* If the root node was just loaded, set pRtree->iDepth to the height
   ** of the r-tree structure. A height of zero means all data is stored on
@@ -1043,24 +1045,34 @@ static int descendToCell(
 ** One of the cells in node pNode is guaranteed to have a 64-bit 
 ** integer value equal to iRowid. Return the index of this cell.
 */
-static int nodeRowidIndex(Rtree *pRtree, RtreeNode *pNode, i64 iRowid){
+static int nodeRowidIndex(
+  Rtree *pRtree, 
+  RtreeNode *pNode, 
+  i64 iRowid,
+  int *piIndex
+){
   int ii;
-  for(ii=0; nodeGetRowid(pRtree, pNode, ii)!=iRowid; ii++){
-    assert( ii<(NCELL(pNode)-1) );
+  int nCell = NCELL(pNode);
+  for(ii=0; ii<nCell; ii++){
+    if( nodeGetRowid(pRtree, pNode, ii)==iRowid ){
+      *piIndex = ii;
+      return SQLITE_OK;
+    }
   }
-  return ii;
+  return SQLITE_CORRUPT;
 }
 
 /*
 ** Return the index of the cell containing a pointer to node pNode
 ** in its parent. If pNode is the root node, return -1.
 */
-static int nodeParentIndex(Rtree *pRtree, RtreeNode *pNode){
+static int nodeParentIndex(Rtree *pRtree, RtreeNode *pNode, int *piIndex){
   RtreeNode *pParent = pNode->pParent;
   if( pParent ){
-    return nodeRowidIndex(pRtree, pParent, pNode->iNode);
+    return nodeRowidIndex(pRtree, pParent, pNode->iNode, piIndex);
   }
-  return -1;
+  *piIndex = -1;
+  return SQLITE_OK;
 }
 
 /* 
@@ -1095,7 +1107,10 @@ static int rtreeNext(sqlite3_vtab_cursor *pVtabCursor){
         }
       }
       pCsr->pNode = pNode->pParent;
-      pCsr->iCell = nodeParentIndex(pRtree, pNode);
+      rc = nodeParentIndex(pRtree, pNode, &pCsr->iCell);
+      if( rc!=SQLITE_OK ){
+        return rc;
+      }
       nodeReference(pCsr->pNode);
       nodeRelease(pRtree, pNode);
       iHeight++;
@@ -1237,7 +1252,7 @@ static int rtreeFilter(
     pCsr->pNode = pLeaf; 
     if( pLeaf ){
       assert( rc==SQLITE_OK );
-      pCsr->iCell = nodeRowidIndex(pRtree, pLeaf, iRowid);
+      rc = nodeRowidIndex(pRtree, pLeaf, iRowid, &pCsr->iCell);
     }
   }else{
     /* Normal case - r-tree scan. Set up the RtreeCursor.aConstraint array 
@@ -1654,16 +1669,20 @@ static int ChooseLeaf(
 ** the node pNode. This function updates the bounding box cells in
 ** all ancestor elements.
 */
-static void AdjustTree(
+static int AdjustTree(
   Rtree *pRtree,                    /* Rtree table */
   RtreeNode *pNode,                 /* Adjust ancestry of this node. */
   RtreeCell *pCell                  /* This cell was just inserted */
 ){
   RtreeNode *p = pNode;
   while( p->pParent ){
-    RtreeCell cell;
     RtreeNode *pParent = p->pParent;
-    int iCell = nodeParentIndex(pRtree, p);
+    RtreeCell cell;
+    int iCell;
+
+    if( nodeParentIndex(pRtree, p, &iCell) ){
+      return SQLITE_CORRUPT;
+    }
 
     nodeGetCell(pRtree, pParent, iCell, &cell);
     if( !cellContains(pRtree, &cell, pCell) ){
@@ -1673,6 +1692,7 @@ static void AdjustTree(
  
     p = pParent;
   }
+  return SQLITE_OK;
 }
 
 /*
@@ -2246,9 +2266,15 @@ static int SplitNode(
     }
   }else{
     RtreeNode *pParent = pLeft->pParent;
-    int iCell = nodeParentIndex(pRtree, pLeft);
-    nodeOverwriteCell(pRtree, pParent, &leftbbox, iCell);
-    AdjustTree(pRtree, pParent, &leftbbox);
+    int iCell;
+    rc = nodeParentIndex(pRtree, pLeft, &iCell);
+    if( rc==SQLITE_OK ){
+      nodeOverwriteCell(pRtree, pParent, &leftbbox, iCell);
+      rc = AdjustTree(pRtree, pParent, &leftbbox);
+    }
+    if( rc!=SQLITE_OK ){
+      goto splitnode_out;
+    }
   }
   if( (rc = rtreeInsertCell(pRtree, pRight->pParent, &rightbbox, iHeight+1)) ){
     goto splitnode_out;
@@ -2293,29 +2319,29 @@ splitnode_out:
 }
 
 /*
-** If node pLeaf is not the root of the r-tree and its pParent
-** pointer is still NULL, locate the parent node of pLeaf and populate 
-** pLeaf->pParent.
+** If node pLeaf is not the root of the r-tree and its pParent pointer is 
+** still NULL, load all ancestor nodes of pLeaf into memory and populate
+** the pLeaf->pParent chain all the way up to the root node.
 */
 static int fixLeafParent(Rtree *pRtree, RtreeNode *pLeaf){
   int rc = SQLITE_OK;
-  if( pLeaf->iNode!=1 && pLeaf->pParent==0 ){
-    int rc2;                      /* sqlite3_reset() return code */
-    sqlite3_bind_int64(pRtree->pReadParent, 1, pLeaf->iNode);
+  RtreeNode *pChild = pLeaf;
+  while( rc==SQLITE_OK && pChild->iNode!=1 && pChild->pParent==0 ){
+    int rc2 = SQLITE_OK;          /* sqlite3_reset() return code */
+    sqlite3_bind_int64(pRtree->pReadParent, 1, pChild->iNode);
     rc = sqlite3_step(pRtree->pReadParent);
     if( rc==SQLITE_ROW ){
+      RtreeNode *pTest;
       i64 iNode = sqlite3_column_int64(pRtree->pReadParent, 0);
-      rc = nodeAcquire(pRtree, iNode, 0, &pLeaf->pParent);
-    }else if( rc==SQLITE_DONE ){
-      rc = SQLITE_CORRUPT;
-    }
-    rc2 = sqlite3_reset(pRtree->pReadParent);
-    if( rc==SQLITE_OK ){
-      rc = rc2;
-    }
-    if( rc==SQLITE_OK ){
-      rc = fixLeafParent(pRtree, pLeaf->pParent);
+      for(pTest=pLeaf; pTest && pTest->iNode!=iNode; pTest=pTest->pParent);
+      if( !pTest ){
+        rc2 = nodeAcquire(pRtree, iNode, 0, &pChild->pParent);
+      }
     }
+    rc = sqlite3_reset(pRtree->pReadParent);
+    if( rc==SQLITE_OK ) rc = rc2;
+    if( rc==SQLITE_OK && !pChild->pParent ) rc = SQLITE_CORRUPT;
+    pChild = pChild->pParent;
   }
   return rc;
 }
@@ -2331,10 +2357,12 @@ static int removeNode(Rtree *pRtree, RtreeNode *pNode, int iHeight){
   assert( pNode->nRef==1 );
 
   /* Remove the entry in the parent cell. */
-  iCell = nodeParentIndex(pRtree, pNode);
-  pParent = pNode->pParent;
-  pNode->pParent = 0;
-  rc = deleteCell(pRtree, pParent, iCell, iHeight+1);
+  rc = nodeParentIndex(pRtree, pNode, &iCell);
+  if( rc==SQLITE_OK ){
+    pParent = pNode->pParent;
+    pNode->pParent = 0;
+    rc = deleteCell(pRtree, pParent, iCell, iHeight+1);
+  }
   rc2 = nodeRelease(pRtree, pParent);
   if( rc==SQLITE_OK ){
     rc = rc2;
@@ -2369,8 +2397,9 @@ static int removeNode(Rtree *pRtree, RtreeNode *pNode, int iHeight){
   return SQLITE_OK;
 }
 
-static void fixBoundingBox(Rtree *pRtree, RtreeNode *pNode){
+static int fixBoundingBox(Rtree *pRtree, RtreeNode *pNode){
   RtreeNode *pParent = pNode->pParent;
+  int rc = SQLITE_OK; 
   if( pParent ){
     int ii; 
     int nCell = NCELL(pNode);
@@ -2382,10 +2411,13 @@ static void fixBoundingBox(Rtree *pRtree, RtreeNode *pNode){
       cellUnion(pRtree, &box, &cell);
     }
     box.iRowid = pNode->iNode;
-    ii = nodeParentIndex(pRtree, pNode);
-    nodeOverwriteCell(pRtree, pParent, &box, ii);
-    fixBoundingBox(pRtree, pParent);
+    rc = nodeParentIndex(pRtree, pNode, &ii);
+    if( rc==SQLITE_OK ){
+      nodeOverwriteCell(pRtree, pParent, &box, ii);
+      rc = fixBoundingBox(pRtree, pParent);
+    }
   }
+  return rc;
 }
 
 /*
@@ -2416,7 +2448,7 @@ static int deleteCell(Rtree *pRtree, RtreeNode *pNode, int iCell, int iHeight){
     if( NCELL(pNode)<RTREE_MINCELLS(pRtree) ){
       rc = removeNode(pRtree, pNode, iHeight);
     }else{
-      fixBoundingBox(pRtree, pNode);
+      rc = fixBoundingBox(pRtree, pNode);
     }
   }
 
@@ -2499,7 +2531,7 @@ static int Reinsert(
     }
   }
   if( rc==SQLITE_OK ){
-    fixBoundingBox(pRtree, pNode);
+    rc = fixBoundingBox(pRtree, pNode);
   }
   for(; rc==SQLITE_OK && ii<nCell; ii++){
     /* Find a node to store this cell in. pNode->iNode currently contains
@@ -2553,11 +2585,13 @@ static int rtreeInsertCell(
     rc = SplitNode(pRtree, pNode, pCell, iHeight);
 #endif
   }else{
-    AdjustTree(pRtree, pNode, pCell);
-    if( iHeight==0 ){
-      rc = rowidWrite(pRtree, pCell->iRowid, pNode->iNode);
-    }else{
-      rc = parentWrite(pRtree, pCell->iRowid, pNode->iNode);
+    rc = AdjustTree(pRtree, pNode, pCell);
+    if( rc==SQLITE_OK ){
+      if( iHeight==0 ){
+        rc = rowidWrite(pRtree, pCell->iRowid, pNode->iNode);
+      }else{
+        rc = parentWrite(pRtree, pCell->iRowid, pNode->iNode);
+      }
     }
   }
   return rc;
@@ -2652,8 +2686,10 @@ static int rtreeUpdate(
     /* Delete the cell in question from the leaf node. */
     if( rc==SQLITE_OK ){
       int rc2;
-      iCell = nodeRowidIndex(pRtree, pLeaf, iDelete);
-      rc = deleteCell(pRtree, pLeaf, iCell, 0);
+      rc = nodeRowidIndex(pRtree, pLeaf, iDelete, &iCell);
+      if( rc==SQLITE_OK ){
+        rc = deleteCell(pRtree, pLeaf, iCell, 0);
+      }
       rc2 = nodeRelease(pRtree, pLeaf);
       if( rc==SQLITE_OK ){
         rc = rc2;
diff --git a/ext/rtree/rtreeA.test b/ext/rtree/rtreeA.test
index 936b8fc9c..d456bf6e2 100644
--- a/ext/rtree/rtreeA.test
+++ b/ext/rtree/rtreeA.test
@@ -184,7 +184,6 @@ populate_t1
 do_test rtreeA-4.1.0 { 
   set_entry_count t1 1 4000
 } {4000}
-breakpoint
 do_corruption_tests rtreeA-4.1 {
   1   "SELECT * FROM t1"
   2   "SELECT * FROM t1 WHERE rowid=5"
@@ -204,4 +203,17 @@ do_corruption_tests rtreeA-5.1 {
   2   "DELETE FROM t1"
 }
 
+#-------------------------------------------------------------------------
+# Add some bad entries to the %_parent table.
+#
+create_t1
+populate_t1
+do_execsql_test rtreeA-6.1.0 { 
+  UPDATE t1_parent set parentnode = parentnode+1
+} {}
+do_corruption_tests rtreeA-6.1 {
+  1   "DELETE FROM t1 WHERE rowid = 5"
+}
+
+
 finish_test
diff --git a/manifest b/manifest
index 209eb10b0..8d03f84bc 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\snew\sfile\srtreeA.test,\sto\stest\sthat\sthe\sr-tree\sextension\sdoesn't\scrash\sif\sit\sencounters\sa\scorrupt\sor\sinconsistent\sdatabase.
-D 2010-09-22T14:19:53
+C Further\stests\sand\schanges\sto\smake\sthe\sr-tree\smodule\smore\srobust.
+D 2010-09-22T19:06:03
 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
 F Makefile.in c599a15d268b1db2aeadea19df2adc3bf2eb6bee
 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -79,7 +79,7 @@ F ext/icu/README.txt bf8461d8cdc6b8f514c080e4e10dc3b2bbdfefa9
 F ext/icu/icu.c 850e9a36567bbcce6bd85a4b68243cad8e3c2de2
 F ext/icu/sqliteicu.h 728867a802baa5a96de7495e9689a8e01715ef37
 F ext/rtree/README 6315c0d73ebf0ec40dedb5aa0e942bc8b54e3761
-F ext/rtree/rtree.c 3733ff121035e83b598089d398293015c4102e6d
+F ext/rtree/rtree.c 4e2bda4a99794d1e618486c13a76987acd547132
 F ext/rtree/rtree.h 834dbcb82dc85b2481cde6a07cdadfddc99e9b9e
 F ext/rtree/rtree1.test dbd4250ac0ad367a262eb9676f7e3080b0368206
 F ext/rtree/rtree2.test acbb3a4ce0f4fbc2c304d2b4b784cfa161856bba
@@ -90,7 +90,7 @@ F ext/rtree/rtree6.test 1ebe0d632a7501cc80ba5a225f028fd4f0fdda08
 F ext/rtree/rtree7.test bcb647b42920b3b5d025846689147778485cc318
 F ext/rtree/rtree8.test 9772e16da71e17e02bdebf0a5188590f289ab37d
 F ext/rtree/rtree9.test df9843d1a9195249c8d3b4ea6aedda2d5c73e9c2
-F ext/rtree/rtreeA.test 2c6c7742957a9970a625762fee625444412b0a89
+F ext/rtree/rtreeA.test ebd9dcff5eaa90d0359e9a30787f5d8da5e013ac
 F ext/rtree/rtree_perf.tcl 6c18c1f23cd48e0f948930c98dfdd37dfccb5195
 F ext/rtree/rtree_util.tcl 06aab2ed5b826545bf215fff90ecb9255a8647ea
 F ext/rtree/sqlite3rtree.h 1af0899c63a688e272d69d8e746f24e76f10a3f0
@@ -166,7 +166,7 @@ F src/pager.h 8167a1e720d0b7a2790079007128e594010220ad
 F src/parse.y 12b7ebd61ea54f0e1b1083ff69cc2c8ce9353d58
 F src/pcache.c 09d38c44ab275db581f7a2f6ff8b9bc7f8c0faaa
 F src/pcache.h c683390d50f856d4cd8e24342ae62027d1bb6050
-F src/pcache1.c d090673e7489940dfaddf3fb72ab550c409b2744
+F src/pcache1.c e9578a3beac26f229ee558a4e16c863f2498185f
 F src/pragma.c 8b24ce00a93de345b6c3bd1e1e2cfba9f63d2325
 F src/prepare.c ce4c35a2b1d5fe916e4a46b70d24a6e997d7c4c6
 F src/printf.c 8ae5082dd38a1b5456030c3755ec3a392cd51506
@@ -863,7 +863,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
 F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f
-P 14e8659e576258b64d67cb3f1222f173089d5127
-R f765e03b8b7edfbba31db768fa2c7c43
+P 68a305fd5ac917317fee2ef6670ac389a120e502
+R 4dc37794fd753db61a2e814773c65213
 U dan
-Z 161dee882fd7434c2ba579743fed4212
+Z f587531c441ccd1803517229a507ab8c
diff --git a/manifest.uuid b/manifest.uuid
index 378d2937d..ce56bca3a 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-68a305fd5ac917317fee2ef6670ac389a120e502
\ No newline at end of file
+7ff3574b9c581b5e1f2b6f98028106c638e59bb7
\ No newline at end of file
diff --git a/src/pcache1.c b/src/pcache1.c
index 633e1202e..abf57dc34 100644
--- a/src/pcache1.c
+++ b/src/pcache1.c
@@ -767,6 +767,7 @@ static void pcache1Truncate(sqlite3_pcache *p, unsigned int iLimit){
 */
 static void pcache1Destroy(sqlite3_pcache *p){
   PCache1 *pCache = (PCache1 *)p;
+  assert( pCache->bPurgeable || (pCache->nMax==0 && pCache->nMin==0) );
   pcache1EnterMutex();
   pcache1TruncateUnsafe(pCache, 0);
   pcache1.nMaxPage -= pCache->nMax;