Многим читателя понравилась моя статья "Последствия использования технологии Copy-Paste при программировании на Си++ и как с этим быть". Обратил на неё внимание и Scott Meyers и задал вопрос о том, как же собственно статический анализ помог выявить описанные в статье ошибки.
Вот его письмо:
Your article on copy-paste of code fragments at http://www.viva64.com/en/a/0068/ was interesting, but it's not clear how most of the errors you use as examples could be detected by static analysis. The only one I see that looks like a good candidate for static analysis is the assignment of too many elements to invModulate. How could static analysis detect the others?
Я написал ему ответ, после чего решил оформить свой ответ в виде поста в блог. Возможно, другим читателям также будет интересно узнать, как были найдены описанные ошибки.
Вот собственно мой ответ:
Все примеры ошибок, используемые в статье про "Copy-Paste", я нашел, исследуя код проектов с помощью анализатора PVS-Studio. Каждая из ошибок была выявлена тем или иным диагностическим правилом.
Первые четыре ошибки были выявлены с помощью диагностического правила V501. Если упрощенно, то это предупреждение выдается в тех случаях, когда слева и справа от операторов &&, ||, -, / и так далее, находятся одинаковые подвыражения. Плюс есть множество исключений, чтобы уменьшить количество ложных срабатываний. Например, предупреждение не будет выдано для этой строчки кода:
if (*p++ == *a++ && *p++ == *a++)
Рассмотрим теперь примеры из статьи.
sampleCount VoiceKey::OnBackward (...) {
...
int atrend = sgn(
buffer[samplesleft - 2]-buffer[samplesleft - 1]);
int ztrend = sgn(
buffer[samplesleft - WindowSizeInt-2]-
buffer[samplesleft - WindowSizeInt-2]);
...
}
PVS-Studio диагностирует это так:
V501 There are identical sub-expressions to the left and to the right of the '-' operator. Audacity voicekey.cpp 304
inline_ bool Contains(const LSS& lss)
{
// We check the LSS contains the two
// spheres at the start and end of the sweep
return
Contains(Sphere(lss.mP0, lss.mRadius)) &&
Contains(Sphere(lss.mP0, lss.mRadius));
}
PVS-Studio диагностирует это так:
V501 There are identical sub-expressions to the left and to the right of the '&&' operator. plgcsopcode icelss.h 69
void COX3DTabViewContainer::OnNcPaint()
{
...
if(rectClient.top<rectClient.bottom &&
rectClient.top<rectClient.bottom)
{
dc.ExcludeClipRect(rectClient);
}
...
}
PVS-Studio диагностирует это так:
V501 There are identical sub-expressions to the left and to the right of the '&&' operator. UT ox3dtabview.cpp 230
void uteTestRunner::StressBayer(uint32 iFlags)
{
...
static EPixelFormat ms_pfList[] =
{ PF_Lub, PF_Lus, PF_Li, PF_Lf, PF_Ld };
const int fsize = sizeof(ms_pfList) / sizeof(ms_pfList);
static EBayerMatrix ms_bmList[] =
{ BM_GRBG, BM_GBRG, BM_RGGB, BM_BGGR, BM_None };
const int bsize = sizeof(ms_bmList) / sizeof(ms_bmList);
...
}
PVS-Studio диагностирует это так:
V501 There are identical sub-expressions to the left and to the right of the '/' operator: sizeof (ms_pfList) / sizeof (ms_pfList) IFF plugins engine.cpp 955
V501 There are identical sub-expressions to the left and to the right of the '/' operator: sizeof (ms_bmList) / sizeof (ms_bmList) IFF plugins engine.cpp 958
Следующие два примера диагностируются с помощью V517. Проверка выявляет последовательности вида "if(A)... else if(A)...". Конечно, это опять упрощенно и существуют специальные исключения из правила.
string TimePeriod::toString() const
{
...
if (_relativeTime <= 143)
os << ((int)_relativeTime + 1) * 5 << _(" minutes");
else if (_relativeTime <= 167)
os << 12 * 60 + ((int)_relativeTime - 143) * 30 << _(" minutes");
else if (_relativeTime <= 196)
os << (int)_relativeTime - 166 << _(" days");
else if (_relativeTime <= 143)
os << (int)_relativeTime - 192 << _(" weeks");
...
}
PVS-Studio диагностирует это так:
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. GSM gsm_sms_codec.cc 175
void DXUTUpdateD3D10DeviceStats(...)
{
...
else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
wcscpy_s( pstrDeviceStats, 256, L"WARP" );
else if( DeviceType == D3D10_DRIVER_TYPE_HARDWARE )
wcscpy_s( pstrDeviceStats, 256, L"HARDWARE" );
else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
wcscpy_s( pstrDeviceStats, 256, L"SOFTWARE" );
...
}
PVS-Studio диагностирует это так:
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. TickerTape dxut.cpp 6217
Следующая ошибка была выявлена анализатором с помощью диагностики V523. Подозрительно, когда then и else ветки условия выполняют одни и те же действия.
BOOL CGridCellBase::PrintCell(...)
{
...
if(IsFixed())
crFG = (GetBackClr() != CLR_DEFAULT) ?
GetTextClr() : pDefaultCell->GetTextClr();
else
crFG = (GetBackClr() != CLR_DEFAULT) ?
GetTextClr() : pDefaultCell->GetTextClr();
...
}
PVS-Studio диагностирует это так:
V523 The 'then' statement is equivalent to the 'else' statement. GridCtrl gridcellbase.cpp 652
Следующий пример содержит явную ошибку "Copy-Paste". Но выявляется она диагностикой, вовсе не предназначенной для выявления опечаток. Можно сказать, что ошибка выявляется косвенно. Ошибка обнаружена, так как имеется явный выход за границы массива. Диагностика V557.
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];
...
}
PVS-Studio диагностирует это так:
V557 Array overrun is possible. The '3' index is pointing beyond array bound. renderer tr_shade_calc.c 679
Последний пример самый интересный. Он выявлен с помощью диагностики V525. Эта диагностика разработана специально, чтобы выявлять схожие участки кода, где с большой вероятностью есть опечатка. Схематически принцип её работ в следующем. Пусть имеется код вида:
if (A == 1)
Q = A + X;
if (A == 2)
Q = A + Y;
if (A == 3)
Q = A + Y;
Три высказывания имеют идентичную структуру. Поэтому рассмотрим этот фрагмент кода, как таблицу, состоящую из имен и чисел и имеющую размерность 5x3:
A 1 Q A X
A 2 Q A Y
A 3 Q A Y
Рассматривая эту таблицу, анализатор, используя эвристический алгоритм, может предположить, что вместо последнего 'Y' должно было быть использовано что-то ещё. Ещё раз повторю, что это очень грубое описание. К сожалению, вынужден признать, что эта проверка нередко даёт ложные срабатывания, которые мы не знаем, как можно устранить. Из-за этого нам пришлось задать для предупреждения V525 третий уровень важности. Тем не менее, она позволяет иногда найти очень интересные ошибки, подобной то, что приведена в статье:
void KeyWordsStyleDialog::updateDlg()
{
...
Style & w1Style =
_pUserLang->_styleArray.getStyler(STYLE_WORD1_INDEX);
styleUpdate(w1Style, _pFgColour[0], _pBgColour[0],
IDC_KEYWORD1_FONT_COMBO, IDC_KEYWORD1_FONTSIZE_COMBO,
IDC_KEYWORD1_BOLD_CHECK, IDC_KEYWORD1_ITALIC_CHECK,
IDC_KEYWORD1_UNDERLINE_CHECK);
Style & w2Style =
_pUserLang->_styleArray.getStyler(STYLE_WORD2_INDEX);
styleUpdate(w2Style, _pFgColour[1], _pBgColour[1],
IDC_KEYWORD2_FONT_COMBO, IDC_KEYWORD2_FONTSIZE_COMBO,
IDC_KEYWORD2_BOLD_CHECK, IDC_KEYWORD2_ITALIC_CHECK,
IDC_KEYWORD2_UNDERLINE_CHECK);
Style & w3Style =
_pUserLang->_styleArray.getStyler(STYLE_WORD3_INDEX);
styleUpdate(w3Style, _pFgColour[2], _pBgColour[2],
IDC_KEYWORD3_FONT_COMBO, IDC_KEYWORD3_FONTSIZE_COMBO,
IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_BOLD_CHECK,
IDC_KEYWORD3_UNDERLINE_CHECK);
Style & w4Style =
_pUserLang->_styleArray.getStyler(STYLE_WORD4_INDEX);
styleUpdate(w4Style, _pFgColour[3], _pBgColour[3],
IDC_KEYWORD4_FONT_COMBO, IDC_KEYWORD4_FONTSIZE_COMBO,
IDC_KEYWORD4_BOLD_CHECK, IDC_KEYWORD4_ITALIC_CHECK,
IDC_KEYWORD4_UNDERLINE_CHECK);
...
}
PVS-Studio диагностирует это так:
V525: The code containing the collection of similar blocks. Check items '7', '7', '6', '7' in lines 576, 580, 584, 588
Продолжение письма к делу не относится, и я не буду приводить текст целиком. Признаю, что эта заметка несколько скучновата, но зато она отлично показывает, что статический анализ можно успешно использовать для выявления ошибок в скопированном коде. При этом такие ошибки находятся как специализированными правилами, такими как V501 или V517, но и косвенно, например правилом V557.
Если вам интересно узнать про другие диагностические проверки, реализованные в PVS-Studio, то вы можете воспользоваться страницей документации.
Дополнительные ресурсы:
0