Комментарии 108
PS: Нескромный вопрос: а у вас в отделе используются какие-нибудь статические анализаторы, или как обычно: все проверки — глазами код-ревьювера?
времени нет, хочу сделать еще одну попытку — после чего выбить соответствующее финансирование, чтоб хотя бы на год в нашей подветке его потестить…
Это я к чему. Нет смысла самому прикручивать пытаться, надо нам написать и все подскажем. Судя по количеству наших статей про Chromium, все-таки PVS-Studio прикручивается :-).
Ну проверку-то разово я тогда прогнал и даже два бага пофиксил. Вот только шума слишком много, и не автоматизировано, надо-то чтоб оно само. В общем, пару выходных я все же убью ещё раз, благо прошлые черновики сохранились.
И обязательно ещё за ключиком схожу ;) дабы демонстрация работала…
Вот только шума слишком много
Да, такое бывает. При этом, не надо героически преодолевать шум. Надо настраивать анализатор. Например, на Chromuim половина! всех ложных срабатываний связана с макросом DCHECK. Если только один этот макрос доработать напильником, жизнь уже сильно улучшится. Я про это скоро напишу.
Основа формата — число секунд с точки Х. Не зря там в тестах после заполнения структуры конвертирует туда и обратно.
Поэтому изначально произвольный таймстамп невозможен
github.com/google/protobuf/pull/4148 — дополнительные тесты
У високосного года есть не только "делится на 4" но ещё условия
Ждёте ли Вы перед публикацией, пока ошибки проанализируют на возможность эксплутирования хотя бы в таких активных и критически важных проектах, как Chromium?
Насколько вообще реально с помощью статического анализатора типа PVS-Studio наткнуться на серьёзную проблему безопасности в открытых проектах?
таких активных и критически важных проектах
наткнуться на серьёзную проблему безопасности в открытых проектах
Навряд ли. Серьёзные проблемы безопасности (уязвимости то бишь), это не опечатка в коде. Это целая цепочка сложных, на первый взгляд (часто) не связанных между собой шагов плюс уязвимости в third-party продуктах.
Если бы всё было так просто — любые уязвимости покрывались бы тестами :)
Я думаю, что иногда (сами того не зная) мы находим уязвимости и помогаем их устранить. Но у нас не хватает сил проводить более тщательное исследование по каждой ошибке, чтобы понять, является она уязвимостью или нет.
Я конечно дико извиняюсь, но юнит тестов нету?
А то, что «1 февраля 1918» на территории ExUSSR — это неверная дата?
Во всех книгах по криптографии написано — «не надо придумывать свою систему шифрования, потому что вы по определению напишете хуже и наделаете кучу ошибок». Вот с разбором дат надо такое-же везде писать.
И вообще со всем чем угодно. Не нужно писать то, что уже кем то написано
Leap second — 61-ая секунда, не поверите.
Для информации — например в 2016 году 31 декабря на часах было 23:59:60
То есть секунда==60 валидна (в очень продвинутых либах), секунда равная 61 нет.
Просто это известная ошибка в библиотеке явы, которая позволяет секунду равную 61.
Но нет, насколько я понимаю, протобаф не жуёт високосные секунды.
Например, в FAT32 или ZIP-архивах можно назначить время создания файла 27:60:62. Будет весело, если программа, агрессивно валидирующая дату, будет падать на таких файлах.
Ну или когда контроллер телекоммуникационной стойки без тени сомнения говорит, что на его часах 12:13:60 — с этим тоже нужно уметь работать.
Заблуждения программистов относительно времени
https://habrahabr.ru/post/146109/
Больше заблуждений программистов относительно времени
https://habrahabr.ru/post/313274/
Вот с разбором дат надо такое-же везде писать.Было бы хорошо, но есть такая интерасная штука, как время в формате 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
Поэтому не получится «один раз написать» и забыть, надо еще страну как минимум учитывать)
То есть
time.day
по-любому будет от 0 до 23, а вот то, что в локализованной строке time.day=12
может записываться и как «12 AM», и как «12 PM» (в зависимости от страны), та и разделители/порядок могут отличаться — это отдельная тема.Я терпеть не могу, когда в 11:17 говорят «уже 12й час идёт», но ведь так оно и есть на самом деле и здесь то же самое. В 12:00 AM идёт первая минута до полудня.
но 12 AM после 11 PM это именно что «логически правильно»Ну ок, допустим. Но тогда совсем логически неправильно это:
12 AM
01 AM
Как будто часы вдруг отмотали на 11 часов назад. Почему 01 больше чем 12, но меньше чем 11? Никакой логики.
Так вот, в программировании давно нужны языки, в которых переменные имеют что-то типа размерности. Чтобы номер месяца и номер дня в месяце нельзя было ни складывать, ни сравнивать.
В языке Паскаль было что-то такое: например, можно было создать перечисляемые типы «месяц» и «день_недели»; их возможные значения выглядели как предопределённые константы, а в применении — контролировался тип.
Например в Rust есть
чтобы например было time.year.isLeap(), time.month.isFebruary().
Вот в чем недостаток примитивных типов в яве. Не в том, что это плохо соотносится с идеей ооп, где все должно быть обьектом. Просто человеку проще сделать сравнение на примитивных числовых типах, чем писать сравнительные методы. Если по пожарной лестнице добраться до квартиры быстрее чем на лифте — это рано или поздно плохо кончится. Схожая история с динамически типизированными языками. Они дают возможность запилить пару багов которые всплывут только во время выполнения.
Ну и конечно много значит сам подход. Попытка быстро избавится от симптома, без особого стремления к качественному улучшению (да, качественное улучшение не тоже самое, что улучшение качества). О, дырка — заклеим. О, пятно — закрасим. О, грязь — подметем. Это же наша работа. А в дипломе написано инженер. Но разве инженер это в первую очередь не подход к решению проблем? Я пишу это как напоминание самому себе.
По работе мне приходится в основном писать тестировочный код и я пришел к тому, что в коде теста («клиенте») должен присутствовать минимум логики. В идеале вообще никакой. Вся логика в методах классов. Сам тест состоит из последовательных вызовов методов. Это повышает поддерживаемость («maintainability») и изменяемость(«changeability) кода. Можно из консоли создать обьекты и перед составлением теста пройти сценарий в шаговом режиме. Улучшается ортогональность: можно увеличивать количество классов и количество тестов независимо друг от друга. Распределять работу: один имплементирует, другой использует в месте назначения (клиенте).
Ну и про сложносоставные логические выражения. Это конечно круто выглядит. Больше сказать о них нечего.
Наверно, проблему надо решать на уровне корпораций, примерно так:
1) Разработчик письменно обязуется использовать языки со строжайшей типизацией.
2) Разработчик имеет право (и это право оговорено в обязательстве, про которое говорит первый пункт) использовать языки без строжайшей типизации. Однако, в этом случае он будет очень строго наказан (допустим, большими штрафами) в случае ошибок, которые не были бы возможны в языках со строжайшей типизацией.
Про автомобили сравнительно недавно сообщали, что там можно перехватить управление через FM-радиоприёмник — вот такая там верификация. Интересно, где там была допущена ошибка, и могла ли строжайшая типизация предотвратить эту уязвимость.
P.S. Мне кажется решение через if было бы не самым плохим:
if ( Month == 2 )
return Day <= 28 + isLeap ();
else
return Day <= ( Month < 8 ? 30 + Month % 2 : 31 - Month % 2 );
Опечатку в таком коде с еще большей вероятностью не заметил бы ни программист, ни PVS
if ( Month == 2 )
return Day <= 28 + isLeap ();
else
return Day <= (int)(sin(Month * M_PI * 0.87) / 2 + 1) + 30;
Даёшь магию во все поля!Да пожалуйста:
return Day <= (((Month & 1) ^ (Month >> 3)) | 30) +
(~(((Month - 2) | (2 - Month)) >> 4) & (isLeap(Year) - 2));
return Day <= 28 + (0x3BBEECC + isLeap(Year) * 0x10 >> Month * 2 & 3);
(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)
Но рядом с битовыми операциями выглядит несколько эклектично.
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)
^ имеет приоритет ниже чем -, пропущены скобкиreturn Day <= 28 + isLeap ();
не знаю насчет языка, для которого это написано, но вообще говоря isLeap() должен (вроде как из названия следует) возвращать boolean и применять к нему арифметическую операцию нехорошо
Если повезёт и булево значение не является кастомным дефайном. Ну и гарантировать что преобразованное значение компилятор точно превратит что-то такое в правильное значение
bool isLeap()
{
return bool(0xDEADBEEF);
}
Если повезёт и булево значение не является кастомным дефайном
Это тогда уже не булево значение.
Ну и гарантировать что преобразованное значение компилятор точно превратит что-то такое в правильное значение
bool isLeap() { return bool(0xDEADBEEF); }
Точно превратит, стандарт говорит, что 0xDEADBEEF преобразуется в true, а true преобразуется в 1.
Точно превратит, стандарт говорит, что 0xDEADBEEF преобразуется в true, а true преобразуется в 1.Если у нас действительно C++, а не C, где не было «настоящего логического типа», так что false == 0, а про true было известно только, что оно не равно нулю.
(Если, например, isLeap() берётся из какой-то старой библиотеки, то там всякого можно ожидать.)
Я уверен что если проанализировать код самого TensorFlow то найдуться ошибки покритичней, ибо чуствуеться еще сыроватость проэкта.
возможно из-за бага с валидацией даты, моя нейросеть не может найти мне девушку…
Подозреваю, что проблема не в нейросети :)
Молодцы ребята, серьёзно :-) Я прокачиваю dataflow для Java в IntelliJ IDEA. Когда IDEA подобное находит, я пищу от радости. У нас, к сожалению, нет ни чтения элемента массива после инициализации, ни выполнения сложения на интервалах. Чтение конкретного элемента в принципе несложно поддержать (частично уже есть, скажем, if(idx == 1) a[idx] = 5; <...> if(a[1] > 0)
подсветит). Создание интервала значений по интервалу индексов интересная мысль, вроде тоже несложно, просто в голову не приходило, что это может такую пользу принести. А вот делать сложение/вычитание на интервалах я пока опасаюсь, потому что состояния опять будут расходиться, если операция в цикле, и надо будет с этим бороться. Плюс у нас анализ в онлайн-режиме, при редактировании кода, поэтому мы несколько ограничены в ресурсах (все и так жалуются, что Идея тормозит =)). Я пока поддержал только битовое И и остаток от деления, потому что эти операции могут существенно сжать интервал значений и расхождений в циклах с ними не наблюдается. Остаток от деления выловил, например, смешной баг вроде if(count % 10 == 11)
.
Ну вот, теперь и мы чего-то можем :-)
Так что думаю наш анализатор сможет порадовать пользователей. IDEA уже 10 лет, но понадобилась наша статья, чтобы добавить полезную диагностику. В общем, мы привнесём свежую кровь в сообщество Java анализаторов :).
Ну, положим, вы кривите душой. Вам специально пришлось подрубать Spoon, генерировать JNI-обёртки с помощью Swig'а и чёрт знает что ещё. А вот давайте блэкбокс-тестирование проведём, мне интересно. Вот вам исходник A и исходник B. Сохраняются ли варнинги в этих случаях?
Ну и очевидно, что Spoon мы юзаем, из-за того, что наш C++ парсер не очень подходит для разбора Java =).
А вот в механизме Data-Flow ничего специально мы не делали, чтобы поддержать такое в Java.
В ваших примерах варнинги не сохраняются, раз вы предоставляете публичный интерфейс для изменения содержимого массива.
Можно, конечно, упомянуть межмодульный анализ, однако вряд ли разработчики библиотеки станут без необходимости делать этот массив публичным.
Вот небольшая заметка на тему синтетических тестов.
Заметку я читал, конечно, она тут совсем ни при чем. Отсутствие варнинга как раз ожидаемо. Откуда же dataflow знает про джавовые уровни доступа?
Сами не планируете выпуститься под GNU GPL?
Всё-таки ошибка возникла не при написании этого кода, а несколько раньше — при планировании проекта. Такие вещи как представление время, часовые пояса, календарь (которые несут в себе не столько сложную функциональность, сколько стандарты, договоренности и культурные особенности) должны находиться только в базовой библиотеке (такой как libc в Линуксе), которую использует миллиард пользователей и просматривает миллион пар глаз. Когда каждый проект начинает городить свои поделки, ошибки статистически неизбежны.
Здесь еще повезло, что PVS заметила странное сравнение, а если бы где-то вместо 30 было написано 31, то вот ну никто бы не помог, потому что код формально верен и имеет смысл, но несет в себе альтернативный стандарт.
Неочевидно, хорошо ли это. Если это реализация строгой спецификации, то лучше навелосипедить, чем полагаться на стороннюю библиотеку и потом напороться на различия в деталях. Например, java.time при всей своей детальной проработанности официально по спецификации не учитывает високосные секунды, допускает годы от -999,999,999 до +999,999,999 и считает, что календарь всегда был Грегорианским (например, отбросит 29 февраля 1400 года как некорректную дату). Соответствует ли это вашей спецификации? Деталей может быть много и всегда есть шанс, что сторонняя реализация их трактует не так, как требуется вам.
Если java.time действительно так себя ведет, то ее реализация как раз нарушает основной принцип единственности источника спецификации.
Деталей может быть много и всегда есть шанс, что сторонняя реализация их трактует не так, как требуется вам.
Календарь и время — это как раз тот случай, когда не нужно хотеть своей спецификации. То есть своя реализация стандартной спецификации — это еще полбеды (там могут вкрасться трудноуловимые ошибки, которые когда-нибудь можно исправить), но вот своя спецификация — это совсем за гранью.
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
github.com/kamailio/kamailio
github.com/OpenSIPS/opensips
Следует задаться вопросом: как можно улучшить стиль, чтобы защититься от подобных ошибок?
Например заводить для дней тип day_t, а для месяцев month_t, чтобы они напрямую не сравнивались. Но это, конечно, накладные расходы на написание таких типов.
31 февраля