From: drh Date: Mon, 27 Sep 2010 21:09:31 +0000 (+0000) Subject: Add assert() statements that fail when a shallow copy is accessed after the X-Git-Tag: version-3.7.4~137^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2b4ded993bab6d6fc2d16be2ea9c311deccb5af1;p=thirdparty%2Fsqlite.git Add assert() statements that fail when a shallow copy is accessed after the original has been modified. These assert() statements should detect the kinds of subtle SCopy bugs such as caused the fault in ticket [b351d95f9cd5ef17e9d9dbae18f]. FossilOrigin-Name: 8b8e1732e8410efd5f8e308c43abe521584e87a0 --- diff --git a/manifest b/manifest index d7aead42bc..15647069d4 100644 --- a/manifest +++ b/manifest @@ -1,8 +1,8 @@ -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 -C Simplifications\sto\sthe\sexpression\scode\sgenerator.\s\sRemove\sabout\s80\slines\sof\nolder\sand\sobsolete\scode. -D 2010-09-27T18:14:12 +C Add\sassert()\sstatements\sthat\sfail\swhen\sa\sshallow\scopy\sis\saccessed\safter\sthe\noriginal\shas\sbeen\smodified.\s\sThese\sassert()\sstatements\sshould\sdetect\sthe\skinds\nof\ssubtle\sSCopy\sbugs\ssuch\sas\scaused\sthe\sfault\nin\sticket\s[b351d95f9cd5ef17e9d9dbae18f]. +D 2010-09-27T21:09:32 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in c599a15d268b1db2aeadea19df2adc3bf2eb6bee F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -228,13 +228,13 @@ F src/update.c 227e6cd512108b84f69421fc6c7aa1b83d60d6e0 F src/utf.c 1baeeac91707a4df97ccc6141ec0f808278af685 F src/util.c 5f5f4db4e799224713582bb49124827b16aa8e54 F src/vacuum.c 924bd1bcee2dfb05376f79845bd3b4cec7b54b2f -F src/vdbe.c 1373d40f5985577a30350bfa78ebd8e1d905ee89 +F src/vdbe.c 00f017d904614e0076d09e40668884a7c61e6899 F src/vdbe.h 4de0efb4b0fdaaa900cf419b35c458933ef1c6d2 -F src/vdbeInt.h a247bd5448039e83394bf4179975b2ae0092874c +F src/vdbeInt.h 7f4cf1b2b69bef3a432b1f23dfebef57275436b4 F src/vdbeapi.c 14040ffad59a9961468a457754d30266e71ebb2d F src/vdbeaux.c de0b06b11a25293e820a49159eca9f1c51a64716 F src/vdbeblob.c 258a6010ba7a82b72b327fb24c55790655689256 -F src/vdbemem.c 2b8210992499e8b846f49fa4b0035bbc2cda0ee0 +F src/vdbemem.c 5005341c08018c9da8c6434e974a38944270b4d5 F src/vdbetrace.c 864cef96919323482ebd9986f2132435115e9cc2 F src/vtab.c 0e8e0cb30dffb078367e843e84e37ef99236c7e4 F src/wal.c 7081f148cb52b0cf2280e6384196402dc58130a3 @@ -870,14 +870,18 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P a7645d293801da64a7579737d0a8b48117af2e2c -R 31b9d8f725b1355a3ecd589f9c768e32 +P 53f5cfe11516d0496c7760378b6305c3088d479f +R e854b2c96b563f29f2e69ca897448670 +T *bgcolor * #ffc0d0 +T *branch * bug-53f5cfe115 +T *sym-bug-53f5cfe115 * +T -sym-trunk * U drh -Z da56ebaf0e041408b3401a85ccd2b2a6 +Z 17ce6c0da33f85f32ad716e4a4d31291 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) -iD8DBQFMoN73oxKgR168RlERAqBjAJ4/xTLrVYlxWxhlWBOiHhf+ogrH4gCdGQFv -0abukVW0mExAm/y5XFe9a7M= -=VfNl +iD8DBQFMoQgRoxKgR168RlERApEWAJ9n5vftkvf9G1CdiZ1ojeecdSsSuwCfRWzj +CY8hXCRBjuCdgYG8Cw1+YgE= +=49Tg -----END PGP SIGNATURE----- diff --git a/manifest.uuid b/manifest.uuid index b42edba870..6b0616b547 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -53f5cfe11516d0496c7760378b6305c3088d479f \ No newline at end of file +8b8e1732e8410efd5f8e308c43abe521584e87a0 \ No newline at end of file diff --git a/src/vdbe.c b/src/vdbe.c index a416020984..81a6952ffa 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -46,6 +46,17 @@ #include "sqliteInt.h" #include "vdbeInt.h" +/* +** Invoke this macro on memory cells just prior to changing the +** value of the cell. This macro verifies that shallow copies are +** not misused. +*/ +#ifdef SQLITE_DEBUG +# define memAboutToChange(P,M) sqlite3VdbeMemPrepareToChange(P,M) +#else +# define memAboutToChange(P,M) +#endif + /* ** The following global variable is incremented every time a cursor ** moves, either by the OP_SeekXX, OP_Next, or OP_Prev opcodes. The test @@ -667,6 +678,7 @@ int sqlite3VdbeExec( assert( pOp->p2>0 ); assert( pOp->p2<=p->nMem ); pOut = &aMem[pOp->p2]; + memAboutToChange(p, pOut); sqlite3VdbeMemReleaseExternal(pOut); pOut->flags = MEM_Int; } @@ -676,25 +688,30 @@ int sqlite3VdbeExec( if( (pOp->opflags & OPFLG_IN1)!=0 ){ assert( pOp->p1>0 ); assert( pOp->p1<=p->nMem ); + assert( memIsValid(&aMem[pOp->p1]) ); REGISTER_TRACE(pOp->p1, &aMem[pOp->p1]); } if( (pOp->opflags & OPFLG_IN2)!=0 ){ assert( pOp->p2>0 ); assert( pOp->p2<=p->nMem ); + assert( memIsValid(&aMem[pOp->p2]) ); REGISTER_TRACE(pOp->p2, &aMem[pOp->p2]); } if( (pOp->opflags & OPFLG_IN3)!=0 ){ assert( pOp->p3>0 ); assert( pOp->p3<=p->nMem ); + assert( memIsValid(&aMem[pOp->p3]) ); REGISTER_TRACE(pOp->p3, &aMem[pOp->p3]); } if( (pOp->opflags & OPFLG_OUT2)!=0 ){ assert( pOp->p2>0 ); assert( pOp->p2<=p->nMem ); + memAboutToChange(p, &aMem[pOp->p2]); } if( (pOp->opflags & OPFLG_OUT3)!=0 ){ assert( pOp->p3>0 ); assert( pOp->p3<=p->nMem ); + memAboutToChange(p, &aMem[pOp->p3]); } #endif @@ -756,6 +773,7 @@ case OP_Goto: { /* jump */ case OP_Gosub: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; assert( (pIn1->flags & MEM_Dyn)==0 ); + memAboutToChange(p, pIn1); pIn1->flags = MEM_Int; pIn1->u.i = pc; REGISTER_TRACE(pOp->p1, pIn1); @@ -1015,6 +1033,8 @@ case OP_Move: { while( n-- ){ assert( pOut<=&aMem[p->nMem] ); assert( pIn1<=&aMem[p->nMem] ); + assert( memIsValid(pIn1) ); + memAboutToChange(p, pOut); zMalloc = pOut->zMalloc; pOut->zMalloc = 0; sqlite3VdbeMemMove(pOut, pIn1); @@ -1060,6 +1080,9 @@ case OP_SCopy: { /* in1, out2 */ pOut = &aMem[pOp->p2]; assert( pOut!=pIn1 ); sqlite3VdbeMemShallowCopy(pOut, pIn1, MEM_Ephem); +#ifdef SQLITE_DEBUG + if( pOut->pScopyFrom==0 ) pOut->pScopyFrom = pIn1; +#endif REGISTER_TRACE(pOp->p2, pOut); break; } @@ -1118,6 +1141,8 @@ case OP_ResultRow: { */ pMem = p->pResultSet = &aMem[pOp->p1]; for(i=0; ip2; i++){ + assert( memIsValid(&pMem[i]) ); + memAboutToChange(p, &pMem[i]); sqlite3VdbeMemNulTerminate(&pMem[i]); sqlite3VdbeMemStoreType(&pMem[i]); REGISTER_TRACE(pOp->p1+i, &pMem[i]); @@ -1348,7 +1373,9 @@ case OP_Function: { assert( pOp->p3p2 || pOp->p3>=pOp->p2+n ); pArg = &aMem[pOp->p2]; for(i=0; ip2+i, pArg); } @@ -1364,6 +1391,7 @@ case OP_Function: { assert( pOp->p3>0 && pOp->p3<=p->nMem ); pOut = &aMem[pOp->p3]; + memAboutToChange(p, pOut); ctx.s.flags = MEM_Null; ctx.s.db = db; ctx.s.xDel = 0; @@ -1483,6 +1511,7 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ */ case OP_AddImm: { /* in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); sqlite3VdbeMemIntegerify(pIn1); pIn1->u.i += pOp->p2; break; @@ -1497,6 +1526,7 @@ case OP_AddImm: { /* in1 */ */ case OP_MustBeInt: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); applyAffinity(pIn1, SQLITE_AFF_NUMERIC, encoding); if( (pIn1->flags & MEM_Int)==0 ){ if( pOp->p2==0 ){ @@ -1523,6 +1553,7 @@ case OP_MustBeInt: { /* jump, in1 */ */ case OP_RealAffinity: { /* in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( pIn1->flags & MEM_Int ){ sqlite3VdbeMemRealify(pIn1); } @@ -1542,6 +1573,7 @@ case OP_RealAffinity: { /* in1 */ */ case OP_ToText: { /* same as TK_TO_TEXT, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( pIn1->flags & MEM_Null ) break; assert( MEM_Str==(MEM_Blob>>3) ); pIn1->flags |= (pIn1->flags&MEM_Blob)>>3; @@ -1564,6 +1596,7 @@ case OP_ToText: { /* same as TK_TO_TEXT, in1 */ */ case OP_ToBlob: { /* same as TK_TO_BLOB, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( pIn1->flags & MEM_Null ) break; if( (pIn1->flags & MEM_Blob)==0 ){ applyAffinity(pIn1, SQLITE_AFF_TEXT, encoding); @@ -1588,6 +1621,7 @@ case OP_ToBlob: { /* same as TK_TO_BLOB, in1 */ */ case OP_ToNumeric: { /* same as TK_TO_NUMERIC, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( (pIn1->flags & (MEM_Null|MEM_Int|MEM_Real))==0 ){ sqlite3VdbeMemNumerify(pIn1); } @@ -1606,6 +1640,7 @@ case OP_ToNumeric: { /* same as TK_TO_NUMERIC, in1 */ */ case OP_ToInt: { /* same as TK_TO_INT, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( (pIn1->flags & MEM_Null)==0 ){ sqlite3VdbeMemIntegerify(pIn1); } @@ -1624,6 +1659,7 @@ case OP_ToInt: { /* same as TK_TO_INT, in1 */ */ case OP_ToReal: { /* same as TK_TO_REAL, in1 */ pIn1 = &aMem[pOp->p1]; + memAboutToChange(p, pIn1); if( (pIn1->flags & MEM_Null)==0 ){ sqlite3VdbeMemRealify(pIn1); } @@ -1716,6 +1752,8 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ pIn1 = &aMem[pOp->p1]; pIn3 = &aMem[pOp->p3]; + memAboutToChange(p, pIn1); + memAboutToChange(p, pIn3); flags1 = pIn1->flags; flags3 = pIn3->flags; if( (pIn1->flags | pIn3->flags)&MEM_Null ){ @@ -1766,6 +1804,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if( pOp->p5 & SQLITE_STOREP2 ){ pOut = &aMem[pOp->p2]; + memAboutToChange(p, pOut); MemSetTypeFlag(pOut, MEM_Int); pOut->u.i = res; REGISTER_TRACE(pOp->p2, pOut); @@ -1838,6 +1877,8 @@ case OP_Compare: { #endif /* SQLITE_DEBUG */ for(i=0; inField ); @@ -2063,6 +2104,7 @@ case OP_Column: { assert( p1nCursor ); assert( pOp->p3>0 && pOp->p3<=p->nMem ); pDest = &aMem[pOp->p3]; + memAboutToChange(p, pDest); MemSetTypeFlag(pDest, MEM_Null); zRec = 0; @@ -2110,6 +2152,7 @@ case OP_Column: { }else if( pC->pseudoTableReg>0 ){ pReg = &aMem[pC->pseudoTableReg]; assert( pReg->flags & MEM_Blob ); + assert( memIsValid(pReg) ); payloadSize = pReg->n; zRec = pReg->z; pC->cacheStatus = (pOp->p5&OPFLAG_CLEARCACHE) ? CACHE_STALE : p->cacheCtr; @@ -2332,6 +2375,8 @@ case OP_Affinity: { pIn1 = &aMem[pOp->p1]; while( (cAff = *(zAffinity++))!=0 ){ assert( pIn1 <= &p->aMem[p->nMem] ); + assert( memIsValid(pIn1) ); + memAboutToChange(p, pIn1); ExpandBlob(pIn1); applyAffinity(pIn1, cAff, encoding); pIn1++; @@ -2398,11 +2443,18 @@ case OP_MakeRecord: { pLast = &pData0[nField-1]; file_format = p->minWriteFileFormat; + /* Identify the output register */ + assert( pOp->p3p1 || pOp->p3>=pOp->p1+pOp->p2 ); + pOut = &aMem[pOp->p3]; + memAboutToChange(p, pOut); + /* Loop through the elements that will make up the record to figure ** out how much space is required for the new record. */ for(pRec=pData0; pRec<=pLast; pRec++){ + assert( memIsValid(pRec) ); if( zAffinity ){ + memAboutToChange(p, pRec); applyAffinity(pRec, zAffinity[pRec-pData0], encoding); } if( pRec->flags&MEM_Zero && pRec->n>0 ){ @@ -2436,8 +2488,6 @@ case OP_MakeRecord: { ** be one of the input registers (because the following call to ** sqlite3VdbeMemGrow() could clobber the value before it is used). */ - assert( pOp->p3p1 || pOp->p3>=pOp->p1+pOp->p2 ); - pOut = &aMem[pOp->p3]; if( sqlite3VdbeMemGrow(pOut, (int)nByte, 0) ){ goto no_mem; } @@ -2985,6 +3035,8 @@ case OP_OpenWrite: { assert( p2>0 ); assert( p2<=p->nMem ); pIn2 = &aMem[p2]; + assert( memIsValid(pIn2) ); + assert( (pIn2->flags & MEM_Int)!=0 ); sqlite3VdbeMemIntegerify(pIn2); p2 = (int)pIn2->u.i; /* The p2 value always comes from a prior OP_CreateTable opcode and @@ -3296,6 +3348,9 @@ case OP_SeekGt: { /* jump, in3 */ assert( oc!=OP_SeekLt || r.flags==0 ); r.aMem = &aMem[pOp->p3]; +#ifdef SQLITE_DEBUG + { int i; for(i=0; ipCursor, &r, 0, 0, &res); if( rc!=SQLITE_OK ){ @@ -3420,6 +3475,9 @@ case OP_Found: { /* jump, in3 */ r.pKeyInfo = pC->pKeyInfo; r.nField = (u16)pOp->p4.i; r.aMem = pIn3; +#ifdef SQLITE_DEBUG + { int i; for(i=0; ip3<=p->nMem ); pMem = &aMem[pOp->p3]; + memAboutToChange(p, pMem); } + assert( memIsValid(pMem) ); REGISTER_TRACE(pOp->p3, pMem); sqlite3VdbeMemIntegerify(pMem); @@ -3809,6 +3872,7 @@ case OP_InsertInt: { pData = &aMem[pOp->p2]; assert( pOp->p1>=0 && pOp->p1nCursor ); + assert( memIsValid(pData) ); pC = p->apCsr[pOp->p1]; assert( pC!=0 ); assert( pC->pCursor!=0 ); @@ -3819,6 +3883,7 @@ case OP_InsertInt: { if( pOp->opcode==OP_Insert ){ pKey = &aMem[pOp->p3]; assert( pKey->flags & MEM_Int ); + assert( memIsValid(pKey) ); REGISTER_TRACE(pOp->p3, pKey); iKey = pKey->u.i; }else{ @@ -3966,6 +4031,7 @@ case OP_RowData: { i64 n64; pOut = &aMem[pOp->p2]; + memAboutToChange(p, pOut); /* Note that RowKey and RowData are really exactly the same instruction */ assert( pOp->p1>=0 && pOp->p1nCursor ); @@ -4294,6 +4360,9 @@ case OP_IdxDelete: { r.nField = (u16)pOp->p3; r.flags = 0; r.aMem = &aMem[pOp->p2]; +#ifdef SQLITE_DEBUG + { int i; for(i=0; ip3]; +#ifdef SQLITE_DEBUG + { int i; for(i=0; iopcode==OP_IdxLT ){ res = -res; @@ -4486,6 +4558,8 @@ case OP_Clear: { if( pOp->p3 ){ p->nChange += nChange; if( pOp->p3>0 ){ + assert( memIsValid(&aMem[pOp->p3]) ); + memAboutToChange(p, &aMem[pOp->p3]); aMem[pOp->p3].u.i += nChange; } } @@ -4848,6 +4922,7 @@ case OP_Program: { /* jump */ pProgram = pOp->p4.pProgram; pRt = &aMem[pOp->p3]; + assert( memIsValid(pRt) ); assert( pProgram->nOp>0 ); /* If the p5 flag is clear, then recursive invocation of triggers is @@ -5017,6 +5092,7 @@ case OP_MemMax: { /* in2 */ }else{ pIn1 = &aMem[pOp->p1]; } + assert( memIsValid(pIn1) ); sqlite3VdbeMemIntegerify(pIn1); pIn2 = &aMem[pOp->p2]; sqlite3VdbeMemIntegerify(pIn2); @@ -5101,7 +5177,9 @@ case OP_AggStep: { apVal = p->apArg; assert( apVal || n==0 ); for(i=0; ip4.pFunc; @@ -5496,6 +5574,7 @@ case OP_VFilter: { /* jump */ pQuery = &aMem[pOp->p3]; pArgc = &pQuery[1]; pCur = p->apCsr[pOp->p1]; + assert( memIsValid(pQuery) ); REGISTER_TRACE(pOp->p3, pQuery); assert( pCur->pVtabCursor ); pVtabCursor = pCur->pVtabCursor; @@ -5551,6 +5630,7 @@ case OP_VColumn: { assert( pCur->pVtabCursor ); assert( pOp->p3>0 && pOp->p3<=p->nMem ); pDest = &aMem[pOp->p3]; + memAboutToChange(p, pDest); if( pCur->nullRow ){ sqlite3VdbeMemSetNull(pDest); break; @@ -5649,6 +5729,7 @@ case OP_VRename: { pVtab = pOp->p4.pVtab->pVtab; pName = &aMem[pOp->p1]; assert( pVtab->pModule->xRename ); + assert( memIsValid(pName) ); REGISTER_TRACE(pOp->p1, pName); assert( pName->flags & MEM_Str ); rc = pVtab->pModule->xRename(pVtab, pName->z); @@ -5700,6 +5781,8 @@ case OP_VUpdate: { apArg = p->apArg; pX = &aMem[pOp->p3]; for(i=0; iflags = ((p)->flags&~(MEM_TypeMask|MEM_Zero))|f) +/* +** Return true if a memory cell is not marked as invalid. This macro +** is for use inside assert() statements only. +*/ +#ifdef SQLITE_DEBUG +#define memIsValid(M) ((M)->flags & MEM_Invalid)==0 +#endif + /* A VdbeFunc is just a FuncDef (defined in sqliteInt.h) that contains ** additional information about auxiliary information bound to arguments @@ -392,6 +403,10 @@ void sqlite3VdbeFrameDelete(VdbeFrame*); int sqlite3VdbeFrameRestore(VdbeFrame *); void sqlite3VdbeMemStoreType(Mem *pMem); +#ifdef SQLITE_DEBUG +void sqlite3VdbeMemPrepareToChange(Vdbe*,Mem*); +#endif + #ifndef SQLITE_OMIT_FOREIGN_KEY int sqlite3VdbeCheckFk(Vdbe *, int); #else diff --git a/src/vdbemem.c b/src/vdbemem.c index da57438a0d..f556edf52b 100644 --- a/src/vdbemem.c +++ b/src/vdbemem.c @@ -593,6 +593,28 @@ int sqlite3VdbeMemTooBig(Mem *p){ return 0; } +#ifdef SQLITE_DEBUG +/* +** This routine prepares a memory cell for modication by breaking +** its link to a shallow copy and by marking any current shallow +** copies of this cell as invalid. +** +** This is used for testing and debugging only - to make sure shallow +** copies are not misused. +*/ +void sqlite3VdbeMemPrepareToChange(Vdbe *pVdbe, Mem *pMem){ + int i; + Mem *pX; + for(i=1, pX=&pVdbe->aMem[1]; i<=pVdbe->nMem; i++, pX++){ + if( pX->pScopyFrom==pMem ){ + pX->flags |= MEM_Invalid; + pX->pScopyFrom = 0; + } + } + pMem->pScopyFrom = 0; +} +#endif /* SQLITE_DEBUG */ + /* ** Size of struct Mem not including the Mem.zMalloc member. */