Для получения триального ключа
заполните форму ниже
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Выходу Dolphin Smalltalk 7 под Open Sou…

Выходу Dolphin Smalltalk 7 под Open Sourсe посвящается

12 Янв 2016

На днях компания ObjectArts полностью открыла исходники и выпустила язык, и среду разработки Dolphin Smalltalk под открытой лицензией MIT! Я не смог пройти мимо, не попробовав проверить этот проект с помощью анализатора кода PVS-Studio. Могу поздравить разработчиков с тем, что у них получилось создать код высокого качества. Мне не удалось найти значимых ошибок. Однако как всегда есть некоторое количество багов и пахнущего кода. Надеюсь благодаря этой статье код станет чуть лучше.

0368_DolphinSmalltalk7_ru/image1.png

О проекте

Dolphin Smalltalk — это среда разработки на собственном диалекте Smalltalk для Windows. Ключевыми особенностями является тесная интеграция с нативными виджетами и подсистемами операционной системы, включая COM и ActiveX, и приятный глазу графический дизайн.Долгое время Dolphin Smalltalk был доступен в двух вариантах: условно-бесплатная ограниченная версия (community edition) и платный пакет для профессиональной разработки. Последний давал доступ ко всем функциям, включая продвинутые редакторы и публикацию приложений в standalone режиме, однако стоил около четырехсот долларов.

С помощью PVS-Studio 6.00 были проверены открытые исходники Dolphin Smalltalk Virtual Machine. Далее представлены результаты проверки статическим анализатором. Несмотря на то, что проект DolphinVM очень маленький, в его коде всё равно встречаются подозрительные места.

Результаты проверки

Предупреждение N1: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] msg;'. compiler.cpp 379

Compiler::StaticType Compiler::FindNameAsStatic(....)
{
  ....
  char* msg = new char[strlen(szPrompt)+name.size()+32];
  ::wsprintf(msg, szPrompt, name.c_str());
  char szCaption[256];
  ::LoadString(GetResLibHandle(), IDR_COMPILER, szCaption, ....);
  int answer = ::MessageBox(NULL, msg, szCaption, ....);
  delete msg;  // <=??
  ....
}

Анализатор обнаружил ошибку, связанную с тем, что память выделяется и освобождается несовместимыми между собой способами.

После вызова оператора "new []" необходимо освобождать память с помощью оператора "delete []".

Предупреждение N2: V716 Suspicious type conversion in return statement: returned BOOL, but function actually returns HRESULT. idolphinstart.cpp 78

#define STDMETHODIMP    HRESULT STDMETHODCALLTYPE

STDMETHODIMP CDolphinSmalltalk::GetVersionInfo(LPVOID pvi)
{
  extern BOOL __stdcall GetVersionInfo(VS_FIXEDFILEINFO* ....);
  return ::GetVersionInfo(static_cast<VS_FIXEDFILEINFO*>(pvi));
}

В этом фрагменте кода тип "BOOL" неявным образом приводится к типу "HRESULT". В то время, как такая операция вполне допустима с точки зрения языка C++, она не имеет практического смысла. Тип HRESULT предназначен для хранения статуса, имеет достаточно сложный формат и не имеет ничего общего с типом BOOL.

Предупреждение N3: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'elems' is lost. Consider assigning realloc() to a temporary pointer. compiler.cpp 2922

POTE Compiler::ParseByteArray()
{
  NextToken();
  while (m_ok && !ThisTokenIsClosing())
  {
    if (elemcount>=maxelemcount)
    {
      _ASSERTE(maxelemcount > 0);
      maxelemcount *= 2;
      elems = (BYTE*)realloc(elems, maxelemcount*sizeof(BYTE));
    }
    ....
  }
  ....
}

Данный код является потенциально опасным: рекомендуется результат функции realloc() сохранять в другой переменной. Функция realloc() производит изменение размера некоторого блока памяти. В случае, если изменение размера блока памяти в данный момент невозможно, функция вернёт нулевой указатель. Главная проблема заключается в том, что при использовании конструкции вида "ptr = realloc(ptr, ...)" указатель ptr на этот блок данных может быть утерян.

Аналогичные опасные места:

  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'm_pAllocations' is lost. Consider assigning realloc() to a temporary pointer. alloc.cpp 436
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'pUnmarked' is lost. Consider assigning realloc() to a temporary pointer. gc.cpp 217

