После выхода Visual Studio 2012 с включенным в состав всех редакций новым модулем статического анализа естественно возникает вопрос: "PVS-Studio еще актуальный инструмент или его сможет заменить встроенный в VS механизм?". Подробный ответ с примерами приведен в этой статье. Выполнено как сравнение интерфейса и удобства использования, так и мощность диагностирования ошибок в коде реальных программ. Для сравнения были выбраны исходные коды трех открытых проектов id Software: Doom 3, Quake 3: Arena, Wolfenstein: Enemy Territory.
Задача сравнения статических анализаторов кода - очень сложная и неблагодарная. Прежде всего, потому что надо разработать методику сравнения, иметь доступ к инструментам, хорошую базу примеров ошибок (причем лучше реальных, а не синтетических). Кроме этого сравнение вообще стоит делать по двум отдельным категориям: диагностические возможности и удобство использования. Сравнение диагностических возможностей "по-хорошему" должно учитывать количество обнаруженных ошибок, количество необнаруженных ошибок, количество ложных срабатываний. Сравнение удобства использования - вообще трудно оценить в цифрах. Кому-то нужна command line версия, а кому-то интегрированная в среду разработки (а ведь их много разных). Кому-то нужно использование в команде, а кому-то - индивидуальное. А если еще вспомнить про разные программные (Windows, Linux) и аппаратные (x86, AMR) платформы... Короче говоря, здесь вполне вакантно место для независимой некоммерческой организации, которая будет заниматься сравнением инструментов анализа кода аналогично компаниям, тестирующим разные антивирусные продукты (например, австрийская AV-Comparatives). Есть, конечно же, Gartner с их (Magic Quadrant for Static Application Security Testing), но этого явно недостаточно. В общем, ниша свободна.
Visual Studio 2012 включает в себя поддержку статического анализа проектов Visual C++ (флаг компилятора /analyze). К сожалению, анализ работает только на проектах Visual C++, использующих непосредственно 11 версию компилятора cl. При открытии проектов более ранних версий анализом удастся воспользоваться только в случае наличия в системе соответствующей такому проекту версии компилятора с поддержкой /analyze. Напомним, что в Visual Studio 2010 Professional статического анализа не было, он был доступен только в версиях Premium и Ultimate. Кроме того, x64-сборки не было возможности проверить вообще ни в какой версии Visual Studio. Сейчас этот недостаток устранен.
Главным преимуществом анализа в 2012 версии Visual Studio, безусловно, является появление нового выделенного окна среды (tool window) Code Analysis. В прошлых версиях анализ выводился в общее окно среды Error List, которое, помимо отсутствия в нём специфичных для анализа кода функционала и интерфейсных средств отображения результатов, обладает весьма ограниченной производительностью.
Стандартный Error List фактически хранит результаты в обычном win32 гриде, что приводит к уже весьма заметным лагам начиная от 1 - 2 тысяч отображаемых сообщений. Новое окно использует динамический WPF Listbox, который должен быть привязан к внутренней структуре данных, а качество его работы теоретически не должно зависеть от размера отображаемого списка. На практике сейчас сложно по-настоящему протестировать его реальную производительность из-за отсутствия крупных проектов под новую версию Visual C++. На данный момент удалось получить только порядка 1000 сообщений одновременно, но даже для такого небольшого количества разница со старым окном уже ощущается.
Каждый элемент списка в новом окне Code Analysis теперь раскрывается при выделении и содержит краткое описание ошибки, а её код отображается в виде удобной гиперссылки на соответствующую статью в MSDN. Раскрытый элемент может также содержать список дочерних listitem'ов, позволяющих показать на несколько участков кода с краткими описаниями и объединить группу схожих срабатываний. Навигация по коду доступна для каждого из этих дочерних элементов.
Отдельно стоит обратить внимание на расположение по-умолчанию окна Code Analysis среди других окон среды. В Visual Studio 11 BETA окно располагалось внизу рабочей области. Элементы нового списка для анализа на наш взгляд были неудачно скомпонованы. На рисунке 1 видно положение окна по умолчанию и что оно излишне растянуто по горизонтали. Это приводит к неэффективному использованию рабочего пространства.
Рисунок 1 — Горизонтальное положение окна Code Analysis
Как видно из рисунка, более половины рабочего пространства окна вообще не используется. При этом, несмотря на то, что окно занимает половину экрана, оно отображает лишь 3 сообщения. Если же развернуть дочерний список выделенного сообщения (ссылка More Information), в нём поместиться ещё меньше. Отсутствие же возможности свернуть выделенное в данный момент сообщение представляется весьма неудобным. Аналогично, выделение нескольких сообщений сразу развёртывает каждое из них. К счастью, часть этих проблем может быть решена перенесением окна, например на панель вкладок Solution Explorer'а с растяжением по вертикали, но неудобства с самопроизвольными развёртываниями элементов всё равно остаются.
В Visual Studio 2012 RC окно по-умолчанию уже располагается по вертикали (рисунок 2) и это намного более логично.
Рисунок 2 – Вертикальное расположение окна Code Analysis
Окно имеет механизм для быстрой фильтрации сообщений (поле Search). Этот механизм позволяет отсеять все сообщения кроме тех, элементы которых содержат введённый в поле поиска запрос. Несмотря на то, что такой механизм позволит увидеть сообщения только из заданного файла либо только с заданным кодом, в окне отсутствует какая либо возможность для сортировки текущих сообщений, например по тем же имени файла и коду. Нет также и возможности отсеять сообщения с определёнными кодами или из определённых файлов. Учитывая, что анализ Visual C++ 2012 иногда генерирует срабатывания и на свои системные файлы, невозможность их исключить из результатов может сильно мешать. Так же не помешала бы и возможность фильтрации сообщений по группам их диагностик. Сейчас получить в окне какую либо отдельную группу диагностик можно только перезапустив анализ, предварительно изменив в настройках решения параметры анализа (Common properties->Code Analysis Settings). Что это значит на практике? Если проанализировать достаточно большой проект со всеми включёнными диагностиками (Microsoft All Rules), то затем найти в результатах только, например, Microsoft Security Rules будет весьма затруднительно из-за большого "шума" от остальных срабатываний.
Code Analysis имеет встроенный механизм подавления ложных срабатываний с помощью добавления в код проекта специальных отметок вида #pragma warning(suppress: xxxx). Отмеченное таким образом сообщение перечёркивается в окне вывода, а при последующем анализе проекта вообще не включается в список результатов. К сожалению мы не нашли возможности очистить окно результатов от таких "вычеркнутых" сообщений кроме как запустить полную проверку заново. Учитывая общую монохромность 2012 студии, после разметки нескольких сообщений как ложных, работа с оставшимися весьма затрудняется. Последующие проверки уже не будут содержать размеченные сообщения, а найти их можно будет только поиском по исходному коду проекта. Если случайно что-то помечено как false alarm, то удалить обратно это нельзя.
Резюмируя всё вышесказанное, можно сказать, что, несмотря на безусловный прогресс по сравнению с используемым ранее выводом результатов анализа в Error List, функционал нового окна Code Analysis выглядит всё ещё достаточно недоработанным и не удобным для регулярного использования.
Другими словами, если вы только начинаете использовать статический анализ, то встроенное в Visual Studio "родное" средство - это хороший старт. Если вы используете статический анализ регулярно и постоянно, то есть смысл посмотреть на специализированные решения, которые имеют выработавшиеся годами интерфейсные решения.
Отдельно сформулируем основные недостатки:
Хотя перечисленные недостатки на первый взгляд могут показаться не существенными, при их наличии работать со статическим анализатором затруднительно.
Для сравнения диагностических возможностей PVS-Studio 4.70 и статического анализа из Visual Studio 2012 (Visual C++ 2012) RC мы взяли исходный код трех проектов id Software с GitHub: Doom 3, Quake 3: Arena, Wolfenstein: Enemy Territory. Прогнали на нем оба анализатора, получили список ошибок. Затем просмотрели (вручную) весь список. И отобрали те ошибки, которые являются реальными. Мы НЕ отбирали неудачный код, возможно некорректные конструкции и т.п. Только явные ошибки.
У нашего сравнения есть недостаток - из-за того, что списки найденных проблем просматривались человеком, то возможно, что некоторые обнаруженные ошибки были пропущены.
Фрагмент 1
C6283 Primitive array-new scalar-delete mismatch. 'testedPlanes' is allocated with array new [], but deleted with scalar delete. brushbsp.cpp 886
testedPlanes = new bool[planeList.Num()];
BuildBrushBSP_r( node, planeList, testedPlanes, skipContents );
delete testedPlanes;Здесь память выделяется для массива, но освобождается так, как будто это был указатель на один объект. Здесь мы имеем дело с неопределённым поведением и результат не предсказуем. В случае с простыми типами это не страшно и конкретно здесь сбой вряд ли произойдет, но вообще это неправильно и надо использовать delete[].
Фрагмент 2
C6283 Primitive array-new scalar-delete mismatch. 'sortIndex' is allocated with array new [], but deleted with scalar delete. image_init.cpp 2214
sortIndex = new int[images.Num()];
delete sortIndex;Такая ошибка уже была описана выше - надо освобождать память с помощью delete[].
Фрагмент 3
C6293 Loop counts down from minimum. Ill-defined for-loop: counts down from minimum. model.cpp 2027
for ( maxY = size-1 ; maxY < size ; maxY-- ) {
for ( i = 0 ; i < size ; i++ ) {
if ( data[maxY*size + i] > 1.0 ) {
break;
}
}
if ( i != size ) {
break;
}
}Здесь очень странно выглядят условия цикла. Хотя теоритически это может быть корректный код, скорее всего здесь ошибка, если смотреть на код рядом с этим фрагментом.
Фрагмент 4
C6269 Pointer dereference ignored. Possibly incorrect order of operations: dereference ignored. model_lwo.cpp 1251
int sgetI1( unsigned char **bp )
{
...
flen += 1;
*bp++;
return i;
}Здесь из-за такого понятия, как приоритет операции происходит не то действие, которое хотел программист при разыменовании и инкременте. Правильный вариант - (*bp)++.
В этом же файле есть еще две похожие ошибки, которые не включены в отчет (как и в случае с PVS-Studio).
Фрагмент 5
C6283 Primitive array-new scalar-delete mismatch. 'sortIndex' is allocated with array new [], but deleted with scalar delete. modelmanager.cpp 617
sortIndex = new int[ localModelManager.models.Num()];
delete sortIndex;Такая ошибка уже была описана выше - надо освобождать память с помощью delete[].
Фрагмент 1
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 524, 533. anim_blend.cpp(524)
const char *idAnim::AddFrameCommand(
const idDeclModelDef *modelDef, int framenum,
idLexer &src, const idDict *def )
{
...
} else if ( token == "muzzle_flash" ) {
if( !src.ReadTokenOnLine( &token ) ) {
return "Unexpected end of line";
}
...
} else if ( token == "muzzle_flash" ) {
fc.type = FC_MUZZLEFLASH;
fc.string = new idStr( "" );
...В этой функции две одинаковые ветки if с разным содержанием. Скорее всего, в одной из них опечатка.
Фрагмент 2
V556 The values of different enum types are compared. af.cpp 895
class idDeclAF_Constraint {
...
declAFConstraintType_t type;
...
};
constraintType_t GetType( void ) const { return type; }
bool idAF::Load( idEntity *ent, const char *fileName )
{
...
if (
file->constraints[j]->name.Icmp(
constraint->GetName() ) == 0 &&
file->constraints[j]->type == constraint->GetType() )
{
...В этом фрагменте кода сравниваются значения разных типов, т.е. принадлежащих разным enum. Хотя в каких-то отдельных случаях это может и работать, но это - явная ошибка.
Фрагмент 3
V528 It is odd that pointer to 'char' type is compared with the '\0' value.
Probably meant: *classname != '\0'. game_local.cpp 1250
const char *classname = mapEnt->epairs.GetString( "classname" );
if ( classname != '\0' ) {
FindEntityDef( classname, false );
}Здесь хотели проверить строку classname, чтобы убедиться, что она не пустая. Однако сравнение не отрабатывает, так как надо разыменовать указатель.
Фрагмент 4
V528 It is odd that pointer to 'char' type is compared with the '\0' value.
Probably meant: *soundShaderName != '\0'. game_local.cpp 1619
soundShaderName = dict->GetString( "s_shader" );
if (soundShaderName != '\0' && dict->GetFloat("s_shakes") != 0.0f)
{
soundShader = declManager->FindSound( soundShaderName );Ошибка повторяет фрагмент 3 - нужно разыменование указателя.
Фрагмент 5
V514 Dividing sizeof a pointer 'sizeof (clientInPVS)' by another value. There is a probability of logical error presence. game_network.cpp 686
void idGameLocal::ServerWriteSnapshot(
int clientNum, int sequence, idBitMsg &msg,
byte *clientInPVS, int numPVSClients )
{
...
memcpy( clientInPVS, snapshot->pvs,
( numPVSClients + 7 ) >> 3 );
LittleRevBytes( clientInPVS, sizeof( int ),
sizeof( clientInPVS ) / sizeof ( int ) );
}Здесь видна целая история из жизни данного фрагмента кода. Когда-то clientInPVS был локальным массивом и sizeof(clientInPVS)/sizeof(int) вычислял действительно количество элементов. Однако со временем clientInPVS стали передавать как параметр в функцию. Но код остался прежним. В результате значение sizeof(clientInPVS)/sizeof(int) всегда равняется или 1 для 32-битной, или 2 для 64-битной платформы. Для исправления надо явно передавать количество элементов.
Фрагмент 6
V599 The destructor was not declared as a virtual one, although the 'BOBrick' class contains virtual functions. gamebustoutwindow.cpp 509
class BOBrick {
...
virtual void WriteToSaveGame( idFile *savefile );
virtual void ReadFromSaveGame( idFile *savefile,
idGameBustOutWindow *game );
};
BOBrick *paddle;
void idGameBustOutWindow::ReadFromSaveGame( idFile *savefile )
{
idWindow::ReadFromSaveGame( savefile );
// Clear out existing paddle and entities from GUI load
delete paddle;В данном фрагменте класс содержит виртуальные функции, но не содержит виртуального деструктора. И хотя это не всегда проблема, лучше всегда заводить виртуальный деструктор в таком случае, чтобы проблема не возникла позже.
Фрагмент 7
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1931, 1933. gamessdwindow.cpp 1931
void idGameSSDWindow::FireWeapon(int key) {
...
} else
if(gameStats.levelStats.targetEnt->type == SSD_ENTITY_ASTRONAUT) {
HitAstronaut(static_cast<SSDAstronaut*>(
gameStats.levelStats.targetEnt), key);
} else
if(gameStats.levelStats.targetEnt->type == SSD_ENTITY_ASTRONAUT) {Опять проверяется одно и то же условие в разных ветках кода. Скорее всего просто неудачно скопировано.
Фрагмент 8
V535 The variable 'i' is being used for this loop and for the outer loop. matrix.cpp 3128
bool idMatX::IsOrthonormal( const float epsilon ) const {
for ( int i = 0; i < numRows; i++ ) {
...
for ( i = 1; i < numRows; i++ ) {Странность кода заключается в том, что счетчик цикла i используется и для внешнего, и для внутреннего цикла.
Фрагмент 9
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. md5.cpp 252
void MD5_Final( MD5_CTX *ctx, unsigned char digest[16] ) {
...
memset( ctx, 0, sizeof( ctx ) ); /* In case it's sensitive */Здесь должно быть sizeof(*ctx), а как написано сейчас - передается размер указателя и обнуляется объект не полностью.
Фрагмент 10
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. model_ase.cpp 731
typedef struct {
...
} aseMesh_t;
aseMesh_t *currentMesh;
...
ase.currentMesh = &ase.currentObject->mesh;
memset( ase.currentMesh, 0, sizeof( ase.currentMesh ) );Не первый раз встречающаяся ошибка, вместо размера объекта в функцию memset передается размер указателя, который всегда один и тот же.
Фрагмент 11
V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. model_lwo.cpp 1251
int sgetI1( unsigned char **bp )
{
...
*bp++;Не менее распространенная проблема - увеличение значения указателя вместо увеличения значения объекта, на который он указывает. Правильный вариант - (*bp)++.
В этом же файле есть еще две похожие ошибки, которые не включены в отчет.
Фрагмент 12
V533 It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'j'. surface_polytope.cpp 65
void idSurface_Polytope::FromPlanes(
const idPlane *planes, const int numPlanes )
{
for ( j = 0; j < w.GetNumPoints(); j++ ) {
for ( k = 0; k < verts.Num(); j++ ) {Здесь внутренний цикл идет по переменной k, однако увеличивается переменная j. Издержки копирования кода.
Фрагмент 13
V535 The variable 'i' is being used for this loop and for the outer loop. weapon.cpp 2533
const char *idWeapon::GetAmmoNameForNum( ammo_t ammonum )
{
...
for ( i = 0; i < 2; i++ ) {
...
for( i = 0; i < num; i++ ) {Вновь для внутреннего и внешнего счетчиков цикла используется одна и та же переменная.
Фрагмент 14
V575 The 'memset' function processes '0' elements. Inspect the third argument. win_shared.cpp 177
void Sys_GetCurrentMemoryStatus( sysMemoryStats_t &stats ) {
...
memset( &statex, sizeof( statex ), 0 );Здесь перепутаны местами второй и третий аргументы - должно быть memset(&statex, 0, sizeof( statex)). Особенность данной ошибки в том, что глазами ее заметить очень сложно.
Фрагмент 15
V512 A call of the 'memset' function will lead to underflow of the buffer '& cluster'. aasfile.cpp 1312
void idAASFileLocal::DeleteClusters( void ) {
aasPortal_t portal;
aasCluster_t cluster;
...
// first portal is a dummy
memset( &portal, 0, sizeof( portal ) );
portals.Append( portal );
// first cluster is a dummy
memset( &cluster, 0, sizeof( portal ) );
clusters.Append( cluster );
}Очень красивая ошибка. Копирование кода до добра не доводит. Во втором блоке забыли заменить sizeof(portal) на sizeof(cluster).
Фрагмент 16
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. megatexture.cpp 542
void idMegaTexture::GenerateMegaMipMaps(
megaTextureHeader_t *header, idFile *outFile )
{
...
byte *newBlock = (byte *)_alloca( tileSize );
...
memset( newBlock, 0, sizeof( newBlock ) );Здесь надо писать sizeof(*newBlock), иначе используется размер указателя.
Фрагмент 17
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. target.cpp 257
#define BIT( num ) ( 1 << ( num ) )
const int BUTTON_ATTACK = BIT(0);
void idTarget_WaitForButton::Think( void ) {
idPlayer *player;
...
if ( player && ( !player->oldButtons & BUTTON_ATTACK ) &&
( player->usercmd.buttons & BUTTON_ATTACK ) ) {
player->usercmd.buttons &= ~BUTTON_ATTACK;Здесь из-за приоритета оператора "!" (который выше, чем у "&") получилось некорректное условие. Хотели проверить, что младший бит равен нулю, а проверили, что все биты равны нулю.
Обнаружено ошибок с помощью Visual Studio 2012: 5.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 1.
Фрагмент 1
C6287 Redundant test. Redundant code: the left and right sub-expressions are identical. be_ai_move.c 3236
if ((result->flags & MOVERESULT_ONTOPOF_FUNCBOB) ||
(result->flags & MOVERESULT_ONTOPOF_FUNCBOB))Здесь очень странное сравнение – скорее всего, условие скопировали, но забыли поменять проверяемый флаг на другое значение.
Фрагмент 2
C6059 Bad concatenation. Misuse of length parameter in call to 'strncat'. Pass the number of remaining characters, not the buffer size of 'path'. l_precomp.c 1013
#define MAX_TOKEN 1024
typedef struct token_s
{
char string[MAX_TOKEN]; //available token
...
}
#define MAX_PATH 64
char path[MAX_PATH];
strncat(path, token.string, MAX_PATH);Неправильный вызов strncat. Для этой функции надо передавать не размер буфера path (который равен MAX_PATH), а количество оставшихся символов.
Фрагмент 2
C6201 Index exceeds stack buffer maximum. Index '32' is out of valid index range '0' to '31' for possibly stack allocated buffer 'bs->teamleader'. ai_cmd.c 1311
...
char teamleader[32]; //netname of the team leader
...
bs->teamleader[sizeof(bs->teamleader)] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 3
C6326 Constant constant comparison. Potential comparison of a constant with another constant. ai_dmq3.c 2513
if ((bs->inventory[INVENTORY_ROCKETLAUNCHER] <= 0 ||
bs->inventory[INVENTORY_ROCKETS < 10]) &&
(bs->inventory[INVENTORY_RAILGUN] <= 0 ||
bs->inventory[INVENTORY_SLUGS] < 10) &&
(bs->inventory[INVENTORY_BFG10K] <= 0 ||
bs->inventory[INVENTORY_BFGAMMO] < 10)) {
return qfalse;
}Здесь ошибка выявилась не совсем так, как написано. Сообщение говорит о том, что странно сравнивать константу с константой. Однако ошибка заключается в том, что скобка закрывается не после индекса массива, а после сравнения. И результат сравнения становится индексом. То есть вместо:
inventory[INVENTORY_ROCKETS < 10]должно быть:
inventory[INVENTORY_ROCKETS] < 10Фрагмент 4
C6200 Index exceeds buffer maximum. Index '3' is out of valid index range '0' to '1' for non-stack buffer 'level.numteamVotingClients'. g_main.c 776
typedef enum {
TEAM_FREE,
TEAM_RED,
TEAM_BLUE,
TEAM_SPECTATOR,
TEAM_NUM_TEAMS // = 4
} team_t;
...
int numteamVotingClients[2];// set by CalculateRanks
...
for ( i = 0; i < TEAM_NUM_TEAMS; i++ ) {
level.numteamVotingClients[i] = 0;
}Здесь банальная ошибка с индексом, выходящим за границу массива. Цикл идет до 4, а массив всего лишь из двух элементов.
Фрагмент 5
C6059 Bad concatenation. Misuse of length parameter in call to 'strncat'. Pass the number of remaining characters, not the buffer size of 'info'. cl_main.c 2609
strncat(info, "\n", sizeof(info));Снова неправильное использование функции strncat().
Фрагмент 6
C6201 Index exceeds stack buffer maximum. Index '3' is out of valid index range '0' to '2' for possibly stack allocated buffer 'invModulate'. tr_shade_calc.c 628
unsigned char invModulate[3];
// this trashes alpha, but the AGEN block fixes it
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];Индекс выходит за границы массива.
Фрагмент 1
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (src)' expression. math_matrix.h 87
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью.
Фрагмент 2
V523 The 'then' statement is equivalent to the 'else' statement. be_aas_sample.c 864
int AAS_TraceAreas(vec3_t start, vec3_t end, int *areas,
vec3_t *points, int maxareas)
{
...
if (front < 0)
frac = (front)/(front-back);
else
frac = (front)/(front-back);Здесь переменная frac рассчитывается одинаковым образом, хотя перед этим проверяется условие. Возможно, она должна рассчитываться по-разному.
Фрагмент 3
V568 It's odd that the argument of sizeof() operator is the '& itemInfo' expression. cg_weapons.c 849
void CG_RegisterItemVisuals( int itemNum ) {
...
itemInfo_t *itemInfo;
memset( itemInfo, 0, sizeof( &itemInfo ) );Третьим аргументом memset оказывается размер указателя, а не размер объекта.
Фрагмент 4
V557 Array overrun is possible. The 'sizeof (bs->teamleader)' index is pointing beyond array bound. ai_cmd.c 1311
char teamleader[32]; //netname of the team leader
void BotMatch_StartTeamLeaderShip(
bot_state_t *bs, bot_match_t *match)
{
...
bs->teamleader[sizeof(bs->teamleader)] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 5
V557 Array overrun is possible. The value of 'i' index could reach 3. g_main.c 776
int numteamVotingClients[2];// set by CalculateRanks
typedef enum {
TEAM_FREE,
TEAM_RED,
TEAM_BLUE,
TEAM_SPECTATOR,
TEAM_NUM_TEAMS
} team_t;
void CalculateRanks( void ) {
...
for ( i = 0; i < TEAM_NUM_TEAMS; i++ ) {
level.numteamVotingClients[i] = 0;
}Массив состоит всего лишь из двух элементов, а значения enum, используемые в качестве счетчика явно больше этого. Из-за чего естественно возникает выход за границы массива.
Фрагмент 6
V579 The Com_Memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cvar.c 763
void Cvar_Restart_f( void ) {
...
cvar_t *var;
...
Com_Memset( var, 0, sizeof( var ) );Опять передается не размер объекта, а размер указателя. Правильный вариант sizeof(*var).
Фрагмент 7
V557 Array overrun is possible. The '3' index is pointing beyond array bound. tr_shade_calc.c 628
void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors )
{
...
unsigned char invModulate[3];
...
invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
// this trashes alpha, but the AGEN block fixes itПромахиваемся по массиву из-за того, что элементов не 4, а 3.
Обнаружено ошибок с помощью Visual Studio 2012: 6.
Обнаружено ошибок с помощью PVS-Studio: 7.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 3.
Фрагмент 1
C6287 Redundant test. Redundant code: the left and right sub-expressions are identical. be_ai_move.c 3572
if ((result->flags & MOVERESULT_ONTOPOF_FUNCBOB) ||
(result->flags & MOVERESULT_ONTOPOF_FUNCBOB))Здесь очень странное сравнение – скорее всего, условие скопировали, но забыли поменять проверяемый флаг на другое значение.
Фрагмент 2
C6059 Bad concatenation. Misuse of length parameter in call to 'strncat'. Pass the number of remaining characters, not the buffer size of 'path'. l_precomp.c 1013
strncat(path, token.string, _MAX_PATH );Неправильный вызов strncat().
Фрагмент 3
C6290 Logical-NOT bitwise-AND precedence. Bitwise operation on logical result: ! has higher precedence than &. Use && or (!(x & y)) instead. bg_pmove.c 3257
if ( !pm->ps->pm_flags & PMF_LIMBO ) {Здесь неправильно сработал приоритет операторов. То есть не так, как ожидал программист. Или так как ожидал. В любом случае программист должен с помощью скобок более явно указать, какого действия он хочет добиться.
Фрагмент 4
C6289 Mutual exclusion over logical-OR is true. Incorrect operator: mutual exclusion over || is always a non-zero constant. Did you intend to use && instead? cg_predict.c 679
if ( ps1->groundEntityNum != ENTITYNUM_WORLD ||
ps1->groundEntityNum != ENTITYNUM_NONE ||
ps2->groundEntityNum != ENTITYNUM_WORLD ||
ps2->groundEntityNum != ENTITYNUM_NONE ) {
return qfalse;
}Здесь, наверное, программист хотел использовать оператор &&, но то ли перепутал, то ли еще по какой причине, но логическое выражение получилось неправильным.
Фрагмент 5
C6201 Index exceeds stack buffer maximum. Index '32' is out of valid index range '0' to '31' for possibly stack allocated buffer 'bs->teamleader'. ai_cmd.c 1037
...
char teamleader[32]; //netname of the team leader
...
bs->teamleader[sizeof(bs->teamleader)] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 6
C6290 Logical-NOT bitwise-AND precedence. Bitwise operation on logical result: ! has higher precedence than &. Use && or (!(x & y)) instead. ai_dmq3.c 5479
if ( !g_entities[client].r.svFlags & SVF_BOT ) {Опять приоритет операторов не дает точно по коду установить, тот ли результат получится, какой и задумывал программист или нет.
Фрагмент 7
C6387 Invalid parameter value. 'params' could be '0': this does not adhere to the specification for the function 'atoi'. ai_script_actions.c 477
qboolean Bot_ScriptAction_Wait( bot_state_t *bs, char *params ) {
if ( !params || !params[0] ) {
Bot_ScriptError( bs, "Wait requires a duration." );
}Здесь в случае params == null вроде бы отработает первая проверка, но при второй проверке произойдет падение.
Фрагмент 8
C6201 Index exceeds stack buffer maximum. Index '3' is out of valid index range '0' to '2' for possibly stack allocated buffer 'invModulate'. tr_shade_calc.c 679
unsigned char invModulate[3];
// this trashes alpha, but the AGEN block fixes it
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];Индекс выходит за границы массива.
Фрагмент 9
C6059 Bad concatenation. Misuse of length parameter in call to 'strncat'. Pass the number of remaining characters, not the buffer size of 'info'. cl_main.c 3791
strncat(info, "\n", sizeof(info));Снова неправильное использование функции strncat().
Фрагмент 1
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (src)' expression. math_matrix.h 94
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}Рассчитать размер массива с помощью sizeof в этом случае просто не возможно и матрица будет скопирована не полностью.
Фрагмент 2
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (result)' expression. bg_animation.c 585
void BG_ParseConditionBits( char **text_pp,
animStringItem_t *stringTable, int condIndex, int result[2] )
{
...
memset( result, 0, sizeof( result ) );Одним из аргументов функции является массив. Его размер пытаются вычислить с помощью sizeof(), хотя надо явно или передавать размер (что более правильно), или жестко написать размер "2", раз в этом коде он все равно прописан.
Фрагмент 3
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. bg_animation.c 776
static void BG_ParseCommands( char **input,
animScriptItem_t *scriptItem,
animModelInfo_t *animModelInfo,
animScriptData_t *scriptData )
{
// TTimo gcc: might be used uninitialized
animScriptCommand_t *command = NULL;
...
memset( command, 0, sizeof( command ) );Здесь вместо размера объекта вычисляется размер указателя.
Фрагмент 4
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. bg_pmove.c 3257
static void PM_Weapon( void ) {
...
if ( !pm->ps->pm_flags & PMF_LIMBO ) {
PM_CoolWeapons();
}Приоритет операторов приводит к тому, что выражение вычисляется не так, как ожидал программист.
Фрагмент 5
V523 The 'then' statement is equivalent to the 'else' statement. bg_pmove.c 4115
static void PM_Weapon( void ) {
...
if ( DotProduct( pml.forward, pm->ps->velocity ) > 0 ) {
VectorScale( pml.forward, -1.f * ( fwdmove_knockback / mass ),
kvel ); // -1 as we get knocked backwards
} else {
VectorScale( pml.forward, -1.f * ( fwdmove_knockback / mass ),
kvel ); // -1 as we get knocked backwards
}Вне зависимости от условия выполняется одинаковая ветка кода. Возможно, здесь должна быть другая ветка.
Фрагмент 6
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cg_character.c 308
static qboolean CG_CheckForExistingAnimModelInfo(
const char *animationGroup, const char *animationScript,
animModelInfo_t **animModelInfo ) {
...
memset( *animModelInfo, 0, sizeof( *animModelInfo ) );Вычисляется размер указателя, а не размер объекта, так как в функцию передается указатель на указатель.
Фрагмент 7
V519 The 'backColor[2]' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3180, 3181. cg_draw.c 3181
typedef vec_t vec4_t[4];
static void CG_DrawObjectiveInfo( void ) {
...
vec4_t backColor;
backColor[0] = 0.2f;
backColor[1] = 0.2f;
backColor[2] = 0.2f;
backColor[2] = 1.f;Вместо четвертого элемента дважды записывают в третий.
Фрагмент 8
V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: ... }. cg_newdraw.c 720
typedef enum {qfalse, qtrue} qboolean;
qboolean eventHandling;
void CG_MouseEvent( int x, int y ) {
switch ( cgs.eventHandling ) {
case CGAME_EVENT_SPEAKEREDITOR:
case CGAME_EVENT_GAMEVIEW:
case CGAME_EVENT_CAMPAIGNBREIFING:
case CGAME_EVENT_FIRETEAMMSG:В switch и в case используются разные enum.
Фрагмент 9
V568 It's odd that the argument of sizeof() operator is the '& itemInfo' expression. cg_weapons.c 1631
void CG_RegisterItemVisuals( int itemNum )
{
itemInfo_t *itemInfo;
...
memset( itemInfo, 0, sizeof( &itemInfo ) );Третьим аргументом memset оказывается размер указателя, а не размер объекта.
Фрагмент 10
V557 Array overrun is possible. The '3' index is pointing beyond array bound. q_math.c
typedef vec_t vec3_t[3];
void RotatePointAroundVertex( vec3_t pnt, float rot_x,
float rot_y, float rot_z, const vec3_t origin )
{
...
// rotate point
pnt[0] = ( tmp[3] * ( tmp[8] - tmp[9] ) + pnt[3] * tmp[2] );Обращение к pnt[3] приводит к промаху мимо массива.
Фрагмент 11
V557 Array overrun is possible. The 'sizeof (bs->teamleader)' index is pointing beyond array bound. ai_cmd.c 1037
char teamleader[32]; //netname of the team leader
...
bs->teamleader[sizeof( bs->teamleader )] = '\0';Промах по массиву - надо было писать sizeof() - 1.
Фрагмент 12
V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. ai_dmq3.c
if ( !g_entities[client].r.svFlags & SVF_BOT ) {
return;
}Из-за приоритета операторов вычисление выражения произойдет не так, как ожидал программист.
Фрагмент 13
V562 It's odd to compare 0 or 1 with a value of 2. ai_main.c 2659
if ( !level.clients[0].pers.connected == CON_CONNECTED ) {
return;
}Приоритет операторов снова меняет суть выражения.
Фрагмент 14
V557 Array overrun is possible. The value of 'i' index could reach 4. g_systemmsg.c 157
#define NUM_PLAYER_CLASSES 5
void G_CheckForNeededClasses( void ) {
qboolean playerClasses[NUM_PLAYER_CLASSES - 1][2];
...
for ( i = 0; i < NUM_PLAYER_CLASSES; i++ ) {
if ( !playerClasses[i][0] ) {
cnt++;
}
}Доступ вне границ массива.
Фрагмент 15
V557 Array overrun is possible. The '3' index is pointing beyond array bound. tr_shade_calc.c 679
void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors )
{
...
unsigned char invModulate[3];
...
invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
// this trashes alpha, but the AGEN block fixes itОпять промахиваемся с размером массива и количеством элементов.
Фрагмент 16
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. cvar.c 905
void Cvar_Restart_f( void ) {
cvar_t *var;
...
memset( var, 0, sizeof( var ) );Опять передается не размер объекта, а размер указателя. Правильный вариант sizeof(*var).
Фрагмент 17
V519 The 'fwdmove_knockback' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 4097, 4098. bg_pmove.c 4098
static void PM_Weapon( void ) {
...
if ( !( pm->ps->eFlags & EF_PRONE ) && (
pml.groundTrace.surfaceFlags & SURF_SLICK ) ) {
float fwdmove_knockback = 0.f;
float bckmove_knockback = 0.f;
switch ( pm->ps->weapon ) {
case WP_MOBILE_MG42: fwdmove_knockback = 4000.f;
fwdmove_knockback = 400.f;
break;
case WP_PANZERFAUST: fwdmove_knockback = 32000.f;
bckmove_knockback = 1200.f;
break;
case WP_FLAMETHROWER: fwdmove_knockback = 2000.f;
bckmove_knockback = 40.f;
break;
}Здесь в ветке WP_MOBILE_MG42 два раза присваивается значение одной и той же переменной.
Обнаружено ошибок с помощью Visual Studio 2012: 9.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 4.
Обнаружено ошибок с помощью Visual Studio 2012: 5.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 1.
Обнаружено ошибок с помощью Visual Studio 2012: 6.
Обнаружено ошибок с помощью PVS-Studio: 7.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 3.
Обнаружено ошибок с помощью Visual Studio 2012: 9.
Обнаружено ошибок с помощью PVS-Studio: 17.
Из них пересекающихся ошибок (обнаружены и Visual Studio 2012, и PVS-Studio): 4.
Ребята из Microsoft проделали большую работу в Visual Studio 2012 с модулем статического анализа. Они молодцы и мы им благодарны за то, что теперь большее количество людей узнают, что такое статический анализ кода. Однако мы тоже активно развиваем PVS-Studio. Поэтому наш инструмент PVS-Studio имеет грамотный удобный интерфейс, а также сильные диагностические возможности, и мы будем улучшать их и далее.