SQLite Forum

Static analysis warnings in lemon parser

Static analysis warnings in lemon parser

(1) By Ian McInerney (imcinerney) on 2020-09-20 11:24:36 [link] [source]

The static analysis tool my project is using is flagging several warnings inside the lemon parser and its generated code:

Inside Lemon.c

  1. The variable tpltname inside the tplt_open function is leaked by the return statements

  2. The variable pathbuf inside the pathsearch function is leaked by the return statements.

  3. The array x inside the Rule_sort function can be overrun because i can be 1 greater than the end of its array and it is then used for the access x[i] = rp. In the example given by my analysis tool x has 32 elements. If all elements of the x array were used previously and must be zeroed, the loop can run its final iteration with x = 31 and then the post increment will make i=32 causing the loop to terminate. Then the access immediately following into x[i] = rp will access the element at x[32], which is one beyond the end of the array.

Inside the generated code

  1. The access if( yyRuleInfoNRhs[yyruleno]==0 ) inside yy_reduce uses yyruleno unguarded by a check to ensure that it actually is a valid rule (e.g. its value is actually inside the array). There is a guarded access to it just above that point inside the debug code though, so it would make sense to guard this access as well.

(2) By Richard Hipp (drh) on 2020-09-20 12:21:04 in reply to 1 [source]

These are all false-positives. Nevertheless, I have implemented changes in at attempt to get your static analyzer to hush-up. Please try again with the latest trunk code.

(3) By Ian McInerney (imcinerney) on 2020-09-21 10:12:04 in reply to 2 [link] [source]

Thanks, that removed 4 of the 5 (but added a new one because pathbufptr is uninitialized). I think the remaining can be fixed by

  1. Initializing pathbufptr to 0
  2. Adding an else clause to the if( (pathbuf != 0) && (path!=0) that frees pathbuf (since the path that leads to this leak is with pathbuf non-zero and path zero - which also really shouldn't happen).

Thanks for your work on this.