/ Check-in [b0b4624f]
Login

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: b0b4624fc5d53bb0cc9fae7dad51984837d946ac
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
Hide Diffs Side-by-Side Diffs Ignore Whitespace Patch

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  +}