Как программисты с PVS-Studio ошибки в проектах искали

    Picture 3Недавно сайт Pinguem.ru совместно с командой PVS-Studio устраивали конкурс, в котором программистам было необходимо в течение месяца использовать статический анализатор PVS-Studio для нахождения и исправления ошибок в коде open-source проектов. Благодаря их стараниям, программы в мире стали чуточку безопаснее и надежнее. В статье мы рассмотрим парочку наиболее интересных ошибок, которые были найдены при помощи PVS-Studio.

    Picture 13

    И как прошел конкурс?


    Конкурс проходил с 23 октября по 27 ноября 2017 года в два этапа для русскоязычной аудитории. На первом этапе конкурсантам было необходимо отправить как можно больше Pull Request'ов авторам проектов. Второй этап был несколько сложнее: нужно было найти ошибку и описать последовательность действий, при которых она бы себя проявила в работе приложения. Лучше всех с заданиями справился Николай Шалакин и стал победителем конкурса. Поздравляем его!

    За время проведения конкурса было отправлено немало хороших Pull Request'ов, желающие могут ознакомиться с ними по этой ссылке. Мы же предлагаем рассмотреть наиболее интересные ошибки, найденные конкурсантами на втором этапе.

    QtCreator


    Многие ли из Вас используют QtCreator для программирования на Python? Как и многие IDE, он тоже подсвечивает некоторые встроенные функции и объекты. Возьмем QtCreator 4.4.1 и напишем несколько зарезервированных слов:

    Picture 3


    Что же такое? Почему встроенные функции oct и chr не подсвечиваются? Взглянем на код:

    // List of python built-in functions and objects
    static const QSet<QString> builtins = {
    "range", "xrange", "int", "float", "long", "hex", "oct" "chr", "ord",
    "len", "abs", "None", "True", "False"
    };

    Функции объявлены, они должны подсвечиваться. И здесь помогает PVS-Studio:

    V653 A suspicious string consisting of two parts is used for initialization. It is possible that a comma is missing. Consider inspecting this literal: «oct» «chr». pythonscanner.cpp 205

    Действительно, между литералами «oct» и «chr» забыли поставить запятую, поэтому два литерала слились в один «octchr», и именно он будет подсвечиваться средой разработки:

    Picture 4


    Здесь можно ознакомиться с Pull Request'ом на исправление.

    ConEmu


    Вы работаете над проектом ConEmu и в отладочной версии решили проверить работу некоторых настроек:

    Picture 5


    Давайте взглянем на код, почему вылетает предупреждение «ListBox was not processed»:

    INT_PTR CSetPgViews::OnComboBox(HWND hDlg, WORD nCtrlId, WORD code)
    {
      switch (code)
      {
      ....
      case CBN_SELCHANGE:
        {
          ....
          UINT val;
          INT_PTR nSel = SendDlgItemMessage(hDlg, 
                                            nCtrlId, 
                                            CB_GETCURSEL,
                                            0,
                                            0);
          switch (nCtrlId)
          {
            ....
            case tThumbMaxZoom:
              gpSet->ThSet.nMaxZoom = max(100,((nSel+1)*100));
            default:
              _ASSERTE(FALSE && "ListBox was not processed");
          }
        }
      }
    }

    Из-за пропущенного break управление перейдет к ветке default после того, как отработают выражения в ветке tThumbMaxZoom. Об этом предупреждает PVS-Studio:

    V796 It is possible that 'break' statement is missing in switch statement. setpgviews.cpp 183

    Здесь можно ознакомиться с Pull Request'ом на исправление.

    UniversalPauseButton


    Довольно интересный проект, особенно полезный для геймеров. При нажатии на кнопку Pause программа приостанавливает работу окна, находящегося на переднем плане:



    Можно переназначить кнопку приостановки/возобновления на другую при помощи файла settings.txt:

    Picture 2


    Если ввести код ключа не менее, чем 20 символов, и не более, чем 30 символов, это приведет к порче стека:

    Picture 8


    Разберемся, почему это происходит. Нас интересует функция LoadPauseKeyFromSettingsFile:

    int LoadPauseKeyFromSettingsFile(_In_ wchar_t* Filename)
    {
      HANDLE FileHandle = CreateFile(Filename, 
                                     GENERIC_READ,
                                     FILE_SHARE_READ,
                                     NULL,
                                     OPEN_EXISTING,
                                     FILE_ATTRIBUTE_NORMAL,
                                     NULL);
    
      if (FileHandle == INVALID_HANDLE_VALUE)
      {
        goto Default;
      }
      
      char  KeyLine[32] = { 0 };
      char  Buffer[2]   = { 0 };
      DWORD ByteRead    = 0;
    
      do
      {
        if (!ReadFile(FileHandle, Buffer, 1, &ByteRead, NULL))
        {
          goto Default;
        }
    
        if (Buffer[0] == '\r' || Buffer[0] == '\n')
        {
          break;
        }
    
        size_t Length = strlen(KeyLine);
        if (Length > 30)                                            // <=
        {
          goto Default;
        }
    
        KeyLine[Length] = Buffer[0];    
        memset(Buffer, 0, sizeof(Buffer));
      } while (ByteRead == 1);
    
      if (!StringStartsWith_AI(KeyLine, "KEY="))
      {
        goto Default;
      }
    
      char KeyNumberAsString[16] = { 0 };                           // <=
    
      for (DWORD Counter = 4; Counter < strlen(KeyLine); Counter++) // <=
      {
        KeyNumberAsString[Counter - 4] = KeyLine[Counter];
      }
      ....
    
      Default:
      if (FileHandle != INVALID_HANDLE_VALUE && FileHandle != NULL)
      {
        CloseHandle(FileHandle);    
      }
      return(0x13);
    }

    В цикле считывается побайтово первая строка. Если она превышает длину в 30 символов, то выполнение переходит по метке Default,при этом освобождается ресурс и возвращается символ с кодом 0x13. Если чтение происходит успешно, и первая строка начинается с «KEY=», то происходит копирование подстроки после символа "=" в 16-байтовый буфер KeyNumberAsString. При вводе ключа от 20 до 30 символов произойдет переполнение буфера. Об этом предупреждает PVS-Studio:

    V557 Array overrun is possible. The value of 'Counter — 4' index could reach 26. main.cpp 146

    Здесь можно ознакомиться с Pull Request'ом на исправление.

    Explorer++


    В этом проекте был найден баг, связанный с сортировкой избранных вкладок:

    Picture 10


    Посмотрим на код сортировки:

    int CALLBACK SortByName(const NBookmarkHelper::variantBookmark_t
                              BookmarkItem1,
                            const NBookmarkHelper::variantBookmark_t
                              BookmarkItem2)
    {
      if (   BookmarkItem1.type() == typeid(CBookmarkFolder)
          && BookmarkItem2.type() == typeid(CBookmarkFolder))
      {
        const CBookmarkFolder &BookmarkFolder1 =
          boost::get<CBookmarkFolder>(BookmarkItem1);
        const CBookmarkFolder &BookmarkFolder2 =
          boost::get<CBookmarkFolder>(BookmarkItem2);
    
        return BookmarkFolder1.GetName()
               .compare(BookmarkFolder2.GetName());
      }
      else
      {
        const CBookmark &Bookmark1 = 
          boost::get<CBookmark>(BookmarkItem1);
        const CBookmark &Bookmark2 =
          boost::get<CBookmark>(BookmarkItem1);
    
        return Bookmark1.GetName().compare(Bookmark2.GetName());
      }
    }

    В ветке else ошиблись и дважды использовали BookmarkItem1, вместо BookmarkItem2. Об этой ошибке и предупреждает PVS-Studio:
    • V537 Consider reviewing the correctness of 'BookmarkItem1' item's usage. bookmarkhelper.cpp 535
    • и еще 5 дополнительных предупреждений.

    Здесь можно ознакомиться с Pull Request'ом на исправление.

    Заключение


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

    Мы также предлагаем всем остальным скачать и попробовать анализатор PVS-Studio, он прост в обращении и может быть Вам полезен.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Phillip Khandeliants. How developers were checking projects for bugs using PVS-Studio

    Прочитали статью и есть вопрос?
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio, версия 2015. Пожалуйста, ознакомьтесь со списком.
    PVS-Studio 383,44
    Ищем ошибки в C, C++ и C# на Windows и Linux
    Поделиться публикацией
    Похожие публикации
    Комментарии 6
    • +2
      И вам спасибо за такой конкурс. И PVS, и Pinguem.
      С удовольствием участвовал и с радостью еще раз приму участие:)
      • +1
        Спасибо за pause button, хорошая чтука, чутка доработать и будет очень удобно
        • 0

          И как, окупилось? Участие похоже на не слишком обширное...

          • +1
            Неизвестно. Такие вещи дают очень неспешные всходы.
          • +9
            FAQ www.viva64.com/ru/order-faq
            Я хочу купить Single User License. Почему ее нет?
            Вам надо использовать лицензию для команды. О причинах этого вы можете прочитать в ответе на вопрос: "Почему у нас нет Single User License?".

            Ссылка ведет на www.viva64.com/ru/b/0135
            Там в первых строках читаем
            Данная статья устарела. С актуальной информацией можно ознакомиться здесь.

            Ссылка ведет на FAQ www.viva64.com/ru/order-faq

            V3110. Possible infinite recursion.

          Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

          Самое читаемое