Пока в мире обсуждают 89-ю церемонию вручения «Оскара», графики актеров и костюмов, мы решили написать обзорную статью об IT-сфере. В статье будут рассмотрены наиболее интересные ошибки, допущенные в проектах с открытым кодом в 2016 году. Этот год был знаменательным для нашего инструмента, так как PVS-Studio стал доступен на ОС Linux. Мы надеемся, что представленные ошибки уже исправлены, но каждый читатель может видеть, насколько серьезны ошибки, допущенные разработчиками.
Итак, посмотрим, какие ошибки удалось обнаружить анализатору PVS-Studio в 2016 году. Помимо фрагмента кода, мы предоставляем диагностику, которая помогла выявить ошибку, и статью, в которой эта ошибка была впервые описана.
Разделы отсортированы по моему представлению о красоте ошибки.
Десятое место
Источник: Поиск ошибок в коде компилятора GCC с помощью PVS-Studio
V519 Переменной bb_copy два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 1076, 1078. cfg.c 1078
void
free_original_copy_tables (void)
{
gcc_assert (original_copy_bb_pool);
delete bb_copy;
bb_copy = NULL; // <=
delete bb_original; // <=
bb_copy = NULL; // <=
delete loop_copy;
loop_copy = NULL;
delete original_copy_bb_pool;
original_copy_bb_pool = NULL;
}Указатель bb_copy дважды устанавливается в ноль, а указатель bb_original остается прежним.
Девятое место
Источник: Долгожданная проверка CryEngine V
V519 Переменной BlendFactor [2] два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 1265, 1266. ccrydxgldevicecontext.cpp 1266
void CCryDXGLDeviceContext::
OMGetBlendState(...., FLOAT BlendFactor[4], ....)
{
CCryDXGLBlendState::ToInterface(ppBlendState, m_spBlendState);
if ((*ppBlendState) != NULL)
(*ppBlendState)->AddRef();
BlendFactor[0] = m_auBlendFactor[0];
BlendFactor[1] = m_auBlendFactor[1];
BlendFactor[2] = m_auBlendFactor[2]; // <=
BlendFactor[2] = m_auBlendFactor[3]; // <=
*pSampleMask = m_uSampleMask;
}Грязная опечатка, которая была быстро исправлена после публикации статьи. Кстати, этот ошибочный код несколько раз копировался в разные фрагменты проекта. Анализатор их тоже нашел.
Восьмое место
Источник: GDB - крепкий орешек: всего несколько ошибок, обнаруженных PVS-Studio
V579 Функция read_memory получает указатель и его размер в качестве аргументов. Возможно, это ошибка. Изучите третий аргумент. jv-valprint.c 111
extern void read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);void java_value_print (....) { .... gdb_byte *buf; buf = ((gdb_byte *) alloca (gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT)); .... read_memory (address, buf, sizeof (buf)); .... }
Оператор sizeof (buf) оценивает не размер буфера, а размер указателя. Следовательно, программе не хватает байтов данных.
Седьмое место
Источник: Команда PVS-Studio вот-вот совершит технический прорыв, а пока давайте перепроверим Blender
V522 Может иметь место разыменование нулевого указателя ve. functions1d.cpp 107
int QuantitativeInvisibilityF1D::operator()(....)
{
ViewEdge *ve = dynamic_cast<ViewEdge*>(&inter);
if (ve) {
result = ve->qi();
return 0;
}
FEdge *fe = dynamic_cast<FEdge*>(&inter);
if (fe) {
result = ve->qi(); // <=
return 0;
}
....
}Опечатка в именах имела более серьезные последствия. Судя по всему, второй фрагмент кода был написан с помощью Copy-Paste. По ошибке программист забыл изменить имя переменной ve на fe. В результате у нас будет неопределенное поведение, которое, например, может привести к сбою.
Шестое место
Источник: Код Toonz оставляет желать лучшего
V546 Член класса инициализируется сам по себе: m_subId (m_subId). tfarmcontroller.cpp 572
class TaskId { int m_id; int m_subId;public: TaskId(int id, int subId = -1) : m_id(id), m_subId(m_subId){};
Интересный баг в списке инициализации класса. Поле m_subld инициализируется само по себе; возможно, программист хотел написать m_subId (subId).
Пятое место
Источник: PVS-Studio на помощь ЦЕРНу: анализ проекта Geant4
V603 Объект создан, но не используется. Если вы хотите вызвать конструктор, следует использовать this-› G4PhysicsModelCatalog :: G4PhysicsModelCatalog (….) . g4physicsmodelcatalog.cc 51
class G4PhysicsModelCatalog { private: .... G4PhysicsModelCatalog(); .... static modelCatalog* catalog; .... };G4PhysicsModelCatalog::G4PhysicsModelCatalog() { if(!catalog) { static modelCatalog catal; catalog = &catal; } }G4int G4PhysicsModelCatalog::Register(const G4String& name) { G4PhysicsModelCatalog(); .... }
Это редкая ошибка, но некоторые программисты все еще думают, что такой вызов конструктора инициализирует поля класса. Вместо доступа к текущему объекту создается новый временный объект, который сразу же уничтожается. В результате поля объекта не будут инициализированы. Если вам нужно использовать инициализацию поля вне конструктора, лучше создать отдельную функцию и получить к ней доступ.
Четвертое место
Источник: Касабланка: Маленький единорог, который умел
V554 Некорректное использование shared_ptr. Память, выделенная с помощью new [], будет очищена с помощью delete. BlackJack_Server140 table.cpp 471
void DealerTable::FillShoe(size_t decks)
{
std::shared_ptr<int> ss(new int[decks * 52]);
....
}По умолчанию интеллектуальный указатель типа shared_ptr для уничтожения объекта вызывает оператор delete без скобок []. В данном случае это неправильно.
Правильный код должен быть:
std::shared_ptr<int> ss(new int[decks * 52],
std::default_delete<int[]>());Третье место
Источник: Юбилей шутера Serious Sam - поиск ошибок в коде Serious Engine v.1.10
V541 Напечатать в себе строку achrDefaultScript опасно. dlgcreateanimatedtexture.cpp 359
BOOL CDlgCreateAnimatedTexture::OnInitDialog()
{
....
// allocate 16k for script
char achrDefaultScript[ 16384];
// default script into edit control
sprintf( achrDefaultScript, ....); // <=
....
// add finishing part of script
sprintf( achrDefaultScript, // <=
"%sANIM_END\r\nEND\r\n", // <=
achrDefaultScript); // <=
....
}В буфере формируется какая-то строка, а затем программист хочет получить новую строку, сохранив предыдущее значение строки и добавив еще два слова. Это кажется действительно простым.
Чтобы объяснить, почему здесь можно получить неожиданный результат, приведу простой и понятный пример из документации по диагностике V541:
char s[100] = "test";
sprintf(s, "N = %d, S = %s", 123, s);В результате мы хотели бы получить строку:
N = 123, S = testНо на практике у нас в буфере будет такая строка:
N = 123, S = N = 123, S =Что будет в нашем случае, сказать сложно, потому что это зависит от реализации функции sprintf. Есть шанс, что код будет работать так, как ожидалось. Но мы также можем получить неправильный вариант или сбой программы. Код можно исправить, если использовать новый буфер для хранения результата.
Второе место
Источник: PVS-Studio копался в ядре FreeBSD
V733 Возможно, что расширение макроса привело к неправильному порядку оценки. Проверить выражение: chan - 1 * 20. isp.c 2301
static void
isp_fibre_init_2400(ispsoftc_t *isp)
....
if (ISP_CAP_VP0(isp))
off += ICB2400_VPINFO_PORT_OFF(chan);
else
off += ICB2400_VPINFO_PORT_OFF(chan - 1); // <=
....
}На первый взгляд, в этом фрагменте кода нет ничего странного. Мы видим, что иногда используется значение chan, иногда меньше на единицу chan - 1, но давайте посмотрим на определение макроса:
#define ICB2400_VPOPT_WRITE_SIZE 20#define ICB2400_VPINFO_PORT_OFF(chan) \ (ICB2400_VPINFO_OFF + \ sizeof (isp_icb_2400_vpinfo_t) + \ (chan * ICB2400_VPOPT_WRITE_SIZE)) // <=
При передаче двоичного выражения в макрос логика оценки резко меняется. Выражение «(chan - 1) * 20» превращается в «chan - 1 * 20», т.е. в «chan - 20», а неправильно оцененный размер используется в дальнейшем в программе.
К сожалению, эта ошибка пока не исправлена. Возможно, разработчики не заметили этого в статье или еще не исправили, но код все равно выглядит странно. Именно поэтому FreeBSD получила вторую награду.
Первое место
Источник: Свежий взгляд на Oracle VM VirtualBox
V547 Выражение всегда ложное. Значение беззнакового типа никогда не <0. dt_subr.c 715
#define vsnprintf RTStrPrintfVint dt_printf(dtrace_hdl_t *dtp, FILE *fp, const char *format, ...) { .... if (vsnprintf(&dtp->dt_buffered_buf[dtp->dt_buffered_offs], // <= avail, format, ap) < 0) { rval = dt_set_errno(dtp, errno); va_end(ap); return (rval); } .... }
Первое место в этом рейтинге 2016 года занимает проект VirtualBox. Его проверял PVS-Studio несколько раз, и каждый раз мы выявляли большое количество ошибок. Однако эта ошибка настолько сбивала с толку, что ввела в заблуждение не только автора кода, но и нас, разработчиков анализатора. Нам действительно пришлось много думать, что не так с кодом и почему PVS-Studio выдал такое странное предупреждение.
В скомпилированном коде в Windows мы увидели замену functions. Новая функция вернула значение беззнакового типа, добавив почти невидимую ошибку. Вот прототипы функций:
size_t RTStrPrintfV(char *, size_t, const char *, va_list args);
int vsnprintf (char *, size_t, const char *, va_list arg );Вывод
В заключение я хотел показать самый популярный снимок, получивший множество восторженных отзывов. Картинка из статьи « PVS-Studio проверила OpenJDK »
Теперь любой желающий может предлагать проекты для проверки через Github в Windows и Linux, что поможет нам найти больше ошибок в проектах с открытым кодом и улучшить качество этих проектов.
Вы можете скачать и попробовать PVS-Studio по этой ссылке.
Если вы хотите обсудить варианты лицензирования, цены и скидки, обращайтесь к нам в поддержку.
Желаем вам безошибочного программирования!
