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

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

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

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

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

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

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


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

>
>
>
Обнаружены ошибки в коде C++Builder

Обнаружены ошибки в коде C++Builder

14 Май 2013

Мы проверили заголовочные файлы, входящие в состав Embarcadero C++Builder XE3. Фактически, это означает только проверку небольшого числа inline-функций. Соответственно найдено совсем немного подозрительных мест, но достаточно для небольшой заметки.

Введение

Мы регулярно проверяем открытые проекты, а так же все другое, что можно проверить. Например, мы проверяли библиотеки, входящие в состав Visual C++ 2012. Результатом стала заметка: "Обнаружены ошибки в библиотеках Visual C++ 2012".

В дистрибутив Visual C++ входят исходные коды библиотек. Ситуация с C++Builder хуже. Есть только заголовочные файлы. Поэтому удалось проверить только некоторые inline-функции. Тем не менее, удалось найти кое-что интересное. Рассмотрим соответствующие участки кода.

Работа с предупреждениями

#pragma warning(disable : 4115)
#include <objbase.h>
#pragma warning(default : 4115)

Диагностическое сообщение, выданное PVS-Studio:

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: 16, 18. iaguid.h 18

Нехорошо устанавливать режим вывода предупреждения в состояние по умолчанию. Правильным подходом, будет сохранить, а затем восстановить предыдущее состояние. Для это следует использовать "#pragma warning(push[ ,n ])" и "#pragma warning(pop)".

Неудачный макрос

#define SET_VTYPE_AND_VARREF(type, val) \
  this->vt = VT_ ## type | VT_BYREF; \
  V_ ## type ## REF (this) = val;

TVariantT& operator=(System::Currency* src)
{
  Clear();
  if(src)
    SET_VTYPE_AND_VARREF(CY,
      reinterpret_cast<tagCY*>(&(src->Val)));
  return* this;
}

Диагностическое сообщение, выданное PVS-Studio:

V640 The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. utilcls.h 1781

Макрос SET_VTYPE_AND_VARREF написан плохо. Его содержимое не обрамлено фигурными скобками { }. В результате условие "if (src)" будет относиться только к первой строчке макроса.

Неопределенное поведение

#define _BITS_BYTE    8
template<class _Uint,
    _Uint _Ax,
    _Uint _Cx,
    _Uint _Mx>
    class linear_congruential
{
  static _CONST_DATA int _Nw =
    (_BITS_BYTE * sizeof (_Uint) + 31) / 32;

  void seed(seed_seq& _Seq)
  {
    _Uint _Arr[3 + _Nw];
    ....
    int _Lsh = _BITS_BYTE * sizeof (_Uint);
    ....

    for (int _Idx = _Nw; 0 < --_Idx; )
      _Arr[3 + _Idx - 1] |=
        _Arr[3 + _Idx] << _Lsh;
    ....
  }
}

Диагностическое сообщение, выданное PVS-Studio:

V610 Instantiate linear_congruential < unsigned long, 40014, 0, 2147483563 >: Undefined behavior. Check the shift operator '<<. The right operand '_Lsh' is greater than or equal to the length in bits of the promoted left operand. random 738

В этой функции, переменная '_Lsh' примет значение 32. Нельзя сдвигать 32-битные типы, более чем на 31 бит. Выдержка из стандарта: The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand.

Также, опасным образом сделан макрос DXVABitMask:

#define DXVABitMask(__n) (~((~0) << __n))

Процитируем соответствующую строчку из стандарта: Otherwise, if E1 has a signed type and non-negative value, and E1*2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.

Из-за этого макроса, PVS-Studio выдает несколько предупреждений. Пример:

V610 Undefined behavior. Check the shift operator '<<. The left operand '(~0)' is negative. dxva.h 1080

Подробнее о сдвигах и неопределенном поведении можно почитать в заметке: Не зная брода, не лезь в воду. Часть третья.

Неучтенное изменение в поведении оператора new

Обнаружилось достаточно много мест, где после вызова оператора 'new', проверяется, что указатель не равен NULL. Сейчас это не имеет смысла и даже вредно. В случае ошибки выделения памяти, оператор 'new' генерирует исключение std::bad_alloc.

Можно вызывать оператор 'new', который не генерирует исключение. В C++Builder даже есть специальный макрос для этого:

#define NEW_NOTHROW(_bytes) new (nothrow) BYTE[_bytes]

Однако видимо не везде удалось навести порядок с выделением памяти. Приведу несколько примеров:

