PVS-Studio - это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#.
PVS-Studio выполняет статический анализ кода и генерирует отчёт, помогающий программисту находить и устранять ошибки. PVS-Studio выполняет широкий спектр проверок кода, но наиболее силён в поисках опечаток и последствий неудачного Copy-Paste. Показательные примеры таких ошибок: V501, V517, V522, V523, V3001.
Анализатор ориентирован на разработчиков, использующих среду Visual Studio, и может в фоновом режиме выполнять анализ измененных файлов после их компиляции. В идеале ошибки будут обнаружены и исправлены ещё до попадания в репозиторий. Однако ничто не мешает использовать анализатор для проверки всего решения целиком или для встраивания в системы непрерывной интеграции. Эти и иные способы использования анализатора описаны в документации.
PVS-Studio выполняет статический анализ кода и генерирует отчёт, помогающий программисту находить и устранять ошибки. PVS-Studio выполняет широкий спектр проверок кода, но наиболее силён в поисках опечаток и последствий неудачного Copy-Paste. Показательные примеры таких ошибок: V501, V517, V522, V523, V3001.
Анализатор ориентирован на разработчиков, использующих среду Visual Studio, и может в фоновом режиме выполнять анализ измененных файлов после их компиляции. В идеале ошибки будут обнаружены и исправлены ещё до попадания в репозиторий. Однако ничто не мешает использовать анализатор для проверки всего решения целиком или для встраивания в системы непрерывной интеграции. Эти и иные способы использования анализатора описаны в документации.
Проверка PHP7
Запись от el_programmer размещена 29.04.2016 в 11:45
Обновил(-а) tezaurismosis 19.06.2016 в 19:22 (Рекламные ссылки удалены)
Обновил(-а) tezaurismosis 19.06.2016 в 19:22 (Рекламные ссылки удалены)
Метки coding, open source, php, programming, с language
| Автор: Сергей Васильев Повторная проверка проектов нередко бывает весьма интересной. Она позволяет узнать, какие новые ошибки были допущены в ходе разработке приложения, а какие ошибки уже были исправлены. Раньше мой коллега уже писал о проверке PHP. С выходом новой версии (PHP7), я решил ещё раз проверить исходный код интерпретатора и нашёл кое-что интересное. [ATTACH]3760[/ATTACH] [size=5]Проверяемый проект[/size] [url=http://php.net/]PHP[/url] - скриптовый язык общего назначения, интенсивно применяемый для разработки веб-приложений. Язык и его интерпретатор разрабатываются в рамках проекта с открытым кодом. [ATTACH]3761[/ATTACH] 3 декабря 2015 года было объявлено о выходе PHP версии 7.0.0. Новая версия основывается на экспериментальной ветке PHP, которая изначально называлась phpng (Следующее поколение PHP), и разрабатывалась с упором на увеличение производительности и уменьшение потребления памяти. Объектом проверки стал интерпретатор PHP, исходный код которого доступен в репозитории на [url=https://github.com/php/php-src]GitHub[/url]. Проверяемая ветвь - master. В качестве инструмента анализа использовался статический анализатор кода PVS-Studio. Для проверки использовалась система мониторинга компиляции, позволяющая осуществлять анализ проекта независимо от того, какая система используется для сборки этого проекта. Пробная версия анализатора доступна для загрузки по ссылке. С предыдущей проверкой проекта можно познакомиться в статье Святослава Размыслова "Заметка про проверку PHP". [size=5]Найденные ошибки[/size] Стоит отметить, что многие ошибки, найденные анализатором, находятся в библиотеках, используемых в PHP. Если все их рассматривать в этой статье, её объем бы порядочно вырос. Но с другой стороны ошибки, допущенные в библиотеках, используемых в проекте, проявят себя и при использовании проекта. Поэтому некоторые из этих ошибок всё же будут выписаны в этой статье. Отдельно хотелось бы отметить, что во время анализа сложилось впечатление, что код [i]целиком[/i] написан на макросах. Они просто повсюду. Это сильно усложняет задачу анализа, про отладку подобного кода я вообще молчу. Их повсеместное использование, к слову, вышло же боком – ошибки из макросов растаскивались по коду. Но об этом ниже. [C]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) } [/C] [b]Предупреждение PVS-Studio:[/b]V506 Pointer to local variable 'tmp' is stored outside the scope of this variable. Such a pointer will become invalid. spl_fixedarray.c 420 При истинности условного выражения оператора [i]if[/i], приведённого выше, указателю [i]offset [/i]может быть присвоен адрес переменной [i]tmp[/i]. При этом время жизни переменной [i]tmp [/i]ограничено её областью видимости, т.е. телом оператора [i]if[/i]. Но ниже по коду вызывается функция, принимающая в качестве одного из параметров указатель [i]offset[/i], который ссылается на уже уничтоженную переменную, что может привести к ошибке при работе с данным указателем. Другой странный код: [C]#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)); .... } [/C] [b]Предупреждение PVS-Studio:[/b]V547 Expression is always false. Unsigned type value is never < 0. spl_directory.c 2886 Логика кода проста - сначала сравнивают 2 величины и берут из них меньшую, после чего полученный результат сравнивают с нулём и записывают в переменную [i]str_len[/i] большее из этих значений. Проблема кроется в том, что [i]size_t[/i]- беззнаковый тип, следовательно, его значение всегда неотрицательно. В итоге использование второго макроса [i]MAX [/i]попросту не имеет смысла. Что это - просто лишняя операция, или какая-то более серьёзная ошибка - судить разработчику, писавшему код. Это не единственное странное сравнение, встретились и другие. [C]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; } } [/C] [b]Предупреждение PVS-Studio:[/b]V605 Consider verifying the expression: ub_wrote > - 1. An unsigned value is compared to the number -1. php_cli.c 307 Переменная [i]ub_wrote [/i]имеет тип [i]size_t[/i], являющийся беззнаковым. Однако ниже выполняется проверка вида [i]ub_wrote > -1. [/i]На первый взгляд может показаться, что это выражение всегда будет истинным, так как [i]ub_wrote[/i] может хранить в себе только неотрицательные значения. На самом деле всё обстоит интереснее. Тип литерала -1 ([i]int[/i]) будет преобразован к типу переменной [i]ub_wrote[/i] ([i]size_t[/i]), то есть в сравнении [i]ub_wrote [/i]с переменной будет участвовать преобразованное значение. В 32-битной программе это будет беззнаковое значение [i]0xFFFFFFFF[/i], а в 64-битной – [i]0xFFFFFFFFFFFFFFFF[/i]. Таким образом, переменная [i]ub_wrote[/i] будет сравниваться с максимальным значением типа [i]unsigned long[/i]. В итоге результатом этого сравнения всегда будет значение [i]false[/i], и оператор [i]return [/i]никогда не выполнится. Схожий код встретился ещё раз. Соответствующее сообщение: V605 Consider verifying the expression: shell_wrote > - 1. An unsigned value is compared to the number -1. php_cli.c 272 Следующий код, на который анализатор выдал предупреждение, также связан с макросом. [C]PHPAPI void php_print_info(int flag) { .... if (!sapi_module.phpinfo_as_text) { php_info_print("<h1>Configuration</h1>\n"); } else { SECTION("Configuration"); } .... } [/C] [b]Предупреждение PVS-Studio:[/b]V571 Recurring check. The 'if (!sapi_module.phpinfo_as_text)' condition was already verified in line 975. info.c 978 На первый взгляд может показаться, что всё нормально, и ошибки нет. Но давайте взглянем на то, что из себя представляет макрос [i]SECTION.[/i] [C] #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(); \ } \ [/C] В итоге, после препроцессирования в *.i-файле будет содержаться код следующего вида: [C]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(); } } .... } [/C] Сейчас проблему заметить стало куда проще. Проверяется некоторое условие [i](!sapi_module.phpinfo_as_text[/i]) и, если оно не выполняется, опять проверяется это условие (которое, естественно, никогда не выполнится). Согласитесь, выглядит, как минимум, странно. Схожая ситуация, связанная с использованием этого макроса, встретилась ещё раз в этой же функции: [C]PHPAPI void php_print_info(int flag) { .... if (!sapi_module.phpinfo_as_text) { SECTION("PHP License"); .... } .... } [/C] [b]Предупреждение PVS-Studio:[/b]V571 Recurring check. The 'if (!sapi_module.phpinfo_as_text)' condition was already verified in line 1058. info.c 1059 Аналогичная ситуация. То же условие, тот же макрос. Раскрываем, получаем код следующего вида: [C]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(); } .... } .... } [/C] Опять дважды проверяется одно и то же условие. Второе будет проверяться только в случае, если истинно первое. Тогда, если истинно первое условие ([i]!sapi_module.phpinfo_as_text[/i]), то всегда будет истинным и второе условие. В таком случае код, находящийся в ветви [i]else[/i] второго оператора [i]if[/i], не будет выполнен вообще никогда. Идём дальше. [C]static int preg_get_backref(char **str, int *backref) { .... register char *walk = *str; .... if (*walk == 0 || *walk != '}') .... } [/C] [b]Предупреждение PVS-Studio:[/b]V590 Consider inspecting the '* walk == 0 || * walk != '}'' expression. The expression is excessive or contains a misprint. php_pcre.c 1033 В данном коде указатель разыменовывается, и его значение сравнивается с некоторыми литералами. Данный код избыточен. Для наглядности перепишем, упростив, выражение: [C]if (a == 0 || a != 125) [/C] Как видите, условие можно упростить до [i]a != 125[/i]. Это может свидетельствовать как об избыточности кода, так и о том, что программист допустил более серьёзную ошибку. [ATTACH]3762[/ATTACH] Источником некоторых проблемных мест стал фреймворк Zend: [C]static zend_mm_heap *zend_mm_init(void) { .... heap->limit = (Z_L(-1) >> Z_L(1)); .... } [/C] [b]Предупреждение PVS-Studio:[/b]V610 Unspecified behavior. Check the shift operator '>>'. The left operand '(- 1)' is negative. zend_alloc.c 1865 В данном коде используется операция правостороннего битового сдвига отрицательного значения. Это случай неуточнённого поведения (unspecified behavior). Хоть с точки зрения языка такой случай и не является ошибочным, в отличии от неопределённого поведения, лучше избегать подобных случаев, так как поведение подобного кода может различаться в зависимости от платформы и компилятора. Другая интересная ошибка содержится в библиотеке PCRE: [C]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), .... }; [/C] [b]Предупреждение PVS-Studio:[/b]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#, и, наверняка, на других языках тоже. Программист допустил опечатку и в выражении продублировал подвыражение [i](1<<ucp_gbL)[/i]. Скорее всего (если судить по остальной части исходного кода), подразумевалось подвыражение [i](1<<ucp_gbT)[/i]. Такие ошибки не бросаются в глаза даже в отдельно выписанном фрагменте кода, а уж на фоне остального - становятся вообще крайне трудно обнаруживаемыми. Об этой ошибке, кстати, писал ещё мой коллега в предыдущей статье, но воз и ныне там. Другое место из той же библиотеки: [C].... firstchar = mcbuffer[0] | req_caseopt; firstchar = mcbuffer[0]; firstcharflags = req_caseopt; .... [/C] [b]Предупреждение PVS-Studio:[/b]V519 The 'firstchar' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 8163, 8164. pcre_compile.c 8164 Согласитесь, код выглядит странно. Записываем результат операции '|' в переменную [i]firstchar[/i], и тут же перезаписываем её значение, игнорируя результат предыдущей операции. Возможно, во втором случае вместо [i]firstchar[/i] подразумевалась другая переменная, но сказать наверняка сложно. Встретились и избыточные условия. Например: [C]PHPAPI php_stream *_php_stream_fopen_with_path(.... const char *path, ....) { .... if (!path || (path && !*path)) { .... } [/C] [b]Предупреждение PVS-Studio:[/b]V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!path' and 'path'. plain_wrapper.c 1487 Данное выражение является избыточным: во втором подвыражении можно убрать проверку указателя [i]path [/i]на неравенство nullptr. Тогда упрощённое выражение примет следующий вид: [C]if (!path || !*path)) { [/C] Не стоит недооценивать подобные ошибки. Вместо переменной [i]path [/i]вполне могло подразумеваться что-то ещё, тогда выражение будет не избыточным, а ошибочным. Кстати, это не единственное место. Встретились ещё несколько: [list][*]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[/list] [size=5]О сторонних библиотеках[/size] Об этом я писал выше, но хочу повториться ещё раз. PHP использует некоторые сторонние библиотеки, которые, увы, не идеальны и содержат ошибки. Многие предупреждения выдавались как раз на код этих библиотек. Можно было бы проанализировать их все, но тогда, поверьте, статья вышла бы очень большой. Определить, находится ли ошибка в коде интерпретатора PHP или сторонней библиотеки достаточно просто – в начале всех исходных файлов находится комментарий, описывающий лицензию, проект, авторов и пр. Отталкиваясь от этих комментариев легко определить, в файле какого проекта притаилась ошибка. С другой стороны - некоторые интересные места всё же рассмотреть стоило. В любом случае, если вы используете какие-то сторонние библиотеки, ответственность перед пользователями за ошибки, допущенные в этих библиотеках, ложится также и на ваши плечи, так как ошибка может проявить себя в ходе использования именно вашего продукта. Поэтому стоит внимательнее относиться к тому, какие зависимости вы тянете в своём проекте. [size=5]Заключение[/size] [ATTACH]3763[/ATTACH] Результаты проверки вышли интересными. На самом деле нашлось много ошибок, в статье рассматривалась только малая часть общих предупреждений высокого и среднего уровней достоверности. Многие из этих ошибок находятся в библиотеках, которые используются в PHP, а значит, неявно, кочуют в его код. В самом коде PHP тоже обнаружились интересные ошибки, некоторые из которых и были выписаны выше. Подводя итог, хотелось бы сказать, что необходимо использовать различные средства, позволяющие повысить продуктивность работы и качества кода. Не стоит ограничиваться тестами и code review. Статический анализатор - один из тех инструментов, которые могут помочь программисту писать более качественный код позволяя полезнее использовать то время, которое было бы потрачено на поиск ошибки, допущенной из-за какой-нибудь опечатки. Но не стоит забывать, что статический анализатор - инструмент регулярного применения. И если вы ещё не используете его - рекомендую загрузить PVS-Studio, проверить свой проект и посмотреть, что же интересного удастся найти. |
Всего комментариев 0
Комментарии
