On 11/16/2016 05:53 AM, Nico Williams wrote:
I don't normally pay attention to warnings when compiling SQLite3, nor
to Coverity or other static analysis tools' output either, as I'm quite
aware that most of these are false positives and thus unwelcome noise
here.
However, I do sample them occasionally, and though usually such reports
are false positives, here are two that don't quite look like false
positives to me. I get these from building the SQLite3 3.15.1
amalgamation.
Uninitialized pointer dereference:
115861 static void generateColumnTypes(
115862 Parse *pParse, /* Parser context */
115863 SrcList *pTabList, /* List of tables */
115864 ExprList *pEList /* Expressions defining the result set */
115865 ){
115866 #ifndef SQLITE_OMIT_DECLTYPE
115867 Vdbe *v = pParse->pVdbe;
115868 int i;
1. var_decl: Declaring variable sNC without initializer.
115869 NameContext sNC;
115870 sNC.pSrcList = pTabList;
115871 sNC.pParse = pParse;
2. Condition i < pEList->nExpr, taking true branch
115872 for(i=0; i<pEList->nExpr; i++){
115873 Expr *p = pEList->a[i].pExpr;
115874 const char *zType;
115875 #ifdef SQLITE_ENABLE_COLUMN_METADATA
115876 const char *zOrigDb = 0;
115877 const char *zOrigTab = 0;
115878 const char *zOrigCol = 0;
115879 zType = columnType(&sNC, p, &zOrigDb, &zOrigTab, &zOrigCol, 0);
115880
115881 /* The vdbe must make its own copy of the column-type and other
115882 ** column specific strings, in case the schema is reset before
this
115883 ** virtual machine is deleted.
115884 */
115885 sqlite3VdbeSetColName(v, i, COLNAME_DATABASE, zOrigDb,
SQLITE_TRANSIENT);
115886 sqlite3VdbeSetColName(v, i, COLNAME_TABLE, zOrigTab,
SQLITE_TRANSIENT);
115887 sqlite3VdbeSetColName(v, i, COLNAME_COLUMN, zOrigCol,
SQLITE_TRANSIENT);
115888 #else
CID 12 301 (#1 of 1): Uninitialized pointer read (UNINIT)
3. uni nit_use_in_call: Using uninitialized value sNC.pNext when calling
columnTypeImpl.
115889 zType = columnType(&sNC, p, 0, 0, 0, 0);
columnType() is a macro expanding to a call to columnTypeImpl().
Anyways, the analysis from here is non-trivial, and I can't convince
myself that sNC.pNext will not be dereferenced.
Thanks for taking the time to look into these.
Some kind of assert() could be helpful there I think. The reason
sNC.pNext will not be accessed is that generateColumnNames() is only
called (a) on a top-level SELECT statement and (b) after all references
have already resolved successfully. Implying that this:
http://www.sqlite.org/src/artifact/672b1af237ad2?ln=1406
is always true.
The obvious fix is to initialize sNC a bit more before the loop at
115872. At least setting sNC.pNext = 0 seems like the right thing to
do.
Another one that I find difficult to analyze is a possible out-of-bounds
read in vdbeSorterCompareInt():
85712 static const u8 aLen[] = {0, 1, 2, 3, 4, 6, 8 };
85713 int i;
85714 res = 0;
85715 for(i=0; i<aLen[s1]; i++){
where s1 is not guaranteed to be less than 7:
85692 const u8 * const p1 = (const u8 * const)pKey1;
85693 const u8 * const p2 = (const u8 * const)pKey2;
85694 const int s1 = p1[1]; /* Left hand serial type */
85695 const int s2 = p2[1]; /* Right hand serial type */
85700 assert( (s1>0 && s1<7) || s1==8 || s1==9 );
85701 assert( (s2>0 && s2<7) || s2==8 || s2==9 );
85702
85703 if( s1>7 && s2>7 ){
85704 res = s1 - s2;
85705 }else{
85706 if( s1==s2 ){
At 85715 we know that (s1 <= 7 || s2 <= 7) && s1 == s2, and we also know
that either s1 or s2 can be 8 or 9, so aLen[s1] at 85715 could very well
have s1 > 6, which would read past the bounds of aLen[].
I think ( ( s1<=7 || s2<=7) && s1==s2 ) implies that s1<=7. And we
assume s1!=7 because there is an assert() that says so.
Dan.
_______________________________________________
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
