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

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

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

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

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

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

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


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

>
>
>
Linux-версия PVS-Studio устроила себе э…

Linux-версия PVS-Studio устроила себе экскурсию по Disney

21 Ноя 2016

Недавно вышла в свет Linux-версия анализатора PVS-Studio. С ее помощью был проверен ряд проектов с открытым исходным кодом. Среди них Chromium, GCC, LLVM (Clang) и другие. И сегодня к этому списку присоединятся проекты, которые были разработаны Walt Disney Animation Studios для сообщества специалистов по созданию виртуальной реальности. Давайте приступим к рассмотрению найденных предупреждений анализатора.

0455_Disney_ru/image1.png

Немного о Disney

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

Программисты Walt Disney Animation Studios оказывают поддержку специалистам по анимации и визуальным эффектам, создавая технологии, доступные в виде программ на C и C++ с открытым кодом для всех представителей отрасли виртуальной реальности. К таким программам можно отнести:

  • Partio (позволяет работать со стандартными форматами файлов частиц через единый интерфейс, реализованный по тому же принципу, что и унифицированные библиотеки изображений)
  • Alembic (открытый формат обмена, который становится индустриальным стандартом для обмена анимированной компьютерной графикой между пакетами по созданию цифрового контента)
  • Universal Scene Description (эффективная система, способная считывать и передавать данные сцены для обмена между различными графическими приложениями)
  • OpenSubdiv (осуществляет детальный рендеринг поверхностей (subdivision surface) на основе уменьшенных моделей)
  • Dinamica (плагин для Autodesk Maya, разработанный на основе физического движка реального времени Bullet Physics Library )
  • PTex (система наложения текстур)

Открытые исходные коды программ от Disney можно скачать на сайте https://disney.github.io/ .

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

Рассмотренные проекты от Walt Disney невелики и насчитывают всего несколько десятков тысяч строк кода на C и C++. Отсюда и такое небольшое количество ошибок по проектам.

Проект Partio

0455_Disney_ru/image2.png

Предупреждение PVS-Studio: V547 Expression '"R"' is always true. PDA.cpp 90

