Re: Static Analysis of PHP_5_4 with CLANG
| From: | Laruence | 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/