31 февраля

    31 февраля

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

    Наша команда написала уже 5 статей (1, 2, 3, 4, 5), посвященных поиску ошибок в открытом проекте Chromium. И, видимо, скоро я напишу ещё несколько заметок в наш блог на эту тему.

    Сейчас я изучаю отчёт, выданный анализатором PVS-Studio и просто выписываю найденные ошибки. Написание заметок — это уже следующий этап. Я люблю в начале просмотреть отчёт, а потом уже решаю, как и о чём лучше написать. Однако, одна ошибка мне особенно понравилась, и я решил описать её сразу, не откладывая на потом.

    Ошибка живёт в библиотеке Protocol Buffers (protobuf), используемой в проекте Chromium. Protocol Buffers — это протокол сериализации (передачи) структурированных данных, предложенный Google как эффективная бинарная альтернатива текстовому формату XML.

    Если бы я увидел эту ошибку пару месяцев назад, я бы прошел мимо неё. Ну ошибка и ошибка. Однако, увидев её, я сразу вспомнил эпический сбой кассовых аппаратов, который недавно произошел в России. 20 декабря крупнейшие ритейлеры и сети АЗС по всей России столкнулись со сбоями в работе новых кассовых аппаратов. Первыми о проблеме узнали жители Владивостока, далее она распространилась по стране по мере начала рабочего дня, затронув Новосибирск, Барнаул, Красноярск, Кемерово и другие крупные города.

    Ошибка в кассовых аппаратах и ошибка в Protocol Buffers это две разные ошибки, никак не связанные между собой. Но мне захотелось показать, как могут возникать подобные ошибки. Ведь часто дефекты кроятся не в хитрых алгоритмах, а являются следствием банальных опечаток. Я не знаю, что там было напутано в коде касс, но зато знаю, как глупая опечатка приводит к неработоспособности функции ValidateDateTime, предназначенной для проверки даты в Protocol Buffers. Давайте посмотрим на код этой функции и разберёмся, в нём.

    static const int kDaysInMonth[13] = {
      0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
    };
    
    bool ValidateDateTime(const DateTime& time) {
      if (time.year < 1 || time.year > 9999 ||
          time.month < 1 || time.month > 12 ||
          time.day < 1 || time.day > 31 ||
          time.hour < 0 || time.hour > 23 ||
          time.minute < 0 || time.minute > 59 ||
          time.second < 0 || time.second > 59) {
        return false;
      }
      if (time.month == 2 && IsLeapYear(time.year)) {
        return time.month <= kDaysInMonth[time.month] + 1;
      } else {
        return time.month <= kDaysInMonth[time.month];
      }
    }

    Функция ValidateDateTime принимает на вход дату и должна определить, корректна она или нет. В начале выполняются основные проверки. Проверяется, что номер месяца лежит в диапазоне [1..12], дни находятся в диапазоне [1..31], минуты находятся в диапазоне [0..59] и так далее. В коде всё это хорошо видно, и не требует специальных пояснений.

    Далее идёт более сложная проверка, есть ли определённый день в определённом месяце. Например, декабрь состоит из 31 дня, а в ноябре 31-ого число нет, так как в месяце только 30 дней.

    Чтобы проверить дни и при этом не писать множество операторов if или длинный switch, используется вспомогательный массив kDaysInMonth. В массиве записано количество дней в различных месяцах. По номеру месяца из массива извлекается максимальное количество дней и используется для проверки.

    Дополнительно учитывается, что год может быть високосным, и тогда в феврале на 1 день больше.

    В общем функция написана просто и красиво. Вот только неправильно.

    Из-за опечатки дни проверяются неправильно. Обратите внимание, что с максимально возможным количеством дней в месяце, сравнивается не день, а месяц.

    Ещё раз взгляните на код:

    if (time.month == 2 && IsLeapYear(time.year)) {
      return time.month <= kDaysInMonth[time.month] + 1;
    } else {
      return time.month <= kDaysInMonth[time.month];
    }

    В сравнениях "time.month <=" надо использовать не член структуры month, а член day. Т.е. корректный код должен выглядеть так:

    if (time.month == 2 && IsLeapYear(time.year)) {
      return time.day <= kDaysInMonth[time.month] + 1;
    } else {
      return time.day <= kDaysInMonth[time.month];
    }

    Естественно номер месяца (от 1 до 12), всегда меньше количества дней в любом из месяцев.

    Из-за этой ошибки корректными будут считаться такие дни, как, например, 31 февраля или 31 ноября.

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

    Данная ошибка (вернее две ошибки) обнаруживается с помощью следующих предупреждений PVS-Studio:

    • V547 / CWE-571 Expression 'time.month <= kDaysInMonth[time.month] + 1' is always true. time.cc 83
    • V547 / CWE-571 Expression 'time.month <= kDaysInMonth[time.month]' is always true. time.cc 85

    Как видите, теперь PVS-Studio также сразу классифицирует предупреждения согласно Common Weakness Enumeration (CWE).

    Ещё хочу обратить внимание, что анализатор PVS-Studio всё более глубоко учится анализировать код. Сама по себе диагностика V547 старая (была реализована в 2010 году). Однако, скажем, год назад анализатор не смог бы найти эту ошибку. Теперь анализатор умеет заглянуть внутрь массива и понять, что извлекаются значения в диапазоне [28..31]. При этом он понимает, что 0 в массиве учитывать не надо, так как возможный диапазон time.month равен [1..12]. Если бы номер месяца был, скажем, равен 100, то произошел выход из функции и анализатор это понимает.

    В итоге, анализатор видит, что происходят следующие сравнения диапазонов:

    • [1..12] <= [28..31]
    • [1..12] <= [29..32]

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

    Следует задаться вопросом: как можно улучшить стиль, чтобы защититься от подобных ошибок?

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

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

    Спасибо за внимание. Пойду дальше изучать отчёт.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. February 31.
    PVS-Studio 383,21
    Ищем ошибки в C, C++ и C# на Windows и Linux
    Поделиться публикацией
    Похожие публикации
    Комментарии 106
    • +22
      Спасибо! Сейчас пофиксю.
      • +3
        вот это скорость!

        PS: Нескромный вопрос: а у вас в отделе используются какие-нибудь статические анализаторы, или как обычно: все проверки — глазами код-ревьювера?
        • +3
          есть статические анализаторы. я пытался прикрутить PVS, сломался на интеграции в некоторых местах.

          времени нет, хочу сделать еще одну попытку — после чего выбить соответствующее финансирование, чтоб хотя бы на год в нашей подветке его потестить…
          • +5
            С проектами типа Chromium никакие инструменты из коробки не работают. Это не моя мысль — это говорили ребята из московской разработки Google, пока ее не разогнали.

            Это я к чему. Нет смысла самому прикручивать пытаться, надо нам написать и все подскажем. Судя по количеству наших статей про Chromium, все-таки PVS-Studio прикручивается :-).
            • 0

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


              И обязательно ещё за ключиком схожу ;) дабы демонстрация работала…

              • +3
                Вот только шума слишком много

                Да, такое бывает. При этом, не надо героически преодолевать шум. Надо настраивать анализатор. Например, на Chromuim половина! всех ложных срабатываний связана с макросом DCHECK. Если только один этот макрос доработать напильником, жизнь уже сильно улучшится. Я про это скоро напишу.
                • 0

                  И в мыслях не было :)
                  Шум надо устранять возле его появления, иначе инструмент только мешает, фильтры для известных вещей, и т.п. — ежу понятно.
                  Макросы одна беда, вторая — местоположение дефолтных библиотек, которые точно надо игнорировать.

        • +3
          Неужели парсер / валидатор не покрыт тестами? Тут они прям так и просятся.
          • +7
            покрыты. и тесты на валидацию есть. только почему-то исключительно валидные случаи проверялись — со всеми вытекающими :)
            • +1
              А что если дата локальная и 60 +1 или +2 секунды в какой-то момент это норма?:)
              • 0

                Основа формата — число секунд с точки Х. Не зря там в тестах после заполнения структуры конвертирует туда и обратно.
                Поэтому изначально произвольный таймстамп невозможен

                • +4
                  61 секунда в минуте — это редкая, но нормальная ситуация.
            • +4
              github.com/google/protobuf/pull/4147 — фикс
              github.com/google/protobuf/pull/4148 — дополнительные тесты
              • +1
                А это такой стиль принят в protobuf, что код и доп тесты в отдельных PR идут? Может не обращал внимание, но вроде бы не попадалось. И если это так сделано намеренно, то в чём основной аргумент за?
                • +5
                  В первую очередь — увидеть что тесты падают до фикса и не падают после.
                  У меня на рабочем лаптопе нет ключа от персонального гитхаба, поэтому редактировал онлайн :D

                  зы: я не настоящий сварщик, я работаю над BigQuery.
                • 0
                  А в чём смысл двойной проверки Non-leap year и Leap year?
                  • +1

                    У високосного года есть не только "делится на 4" но ещё условия

                    • +1
                      Но ведь эти тесты не для проверки работы IsLeapYear, по идее? Вот там эти проверки были бы уместны (и они там есть), а здесь они только вопросы вызывают — разве поведение этой функции меняется в зависимости от того, какой именно високосный год ей дадут?
                      • +1

                        Нет, по крайней мере сейчас. Но это один из антипаттернов — тестировать код, зная как он устроен. Такие тесты ломаются или не ловят поломки при рефакторинге или смене алгоритма.
                        Мои тесты тоже не верх качества, так как в идеале надо проверять на все месяцы независимо.

              • +3
                Good. Я скоро ещё принесу. Около 250 штук. :)
                • +2

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

              • 0
                Andrey2008, попадали ли ошибки, найденные PVS Studio под программы Bug Bounty, или в базы наподобие CVE?
                Ждёте ли Вы перед публикацией, пока ошибки проанализируют на возможность эксплутирования хотя бы в таких активных и критически важных проектах, как Chromium?
                Насколько вообще реально с помощью статического анализатора типа PVS-Studio наткнуться на серьёзную проблему безопасности в открытых проектах?
                • +1
                  таких активных и критически важных проектах

                  наткнуться на серьёзную проблему безопасности в открытых проектах

                  Навряд ли. Серьёзные проблемы безопасности (уязвимости то бишь), это не опечатка в коде. Это целая цепочка сложных, на первый взгляд (часто) не связанных между собой шагов плюс уязвимости в third-party продуктах.
                  Если бы всё было так просто — любые уязвимости покрывались бы тестами :)

                  • 0
                    они точно попадают под программу «Buy License» в качестве Bounty
                    • +3
                      Да, анализатор PVS-Studio может выявить уязвимости. Раз он выявляет большое количество CWE-дефектов, то часть из них вполне могут превращаться в CVE. Подробнее.

                      Я думаю, что иногда (сами того не зная) мы находим уязвимости и помогаем их устранить. Но у нас не хватает сил проводить более тщательное исследование по каждой ошибке, чтобы понять, является она уязвимостью или нет.
                    • +1

                      Я конечно дико извиняюсь, но юнит тестов нету?

                      • +4
                        В них наверное тот же самый код :)
                      • +8
                        А то, что в минуте бывает 61 секунда — учитывать не надо?
                        А то, что «1 февраля 1918» на территории ExUSSR — это неверная дата?
                        Во всех книгах по криптографии написано — «не надо придумывать свою систему шифрования, потому что вы по определению напишете хуже и наделаете кучу ошибок». Вот с разбором дат надо такое-же везде писать.
                        • 0

                          И вообще со всем чем угодно. Не нужно писать то, что уже кем то написано

                          • 0
                            Бывает только 60я секунда. Даже если в году две високосных секунды — между ними пол года будет
                            • +3
                              В 1 минуте обычно 60 секунд.
                              Leap second — 61-ая секунда, не поверите.
                              Для информации — например в 2016 году 31 декабря на часах было 23:59:60
                              • 0
                                Ага. Мы о разном говорим. Я не про количество оных, а про индекс.
                                То есть секунда==60 валидна (в очень продвинутых либах), секунда равная 61 нет.
                                Просто это известная ошибка в библиотеке явы, которая позволяет секунду равную 61.

                                Но нет, насколько я понимаю, протобаф не жуёт високосные секунды.
                                • –1
                                  секунда равная 61

                                  Вы не поверите, но это 62-ая. Они на часах с нуля идут.

                                  • 0

                                    См.мой комментарий. Индекс vs количество. "первая секунда" не детерминированная фраза, может означать и нулевую и первую.

                                    • +2
                                      Не поверим. Логично, что 1я — это между 00:00 и 00:01, 60я — между 00:59 и 01:00.
                                  • +1
                                    Бывают случаи, когда в 1 минуте возможно не 60, а 64 секунды, да вообще, когда на входе задана невалидная дата, и с этой датой нужно полноценно работать, а не поднимать лапки.

                                    Например, в FAT32 или ZIP-архивах можно назначить время создания файла 27:60:62. Будет весело, если программа, агрессивно валидирующая дату, будет падать на таких файлах.

                                    Ну или когда контроллер телекоммуникационной стойки без тени сомнения говорит, что на его часах 12:13:60 — с этим тоже нужно уметь работать.
                                    • 0

                                      Нет, это не в одной минуте, а где-то нужно хранение невалидных дат. Там надо подходить соответствующим образом для эмуляции багов

                                • +4

                                  Заблуждения программистов относительно времени
                                  https://habrahabr.ru/post/146109/


                                  Больше заблуждений программистов относительно времени
                                  https://habrahabr.ru/post/313274/

                                  • 0

                                    Объясните, пожалуйста, откуда берется 61 (62,63,64) секунда? Все прочел, но так и не понял — ведь 1 минута = 60 секунд и это чуть ли не метрическая система единиц.

                                  • 0
                                    Вот с разбором дат надо такое-же везде писать.
                                    Было бы хорошо, но есть такая интерасная штука, как время в формате PM/AM:
                                    11:00 AM
                                    12:00 AM
                                    01:00 AM

                                    02:00 AM
                                    03:00 AM

                                    10:00 AM
                                    11:00 AM
                                    12:00 PM
                                    01:00 PM

                                    02:00 PM…

                                    Т.е. 12 PM почти всегда меньше 01 PM.
                                    А 12 AM меньше 01 AM.
                                    И всё бы хорошо, можно захардкодить, НО: в некоторых странах(Австралия), где то же принято использовать AM/PM, нумерация может отличаться, и идти логически правильно:
                                    11 PM
                                    12 PM
                                    01 AM

                                    Поэтому не получится «один раз написать» и забыть, надо еще страну как минимум учитывать)
                                    • +1
                                      Ну, это уже, ИМХО, вопрос парсинга/генерации локализованных строковых форматов (немного другая тема), а не проверки валидности структуры для хранения.

                                      То есть time.day по-любому будет от 0 до 23, а вот то, что в локализованной строке time.day=12 может записываться и как «12 AM», и как «12 PM» (в зависимости от страны), та и разделители/порядок могут отличаться — это отдельная тема.
                                      • +1

                                        Time.hour а не .day ;))

                                        • 0
                                          Ой, ну да.
                                          Я просто копировал из текста статьи дважды: первый раз Ctrl+C не отработало, а второй раз, видимо, моё внимание.
                                      • 0
                                        Хочу заметить, что это хоть и контринтуитивно, но 12 AM после 11 PM это именно что «логически правильно» так-как любой момент сразу после полуночи или полудня будет уже после, а сами по-себе полдень и полночь временной продолжительности не имеют. Если на то пошло, то идеально точное 12:00 будет просто meridiem без ante или post.

                                        Я терпеть не могу, когда в 11:17 говорят «уже 12й час идёт», но ведь так оно и есть на самом деле и здесь то же самое. В 12:00 AM идёт первая минута до полудня.
                                        • 0
                                          но 12 AM после 11 PM это именно что «логически правильно»
                                          Ну ок, допустим. Но тогда совсем логически неправильно это:
                                          12 AM
                                          01 AM

                                          Как будто часы вдруг отмотали на 11 часов назад. Почему 01 больше чем 12, но меньше чем 11? Никакой логики.
                                          • 0
                                            Я так понимаю это возникло из-за повсеместного использования стрелочных часов у которых вместо ноля было 12. Мне тоже это не нравится, в том числе на стрелочных часах.
                                    • +3
                                      В кассовых аппаратах ничего не было напутано, там обиженный разработчик закладку сделал.
                                      • 0
                                        Теперь анализатор умеет заглянуть внутрь массива

                                        Неужели эмулятор?
                                      • 0
                                        В физике есть «теория размерностей», которая говорит, что килограммы нельзя сравнивать с метрами (хотя в общей теории поля размерности пропадают — например, метры переходят в секунды со скоростью света).

                                        Так вот, в программировании давно нужны языки, в которых переменные имеют что-то типа размерности. Чтобы номер месяца и номер дня в месяце нельзя было ни складывать, ни сравнивать.
                                        В языке Паскаль было что-то такое: например, можно было создать перечисляемые типы «месяц» и «день_недели»; их возможные значения выглядели как предопределённые константы, а в применении — контролировался тип.
                                        • 0

                                          Например в Rust есть

                                          • 0

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

                                          • 0
                                            Просто вместо «int day» нужно использовать «class Day day». Или class enum Day (не помню синтаксис для строгих перечислений), но полноценный класс мне больше в данном случае нравится. Другое дело, что слишком много неудобств появляется, помимо кучи удобств.
                                            • 0
                                              Когда я пытался понять как можно было бы избежать такой проблемы в общем, моя первая мысль тоже была, что нужно чтобы все данные имели разный тип,
                                              чтобы например было time.year.isLeap(), time.month.isFebruary().

                                              Вот в чем недостаток примитивных типов в яве. Не в том, что это плохо соотносится с идеей ооп, где все должно быть обьектом. Просто человеку проще сделать сравнение на примитивных числовых типах, чем писать сравнительные методы. Если по пожарной лестнице добраться до квартиры быстрее чем на лифте — это рано или поздно плохо кончится. Схожая история с динамически типизированными языками. Они дают возможность запилить пару багов которые всплывут только во время выполнения.
                                              Ну и конечно много значит сам подход. Попытка быстро избавится от симптома, без особого стремления к качественному улучшению (да, качественное улучшение не тоже самое, что улучшение качества). О, дырка — заклеим. О, пятно — закрасим. О, грязь — подметем. Это же наша работа. А в дипломе написано инженер. Но разве инженер это в первую очередь не подход к решению проблем? Я пишу это как напоминание самому себе.

                                              По работе мне приходится в основном писать тестировочный код и я пришел к тому, что в коде теста («клиенте») должен присутствовать минимум логики. В идеале вообще никакой. Вся логика в методах классов. Сам тест состоит из последовательных вызовов методов. Это повышает поддерживаемость («maintainability») и изменяемость(«changeability) кода. Можно из консоли создать обьекты и перед составлением теста пройти сценарий в шаговом режиме. Улучшается ортогональность: можно увеличивать количество классов и количество тестов независимо друг от друга. Распределять работу: один имплементирует, другой использует в месте назначения (клиенте).

                                              Ну и про сложносоставные логические выражения. Это конечно круто выглядит. Больше сказать о них нечего.
                                              • 0
                                                Проблема даже в том, что если не сделать преобразование в/из примитивных типов, то пользоваться будет невозможно, а если сделать, пусть и явным, преобразование, то проблема чуть уменьшится, но никуда не денется.
                                            • 0

                                              Такие языки давно есть, просто C++ не один из них.
                                              Вот, например, на Idris. Объявлен тип данных "день месяца" и доказана теорема (goal) "номер дня февраля всегда меньше 29":


                                              Заголовок спойлера
                                              import Data.Fin
                                              import Data.List
                                              
                                              Year : Type
                                              Leap : Year -> Type
                                              
                                              data Month = Jan | Feb | Mar | Apr | May | Jun
                                                         | Jul | Avg | Sep | Oct | Nov | Dec
                                              
                                              Long : Month -> Type
                                              Long m = Elem m [Jan, Mar, May, Jul, Avg, Oct, Dec]
                                              
                                              Short : Month -> Type
                                              Short m = Elem m [Apr, Jun, Sep, Nov]
                                              
                                              data DayOfMonth : Year -> Month -> Type where
                                                LongMonthDay  : (day : Fin 31) -> {auto pf : Long month} -> DayOfMonth _ month
                                                ShortMonthDay : (day : Fin 30) -> {auto pf : Short month} -> DayOfMonth _ month
                                                FebDay        : (day : Fin 28) -> DayOfMonth _ Feb
                                                FebThe29th    : {auto pf : Leap year} -> DayOfMonth year Feb
                                              
                                              domToNat : DayOfMonth y m -> Nat
                                              domToNat (LongMonthDay day) = finToNat day + 1
                                              domToNat (ShortMonthDay day) = finToNat day + 1
                                              domToNat (FebDay day) = finToNat day + 1
                                              domToNat FebThe29th = 29
                                              
                                              goal : (day : DayOfMonth _ Feb) -> LTE (domToNat day) 29
                                              goal (LongMonthDay _ {pf}) = absurd (lemma pf) where
                                                lemma : Not (Long Feb)
                                                lemma (There (There (There (There (There (There (There Here))))))) impossible
                                                lemma (There (There (There (There (There (There (There (There _)))))))) impossible
                                              goal (ShortMonthDay _ {pf}) = absurd (lemma pf) where
                                                lemma : Not (Short Feb)
                                                lemma (There (There (There (There Here)))) impossible
                                                lemma (There (There (There (There (There _))))) impossible
                                              goal (FebDay day) =
                                                rewrite sym (plusSuccRightSucc (finToNat day) 0) in
                                                rewrite plusZeroRightNeutral (finToNat day) in
                                                LTESucc (lemma day)
                                                where
                                                lemma : (k : Fin n) -> LTE (finToNat k) n
                                                lemma FZ     {n = Z} impossible
                                                lemma (FS _) {n = Z} impossible
                                                lemma FZ     {n = S _} = LTEZero
                                                lemma (FS k) {n = S _} = LTESucc (lemma k)
                                              goal FebThe29th = lteRefl
                                              • 0
                                                Фразу «в программировании давно нужны языки» надо понимать как «эти языки надо использовать», а не как «надо, чтобы эти языки просто были где-то в музее никогда не использовавшихся языков».
                                                • 0

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

                                                  • 0
                                                    Вы опять меня не поняли. Вопрос не в том, чтобы эти языки использовал я. Вопрос в том, чтобы языки без строжайшей типизации вообще не использовали в сложных и важных проектах. В идеале — хорошо бы запретить это гос.законом; но это нереально, т.к. гос.структуры работают сами знаете как: например, вместо того, чтобы уничтожить спамеров, принимают «закон Яровой».

                                                    Наверно, проблему надо решать на уровне корпораций, примерно так:
                                                    1) Разработчик письменно обязуется использовать языки со строжайшей типизацией.
                                                    2) Разработчик имеет право (и это право оговорено в обязательстве, про которое говорит первый пункт) использовать языки без строжайшей типизации. Однако, в этом случае он будет очень строго наказан (допустим, большими штрафами) в случае ошибок, которые не были бы возможны в языках со строжайшей типизацией.

                                                    Про автомобили сравнительно недавно сообщали, что там можно перехватить управление через FM-радиоприёмник — вот такая там верификация. Интересно, где там была допущена ошибка, и могла ли строжайшая типизация предотвратить эту уязвимость.
                                              • 0
                                                Так в любом языке, где есть User Defined Types — заводишь, и вперед. В той же Джаве для этого отлично подходят перечисления
                                              • –1
                                                Интересная находка, мощный у вас инструмент!

                                                P.S. Мне кажется решение через if было бы не самым плохим:
                                                	if ( Month == 2 )
                                                		return Day <= 28 + isLeap ();
                                                	else
                                                		return Day <= ( Month < 8 ? 30 + Month % 2 : 31 - Month % 2 );
                                                
                                                • +9

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

                                                  • +1
                                                    Может и так, но это можно сказать про любой код. Определение дней в месяце легко вычисляется и логику можно сосредоточить в одном месте, а не глобальный массив с 13 элементами для 12 месяцев только невисокосного года, который еще 'доучитывается' в теле функции.
                                                  • +23
                                                    Слишком примитивно. Даёшь магию во все поля!
                                                    	if ( Month == 2 )
                                                    		return Day <= 28 + isLeap ();
                                                    	else
                                                    		return Day <= (int)(sin(Month * M_PI * 0.87) / 2 + 1) + 30;
                                                    
                                                    • +1
                                                      Даёшь магию во все поля!
                                                      Да пожалуйста:
                                                      return Day <= (((Month & 1) ^ (Month >> 3)) | 30) + 
                                                      (~(((Month - 2) | (2 - Month)) >> 4) & (isLeap(Year) - 2));
                                                      • +2
                                                        Можно и так:

                                                        return Day <= 28 + (0x3BBEECC + isLeap(Year) * 0x10 >> Month * 2 & 3);
                                                        • 0
                                                          Парни, шутки-шутками, но определить, что число четное от 1 до 12 с поправкой на месяц…ну ведь это с++, а не c++script…
                                                          Не хочу критиковать, но и массив, с закладкой что месяц не индекс, плюс отдельная подработка внутри функции…неужели такое красивое решение?
                                                      • 0
                                                        Прекрасно! Вот только функция isLeap меня смущает. Слишком очевидно и легко сопровождаемо. Нужно избавиться и от неё. Естественно, всякие
                                                        (Year % 4 == 0 && (Year % 100 != 0 || Year % 400 == 0) ? 1 : 0)
                                                        это слишком примитивно. Можно остановиться на
                                                        !(Year & 3) - !(Year % 100) + !(Year % 400)
                                                        Но можно пойти ещё чуть дальше:
                                                        (Year & 1 | (Year >> 1) & 1) ^ 1 - !(Year % 100) + !(Year % 400)
                                                        Тут у меня фантазия пока остановилась. Может, кто-нибудь придумает, как избавиться от этих слишком очевидных 100 и 400?

                                                        P.S. Можно, конечно, использовать и старые добрые синусы:
                                                        (int)(sin(-(Year - 1) * M_PI / 2) / 2 + .5) - (int)(sin(-(Year - 25) * M_PI / 50) / 2 + .5) + (sin(-(Year - 100) * M_PI / 200) / 2 + .5)

                                                        Но рядом с битовыми операциями выглядит несколько эклектично.
                                                        • 0
                                                          Так как я хочу, чтобы мой код ещё и поняли, я не буду писать всё в одну строку.
                                                          static inline int f(int x) {
                                                          //	return x ? -1 : 0;
                                                          	static const int INT_BIT = CHAR_BIT*sizeof(int) - 1;
                                                          	return (x|-x) >> INT_BIT;
                                                          }
                                                          static inline int isLeap(int Year) {
                                                          	int YearDiv100 = Year * 1374389535LL >> 37;
                                                          	return 1 + (f(Year & 3) ^ f(Year - YearDiv100 * 100) \
                                                          	       ^ f(Year - (YearDiv100 >> 2) * 400));
                                                          }

                                                          P.S.
                                                          (Year & 1 | (Year >> 1) & 1) ^ 1 - !(Year % 100) + !(Year % 400)
                                                          ^ имеет приоритет ниже чем -, пропущены скобки
                                                          • 0
                                                            P.P.S.
                                                            !(Year & 3) - !(Year % 100) + !(Year % 400)

                                                            (Year & 1 | (Year >> 1) & 1) ^ (1 - !(Year % 100) + !(Year % 400))

                                                            hdfan2, Хоть это и эквивалентные строчки, но логический переход весьма нетривиален!
                                                    • +3
                                                      return Day <= 28 + isLeap ();


                                                      не знаю насчет языка, для которого это написано, но вообще говоря isLeap() должен (вроде как из названия следует) возвращать boolean и применять к нему арифметическую операцию нехорошо
                                                      • +1
                                                        В C++ bool преобразуется в 0, если false и 1, если true
                                                        • 0

                                                          Если повезёт и булево значение не является кастомным дефайном. Ну и гарантировать что преобразованное значение компилятор точно превратит что-то такое в правильное значение


                                                          bool isLeap()
                                                          {
                                                             return bool(0xDEADBEEF);
                                                          }
                                                          • 0
                                                            Если повезёт и булево значение не является кастомным дефайном

                                                            Это тогда уже не булево значение.
                                                            Ну и гарантировать что преобразованное значение компилятор точно превратит что-то такое в правильное значение
                                                            bool isLeap()
                                                            {
                                                               return bool(0xDEADBEEF);
                                                            }
                                                            


                                                            Точно превратит, стандарт говорит, что 0xDEADBEEF преобразуется в true, а true преобразуется в 1.
                                                            • 0
                                                              Точно превратит, стандарт говорит, что 0xDEADBEEF преобразуется в true, а true преобразуется в 1.
                                                              Если у нас действительно C++, а не C, где не было «настоящего логического типа», так что false == 0, а про true было известно только, что оно не равно нулю.
                                                              (Если, например, isLeap() берётся из какой-то старой библиотеки, то там всякого можно ожидать.)

                                                      • 0
                                                        Если интерпретировать выражение «не самым плохим» буквально, то тут я согласен, можно написать ещё хуже (чуть выше есть примеры) :)
                                                      • 0
                                                        Как насчёт проверить новый Firefox (тот что на Quantum)?
                                                        • 0
                                                          У них все новые компоненты стараются писать на Rust. Статического анализатора для него ещё нету(имею ввиду PVS).
                                                        • 0
                                                          Если глобально смотреть, то календарь у нас так себе. Постоянно сдвиги, на середину недели могут выпадать стыки кварталов/месяцев… А поменять никаких шансов.
                                                        • 0
                                                          Кстати Protocol Buffers (protobuf) используеться в TensorFlow, возможно из-за бага с валидацией даты, моя нейросеть не может найти мне девушку…
                                                          Я уверен что если проанализировать код самого TensorFlow то найдуться ошибки покритичней, ибо чуствуеться еще сыроватость проэкта.
                                                            • 0
                                                              Благодарю, интересная статья, если выходит что само C++ ядро у TensorFlow стабильное значит Python обертка не отшлифованная. Я недавно выхвачивал segmentation fault только из-за того что переименовал имя checkpoint файла из “model.data-00000-of-00001” в “model”. Я уже собирался анализировать core дампы с помощью GDB или побайтно смотреть diff-ы checkpoint файлов. Но потом понял что просто Machine Learning Python программисты не любят мирские входные проверки.
                                                            • +1
                                                              возможно из-за бага с валидацией даты, моя нейросеть не может найти мне девушку…

                                                              Подозреваю, что проблема не в нейросети :)
                                                            • 0
                                                              Уверен, что если бросить писать нейронку, выйти на улицу или, на крайний случай, поставить Tinder, то больше шансов найти девушку! ;)

                                                              UPD: коментарий для JaroslavB
                                                              • +4

                                                                Молодцы ребята, серьёзно :-) Я прокачиваю dataflow для Java в IntelliJ IDEA. Когда IDEA подобное находит, я пищу от радости. У нас, к сожалению, нет ни чтения элемента массива после инициализации, ни выполнения сложения на интервалах. Чтение конкретного элемента в принципе несложно поддержать (частично уже есть, скажем, if(idx == 1) a[idx] = 5; <...> if(a[1] > 0) подсветит). Создание интервала значений по интервалу индексов интересная мысль, вроде тоже несложно, просто в голову не приходило, что это может такую пользу принести. А вот делать сложение/вычитание на интервалах я пока опасаюсь, потому что состояния опять будут расходиться, если операция в цикле, и надо будет с этим бороться. Плюс у нас анализ в онлайн-режиме, при редактировании кода, поэтому мы несколько ограничены в ресурсах (все и так жалуются, что Идея тормозит =)). Я пока поддержал только битовое И и остаток от деления, потому что эти операции могут существенно сжать интервал значений и расхождений в циклах с ними не наблюдается. Остаток от деления выловил, например, смешной баг вроде if(count % 10 == 11).

                                                                • +2

                                                                  Ну вот, теперь и мы чего-то можем :-)


                                                                  image

                                                                  • +4
                                                                    А нам даже ничего специально делать не пришлось. Возможность сама «подхватилась» из C++ — ядра.

                                                                    image

                                                                    Так что думаю наш анализатор сможет порадовать пользователей. IDEA уже 10 лет, но понадобилась наша статья, чтобы добавить полезную диагностику. В общем, мы привнесём свежую кровь в сообщество Java анализаторов :).
                                                                    • 0

                                                                      Ну, положим, вы кривите душой. Вам специально пришлось подрубать Spoon, генерировать JNI-обёртки с помощью Swig'а и чёрт знает что ещё. А вот давайте блэкбокс-тестирование проведём, мне интересно. Вот вам исходник A и исходник B. Сохраняются ли варнинги в этих случаях?

                                                                      • +2
                                                                        На самом деле, генерировать JNI обертки с помощью SWIG не так уж сложно.
                                                                        Ну и очевидно, что Spoon мы юзаем, из-за того, что наш C++ парсер не очень подходит для разбора Java =).
                                                                        А вот в механизме Data-Flow ничего специально мы не делали, чтобы поддержать такое в Java.

                                                                        В ваших примерах варнинги не сохраняются, раз вы предоставляете публичный интерфейс для изменения содержимого массива.
                                                                        Можно, конечно, упомянуть межмодульный анализ, однако вряд ли разработчики библиотеки станут без необходимости делать этот массив публичным.
                                                                        Вот небольшая заметка на тему синтетических тестов.
                                                                        • 0

                                                                          Заметку я читал, конечно, она тут совсем ни при чем. Отсутствие варнинга как раз ожидаемо. Откуда же dataflow знает про джавовые уровни доступа?

                                                                          • 0
                                                                            Эта информация собирается визитором на стороне Java. А механизм Data-Flow просто не будет задействован в таком случае. Думаю, по возможности, мы как-нибудь расскажем обо всем поподробнее.
                                                                            • 0

                                                                              Ну так существенная часть моего патча состояла в том, чтобы определить, когда можно доверять значениям из массива, а когда нельзя. Собственно в dataflow может один вызов добавился. Визитор и вам написать пришлось, так что вы всё-таки что-то сделали специально ;-)

                                                                • 0
                                                                  Товарищи, исправили уже нарушения GNU GPL? Те, связанные со статической линковкой glibc?

                                                                  Сами не планируете выпуститься под GNU GPL?
                                                                • +1

                                                                  Всё-таки ошибка возникла не при написании этого кода, а несколько раньше — при планировании проекта. Такие вещи как представление время, часовые пояса, календарь (которые несут в себе не столько сложную функциональность, сколько стандарты, договоренности и культурные особенности) должны находиться только в базовой библиотеке (такой как libc в Линуксе), которую использует миллиард пользователей и просматривает миллион пар глаз. Когда каждый проект начинает городить свои поделки, ошибки статистически неизбежны.


                                                                  Здесь еще повезло, что PVS заметила странное сравнение, а если бы где-то вместо 30 было написано 31, то вот ну никто бы не помог, потому что код формально верен и имеет смысл, но несет в себе альтернативный стандарт.

                                                                  • 0

                                                                    Неочевидно, хорошо ли это. Если это реализация строгой спецификации, то лучше навелосипедить, чем полагаться на стороннюю библиотеку и потом напороться на различия в деталях. Например, java.time при всей своей детальной проработанности официально по спецификации не учитывает високосные секунды, допускает годы от -999,999,999 до +999,999,999 и считает, что календарь всегда был Грегорианским (например, отбросит 29 февраля 1400 года как некорректную дату). Соответствует ли это вашей спецификации? Деталей может быть много и всегда есть шанс, что сторонняя реализация их трактует не так, как требуется вам.

                                                                    • 0

                                                                      Если java.time действительно так себя ведет, то ее реализация как раз нарушает основной принцип единственности источника спецификации.


                                                                      Деталей может быть много и всегда есть шанс, что сторонняя реализация их трактует не так, как требуется вам.
                                                                      Календарь и время — это как раз тот случай, когда не нужно хотеть своей спецификации. То есть своя реализация стандартной спецификации — это еще полбеды (там могут вкрасться трудноуловимые ошибки, которые когда-нибудь можно исправить), но вот своя спецификация — это совсем за гранью.
                                                                  • +2
                                                                    Кстати, вдруг вспомнил, что в своё время встретили VirtualBox другую разновидность бага работы с днями.

                                                                    static const uint8_t g_acDaysInMonths[12] =
                                                                    {
                                                                      /*Jan Feb Mar Arp May Jun Jul Aug Sep Oct Nov Dec */
                                                                        31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
                                                                    };
                                                                    
                                                                    static const uint8_t g_acDaysInMonthsLeap[12] =
                                                                    {
                                                                      /*Jan Feb Mar Arp May Jun Jul Aug Sep Oct Nov Dec */
                                                                        31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
                                                                    };
                                                                    
                                                                    static PRTTIME rtTimeNormalizeInternal(PRTTIME pTime)
                                                                    {
                                                                      ....
                                                                      unsigned cDaysInMonth = fLeapYear
                                                                        ? g_acDaysInMonthsLeap[pTime->u8Month - 1]          // <=
                                                                        : g_acDaysInMonthsLeap[pTime->u8Month - 1];         // <=
                                                                      ....
                                                                    }
                                                                    


                                                                    V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: g_acDaysInMonthsLeap[pTime->u8Month — 1]. time.cpp 453
                                                                    • 0
                                                                      Пользуясь случаем — сравните пожалуйста качество кода 2-х популярных SIP-серверов:
                                                                      github.com/kamailio/kamailio
                                                                      github.com/OpenSIPS/opensips
                                                                      • 0
                                                                        Следует задаться вопросом: как можно улучшить стиль, чтобы защититься от подобных ошибок?

                                                                        Например заводить для дней тип day_t, а для месяцев month_t, чтобы они напрямую не сравнивались. Но это, конечно, накладные расходы на написание таких типов.

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

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