Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57758
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.170 as permitted sender)
MIME-Version: 1.0
Sender: laruence@gmail.com
In-Reply-To: <4F2F0176.1000104@php.net>
References: <4F2EDE20.50207@php.net> <4F2EFB75.6010204@sugarcrm.com> <4F2F0176.1000104@php.net>
Date: Mon, 6 Feb 2012 13:52:15 +0800
Message-ID: <CABBJUpcThmGkv0-_UFgO_B6+H3n=xSEm=Eek-DAY0hk=iiUGjA@mail.gmail.com>
To: Rasmus Lerdorf <rasmus@php.net>, Dmitry Stogov <dmitry@php.net>
Cc: Stas Malyshev <smalyshev@sugarcrm.com>, 
	"internals@lists.php.net" <internals@lists.php.net>, Sebastian Bergmann <sebastian@php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Static Analysis of PHP_5_4 with CLANG
From: laruence@php.net (Laruence)

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!
>>
>>> =C2=A0 http://clang-php54.phpunit.de/ might be of interest to some.
>>> =C2=A0 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 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if
> (!strcasecmp(arg_info->class_name, "self") && fptr->common.scope ) {
> 3076 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cl=
ass_name =3D
> fptr->common.scope->name;
> 3077 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cl=
ass_name_len =3D
> fptr->common.scope->name_length;
> 3078 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} 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
>



--=20
Laruence =C2=A0Xinchen Hui
http://www.laruence.com/