Предупреждение N4: V547 Expression 'i >= 0' is always true. Unsigned type value is always >= 0. compact.cpp 35

// Answer the index of the last occuppied OT entry
unsigned __stdcall ObjectMemory::lastOTEntry()
{
  HARDASSERT(m_pOT);
//  HARDASSERT(m_nInCritSection > 0);

  unsigned i = m_nOTSize-1;
  const OTE* pOT = m_pOT;
  while (pOT[i].isFree())
  {
    ASSERT(i >= 0);
    i--;
  }

  return i;
}

Скорее всего ошибки здесь нет, но код подозрительный. По очереди перебираются элементы массива до тех пор, пока функция isFree() для одного из элементов не вернёт false. Неправильным здесь является ASSERT. Он, на самом деле, ничего не проверяет. Переменная 'i' беззнаковая, а значит она всегда больше или равна 0.

Ещё одно сравнение '>=0' с беззнаковым типом:

  • V547 Expression is always true. Unsigned type value is always >= 0. loadimage.cpp 343

Предупреждение N5: V730 Not all members of a class are initialized inside the constructor. Consider inspecting: m_dwSize. imagefilemapping.h 13

class ImageFileMapping
{
  HANDLE m_hFile;
  HANDLE m_hMapping;
  LPVOID m_pData;
  DWORD  m_dwSize;

public:
  ImageFileMapping() : m_hFile(0), m_hMapping(0), m_pData(NULL){}
  ~ImageFileMapping() { Close(); }
  ....
};

Ещё один случай потенциально опасного кода. Класс "ImageFileMapping" содержит всего 4 поля, но в конструкторе присваивают начальное значение только трём из них. Член 'm_dwSize' остается неинициализированным.

Это достаточно распространенная практика, когда в классе не работают с "размером", если указатель на массив ещё нулевой. Однако, очень легко ошибиться, поэтому лучше проинициализировать все члены класса.

Похожие классы:

  • V730 Not all members of a class are initialized inside the constructor. Consider inspecting: m_flags, m_oopWorkspacePools, m_context, m_compiledMethodClass. compiler.cpp 84
  • V730 Not all members of a class are initialized inside the constructor. Consider inspecting: m_tokenType, m_integer, tp, m_cc, m_base. lexer.cpp 40

Предупреждение N6: V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 99, 101. compact.cpp 101

// Perform a compacting GC
size_t ObjectMemory::compact()
{
  ....
  #pragma warning (disable : 4127)
  while(true)
  #pragma warning (default : 4127)
  ....
}

Программисты часто считают, что после директивы "pragma warning(default : X)" опять начнут действовать предупреждения, отключенные ранее помощью "pragma warning(disable: X)". Это не так. Директива 'pragma warning(default : X)' устанавливает предупреждение с номером 'X' в состояние, которое действует ПО УМОЛЧАНИЮ. Это далеко не одно и то же.

Корректный вариант кода:

size_t ObjectMemory::compact()
{
  ....
  #pragma warning(push)
  #pragma warning (disable : 4127)
  while(true)
  #pragma warning(pop)
  ....
}

Хорошая статья по данной теме: "Итак, вы хотите заглушить это предупреждение в Visual C++".

Весь список таких мест:

  • V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 244, 246. expire.cpp 246
  • V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 226, 241. expire.cpp 241
  • V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 126, 128. interfac.cpp 128
  • V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 385, 387. interprt.cpp 387

Предупреждение N7: V576 Incorrect format. Consider checking the fourth actual argument of the 'wsprintfA' function. To print the value of pointer the '%p' should be used. interfac.cpp 679

inline DWORD __stdcall
Interpreter::GenericCallbackMain(SMALLINTEGER id, BYTE* lpArgs)
{
  ....
#ifdef _DEBUG
  {
    char buf[128];
    wsprintf(buf, "WARNING: .... (%d, %x)\n", id, lpArgs);
    WarningWithStackTrace(buf);
  }
  #endif
  ....
}

Очень часто значение указателя пытаются распечатать, используя спецификатор '%x'.

Этот код ошибочен, поскольку будет работать только в тех системах, где размер указателя совпадает с размером типа 'int'. А, например, в Win64 этот код уже распечатает только младшую часть указателя 'ptr'. В этом случае необходимо использовать спецификатор '%p'.