inline void _bstr_t::Assign(BSTR s) throw(_com_error)
{
  if (m_Data != NULL) {
    m_Data->Assign(s); 
  } 
  else {
    m_Data = new Data_t(s, TRUE);
    if (m_Data == NULL) {
      _com_issue_error(E_OUTOFMEMORY);
    }
  }
}

Диагностическое сообщение, выданное PVS-Studio:

V668 There is no sense in testing the 'm_Data' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. comutil.h 454

Строчка "_com_issue_error(E_OUTOFMEMORY);" никогда не выполняется. В случае ошибки будет сгенерировано исключение std::bad_alloc().


static inline BYTE *__CorHlprNewThrows(size_t bytes)
{
  BYTE *pbMemory = new BYTE[bytes];
  if (pbMemory == NULL)
    __CorHlprThrowOOM();
  return pbMemory;
}

Диагностическое сообщение, выданное PVS-Studio:

V668 There is no sense in testing the 'pbMemory' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. corhlpr.h 56

template<class TYPE, class ARG_TYPE>
void CDXArray<TYPE, ARG_TYPE>::SetSize(int nNewSize, int nGrowBy)
{
  ....
  TYPE* pNewData = (TYPE*) new BYTE[nNewMax * sizeof(TYPE)];

  // oh well, it's better than crashing
  if (pNewData == NULL)
    return;
  ....
}

Диагностическое сообщение, выданное PVS-Studio:

V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 338

Остальные фрагменты кода похожи, и приводить их смысла нет. Ограничусь только диагностическими сообщениями:

  • V668 There is no sense in testing the 'p' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. d3dx10math.inl 1008
  • V668 There is no sense in testing the 'p' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 123
  • V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 395
  • V668 There is no sense in testing the 'm_pHashTable' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 1126
  • V668 There is no sense in testing the 'newBrush' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 44
  • V668 There is no sense in testing the 'retimage' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 374
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 615
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 645
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1196
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1231
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1372
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1405
  • V668 There is no sense in testing the 'newLineCap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluslinecaps.h 153
  • V668 There is no sense in testing the 'nativeRegions' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusgraphics.h 1415
  • V668 There is no sense in testing the 'newRegion' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusregion.h 89
  • V668 There is no sense in testing the 'nativeFamilyList' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusfontcollection.h 57
  • V668 There is no sense in testing the 'newImage' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 334
  • V668 There is no sense in testing the 'bitmap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 819
  • V668 There is no sense in testing the 'bitmap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 862
  • V668 There is no sense in testing the 'm_pData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. spcollec.h 266
  • V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. spcollec.h 325

И это только в inline-функциях! Представляю, что творится в *.cpp файлах. :)

Примечание

В тот момент, когда я закончил писать эту статью, вышел Embarcadero C++Builder XE4. Впрочем, это не отменяет пользу от проделанного анализа и хорошо демонстрирует возможности PVS-Studio.

Заключение

Спасибо всем за внимание. Надеюсь, разработчики C++Builder обратят на нас внимание и захотят проверить с помощью PVS-Studio исходные коды компилятора и библиотек. В заключении, хочу предложить вниманию несколько полезных ссылок:

Популярные статьи по теме
Выявляем ошибки в релизе LLVM 13.0.0

Дата: 08 Окт 2021

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

Задача коммерческих статических анализаторов выполнять более глубокий и полный анализ кода, чем компиляторы. Давайте посмотрим, что смог обнаружить PVS-Studio в исходном коде проекта LLVM 13.0.0.
Проверка Clang 11 с помощью PVS-Studio

Дата: 27 Окт 2020

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

Время от времени нам приходится писать статьи о проверке очередной версии какого-то компилятора. Это неинтересно. Однако, как показывает практика, если этого долго не делать, люди начинают сомневатьс…
PVS-Studio теперь в Compiler Explorer!

Дата: 06 Июл 2020

Автор: Георгий Грибков

Совсем недавно произошло знаменательное событие: PVS-Studio появился в Compiler Explorer! Теперь вы можете быстро и легко проанализировать код на наличие ошибок прямо на сайте godbolt.org (Compiler E…
Анализатор PVS-Studio на сайте godbolt.org (Compiler Explorer) и предостережение

Дата: 08 Июн 2020

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

Мы добавили возможность экспериментировать со статическим анализатором кода PVS-Studio на сайте godbolt.org (Compiler Explorer). Поддерживается анализ C и C++ кода. Уверены, что это интересный и очен…
Проверка компилятора GCC 10 с помощью PVS-Studio

Дата: 16 Апр 2020

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

Компилятор GCC написан с обильным использованием макросов. Очередная проверка кода GCC с помощью PVS-Studio вновь подтверждает мнение нашей команды, что макросы – это плохо. В таком коде тяжело разби…

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

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