# Проверка PHP7

Повторная проверка проектов нередко бывает весьма интересной\. Она позволяет узнать, какие новые ошибки были допущены в ходе разработке приложения, а какие ошибки уже были исправлены\. Раньше мой коллега уже писал о проверке PHP\. С выходом новой версии \(PHP7\), я решил ещё раз проверить исходный код интерпретатора и нашёл кое\-что интересное\.

![0392_PHP_ru/image1.png](https://import.viva64.com/docx/blog/0392_PHP_ru/image1.png)

## Проверяемый проект

[PHP](https://www.php.net/) \-  скриптовый язык общего назначения, интенсивно применяемый для разработки веб\-приложений\. Язык и его интерпретатор разрабатываются в рамках проекта с открытым кодом\. 

![0392_PHP_ru/image2.png](https://import.viva64.com/docx/blog/0392_PHP_ru/image2.png)

3 декабря 2015 года было объявлено о выходе PHP версии 7\.0\.0\. Новая версия основывается на экспериментальной ветке PHP, которая изначально называлась phpng \(Следующее поколение PHP\), и разрабатывалась с упором на увеличение производительности и уменьшение потребления памяти\.

Объектом проверки стал интерпретатор PHP, исходный код которого доступен в репозитории на [GitHub](https://github.com/php/php-src)\. Проверяемая ветвь \- master\.

В качестве инструмента анализа использовался статический анализатор кода [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. Для проверки использовалась [система мониторинга компиляции](https://pvs-studio.ru/ru/docs/manual/0031/), позволяющая осуществлять анализ проекта независимо от того, какая система используется для сборки этого проекта\. Пробная версия анализатора доступна для загрузки по [ссылке](https://pvs-studio.ru/ru/pvs-studio/download/)\. 

С предыдущей проверкой проекта можно познакомиться в статье Святослава Размыслова "[Заметка про проверку PHP](https://pvs-studio.ru/ru/blog/posts/cpp/0277/)"\.

## Найденные ошибки

Стоит отметить, что многие ошибки, найденные анализатором, находятся в библиотеках, используемых в PHP\. Если все их рассматривать в этой статье, её объем бы порядочно вырос\. Но с другой стороны ошибки, допущенные в библиотеках, используемых в проекте, проявят себя и при использовании проекта\. Поэтому некоторые из этих ошибок всё же будут выписаны в этой статье\.

Отдельно хотелось бы отметить, что во время анализа сложилось впечатление, что код _целиком_ написан на макросах\. Они просто повсюду\. Это сильно усложняет задачу анализа, про отладку подобного кода я вообще молчу\. Их повсеместное использование, к слову, вышло же боком – ошибки из макросов растаскивались по коду\. Но об этом ниже\.

```cpp
static void spl_fixedarray_object_write_dimension(zval *object, 
                                                  zval *offset, 
                                                  zval *value) 
{
  ....
  if (intern->fptr_offset_set) {
    zval tmp;
    if (!offset) {
      ZVAL_NULL(&tmp);
      offset = &tmp;
    } else {
      SEPARATE_ARG_IF_REF(offset);
  }
  ....
  spl_fixedarray_object_write_dimension_helper(intern, offset, value)
}
```

**Предупреждение PVS\-Studio:** [V506](https://pvs-studio.ru/ru/docs/warnings/v506/) Pointer to local variable 'tmp' is stored outside the scope of this variable\. Such a pointer will become invalid\. spl\_fixedarray\.c 420

При истинности условного выражения оператора _if_, приведённого выше, указателю _offset_ может быть присвоен адрес переменной _tmp_\. При этом время жизни переменной _tmp_ ограничено её областью видимости, т\.е\. телом оператора _if_\. Но ниже по коду вызывается функция, принимающая в качестве одного из параметров указатель _offset_, который ссылается на уже уничтоженную переменную, что может привести к ошибке при работе с данным указателем\.

Другой странный код:

```cpp
#define MIN(a, b)  (((a)<(b))?(a):(b))
#define MAX(a, b)  (((a)>(b))?(a):(b))
SPL_METHOD(SplFileObject, fwrite)
{
  ....
  size_t str_len;
  zend_long length = 0;
  ....
  str_len = MAX(0, MIN((size_t)length, str_len));
  ....
}
```

**Предупреждение PVS\-Studio:** [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression is always false\. Unsigned type value is never < 0\. spl\_directory\.c 2886

Логика кода проста \- сначала сравнивают 2 величины и берут из них меньшую, после чего полученный результат сравнивают с нулём и записывают в переменную _str\_len_ большее из этих значений\. Проблема кроется в том, что _size\_t_ \- беззнаковый тип, следовательно, его значение всегда неотрицательно\. В итоге использование второго макроса _MAX_ попросту не имеет смысла\. Что это \- просто лишняя операция, или какая\-то более серьёзная ошибка \- судить разработчику, писавшему код\.

Это не единственное странное сравнение, встретились и другие\.

```cpp
static size_t sapi_cli_ub_write(const char *str, size_t str_length)
{
  ....
  size_t ub_wrote;
  ub_wrote = cli_shell_callbacks.cli_shell_ub_write(str, str_length);
  if (ub_wrote > -1) {
    return ub_wrote;
  }
}
```

**Предупреждение PVS\-Studio:** [V605](https://pvs-studio.ru/ru/docs/warnings/v605/) Consider verifying the expression: ub\_wrote \> \- 1\. An unsigned value is compared to the number \-1\. php\_cli\.c 307

Переменная _ub\_wrote _имеет тип _size\_t_, являющийся беззнаковым\. Однако ниже выполняется проверка вида _ub\_wrote \> \-1\. _На первый взгляд может показаться, что это выражение всегда будет истинным, так как _ub\_wrote_ может хранить в себе только неотрицательные значения\. На самом деле всё обстоит интереснее\. 

Тип литерала \-1 \(_int_\) будет преобразован к типу переменной _ub\_wrote_ \(_size\_t_\), то есть в сравнении _ub\_wrote _с переменной будет участвовать преобразованное значение\. В 32\-битной программе это будет беззнаковое значение _0xFFFFFFFF_, а в 64\-битной – _0xFFFFFFFFFFFFFFFF_\. Таким образом, переменная _ub\_wrote_ будет сравниваться с максимальным значением типа _unsigned long_\. В итоге результатом этого сравнения всегда будет значение _false_, и оператор _return_ никогда не выполнится\.

Схожий код встретился ещё раз\. Соответствующее сообщение: [V605](https://pvs-studio.ru/ru/docs/warnings/v605/) Consider verifying the expression: shell\_wrote \> \- 1\. An unsigned value is compared to the number \-1\. php\_cli\.c 272

Следующий код, на который анализатор выдал предупреждение, также связан с макросом\.

```cpp
PHPAPI void php_print_info(int flag)
{
  ....
  if (!sapi_module.phpinfo_as_text) {
    php_info_print("<h1>Configuration</h1>\n");
  } else {
    SECTION("Configuration");
  }
  ....
}
```

**Предупреждение PVS\-Studio:** [V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(\!sapi\_module\.phpinfo\_as\_text\)' condition was already verified in line 975\. info\.c 978

На первый взгляд может показаться, что всё нормально, и ошибки нет\. Но давайте взглянем на то, что из себя представляет макрос _SECTION\._

```cpp
#define SECTION(name) if (!sapi_module.phpinfo_as_text) { \
                        php_info_print("<h2>" name "</h2>\n"); \
                      } else { \
                        php_info_print_table_start(); \
                        php_info_print_table_header(1, name); \
                        php_info_print_table_end(); \
                      } \
```

В итоге, после препроцессирования в \*\.i\-файле будет содержаться код следующего вида:

```cpp
PHPAPI void php_print_info(int flag)
{
  ....
  if (!sapi_module.phpinfo_as_text) {
    php_info_print("<h1>Configuration</h1>\n");
  } else {
    if (!sapi_module.phpinfo_as_text) { 
      php_info_print("<h2>Configuration</h2>\n"); 
    } else { 
      php_info_print_table_start(); 
      php_info_print_table_header(1, "Configuration"); 
      php_info_print_table_end(); 
    } 
  }
  ....
}
```

Сейчас проблему заметить стало куда проще\. Проверяется некоторое условие _\(\!sapi\_module\.phpinfo\_as\_text_\) и, если оно не выполняется, опять проверяется это условие \(которое, естественно, никогда не выполнится\)\. Согласитесь, выглядит, как минимум, странно\.

Схожая ситуация, связанная с использованием этого макроса, встретилась ещё раз в этой же функции: 

```cpp
PHPAPI void php_print_info(int flag)
{
  ....
  if (!sapi_module.phpinfo_as_text) {
    SECTION("PHP License");
    ....
  }
  ....
}
```

**Предупреждение PVS\-Studio:** [V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(\!sapi\_module\.phpinfo\_as\_text\)' condition was already verified in line 1058\. info\.c 1059

Аналогичная ситуация\. То же условие, тот же макрос\. Раскрываем, получаем код следующего вида:

```cpp
PHPAPI void php_print_info(int flag)
{
  ....
  if (!sapi_module.phpinfo_as_text) {
    if (!sapi_module.phpinfo_as_text) { 
      php_info_print("<h2>PHP License</h2>\n"); 
    } else { 
      php_info_print_table_start(); 
      php_info_print_table_header(1, "PHP License"); 
      php_info_print_table_end(); 
    }
    ....
  }
  ....
}
```

Опять дважды проверяется одно и то же условие\. Второе будет проверяться только в случае, если истинно первое\. Тогда, если истинно первое условие \(_\!sapi\_module\.phpinfo\_as\_text_\), то всегда будет истинным и второе условие\. В таком случае код, находящийся в ветви _else_ второго оператора _if_, не будет выполнен вообще никогда\.

Идём дальше\.

```cpp
static int preg_get_backref(char **str, int *backref)
{
  ....
  register char *walk = *str;
  ....
  if (*walk == 0 || *walk != '}')
  ....
}
```

**Предупреждение PVS\-Studio:** [V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting the '\* walk \=\= 0 \|\| \* walk \!\= '\}'' expression\. The expression is excessive or contains a misprint\. php\_pcre\.c 1033

В данном коде указатель разыменовывается, и его значение сравнивается с некоторыми литералами\. Данный код избыточен\. Для наглядности перепишем, упростив, выражение:

```cpp
if (a == 0 || a != 125)
```

Как видите, условие можно упростить до _a \!\= 125_\.

Это может свидетельствовать как об избыточности кода, так и о том, что программист допустил более серьёзную ошибку\. 

![0392_PHP_ru/image3.png](https://import.viva64.com/docx/blog/0392_PHP_ru/image3.png)

Источником некоторых проблемных мест стал Zend Engine:

```cpp
static zend_mm_heap *zend_mm_init(void)
{
  ....
  heap->limit = (Z_L(-1) >> Z_L(1));
  ....
}
```

**Предупреждение PVS\-Studio:** [V610](https://pvs-studio.ru/ru/docs/warnings/v610/) Unspecified behavior\. Check the shift operator '\>\>'\. The left operand '\(\- 1\)' is negative\. zend\_alloc\.c 1865

В данном коде используется операция правостороннего битового сдвига отрицательного значения\. Это случай неуточнённого поведения \(unspecified behavior\)\. Хоть с точки зрения языка такой случай и не является ошибочным, в отличии от неопределённого поведения, лучше избегать подобных случаев, так как поведение подобного кода может различаться в зависимости от платформы и компилятора\.

Другая интересная ошибка содержится в библиотеке PCRE:

```cpp
const pcre_uint32 PRIV(ucp_gbtable[]) = {
  ....
  (1<<ucp_gbExtend)|(1<<ucp_gbSpacingMark)|(1<<ucp_gbL)|   /*  6 L */
  (1<<ucp_gbL)|(1<<ucp_gbV)|(1<<ucp_gbLV)|(1<<ucp_gbLVT),
  ....
};
```

**Предупреждение PVS\-Studio:** [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(1 << ucp\_gbL\)' to the left and to the right of the '\|' operator\. pcre\_tables\.c 161

Ошибки подобного рода можно назвать классическими\. Они [находились](https://pvs-studio.ru/ru/blog/examples/v501/) и находятся в C\+\+ проектах, [не избавлены от них](https://pvs-studio.ru/ru/blog/examples/v3001/) проекты, написанные на C\#, и, наверняка, на других языках тоже\. Программист допустил опечатку и в выражении продублировал подвыражение _\(1<<ucp\_gbL\)_\. Скорее всего \(если судить по остальной части исходного кода\), подразумевалось подвыражение _\(1<<ucp\_gbT\)_\. Такие ошибки не бросаются в глаза даже в отдельно выписанном фрагменте кода, а уж на фоне остального \- становятся вообще крайне трудно обнаруживаемыми\.

Об этой ошибке, кстати, писал ещё мой коллега в предыдущей статье, но воз и ныне там\. 

Другое место из той же библиотеки:

```cpp
....
firstchar = mcbuffer[0] | req_caseopt;
firstchar = mcbuffer[0];
firstcharflags = req_caseopt;
....
```

**Предупреждение PVS\-Studio:** [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'firstchar' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 8163, 8164\. pcre\_compile\.c 8164

Согласитесь, код выглядит странно\. Записываем результат операции '\|' в переменную _firstchar_, и тут же перезаписываем её значение, игнорируя результат предыдущей операции\. Возможно, во втором случае вместо _firstchar_ подразумевалась другая переменная, но сказать наверняка сложно\. 

Встретились и избыточные условия\. Например:

```cpp
PHPAPI php_stream *_php_stream_fopen_with_path(.... const char *path, 
                                               ....)
{
  ....
  if (!path || (path && !*path)) {
  ....
}
```

**Предупреждение PVS\-Studio:** [V728](https://pvs-studio.ru/ru/docs/warnings/v728/) An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!path' and 'path'\.  plain\_wrapper\.c 1487

Данное выражение является избыточным: во втором подвыражении можно убрать проверку указателя _path_ на неравенство nullptr\. Тогда упрощённое выражение примет следующий вид:

```cpp
if (!path || !*path)) {
```

Не стоит недооценивать подобные ошибки\. Вместо переменной _path_ вполне могло подразумеваться что\-то ещё, тогда выражение будет не избыточным, а ошибочным\. Кстати, это не единственное место\. Встретились ещё несколько:

* V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!path' and 'path'\.  fopen\_wrappers\.c 643
* V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!headers\_lc' and 'headers\_lc'\.  sendmail\.c 728

## О сторонних библиотеках

Об этом я писал выше, но хочу повториться ещё раз\. PHP использует некоторые сторонние библиотеки, которые, увы, не идеальны и содержат ошибки\. Многие предупреждения выдавались как раз на код этих библиотек\. Можно было бы проанализировать их все, но тогда, поверьте, статья вышла бы очень большой\.

Определить, находится ли ошибка в коде интерпретатора PHP или сторонней библиотеки достаточно просто – в начале всех исходных файлов находится комментарий, описывающий лицензию, проект, авторов и пр\. Отталкиваясь от этих комментариев легко определить, в файле какого проекта притаилась ошибка\.

С другой стороны \- некоторые интересные места всё же рассмотреть стоило\. В любом случае, если вы используете какие\-то сторонние библиотеки, ответственность перед пользователями за ошибки, допущенные в этих библиотеках, ложится также и на ваши плечи, так как ошибка может проявить себя в ходе использования именно вашего продукта\. Поэтому стоит внимательнее относиться к тому, какие зависимости вы тянете в своём проекте\.

## Заключение

![0392_PHP_ru/image4.png](https://import.viva64.com/docx/blog/0392_PHP_ru/image4.png)

Результаты проверки вышли интересными\. На самом деле нашлось много ошибок, в статье рассматривалась только малая часть общих предупреждений высокого и среднего уровней достоверности\. Многие из этих ошибок находятся в библиотеках, которые используются в PHP, а значит, неявно, кочуют в его код\. В самом коде PHP тоже обнаружились интересные ошибки, некоторые из которых и были выписаны выше\.

Подводя итог, хотелось бы сказать, что необходимо использовать различные средства, позволяющие повысить продуктивность работы и качества кода\. Не стоит ограничиваться тестами и code review\. Статический анализатор \- один из тех инструментов, которые могут помочь программисту писать более качественный код позволяя полезнее использовать то время, которое было бы потрачено на поиск ошибки, допущенной из\-за какой\-нибудь опечатки\. Но не стоит забывать, что статический анализатор \- инструмент регулярного применения\. И если вы ещё не используете его \- рекомендую [загрузить](https://pvs-studio.ru/ru/pvs-studio/download/) PVS\-Studio, проверить свой проект и посмотреть, что же интересного удастся найти\.

**P\.S\.** Разработчики, занимающиеся разработкой [Zend Engine](https://www.zend.com/), связались с нами и сообщили, что ошибки, которые были описаны в статье, уже исправлены\. Так держать\!