Re: Usin ClangStatic Analyzer for zend_parse_parameters

On Mon, Feb 17, 2014 at 9:42 PM, Johannes Schlüter <johannes@schlueters.de> wrote: > Hi, > > we have the plan to change types we use for zval data. A common place we > use this in is the family of zend_parse_[method_]parameters[_ex] > functions. The issue there is that those a variadic so the compiler > won't notice mistakes. As I was looking into clang internals a bit I > decided to create a plugin to clang's static analyzer to help with this. > The result can be found on > https://github.com/johannes/clang-php-checker/ > > If you get it compiled and working it can detect such things: > > $ cat foo.c > int zend_parse_parameters(int ht, char *, ...); > > #define FORMAT "lsd" > > char *get_format(int a) { > if (!a) { > return 0; > } > return FORMAT; > } > > int **get_location_for_int(); > > void foo() { > char *c; > int i; > > extern int x; > char *format = get_format(x); > zend_parse_parameters(0, format, get_location_for_int(), &c, > &i); > } > > > $ clang -cc1 -analyze -analyzer-checker=php.ZPPChecker \ > -load ./PHPChecker.so foo.c > foo.c:20:3: warning: Type of passed argument &SymRegion{conj_$3{int **}} is of type > int ** which did not match expected long * (aka. long *) for modifier 'l' at offset 3 > zend_parse_parameters(0, format, get_location_for_int(), &c, &i); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > foo.c:20:3: warning: Too few arguments for format "lsd" while checking for > modifier 'd' > zend_parse_parameters(0, format, get_location_for_int(), &c, &i); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 2 warnings generated. > > > The part of "&SymRegion{conj_$3{int **}}" isn't really nice, yet, > usually you will have variable names there which will be printed > instead, this example was supposed to show a more complex case. (this > example is held simple and missing i.e. TSRM parameters, they are > supported in ZTS and non-ZTS-mode) > > Running this over one of my PHP builds I got those error reports: > http://schlueters.de/zppcheck/ maybe somebody wants to go > through them > and fix at least the trivial ones. > > > The checker plugin currently only knows about PHP 5.5's zpp modifiers, > support for the size_t branch will be added (it's trivial to add, I'm > happy about pull requests, check for PHPSample in the code!) > > If there is interest we can add such checks for other PHP-specific > things. > > johannes > > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > Not sure about some :-p https://github.com/jpauli/php-src/compare/zpp_type_fixed Julien