Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Проверка PHP7

Проверка PHP7

28 Апр 2016

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

0392_PHP_ru/image1.png

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

PHP - скриптовый язык общего назначения, интенсивно применяемый для разработки веб-приложений. Язык и его интерпретатор разрабатываются в рамках проекта с открытым кодом.

0392_PHP_ru/image2.png

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

Объектом проверки стал интерпретатор PHP, исходный код которого доступен в репозитории на GitHub. Проверяемая ветвь - master.

В качестве инструмента анализа использовался статический анализатор кода PVS-Studio. Для проверки использовалась система мониторинга компиляции, позволяющая осуществлять анализ проекта независимо от того, какая система используется для сборки этого проекта. Пробная версия анализатора доступна для загрузки по ссылке.

С предыдущей проверкой проекта можно познакомиться в статье Святослава Размыслова "Заметка про проверку PHP".

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

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

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

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 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, который ссылается на уже уничтоженную переменную, что может привести к ошибке при работе с данным указателем.

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

#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 Expression is always false. Unsigned type value is never < 0. spl_directory.c 2886

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

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

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 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 Consider verifying the expression: shell_wrote > - 1. An unsigned value is compared to the number -1. php_cli.c 272

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

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 Recurring check. The 'if (!sapi_module.phpinfo_as_text)' condition was already verified in line 975. info.c 978

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

#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-файле будет содержаться код следующего вида:

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) и, если оно не выполняется, опять проверяется это условие (которое, естественно, никогда не выполнится). Согласитесь, выглядит, как минимум, странно.

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

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

Предупреждение PVS-Studio: V571 Recurring check. The 'if (!sapi_module.phpinfo_as_text)' condition was already verified in line 1058. info.c 1059

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

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, не будет выполнен вообще никогда.

Идём дальше.

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

Предупреждение PVS-Studio: V590 Consider inspecting the '* walk == 0 || * walk != '}'' expression. The expression is excessive or contains a misprint. php_pcre.c 1033

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

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

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

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

0392_PHP_ru/image3.png

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

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

Предупреждение PVS-Studio: V610 Unspecified behavior. Check the shift operator '>>'. The left operand '(- 1)' is negative. zend_alloc.c 1865

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

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

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 There are identical sub-expressions '(1 << ucp_gbL)' to the left and to the right of the '|' operator. pcre_tables.c 161

Ошибки подобного рода можно назвать классическими. Они находились и находятся в C++ проектах, не избавлены от них проекты, написанные на C#, и, наверняка, на других языках тоже. Программист допустил опечатку и в выражении продублировал подвыражение (1<<ucp_gbL). Скорее всего (если судить по остальной части исходного кода), подразумевалось подвыражение (1<<ucp_gbT). Такие ошибки не бросаются в глаза даже в отдельно выписанном фрагменте кода, а уж на фоне остального - становятся вообще крайне трудно обнаруживаемыми.

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

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

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

Предупреждение PVS-Studio: V519 The 'firstchar' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 8163, 8164. pcre_compile.c 8164

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

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

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

Предупреждение PVS-Studio: V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!path' and 'path'. plain_wrapper.c 1487

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

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

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

Подводя итог, хотелось бы сказать, что необходимо использовать различные средства, позволяющие повысить продуктивность работы и качества кода. Не стоит ограничиваться тестами и code review. Статический анализатор - один из тех инструментов, которые могут помочь программисту писать более качественный код позволяя полезнее использовать то время, которое было бы потрачено на поиск ошибки, допущенной из-за какой-нибудь опечатки. Но не стоит забывать, что статический анализатор - инструмент регулярного применения. И если вы ещё не используете его - рекомендую загрузить PVS-Studio, проверить свой проект и посмотреть, что же интересного удастся найти.

P.S. Разработчики, занимающиеся разработкой Zend Engine, связались с нами и сообщили, что ошибки, которые были описаны в статье, уже исправлены. Так держать!

Популярные статьи по теме
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…

Комментарии (0)

Следующие комментарии

На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять