Static Analysis of PHP_5_4 with CLANG

Another thing - is there any way to give CLANG some hints about some
functions - such as the fact that zend_error(E_ERROR) does not return
or just make some exceptions when we know some situation that it
thinks can happen does not in fact happen - such as revtal_ptr_ptr =
&retval_ptr and then passing retval_ptr_ptr would actually change
retval_ptr somewhere on the way, etc.
The idea is if we could weed out false positives and somehow mark
them we could use this tool in CI for detecting real errors.

Perhaps those instances could be replaced by zend_error_noreturn. That
variant has the "noreturn" attribute, which signals the function never
returns. This useful for suppressing those kind of warnings (note C11
also added a standard __Noreturn).

--
Gustavo Lopes

Hi!

Another thing - is there any way to give CLANG some hints about some
functions - such as the fact that zend_error(E_ERROR) does not return or
just make some exceptions when we know some situation that it thinks can
happen does not in fact happen - such as revtal_ptr_ptr = &retval_ptr and
then passing retval_ptr_ptr would actually change retval_ptr somewhere on
the way, etc.
The idea is if we could weed out false positives and somehow mark them we
could use this tool in CI for detecting real errors.

Clang (and its static analyzer) has a huge number of annotative
attributes for everything which can be easily ifdef'd to nothing on
non-Clang. http://clang.llvm.org/docs/LanguageExtensions.html gives
most of them, and Clang also has the (nearly) full range of GCC's
attributes. You can also disable the analyzer for non-annotatable
false positives with a #if block (I forget the name of the macro off
the top of my head).

The most recent versions of clang should be able to trace aliased
pointer paths to some extent, so if you're getting false positives
there, it's possible there's a subtle issue (I haven't looked at the
actual output).

-- Gwynne

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

--

--
Laruence  Xinchen Hui
http://www.laruence.com/