Many hyperlinks are disabled.
Use anonymous login
to enable hyperlinks.
Overview
| Comment: | Add a utility program that looks for assert(), NEVER(), ALWAYS(), and testcase() macros that have side-effects, and reports errors when they are found. Also fix a bug that this utility detected as it was being tested. |
|---|---|
| Downloads: | Tarball | ZIP archive | SQL archive |
| Timelines: | family | ancestors | descendants | both | trunk |
| Files: | files | file ages | folders |
| SHA1: |
b0b4624fc5d53bb0cc9fae7dad519848 |
| User & Date: | drh 2016-02-06 22:32:06 |
Context
|
2016-02-07
| ||
| 00:08 | Add the sourcetest target to Makefile.msc. check-in: ab269e72 user: drh tags: trunk | |
|
2016-02-06
| ||
| 22:32 | Add a utility program that looks for assert(), NEVER(), ALWAYS(), and testcase() macros that have side-effects, and reports errors when they are found. Also fix a bug that this utility detected as it was being tested. check-in: b0b4624f user: drh tags: trunk | |
| 19:48 | Make sure variable declarations occur at the beginning of blocks, even with SQLITE_DEBUG enabled. check-in: 2f7778e6 user: drh tags: trunk | |
Changes
Changes to Makefile.in.
578 578 sqlite3$(TEXE): $(TOP)/src/shell.c libsqlite3.la sqlite3.h 579 579 $(LTLINK) $(READLINE_FLAGS) $(SHELL_OPT) -o $@ \ 580 580 $(TOP)/src/shell.c libsqlite3.la \ 581 581 $(LIBREADLINE) $(TLIBS) -rpath "$(libdir)" 582 582 583 583 sqldiff$(TEXE): $(TOP)/tool/sqldiff.c sqlite3.c sqlite3.h 584 584 $(LTLINK) -o $@ $(TOP)/tool/sqldiff.c sqlite3.c $(TLIBS) 585 + 586 +srcck1$(BEXE): $(TOP)/tool/srcck1.c 587 + $(BCC) -o srcck1$(BEXE) $(TOP)/tool/srcck1.c 588 + 589 +sourcetest: srcck1$(BEXE) sqlite3.c 590 + ./srcck1 sqlite3.c 585 591 586 592 fuzzershell$(TEXE): $(TOP)/tool/fuzzershell.c sqlite3.c sqlite3.h 587 593 $(LTLINK) -o $@ $(FUZZERSHELL_OPT) \ 588 594 $(TOP)/tool/fuzzershell.c sqlite3.c $(TLIBS) 589 595 590 596 fuzzcheck$(TEXE): $(TOP)/test/fuzzcheck.c sqlite3.c sqlite3.h 591 597 $(LTLINK) -o $@ $(FUZZCHECK_OPT) $(TOP)/test/fuzzcheck.c sqlite3.c $(TLIBS) ................................................................................ 1079 1085 # 1080 1086 quicktest: ./testfixture$(TEXE) 1081 1087 ./testfixture$(TEXE) $(TOP)/test/extraquick.test $(TESTOPTS) 1082 1088 1083 1089 # This is the common case. Run many tests that do not take too long, 1084 1090 # including fuzzcheck, sqlite3_analyzer, and sqldiff tests. 1085 1091 # 1086 -test: $(TESTPROGS) fastfuzztest 1092 +test: $(TESTPROGS) sourcetest fastfuzztest 1087 1093 ./testfixture$(TEXE) $(TOP)/test/veryquick.test $(TESTOPTS) 1088 1094 1089 1095 # Run a test using valgrind. This can take a really long time 1090 1096 # because valgrind is so much slower than a native machine. 1091 1097 # 1092 1098 valgrindtest: $(TESTPROGS) valgrindfuzz 1093 1099 OMIT_MISUSE=1 valgrind -v ./testfixture$(TEXE) $(TOP)/test/permutations.test valgrind $(TESTOPTS)
Changes to main.mk.
475 475 sqlite3$(EXE): $(TOP)/src/shell.c libsqlite3.a sqlite3.h 476 476 $(TCCX) $(READLINE_FLAGS) -o sqlite3$(EXE) $(SHELL_OPT) \ 477 477 $(TOP)/src/shell.c libsqlite3.a $(LIBREADLINE) $(TLIBS) $(THREADLIB) 478 478 479 479 sqldiff$(EXE): $(TOP)/tool/sqldiff.c sqlite3.c sqlite3.h 480 480 $(TCCX) -o sqldiff$(EXE) -DSQLITE_THREADSAFE=0 \ 481 481 $(TOP)/tool/sqldiff.c sqlite3.c $(TLIBS) $(THREADLIB) 482 + 483 +srcck1$(EXE): $(TOP)/tool/srcck1.c 484 + $(BCC) -o srcck1$(EXE) $(TOP)/tool/srcck1.c 485 + 486 +sourcetest: srcck1$(EXE) sqlite3.c 487 + ./srcck1 sqlite3.c 482 488 483 489 fuzzershell$(EXE): $(TOP)/tool/fuzzershell.c sqlite3.c sqlite3.h 484 490 $(TCCX) -o fuzzershell$(EXE) -DSQLITE_THREADSAFE=0 -DSQLITE_OMIT_LOAD_EXTENSION \ 485 491 $(FUZZERSHELL_OPT) $(TOP)/tool/fuzzershell.c sqlite3.c \ 486 492 $(TLIBS) $(THREADLIB) 487 493 488 494 fuzzcheck$(EXE): $(TOP)/test/fuzzcheck.c sqlite3.c sqlite3.h ................................................................................ 764 770 # 765 771 quicktest: ./testfixture$(EXE) 766 772 ./testfixture$(EXE) $(TOP)/test/extraquick.test $(TESTOPTS) 767 773 768 774 # The default test case. Runs most of the faster standard TCL tests, 769 775 # and fuzz tests, and sqlite3_analyzer and sqldiff tests. 770 776 # 771 -test: $(TESTPROGS) fastfuzztest 777 +test: $(TESTPROGS) sourcetest fastfuzztest 772 778 ./testfixture$(EXE) $(TOP)/test/veryquick.test $(TESTOPTS) 773 779 774 780 # Run a test using valgrind. This can take a really long time 775 781 # because valgrind is so much slower than a native machine. 776 782 # 777 783 valgrindtest: $(TESTPROGS) valgrindfuzz 778 784 OMIT_MISUSE=1 valgrind -v \
Changes to src/btree.c.
6144 6144 ** Use a call to btreeParseCellPtr() to verify that the values above 6145 6145 ** were computed correctly. 6146 6146 */ 6147 6147 #if SQLITE_DEBUG 6148 6148 { 6149 6149 CellInfo info; 6150 6150 pPage->xParseCell(pPage, pCell, &info); 6151 - assert( nHeader=(int)(info.pPayload - pCell) ); 6151 + assert( nHeader==(int)(info.pPayload - pCell) ); 6152 6152 assert( info.nKey==nKey ); 6153 6153 assert( *pnSize == info.nSize ); 6154 6154 assert( spaceLeft == info.nLocal ); 6155 6155 } 6156 6156 #endif 6157 6157 6158 6158 /* Write the payload into the local Cell and any extra into overflow pages */ ................................................................................ 7803 7803 */ 7804 7804 static int balance(BtCursor *pCur){ 7805 7805 int rc = SQLITE_OK; 7806 7806 const int nMin = pCur->pBt->usableSize * 2 / 3; 7807 7807 u8 aBalanceQuickSpace[13]; 7808 7808 u8 *pFree = 0; 7809 7809 7810 - TESTONLY( int balance_quick_called = 0 ); 7811 - TESTONLY( int balance_deeper_called = 0 ); 7810 + VVA_ONLY( int balance_quick_called = 0 ); 7811 + VVA_ONLY( int balance_deeper_called = 0 ); 7812 7812 7813 7813 do { 7814 7814 int iPage = pCur->iPage; 7815 7815 MemPage *pPage = pCur->apPage[iPage]; 7816 7816 7817 7817 if( iPage==0 ){ 7818 7818 if( pPage->nOverflow ){ 7819 7819 /* The root page of the b-tree is overfull. In this case call the 7820 7820 ** balance_deeper() function to create a new child for the root-page 7821 7821 ** and copy the current contents of the root-page to it. The 7822 7822 ** next iteration of the do-loop will balance the child page. 7823 7823 */ 7824 - assert( (balance_deeper_called++)==0 ); 7824 + assert( balance_deeper_called==0 ); 7825 + VVA_ONLY( balance_deeper_called++ ); 7825 7826 rc = balance_deeper(pPage, &pCur->apPage[1]); 7826 7827 if( rc==SQLITE_OK ){ 7827 7828 pCur->iPage = 1; 7828 7829 pCur->aiIdx[0] = 0; 7829 7830 pCur->aiIdx[1] = 0; 7830 7831 assert( pCur->apPage[1]->nOverflow ); 7831 7832 } ................................................................................ 7856 7857 ** buffer. 7857 7858 ** 7858 7859 ** The purpose of the following assert() is to check that only a 7859 7860 ** single call to balance_quick() is made for each call to this 7860 7861 ** function. If this were not verified, a subtle bug involving reuse 7861 7862 ** of the aBalanceQuickSpace[] might sneak in. 7862 7863 */ 7863 - assert( (balance_quick_called++)==0 ); 7864 + assert( balance_quick_called==0 ); 7865 + VVA_ONLY( balance_quick_called++ ); 7864 7866 rc = balance_quick(pParent, pPage, aBalanceQuickSpace); 7865 7867 }else 7866 7868 #endif 7867 7869 { 7868 7870 /* In this case, call balance_nonroot() to redistribute cells 7869 7871 ** between pPage and up to 2 of its sibling pages. This involves 7870 7872 ** modifying the contents of pParent, which may cause pParent to ................................................................................ 9323 9325 BtShared *pBt = p->pBt; 9324 9326 int savedDbFlags = pBt->db->flags; 9325 9327 char zErr[100]; 9326 9328 VVA_ONLY( int nRef ); 9327 9329 9328 9330 sqlite3BtreeEnter(p); 9329 9331 assert( p->inTrans>TRANS_NONE && pBt->inTransaction>TRANS_NONE ); 9330 - assert( (nRef = sqlite3PagerRefcount(pBt->pPager))>=0 ); 9332 + VVA_ONLY( nRef = sqlite3PagerRefcount(pBt->pPager) ); 9333 + assert( nRef>=0 ); 9331 9334 sCheck.pBt = pBt; 9332 9335 sCheck.pPager = pBt->pPager; 9333 9336 sCheck.nPage = btreePagecount(sCheck.pBt); 9334 9337 sCheck.mxErr = mxErr; 9335 9338 sCheck.nErr = 0; 9336 9339 sCheck.mallocFailed = 0; 9337 9340 sCheck.zPfx = 0;
Changes to src/main.c.
3562 3562 ** assert() is disabled, then the return value is zero. If X is 3563 3563 ** false and assert() is enabled, then the assertion fires and the 3564 3564 ** process aborts. If X is false and assert() is disabled, then the 3565 3565 ** return value is zero. 3566 3566 */ 3567 3567 case SQLITE_TESTCTRL_ASSERT: { 3568 3568 volatile int x = 0; 3569 - assert( (x = va_arg(ap,int))!=0 ); 3569 + assert( /*side-effects-ok*/ (x = va_arg(ap,int))!=0 ); 3570 3570 rc = x; 3571 3571 break; 3572 3572 } 3573 3573 3574 3574 3575 3575 /* 3576 3576 ** sqlite3_test_control(SQLITE_TESTCTRL_ALWAYS, int X)
Added tool/srcck1.c.
1 +/* 2 +** The program does some simple static analysis of the sqlite3.c source 3 +** file looking for mistakes. 4 +** 5 +** Usage: 6 +** 7 +** ./srcck1 sqlite3.c 8 +** 9 +** This program looks for instances of assert(), ALWAYS(), NEVER() or 10 +** testcase() that contain side-effects and reports errors if any such 11 +** instances are found. 12 +** 13 +** The aim of this utility is to prevent recurrences of errors such 14 +** as the one fixed at: 15 +** 16 +** https://www.sqlite.org/src/info/a2952231ac7abe16 17 +** 18 +** Note that another similar error was found by this utility when it was 19 +** first written. That other error was fixed by the same check-in that 20 +** committed the first version of this utility program. 21 +*/ 22 +#include <stdlib.h> 23 +#include <ctype.h> 24 +#include <stdio.h> 25 + 26 +/* Read the complete text of a file into memory. Return a pointer to 27 +** the result. Panic if unable to read the file or allocate memory. 28 +*/ 29 +static char *readFile(const char *zFilename){ 30 + FILE *in; 31 + char *z; 32 + long n; 33 + size_t got; 34 + 35 + in = fopen(zFilename, "rb"); 36 + if( in==0 ){ 37 + fprintf(stderr, "unable to open '%s' for reading\n", zFilename); 38 + exit(1); 39 + } 40 + fseek(in, 0, SEEK_END); 41 + n = ftell(in); 42 + rewind(in); 43 + z = malloc( n+1 ); 44 + if( z==0 ){ 45 + fprintf(stderr, "cannot allocate %d bytes to store '%s'\n", 46 + (int)(n+1), zFilename); 47 + exit(1); 48 + } 49 + got = fread(z, 1, n, in); 50 + fclose(in); 51 + if( got!=(size_t)n ){ 52 + fprintf(stderr, "only read %d of %d bytes from '%s'\n", 53 + (int)got, (int)n, zFilename); 54 + exit(1); 55 + } 56 + z[n] = 0; 57 + return z; 58 +} 59 + 60 +/* Change the C code in the argument to see if it might have 61 +** side effects. The only accurate way to know this is to do a full 62 +** parse of the C code, which this routine does not do. This routine 63 +** uses a simple heuristic of looking for: 64 +** 65 +** * '=' not immediately after '>', '<', '!', or '='. 66 +** * '++' 67 +** * '--' 68 +** 69 +** If the code contains the phrase "side-effects-ok" is inside a 70 +** comment, then always return false. This is used to disable checking 71 +** for assert()s with deliberate side-effects, such as used by 72 +** SQLITE_TESTCTRL_ASSERT - a facility that allows applications to 73 +** determine at runtime whether or not assert()s are enabled. 74 +** Obviously, that determination cannot be made unless the assert() 75 +** has some side-effect. 76 +** 77 +** Return true if a side effect is seen. Return false if not. 78 +*/ 79 +static int hasSideEffect(const char *z, unsigned int n){ 80 + unsigned int i; 81 + for(i=0; i<n; i++){ 82 + if( z[i]=='/' && strncmp(&z[i], "/*side-effects-ok*/", 19)==0 ) return 0; 83 + if( z[i]=='=' && i>0 && z[i-1]!='=' && z[i-1]!='>' 84 + && z[i-1]!='<' && z[i-1]!='!' && z[i+1]!='=' ) return 1; 85 + if( z[i]=='+' && z[i+1]=='+' ) return 1; 86 + if( z[i]=='-' && z[i+1]=='-' ) return 1; 87 + } 88 + return 0; 89 +} 90 + 91 +/* Return the number of bytes in string z[] prior to the first unmatched ')' 92 +** character. 93 +*/ 94 +static unsigned int findCloseParen(const char *z){ 95 + unsigned int nOpen = 0; 96 + unsigned i; 97 + for(i=0; z[i]; i++){ 98 + if( z[i]=='(' ) nOpen++; 99 + if( z[i]==')' ){ 100 + if( nOpen==0 ) break; 101 + nOpen--; 102 + } 103 + } 104 + return i; 105 +} 106 + 107 +/* Search for instances of assert(...), ALWAYS(...), NEVER(...), and/or 108 +** testcase(...) where the argument contains side effects. 109 +** 110 +** Print error messages whenever a side effect is found. Return the number 111 +** of problems seen. 112 +*/ 113 +static unsigned int findAllSideEffects(const char *z){ 114 + unsigned int lineno = 1; /* Line number */ 115 + unsigned int i; 116 + unsigned int nErr = 0; 117 + char c, prevC = 0; 118 + for(i=0; (c = z[i])!=0; prevC=c, i++){ 119 + if( c=='\n' ){ lineno++; continue; } 120 + if( isalpha(c) && !isalpha(prevC) ){ 121 + if( strncmp(&z[i],"assert(",7)==0 122 + || strncmp(&z[i],"ALWAYS(",7)==0 123 + || strncmp(&z[i],"NEVER(",6)==0 124 + || strncmp(&z[i],"testcase(",9)==0 125 + ){ 126 + unsigned int j, n; 127 + const char *z2 = &z[i+5]; 128 + while( z2[0]!='(' ){ z2++; } 129 + z2++; 130 + n = findCloseParen(z2); 131 + if( hasSideEffect(z2, n) ){ 132 + nErr++; 133 + fprintf(stderr, "side-effect line %u: %.*s\n", lineno, 134 + (int)(&z2[n+1] - &z[i]), &z[i]); 135 + } 136 + } 137 + } 138 + } 139 + return nErr; 140 +} 141 + 142 +int main(int argc, char **argv){ 143 + char *z; 144 + unsigned int nErr = 0; 145 + if( argc!=2 ){ 146 + fprintf(stderr, "Usage: %s FILENAME\n", argv[0]); 147 + return 1; 148 + } 149 + z = readFile(argv[1]); 150 + nErr = findAllSideEffects(z); 151 + free(z); 152 + if( nErr ){ 153 + fprintf(stderr, "Found %u undesirable side-effects\n", nErr); 154 + return 1; 155 + } 156 + return 0; 157 +}