Предупреждение N8: V547 Expression 'ch > 127' is always false. The value range of char type: [-128, 127]. decode.cpp 55

ostream& operator<<(ostream& stream, const VariantCharOTE* oteChars)
{
  ....
  char ch = string->m_characters[i];
  //if (ch = '\0') break;
  if (ch < 32 || ch > 127)  // <=
  {
    static char hexChars[16+1] = "0123456789ABCDEF";
    ....
  }
  ....
}

По умолчанию тип 'char' имеет диапазон значений, равный [-127;127]. С помощью флага компиляции /J можно сказать компилятору, чтобы использовался диапазон [0;255]. Но для компиляции этого исходного файла такой флаг не указывается, поэтому проверка "ch > 127" не имеет смысла.

Предупреждение N9: V688 The 'prev' function argument possesses the same name as one of the class members, which can result in a confusion. thrdcall.h 126

void LinkAfter(T* prev)
{
  T* pThis = static_cast<T*>(this);
  this->next = prev->next;
  if (this->next)
    this->next->prev = pThis;
  this->prev = prev;
  prev->next = pThis;
}

Скорее всего, в этой функции нет ошибки, но называть одинаковым именем параметры функций класса и членов класса - не очень хороший стиль написания кода. Это потенциально может привести к опечатке, из-за который будет использоваться или изменяться значение не той переменной.

Предупреждение N10: V601 The 'false' value is implicitly cast to the integer type. compiler.cpp 1940

int Compiler::ParseUnaryContinuation(...., int textPosition)
{
  int continuationPointer = m_codePointer;
  MaybePatchLiteralMessage();
  while (m_ok && (ThisToken()==NameConst)) 
  {
    int specialCase=false;  // <=
    ....
    if (!specialCase)       // <=
    {
      int sendIP = GenMessage(ThisTokenText(), 0, textPosition);
      AddTextMap(sendIP, textPosition, ThisTokenRange().m_stop);
    }
    ....
  }
  ....
}

В данном случае, это предупреждение носит рекомендательный характер. Если с переменной 'specialCase' везде работает как с логической переменной, то лучше использовать стандартный тип 'bool' для этого.

Заключение

Ещё один проект отправился в список проверенных нами открытых проектов.

Подготавливая такие статьи, мы приводим в них информацию далеко не о всех предупреждениях, которые выдаёт статический анализатор. Поэтому мы рекомендуем авторам проекта самостоятельно выполнить анализ и изучить все выдаваемые анализатором сообщения.

И как всегда напоминаю читателем, что ценность анализаторов не в однократных проверках, а в регулярном использовании.

Популярные статьи по теме
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

В этой статье постараюсь затронуть все вещи, которые можно без зазрения совести выкинуть из С++, не потеряв ничего (кроме боли), уменьшить стандарт, нагрузку на создателей компиляторов, студентов, из…
Продление жизни временных значений в С++: рецепты и подводные камни

Дата: 01 Ноя 2022

Автор: Гость

Прочитав эту статью, вы узнаете следующее: способы, которыми можно продлить время жизни временного объекта в С++; рекомендации и подводные камни этого механизма, с которыми может столкнуться С++ прог…
Как мы баг в PVS-Studio искали или 278 Гигабайтов логов

Дата: 28 Окт 2022

Автор: Григорий Семенчев, Сергей Ларин, Филипп Хандельянц

Предлагаем вашему вниманию интересную историю о поиске бага внутри анализатора PVS-Studio. Да, мы тоже допускаем ошибки, но мы готовы засучить рукава и залезть в самую глубину "кроличьей норы".
0, 1, 2, Фредди забрал Blender

Дата: 26 Окт 2022

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

Эта статья могла бы получить название "Как PVS-Studio защищает от поспешных правок кода, пример N7". Однако так именовать статьи становится скучновато. Поэтому сейчас вы узнаете, причём здесь Фредди …
Примеры ошибок, которые может обнаружить PVS-Studio в коде LLVM 15.0

Дата: 25 Окт 2022

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

Компиляторы развиваются и выдают всё больше предупреждений. Остаются ли преимущества от использования статических анализаторов кода, таких как PVS-Studio? Да, так как анализаторы тоже развиваются. Пе…

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

Следующие комментарии
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо