From bb246c4da04812bfd666968e586856944a7283e9 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 12 Jan 2012 14:25:55 +0000 Subject: [PATCH] Remove some assert() statements that can fail with corrupt databases. FossilOrigin-Name: 7654ae71bd26ae5d713935624d2b6cb8d2e4633f --- manifest | 15 ++--- manifest.uuid | 2 +- src/btree.c | 20 +++++- test/corruptF.test | 150 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 test/corruptF.test diff --git a/manifest b/manifest index 97da04f66c..8aa952acc4 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Remove\scode\smade\sunreachable\sby\sthe\senhancement\sof\sthe\sprevious\scheck-in. -D 2012-01-11T16:16:08.656 +C Remove\ssome\sassert()\sstatements\sthat\scan\sfail\swith\scorrupt\sdatabases. +D 2012-01-12T14:25:55.908 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in 5b4a3e12a850b021547e43daf886b25133b44c07 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -125,7 +125,7 @@ F src/auth.c 523da7fb4979469955d822ff9298352d6b31de34 F src/backup.c e9538bad2d4a4fcd4308f1aed7cb18a0fbc968f9 F src/bitvec.c af50f1c8c0ff54d6bdb7a80e2fceca5a93670bef F src/btmutex.c 976f45a12e37293e32cae0281b15a21d48a8aaa7 -F src/btree.c 378a0f39d6e25cc3f7c40db3d90ec6f0a29e7733 +F src/btree.c 8276f939669ed652dfe073b1290fe0094ff92458 F src/btree.h f5d775cd6cfc7ac32a2535b70e8d2af48ef5f2ce F src/btreeInt.h 6c9960645c431c9456ca56498f43a2b3bf1fa8c2 F src/build.c 8e2a4dedad860fed982270ef43968505f35ec57f @@ -349,6 +349,7 @@ F test/corruptB.test 20d4a20cbed23958888c3e8995b424a47223d647 F test/corruptC.test 62a767fe64acb1975f58cc6171192839c783edbb F test/corruptD.test 99b1999dbfa7cc04aaeac9d695a2445d4e7c7458 F test/corruptE.test 1b9eb20a8711251ce57b44a257e241085b39b52d +F test/corruptF.test 984b1706c9c0e4248141b056c21124612628d12e F test/count.test 454e1ce985c94d13efeac405ce54439f49336163 F test/crash.test 519dc29f6fea151f015a23236e555239353946eb F test/crash2.test 5b14d4eb58b880e231361d3b609b216acda86651 @@ -986,7 +987,7 @@ F tool/tostr.awk e75472c2f98dd76e06b8c9c1367f4ab07e122d06 F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f F tool/warnings-clang.sh 9f406d66e750e8ac031c63a9ef3248aaa347ef2a F tool/warnings.sh fbc018d67fd7395f440c28f33ef0f94420226381 -P 629108c8e5376f989cd66e11437949a54c33a5b8 -R 0e3f2b6eff759aa9860b31ea731b9403 -U drh -Z 39b800b2fc63936bd07e49ea34b13ab2 +P 9e31a275ef494ea8713a1d60a15b84157e57c3ff +R 04391ebd30f10ed455a6246275bb3b80 +U dan +Z 8b2af6d3a0a6ab469514bfcff8c006db diff --git a/manifest.uuid b/manifest.uuid index 0568c3c8a9..6f9bf51b9b 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -9e31a275ef494ea8713a1d60a15b84157e57c3ff \ No newline at end of file +7654ae71bd26ae5d713935624d2b6cb8d2e4633f \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 64ed28ea24..6cef1a1767 100644 --- a/src/btree.c +++ b/src/btree.c @@ -4194,7 +4194,7 @@ static int moveToChild(BtCursor *pCur, u32 newPgno){ return SQLITE_OK; } -#ifndef NDEBUG +#if 0 /* ** Page pParent is an internal (non-leaf) tree page. This function ** asserts that page number iChild is the left-child if the iIdx'th @@ -4227,11 +4227,21 @@ static void moveToParent(BtCursor *pCur){ assert( pCur->eState==CURSOR_VALID ); assert( pCur->iPage>0 ); assert( pCur->apPage[pCur->iPage] ); + + /* UPDATE: It is actually possible for the condition tested by the assert + ** below to be untrue if the database file is corrupt. This can occur if + ** one cursor has modified page pParent while a reference to it is held + ** by a second cursor. Which can only happen if a single page is linked + ** into more than one b-tree structure in a corrupt database. */ +#if 0 assertParentIndex( pCur->apPage[pCur->iPage-1], pCur->aiIdx[pCur->iPage-1], pCur->apPage[pCur->iPage]->pgno ); +#endif + testcase( iIdx>pParent->nCell ); + releasePage(pCur->apPage[pCur->iPage]); pCur->iPage--; pCur->info.nSize = 0; @@ -4701,7 +4711,13 @@ int sqlite3BtreeNext(BtCursor *pCur, int *pRes){ pPage = pCur->apPage[pCur->iPage]; idx = ++pCur->aiIdx[pCur->iPage]; assert( pPage->isInit ); - assert( idx<=pPage->nCell ); + + /* If the database file is corrupt, it is possible for the value of idx + ** 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 ); pCur->info.nSize = 0; pCur->validNKey = 0; diff --git a/test/corruptF.test b/test/corruptF.test new file mode 100644 index 0000000000..33eef39bdd --- /dev/null +++ b/test/corruptF.test @@ -0,0 +1,150 @@ +# 2012 January 12 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set testprefix corruptF + +# Do not use a codec for tests in this file, as the database file is +# manipulated directly using tcl scripts (using the [hexio_write] command). +# +do_not_use_codec + +proc str {i} { format %08d $i } + +# Create a 6 page database containing a single table - t1. Table t1 +# consists of page 2 (the root page) and pages 5 and 6 (leaf pages). +# Database pages 3 and 4 are on the free list. +# +proc create_test_db {} { + catch { db close } + forcedelete test.db + sqlite3 db test.db + db func str str + execsql { + PRAGMA auto_vacuum = 0; + PRAGMA page_size = 1024; + CREATE TABLE t1(x); /* root page = 2 */ + CREATE TABLE t2(x); /* root page = 3 */ + CREATE TABLE t3(x); /* root page = 4 */ + + INSERT INTO t1 VALUES(str(1)); + INSERT INTO t1 SELECT str(rowid+1) FROM t1; + INSERT INTO t1 SELECT str(rowid+2) FROM t1; + INSERT INTO t1 SELECT str(rowid+4) FROM t1; + INSERT INTO t1 SELECT str(rowid+8) FROM t1; + INSERT INTO t1 SELECT str(rowid+16) FROM t1; + INSERT INTO t1 SELECT str(rowid+32) FROM t1; + INSERT INTO t1 SELECT str(rowid+64) FROM t1; + DROP TABLE t2; + DROP TABLE t3; + } + db close +} + +do_test 1.1 { create_test_db } {} + +# Check the db is as we expect. 6 pages in total, with 3 and 4 on the free +# list. Page 3 is the free list trunk and page 4 is a leaf. +# +do_test 1.2 { file size test.db } [expr 6*1024] +do_test 1.3 { hexio_read test.db 32 4 } 00000003 +do_test 1.4 { hexio_read test.db [expr 2*1024] 12 } 000000000000000100000004 + +# Change the free-list entry to page 6 and reopen the db file. +do_test 1.5 { + hexio_write test.db [expr 2*1024 + 8] 00000006 + sqlite3 db test.db +} {} + +# Now create a new table in the database file. The root of the new table +# is page 6, which is also the right-most leaf page in table t1. +# +do_execsql_test 1.6 { + CREATE TABLE t4(x); + SELECT * FROM sqlite_master; +} { + table t1 t1 2 {CREATE TABLE t1(x)} + table t4 t4 6 {CREATE TABLE t4(x)} +} + +# At one point this was causing an assert to fail. +# +# This statement opens a cursor on table t1 and does a full table scan. As +# each row is visited, it is copied into table t4. There is no temporary +# table. +# +# When the t1 cursor reaches page 6 (which is both the right-most leaf of +# t1 and the root of t4), it continues to iterate through the keys within +# it (which at this point are keys that have been inserted into t4). And +# for each row visited, another row is inserted into page 6 - it being the +# root page of t4. Eventually, page 6 becomes full and the height of the +# b-tree for table t4 increased. From the point of view of the t1 cursor, +# this unexpectedly reduces the number of keys on page 6 in the middle of +# its iteration, which causes an assert() to fail. +# +db_save_and_close +if 1 { +for {set i 0} {$i < 128} {incr i} { + db_restore_and_reopen + do_test 1.7.$i { + set res [ + catchsql { INSERT INTO t4 SELECT x FROM t1 WHERE rowid>$i } + ] + if {$res == "0 {}" || $res == "1 {database disk image is malformed}"} { + set res "" + } + set res + } {} +} +} + +do_test 2.1 { create_test_db } {} +do_test 2.2 { file size test.db } [expr 6*1024] +do_test 2.3 { hexio_read test.db 32 4 } 00000003 +do_test 2.4 { hexio_read test.db [expr 2*1024] 12 } 000000000000000100000004 + +# Change the free-list entry to page 5 and reopen the db file. +do_test 2.5 { + hexio_write test.db [expr 2*1024 + 8] 00000005 + sqlite3 db test.db +} {} + +# Now create a new table in the database file. The root of the new table +# is page 5, which is also the right-most leaf page in table t1. +# +do_execsql_test 2.6 { + CREATE TABLE t4(x); + SELECT * FROM sqlite_master; +} { + table t1 t1 2 {CREATE TABLE t1(x)} + table t4 t4 5 {CREATE TABLE t4(x)} +} + +db_save_and_close +for {set i 127} {$i >= 0} {incr i -1} { + db_restore_and_reopen + do_test 2.7.$i { + set res [ + catchsql { + INSERT INTO t4 SELECT x FROM t1 WHERE rowid<$i ORDER BY rowid DESC + } + ] + if {$res == "0 {}" || $res == "1 {database disk image is malformed}"} { + set res "" + } + set res + } {} +} + +finish_test +