Re: Static Analysis of PHP_5_4 with CLANG

From: Date: Mon, 06 Feb 2012 05:52:15 +0000
Subject: Re: Static Analysis of PHP_5_4 with CLANG
References: 1 2 3  Groups: php.internals 
Request: Send a blank email to internals+get-57758@lists.php.net to get a copy of this message
On Mon, Feb 6, 2012 at 6:23 AM, Rasmus Lerdorf <rasmus@php.net> wrote: > On 02/05/2012 01:58 PM, Stas Malyshev wrote: >> Hi! >> >>>   http://clang-php54.phpunit.de/ might be of interest >>> to some. >>>   http://bit.ly/u06eCD has details of how to produce >>> this report. >> >> Thanks, definitely is a useful thing. Though I'm not sure if this tool >> is always right. For example, I looked at >> http://clang-php54.phpunit.de/report-dqJzsw.html#EndPath, >> and it claims >> in line 1331 replacement is garbage. However, the only way to get there >> is to pass through (flags & ENT_HTML_SUBSTITUTE_DISALLOWED_CHARS), and >> if (flags & ENT_HTML_SUBSTITUTE_DISALLOWED_CHARS) is not 0 then >> replacement is initialized in lines 1245-1252 by one of the clauses. >> Looks like this tool does not remember the branches it took before. Am I >> missing something here or should we submit a bug report to CLANG devs? > > I checked scan.coverity and it didn't find any problems in > ext/standard/html.c > > The Coverity scan does find a bunch of possible null-dereferences. > > For example, in zend_compile.c: > > 3075                                if > (!strcasecmp(arg_info->class_name, "self") && fptr->common.scope ) { > 3076                                        class_name = > fptr->common.scope->name; > 3077                                        class_name_len = > fptr->common.scope->name_length; > 3078                                } else if > (!strcasecmp(arg_info->class_name, "parent") && > fptr->common.scope->parent) { > > The if on 3075 implies that fptr->common.scope could be null and then in > the else if on 3078 fptr->common.scope->parent is checked. Now, it > probably means that every time arg_info->class_name is set to "parent" > fptr->common.scope will be defined, but a static analyzer isn't able to > detect that. Note also that this code only appears in trunk. The 5.4 > code is completely different. Was this a missed trunk commit? Rasmus: this is a fix for #60573. since 5.4 is in freeze period, so I hold from 5.4. and IMO if the classname is set to "self", then the scope could not be NULL, obviously the code is in-appropriate. I will fix it. Dmitry, could you give me some advise on this plz? thanks > > -Rasmus > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- Laruence  Xinchen Hui http://www.laruence.com/

« previous php.internals (#57758) next »