Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72662
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain schlueters.de from 217.114.215.10 cause and error)
To: PHP internals list <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Date: Mon, 17 Feb 2014 21:42:35 +0100
Message-ID: <1392669755.3990.291.camel@guybrush>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Usin ClangStatic Analyzer for zend_parse_parameters
From: johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)

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