Как в PVS-Studio мы решали одну инженерную задачу в течение нескольких лет


    Сначала я хотел назвать эту заметку «Как PVS-Studio позволяет ДЕШЕВО внедрить статический анализ кода в процесс разработки», но не решился из-за двусмысленного толкования слова «дешево». Поэтому я расскажу об одной инженерной проблеме, которую мы постоянно должны были решать для того, чтобы люди пользовались нашим продуктом. Забегая вперед, скажу, что, как мне кажется, мы ее решили.



    Итак, разработав в начале 2007 года первую версию статического анализатора кода (который тогда назывался Viva64 и выявлял 64-битные ошибки), мы столкнулись с проблемой внедрения инструмента у клиентов. Наш клиент – это компания, в которой как минимум несколько десятков разработчиков, и хотя бы несколько сотен тысяч строк кода. Любой статический анализатор на такой код выдаст кучу предупреждений. Например, мы своим инструментом получали до нескольких тысяч сообщений на один проект.

    Да, конечно же, здесь проблема с ложными срабатываниями анализатора. Однако у любого анализатора есть ложные срабатывания и от них никуда не деться. Встал вопрос – что делать с большим количеством сообщений, которые получает пользователь? То есть проблема выглядит так. Потенциальный пользователь скачивает программу (trial), запускает ее и получает десять тысяч сообщений. Естественно, его это печалит, затем uninstall и мы потеряли клиента.

    Первое что мы сделали – это сразу же убрали дублирующиеся сообщения. Анализатор проверяет проекты на C/C++ и бывает, что ошибка в .h-файле может выдаваться при проверке нескольких .cpp-файлов, которые его используют. У нас это не дублируется. Затем мы добавили возможности фильтрации результатов анализа (и постоянно их совершенствовали): фильтрация по коду ошибки, по тексту сообщения, возможность не проверять файлы по маске и т.п. Все это позволяло существенно снизить количество сообщений, но только после настройки. Пользователь в первый раз все так же получал гору сообщений. Таким образом, фильтрация сообщений – это важный инструмент, но он не решал исходную проблему – трудность внедрения инструмента в процесс разработки.

    Затем в анализаторе появился новый механизм «Mark as False Alarm». Это вставка в код комментариев специального вида (//-V112) для подавления сообщений анализатора. Разметив код таким образом в дальнейшем можно получать сообщения о проблемах только в тех фрагментах кода, где такой разметки пока нет. В идеале это только новый код. Хотя проблема внедрения анализатора в процесс разработки команды стала чуть более простой, все равно, нужно чтобы несколько человек из команды сначала разметили код, избавив от явно мусорных сообщений.

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

    Поэтому мы пошли дальше и сделали новую супер возможность «Incremental Analysis after Build». Теперь анализатор запускается сразу после компиляции и проверяет только те файлы, которые были «задеты» правками пользователя. В отличие от проверки файлов за несколько дней (когда могли проверяться правки команды разработчиков), теперь пользователь видит ТОЛЬКО ошибки в том коде, который он непосредственно трогает.

    Программист теперь не будет беспокоиться о большом объеме кода, который он не трогает. Возможно этому коду уже более 5 лет. Он практически не модифицируется и большинство дефектов в нем уже исправлено. Этот код не надо бросаться проверять в первую очередь и анализатор этого и не делает. Программист будет видеть предупреждения только в свежем коде. А уж если у него будет дополнительное время, он всегда сможет проверить проект целиком, заглянув в самые редко посещаемые места.

    Да, все равно есть ложные срабатывания анализатора. Да, все равно не потеряли свою актуальность фильтры. Но важное другое. Стоимость ВНЕДРЕНИЯ (затраты сил людей на ввод в эксплуатацию) статического анализатора нам удалось сократить до нуля. То есть теперь человек скачивает статический анализатор, устанавливает его и СРАЗУ же начинает получать выгоду от него без каких-либо дополнительных действий.

    Однако не хватало завершающего штриха. Всё бы хорошо, но вот только то, что анализатор нашёл ошибки, было очень плохо заметно. Смена цвета иконки окна PVS-Studio (как мы сделали сначала) не так заметна, а в Visual Studio 2005 это и вовсе не наработает. Решением стало всплывающее уведомление.

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

    Таким образом, инженерная задача по внедрению статического анализа в процесс разработки была решена. Поэтому в PVS-Studio 4.34 режим Incremental Analysis after Build будет по умолчанию включен.

    Вывод прост. Теперь разработчикам можно не бояться сложностей внедрения статического анализа кода, а просто скачать PVS-Studio, установить его и смотреть на ошибки, которые будут обнаруживаться в новом только что разработанном коде.
    Ads
    AdBlock has stolen the banner, but banners are not teeth — they will be back

    More

    Comments 33

      0
      Спасибо за идею — «Incremental Analysis after Build».
      Мы к сожалению на С++ не пишем, но интересно попробовать настроить эту возможность на нашем билд сервере.
        +37
        ой-ой-ой

        Текст на всплывающем окошке — крайне плохой.

        «1 error in your source code»

        что мы видим?
        — прямое гнобление человека. _твой_ код — говно. это недопустимо само по себе.
        — полстатьи посвящено ложным срабатываниям. почему тогда error?
        — error(s) — вы серьёзно? что, правда? не посчитать одна или несколько? сейчас точно 2011 год?

        Когда я работал в одной крупной компании, у нас были чёткие правила поведения на code-review:
        — анализируется код и только код
        — категорически запрещены (под страхом увольнения) какие-либо персональные оценки вида: «что за дебил это писал?», «бред какой-то», «руки бы поотрывал»

        Я бы переделал на что-нибудь приличнее:

        One possible error found
        — Recently compiled code may introduce 1 possible error. Click here to get details.

        А ещё лучше, если ошибка ровно одна — просто показать её!

        One possible error found
        — Recently compiled code may be affected by stack overflow error in buffer.cpp.
        Click here to get details.

        Ну и вообще, не пожалейте денег и наймите английского native-speaker'а проверить дух и лексику во всех сообщениях.
          +4
          >> Текст на всплывающем окошке — крайне плохой.

          Большое спасибо за взгляд со стороны. Обязательно учтем это.

            0
            Вообще можно, если есть вероятность ошибочного срабатывания, писать вместо 'error', 'suspicious code' или что-нибудь подобное.
            –2
            «Текст на всплывающем окошке — крайне плохой.»
            что мы видим?
            — прямое гнобление человека. _твой_ текст в UI — говно.

            «1 error» по-моему не так гнобил. =)
              0
              kosiakk на личном примере показал, как НЕ надо писать сообщения. И был прав.
                +1
                >что мы видим? — прямое гнобление человека. _твой_ текст в UI — говно.

                Уж не знаю, что там видите вы, но мы ничего подобного мы там не видим. Мы видим оценку текста («текст — крайне плохой»), но не оценку человека («вы написали крайне плохой текст»).
                "Твой текст" — это вы додумали сами.
                  0
                  Соглашусь, параллель получилась не очень тонкой. Но все же, «1 error» — это не такое уж и «гнобление».
                    0
                    «Гнобление» не в «1 error», а в «in your source code».
                      0
                      Моя претензия к «1 error» в том, что это может оказаться ложным срабатыванием (по признанию самого автора), но сообщается об этом, как об однозначной катастрофе.
                0
                За два года можно было сделать интеграция в «bug tracking systems», системы контроля версий и централизованный «build environment».

                Непонятно как работать с вашим продуктам тестерам/QA. В некоторых компаниях есть тестеры. Они ищут приоритетных дефекты, назначают приоритеты и риски. И только после оценки влияния дефекта на бизнес, программисты начинают исправят ошибку.
                  +6
                  >> За два года можно было сделать интеграция в «bug tracking systems».

                  Никто не просил пока, а выплевывать весь отчет в баг-треккер — явный спам и мусор.

                  >> системы контроля версий

                  есть конечно же

                  >> централизованный «build environment».

                  с этим тоже никаких проблем.

                  >> Непонятно как работать с вашим продуктам тестерам/QA.

                  Статический анализ кода — это инструмент для программиста. Точно также и про отладчик можно сказать: «Не понятно как с ним работать тестерам/QA».
                    –1
                    >Никто не просил пока, а выплевывать весь отчет в баг-треккер — явный спам и мусор.
                    После проверки и назначение приоритета, можно автоматики импортировать.
                    >Статический анализ кода — это инструмент для программиста.
                    Вашим анализатором кода, могут пользоваться только программисты. Я работал в компании, где только тестеры пользовались статическим анализатором кода. Это очень полезно, для программиста. Им не нужно отсеивать мусор, а работа над тикетами приучает не допускать аналогичные повторные ошибки.
                      +4
                      Расскажите, пожалуйста, подробнее — как тестеры могут пользоваться статическим анализатором? Я не очень представляю это. Или тестеры — программисты?

                      И скажите название анализатора, которым они пользовались.
                        0
                        coverity prevent, наверно сейчас это Coverity Static Analysis.

                        Каждое утро или по заданию, тестеры классифицировали все дефекты. Дальше программисты работали с тикетами.
                          0
                          Ну а тестеры — это программисты?
                            0
                            Бывают кстати и программисты, но это наверное в единичных компаниях. В Parallels например большая часть QA отдела программисты, даже и реверсеры есть, которые в случае чего могут и код сторонних приложений глянуть, ну или автоматизировать что-то изнутри.
                              +1
                              Есть ещё компании где тестеры пишут код (на java в конкретном случае) =)
                              0
                              Если имеются в виду внешние тестеры, то они могут ВООБЩЕ не знать и не понимать программный код. В этом случае дать им анализатор кода — равносильно одному из двух: либо все предупреждения трансформируются в ошибки, либо на него просто забьют.
                      –1
                      >Теперь анализатор запускается сразу после компиляции и проверяет только те файлы, которые были «задеты» правками пользователя.

                      тогда может стоит подумать, о плагине к системам контроля версий, например проверка кода при коммите в меркуриале «pre-commit hook», которая будет проверять измененные файлы или вообще дифы и не давать делать коммит.
                      Просто разработчики часто, билд жмут после нескольких строчек изменений, еще не закончив реализацию фичи, и ошибки от вашего анализатора на неготовом коде могут раздражать.
                      +1
                      Давно читаю посты про PVS-Studio и радуюсь, какая классная штука!

                      А есть что-нибудь подобное для PHP?
                      0
                      А можно где нить почитать, про внутренности PVS? Например как вы код код анализируете — методов достаточно много.
                      Как набираете список правил?

                      ЗЫ Кстати на вашем сайте достаточно интересный набор правил живет — странно, что вы про него рассказываете. www.viva64.com/ru/d/
                        0
                        Кстати, я правильно понимаю, что PVS частично основана на OpenC++?
                          0
                          Да. PVS-Studio основана на нашей же VivaCore, которая в свою очередь основана на OpenC++. Но чуть-ли не весь код OpenC++ уже переписан. Как минимум мы добавили: поддержку C, поддержку C++0x, поддержку Visual Studio ну и много чего еще.
                          +1
                          >> А можно где нить почитать, про внутренности PVS?

                          Пока нет, но у нас уже есть в планах статья про это.

                          >> Как набираете список правил?

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

                          >> Кстати на вашем сайте достаточно интересный набор правил живет — странно, что вы про него рассказываете.

                          Подскажите, пожалуйста, каким образом про него рассказать так, чтобы это было интересно? Ведь просто «вот тут у нас больше сотни правил» — это же скучновато.
                            0
                            Думаю взять пяток с самой интересной историей создания. Например, появишиеся в результате поиска реальных ошибок.
                            Тем более, многие правила очень всеобъемлющи и могут покрывать с десяток различных случаев. Не только указанные в примерах.
                          0
                          false positives надо методично уничтожать. Сделайте сервис который принимает все отвергнутые срабатывания, у вас будет накапливаться статистика и опыт по ошибкам, если ошибка на самом деле присутствует можно отправить пользователю обратный ответ. чтобы он ее исправил (по сути интерактивный support). Это можно объединить в подобие форума, вики и faq — тогда и другие пользователи смогут участвовать в обсуждении чужих ошибок и принятии правильного решения, генерации набора правил работы с подобным кодом. В продолжении интерактивного support'а пользователь может получать обновления из соответствующей темы в виде RSS или еще чего. Посмотрите в сторону формирования практик безопасной, быстрой, безошибочной разработки на C++. Начните с простых примеров типа banned.h как в SDL или нескольких примеров работы с массивами, указателями (это есть во многих книгах, но чаще короткий емкий и выверенный кусок кода лучше любых талмудов). Смысл в том чтобы сформировать community которое будет заходить на сайт с целью прокачивания скиллов C++, а это и реклама, и feedback, и crowdsourcing.
                            +3
                            Только не «PVS-Studio have found error», а «PVS-Studio has found error». А так классная идея, хотел бы такой анализатор для C# кода. ;)
                              0
                              Даже лучше PVS-Studio has found errors.
                                +2
                                Ещё лучше: PVS-Studio hasn't found errors. :)
                                  0
                                  Еще лучше: PVS-Studio correts all found errors. :)

                            Only users with full accounts can post comments. Log in, please.