Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57751
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain lerdorf.com from 209.85.210.170 cause and error)
Sender: Rasmus Lerdorf <rasmus@lerdorf.com>
Message-ID: <4F2F0176.1000104@php.net>
Date: Sun, 05 Feb 2012 14:23:50 -0800
Organization: PHP Development Team
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111229 Thunderbird/9.0
MIME-Version: 1.0
To: Stas Malyshev <smalyshev@sugarcrm.com>
CC: "internals@lists.php.net" <internals@lists.php.net>, 
 Sebastian Bergmann <sebastian@php.net>
References: <4F2EDE20.50207@php.net> <4F2EFB75.6010204@sugarcrm.com>
In-Reply-To: <4F2EFB75.6010204@sugarcrm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Static Analysis of PHP_5_4 with CLANG
From: rasmus@php.net (Rasmus Lerdorf)

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