ParticlesDataMutable* readPDA(....)
{
  ....
  while(input->good())
  {
    *input>>word;
    ....
    if(word=="V"){
        attrs.push_back(simple->addAttribute(....);
    }else if("R"){                                 // <=
        attrs.push_back(simple->addAttribute(....);
    }else if("I"){                                 // <=
        attrs.push_back(simple->addAttribute(....);
    }
    index++;
  }
  ....
}

Анализатор выдал сообщение, что условие всегда истина. Это будет приводить к тому, что действие, которое определено в else ветке, никогда не будет выполнено. Я считаю, такая ситуация возникла из-за невнимательности программиста, и условия, которые не будут приводить к такой ошибке, должны выглядеть следующим образом:

....
if(word=="V"){
    attrs.push_back(simple->addAttribute(....);
}else if(word=="R"){                                // <=
    attrs.push_back(simple->addAttribute(....);
}else if(word=="I"){                                // <=
    attrs.push_back(simple->addAttribute(....);
}
....

Предупреждение PVS-Studio: V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *charArray[i] != '\0'. MC.cpp 109

int CharArrayLen(char** charArray)
{
  int i = 0;
  if(charArray != false)
  {
    while(charArray[i] != '\0')   // <=
    {
      i++;
    }
  }
  return i;
}

Если я правильно понимаю, функция CharArrayLen подсчитывает количество символов в строке charArray. Но действительно ли это так? По-моему, в условии цикла while есть ошибка, связанная с тем, что указатель на тип char сравнивается со значением '\0'. Высока вероятность, что забыта операция разыменования указателя. Поэтому условие цикла while должно выглядеть, например, так:

while ((*charArray)[i] != '\0')

Кстати проверка, расположенная чуть выше, тоже весьма странная:

if(charArray != false)

Проверка, конечно, работает, но будет намного лучше заменить её на такую:

if(charArray != nullptr)

В целом, создается впечатление, что функцию разрабатывал стажёр, или она не дописана. Не понятно, почему бы просто не написать код с использованием функции strlen():

int CharArrayLen(const char** charArray)
{
  if (charArray == nullptr)
    return 0;
  return strlen(*charArray);
}

Предупреждение PVS-Studio: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'attributeData[i]' is lost. Consider assigning realloc() to a temporary pointer. ParticleSimple.cpp 266

ParticleIndex ParticlesSimple::
addParticle()
{
  ....
  for(unsigned int i=0;i<attributes.size();i++)
    attributeData[i]=
                  (char*)realloc(attributeData[i],       // <=
                                (size_t)attributeStrides[i]*
                                (size_t)allocatedCount);
  ....
}

Анализатор выявил в коде опасное использование realloc. Конструкция foo = realloc(foo, ...) опасна тем, что в случае невозможности выделения памяти функция вернет nullptr, тем самым перезаписав предыдущее значение указателя, что может привести к утечке памяти, а то и вовсе к падению программы. Возможно, такая ситуация крайне редка для многих случаев, но перестраховаться, я думаю, все же стоит. Чтобы предотвратить подобную ситуацию, рекомендуется сохранять значение указателя в дополнительной переменной перед использованием realloc.

Аналогичные предупреждения анализатора:

  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'attributeData[i]' is lost. Consider assigning realloc() to a temporary pointer. ParticleSimple.cpp 280
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'data' is lost. Consider assigning realloc() to a temporary pointer. ParticleSimpleInterleave.cpp 281
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'data' is lost. Consider assigning realloc() to a temporary pointer. ParticleSimpleInterleave.cpp 292

Проект Alembic

0455_Disney_ru/image3.png

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'm_uKnot' to the left and to the right of the '||' operator. ONuPatch.h 253

class Sample
{
  public:
    ....
    bool hasKnotSampleData() const
    {
      if( (m_numU != ABC_GEOM_NUPATCH_NULL_INT_VALUE) ||
          (m_numV != ABC_GEOM_NUPATCH_NULL_INT_VALUE) ||
          (m_uOrder != ABC_GEOM_NUPATCH_NULL_INT_VALUE) ||
          (m_vOrder != ABC_GEOM_NUPATCH_NULL_INT_VALUE) ||
           m_uKnot || m_uKnot)                            // <=
           return true;
      else
          return false;
    }
    ....
  protected:
    ....
    int32_t m_numU;
    int32_t m_numV;
    int32_t m_uOrder;
    int32_t m_vOrder;
    Abc::FloatArraySample m_uKnot;
    Abc::FloatArraySample m_vKnot;
    ....
}

И снова ошибка, связанная с рассеянностью программиста. Несложно догадаться, что вместо повторяющегося поля m_uKnot в условии должно стоять m_vKnot.

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. OFaceSet.cpp 230

void OFaceSetSchema::set( const Sample &iSamp )
{
  ....
  if ( iSamp.getSelfBounds().hasVolume() )
  {
      // Caller explicity set bounds for this sample of the faceset.
      
      m_selfBoundsProperty.set( iSamp.getSelfBounds() );   // <=
  }
  else                                       
  {
      m_selfBoundsProperty.set( iSamp.getSelfBounds() );   // <=
      
      // NYI compute self bounds via parent mesh's faces
  }
  ....
}

PVS-Studio обнаружил в коде оператор if..else, в котором в обоих исходах выполняется одно и то же, несмотря на разные комментарии. Вполне вероятно, что этот участок кода томится в очереди ближайших задач команды программистов, ну а пока этот участок кода ошибочен и требует доработки.

Предупреждение PVS-Studio: V629 Consider inspecting the '1 << iStreamID' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. StreamManager.cpp 176

void StreamManager::put( std::size_t iStreamID )
{
  ....
  // CAS (compare and swap) non locking version
  Alembic::Util::int64_t oldVal = 0;
  Alembic::Util::int64_t newVal = 0;

  do
  {
    oldVal = m_streams;
    newVal = oldVal | ( 1 << iStreamID );             // <=
  }
  while ( ! COMPARE_EXCHANGE( m_streams, oldVal, newVal ) );
}

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

В выражении newVal = oldVal | (1 << iStreamID ) смещается единица, представленная как int, и далее результат сдвига преобразуется к 64-битному типу. Потенциальная ошибка здесь заключается в том, что если значение переменной iStreamID может быть больше 32, то данный участок кода будет работать некорректно из-за возникновения неопределенного поведения.

Код станет безопаснее, если число 1 будет представлено 64-битным беззнаковым типом данных:

 newVal = oldVal | (  Alembic::Util::int64_t(1) << iStreamID );

Анализатор выдал еще одно такое предупреждение:

  • V629 Consider inspecting the '1 << (val - 1)' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. StreamManager.cpp 148

Проект Universal Scene Description

0455_Disney_ru/image4.png

Предупреждение PVS-Studio: V668 There is no sense in testing the '_rawBuffer' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. uvTextureStorageData.cpp 118

bool GlfUVTextureStorageData::Read(....) 
{
  ....
  _rawBuffer = new unsigned char[_size];                   // <=
  if (_rawBuffer == nullptr) {                             // <=
      TF_RUNTIME_ERROR("Unable to allocate buffer.");
      return false;
  }
  ....
  return true; 
}

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

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'HdChangeTracker::DirtyPrimVar' to the left and to the right of the '|' operator. basisCurves.cpp 563

HdBasisCurves::_GetInitialDirtyBits() const
{
  int mask = HdChangeTracker::Clean;

  mask |= HdChangeTracker::DirtyPrimVar     // <=
       |  HdChangeTracker::DirtyWidths
       |  HdChangeTracker::DirtyRefineLevel
       |  HdChangeTracker::DirtyPoints
       |  HdChangeTracker::DirtyNormals
       |  HdChangeTracker::DirtyPrimVar     // <=
       |  HdChangeTracker::DirtyTopology
       ....
      ;

  return (HdChangeTracker::DirtyBits)mask;
}

Для определения mask использовано множество полей, среди которых есть повторяющиеся. Так не должно быть, поэтому программист или лишний раз по невнимательности использует одно и тоже поле, или, что скорее всего, вместо повторяющегося поля DirtyPrimVar должно быть другое поле.

Аналогичное предупреждение:

  • V501 There are identical sub-expressions 'HdChangeTracker::DirtyPrimVar' to the left and to the right of the '|' operator. mesh.cpp 1199

Проект OpenSubdiv

0455_Disney_ru/image5.png

Предупреждение PVS-Studio: V595 The 'destination' pointer was utilized before it was verified against nullptr. Check lines: 481, 483. hbr_utils.h 481

template <class T> void
createTopology(....) 
{
  ....
  OpenSubdiv::HbrVertex<T> * destination = 
                        mesh->GetVertex( fv[(j+1)%nv] );
  OpenSubdiv::HbrHalfedge<T> * opposite  = 
                        destination->GetEdge(origin);  // <=

  if(origin==NULL || destination==NULL)                // <=
  {              
    printf(....);
    valid=false;
    break;
  }
  ....
}

Пожалуй, V595 является самым распространенным предупреждением, выдаваемым анализатором. PVS-Studio считает код опасным, если указатель разыменовывается, а потом ниже по коду проверяется. Если указатель проверяют, то предполагают, что он может быть равен нулю.

Так и происходит в вышеприведенном участке кода. Для инициализации указателя opposite происходит разыменование указателя destination, а далее идет проверка этих указателей на равенство NULL.

И еще парочка предупреждений:

  • V595 The 'destination' pointer was utilized before it was verified against nullptr. Check lines: 145, 148. hbr_tutorial_1.cpp 145
  • V595 The 'destination' pointer was utilized before it was verified against nullptr. Check lines: 215, 218. hbr_tutorial_2.cpp 215

Предупреждение PVS-Studio: V547 Expression 'buffer[0] == '\r' && buffer[0] == '\n ' ' is always false. Probably the '||' operator should be used here. hdr_reader.cpp 84

unsigned char *loadHdr(....)
{
  ....
  char buffer[MAXLINE];
  // read header
  while(true) 
  {
    if (! fgets(buffer, MAXLINE, fp)) goto error;
    if (buffer[0] == '\n') break;
    if (buffer[0] == '\r' && buffer[0] == '\n') break;   // <=
    ....
  }
  ....
}

Программист допустил ошибку в написании условия, которая приводит к тому, что условие всегда равно false. Скорее всего программист хотел сделать так, что если встречаются такие маркеры конца строки, как \n или \r\n, то необходимо выйти из цикла while. Поэтому ошибочное условие должно быть записано следующим образом:

 if (buffer[0] == '\r' && buffer[1] == '\n') break;

Предупреждение PVS-Studio: V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. main.cpp 652

main(int argc, char ** argv) 
{
  ....
  #if defined(OSD_USES_GLEW)
  if (GLenum r = glewInit() != GLEW_OK) {                 // <=
      printf("Failed to initialize glew. error = %d\n", r);
      exit(1);
  }
  #endif
....
}

Анализатор обнаружил потенциальную ошибку в выражении GLenum r = glewInit() != GLEW_OK, которое, скорее всего, работает не так, как задумывал программист. Создавая такой код, программист, как правило, хочет выполнить действия в следующем порядке:

(GLenum r = glewInit()) != GLEW_OK

Но приоритет оператора '!=' выше, чем приоритет оператора присваивания. Поэтому выражение вычисляется так:

GLenum r = (glewInit() != GLEW_OK)

Поэтому, если функция glewInit() отработает неправильно, на экране будет распечатан неверный код ошибки. Точнее, всегда будет напечатана единица.

Для исправления ошибки можно использовать скобки или вынести создание объекта за пределы условия, что придаст коду более читабельный вид. См. также главу 16 в книге "Главный вопрос программирования, рефакторинга и всего такого".

PVS-Studio обнаружил еще несколько подобных мест:

  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. glEvalLimit.cpp 1419
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. glStencilViewer.cpp 1128
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. farViewer.cpp 1406

Предупреждение PVS-Studio: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'm_blocks' is lost. Consider assigning realloc() to a temporary pointer. allocator.h 145

template <typename T>
T*
HbrAllocator<T>::Allocate() 
{
  if (!m_freecount) 
  {
    ....
    // Keep track of the newly allocated block
    if (m_nblocks + 1 >= m_blockCapacity) {
        m_blockCapacity = m_blockCapacity * 2;
        if (m_blockCapacity < 1) m_blockCapacity = 1;
        m_blocks = (T**) realloc(m_blocks,                // <=
                                 m_blockCapacity * sizeof(T*));
    }
    m_blocks[m_nblocks] = block;                          // <=
    ....
  }
  ....
}

И снова опасное использование функции realloc. А почему оно опасное - описано выше в разделе 'Проект Partio'.

Проект Dynamica

0455_Disney_ru/image6.png

Предупреждение PVS-Studio: V512 A call of the 'memset' function will lead to overflow of the buffer 'header.padding'. pdbIO.cpp 249

struct pdb_header_t
{
  int       magic;
  unsigned short swap;
  float       version;
  float       time;
  unsigned int data_size;
  unsigned int num_data;
  char      padding[32];
  //pdb_channel_t   **data;
  int             data;
};

bool pdb_io_t::write(std::ostream &out)
{
  pdb_header_t            header;
  ....
  header.magic = PDB_MAGIC;
  header.swap = 0;
  header.version = 1.0;
  header.time = m_time;
  header.data_size = m_num_particles;
  header.num_data = m_attributes.size();
  memset(header.padding, 0, 32 * sizeof(char) + sizeof(int));
  ....
}

Анализатор обнаружил потенциально возможную ошибку, связанную с заполнением буфера памяти header.padding. Через memset программист обнуляет 36 байтов в буфере header.padding, состоящий всего из 32 байт. На первый взгляд такое использование ошибочно, но, на самом деле, программист оказался хитрецом и вместе с header.padding обнуляет переменную data. Ведь поля padding и data структуры pdb_header_t расположены последовательно, а значит последовательно расположены и в памяти. Да! Ошибки нет в данной ситуации, но из-за такой хитрости в этом месте потенциально может появиться ошибка. Например, если другой программист изменит структуру pdb_header_t, добавив между полями padding и data свои поля, и не заметит хитрости своего коллеги. Поэтому лучше обнулять каждую переменную по отдельности.

Проект Ptex

0455_Disney_ru/image7.png

Предупреждение PVS-Studio: V612 An unconditional 'return' within a loop. PtexHashMap.h 292

Entry* lockEntriesAndGrowIfNeeded(size_t& newMemUsed)
{
  while (_size*2 >= _numEntries) {
      Entry* entries = lockEntries();
      if (_size*2 >= _numEntries) {
          entries = grow(entries, newMemUsed);
      }
      return entries;
  }
  return lockEntries();
}

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

Заключение

Статический анализ кода при написании качественного ПО играет очень важную роль, так как используя статический анализ на регулярной основе, вы сокращаете трудозатраты на устранение глупых или тяжело обнаруживаемых ошибок и сможете потратить больше времени на что-то полезное.

Если вы еще не проверяли свой проект на наличие ошибок и не пускались в увлекательные поиски багов, то советую Вам скачать PVS-Studio для Linux и обязательно это сделать.

Популярные статьи по теме
Зло живёт в функциях сравнения

Дата: 19 Май 2017

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

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

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

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Эффект последней строки

Дата: 31 Май 2014

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

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
PVS-Studio ROI

Дата: 30 Янв 2019

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

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

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

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

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

Следующие комментарии

На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять