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

28 Июн 2011

Теги:

#Cpp

Пояснения к статье про Copy-Paste

28 Июн 2011

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

Многим читателя понравилась моя статья "Последствия использования технологии 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, то вы можете воспользоваться страницей документации.

Дополнительные ресурсы:

Андрей Карпов. Последствия использования технологии Copy-Paste при программировании на Си++ и как с этим быть. http://www.viva64.com/ru/a/0068/
Wikipedia. Скотт Майерс. https://ru.wikipedia.org/wiki/%D0%9C%D0%B0%D0%B9%D0%B5%D1%80%D1%81,_%D0%A1%D0%BA%D0%BE%D1%82%D1%82

#Cpp

Теги:

#Cpp

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Тип желаемой лицензии:

Пояснения к статье про Copy-Paste

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

Хотите получить бесплатную лицензию?