Неопределенное поведение, пронесенное сквозь года

    У вас бывают в разработке такие периоды, когда что-то в коде идет не так, ты ищешь баг, а потом оказывается, что за ним стоял еще один баг? Мне нравится искать баги. Это создает ощущение словно ты Шерлок Холмс и являешься главным героем в детективе, где кто-то из обширного списка на вид безобидных классов и функций вызывает неожиданное и даже неопределенное поведение программы, а ты своим зорким взглядом и экспериментами пытаешься вычислить этого мерзавца в кратчайшие сроки. Можно выделить несколько стадий поиска бага:

    • удивление (не знаю как вы, но я каждый раз как в первый раз удивляюсь когда что-то вдруг в моем коде работает не так, как ожидается);

    • обвинение всех кругом в баге (коллег по проекту, github, сторонние либы, компилятор), но только не себя;

    • смирение с тем, что возможно баг появился из-за меня и поиск бага: анализ выдаваемого результата, локализация ошибки, эксперименты с входными данными; в общем, все, что делает нормальный детектив, только в сфере программирования;

    • если причина бага найдена быстро, то я хвалю себя за то, что нашел баг, при этом, я не напоминаю себе, что причиной бага стал тоже я, а не коллеги по проекту, не github, не сторонние либы и не компилятор;

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

    Меня зову Женя, я open source разработчик. Я занимаюсь разработкой библиотеки sqlite_orm на С++ https://github.com/fnc12/sqlite_orm в свое свободное время.

    Библиотека sqlite_orm предоставляет более удобный API на С++, чем «родная» SQLite3-библиотека, написанная на чистом С. Конечно, я и другие контрибьюторы еще не покрыли весь API SQLite, потому работа не останавливается никогда. Меня очень давно просили добавить в sqlite_orm поддержку пользовательских функций. Это возможность привязывать колбэки на чистом C в виде функций, доступных внутри SQLite-запросов. И вот я решил, что хватит тянуть кота за хвост, что когда-нибудь все-равно придется взяться за это, а значит почему бы не взяться за это прямо сейчас? Сказано - сделано. Я сел, начал кодить. Фича состоит из условных трех частей:

    1. скалярные функции;

    2. агрегатные функции;

    3. скалярные и агрегатные функции с варьирующимся количеством аргументов (первые два пункта имели константное количество аргументов).

    Я честно сделал все три шага. Это было три последовательных пулл-реквеста. И, если коротко, то третий ПР я так и не влил все еще, потому что с ним начали происходить, если выражаться мягко, волшебные странности.

    AppVeyor вдруг стал говорить, что юнит-тесты упали. Хм, ок, я пошел смотреть. Конечно, меня это удивило, потому что локально все проходило. 

    Однако, локально я работаю на macOS, а AppVeyor мне собирает Windows. Значит ошибка платформозависимая, а это значит, что на носу у меня «веселая» детективная история, потому что платформозависимые ошибки самые вредные. Однако, я даже представить себе не мог во что это все выльется.

    Ок, смотрю я в логи AppVeyor. Логи говорят: 1 тест не пройден. Ок, смотрим какой тест. Ответ: тот самый, который я добавлял вместе с пользовательскими функциями. Если точно, то вот логи:

    Для тех, кому интересны технические детали того, что тут происходит:

    Запрос SELECT FIRST(‘Vanotek’, ‘Tinashe’, ‘Pitbull’) где функция FIRST это скалярная функция с варьирующимся количеством аргументов, которая принимает строки и возвращает строку, состоящую из первых символов всех аргументов в том же порядке, в котором они подставлены в функцию. На маке результат равен «VTP», что логично, А вот на венде - нет. 

    Я в недоумении, ругаюсь на венду, думаю забить на ее поддержку, удалить этот юнит-тест и влить ПР как есть. Но я успокаиваюсь, отбрасываю дурацкие мысли и начинаю пытаться найти причину косяка.

    Давайте я не буду описывать подробности того, что было дальше. Расскажу коротко. Я сначала добавлял cout логи чтобы посмотреть на то, что происходит прямо на AppVeyor и по-быстрому решить проблему. Не вышло. Потом я таки расчехлил свою венду, собрал проект в Visual Studio и стал дебажить (к счастью, на моей венде баг тоже воспроизводился). В общем, косяк оказался в том, что когда для результата отдаем строку, то ее надо копировать и предоставлять указатель на функцию-деструктор. Самое забавное в этом баге то, что в процессе дебага я долго не мог понять почему целые числа отлично заходят в качестве результата, а строки ломаются, при этом только на венде. Мак, как я и говорил, стабильно выдавал мне 0 непройденных тестов. Я даже в процессе написал одному из разработчиков SQLite о том, что я нашел ошибку в SQLite, которая воспроизводится только на венде. Потом, когда я уже понял в чем косяк, я отписался ему вновь о том, что я сам дурак, «извини меня за беспокойство и за то, что я закидал тебе личку кусками своего кода».

    Это был очень глупый баг, в котором был виноват исключительно я сам, а не коллеги по проекту, не github, не сторонние либы и не компилятор. Я сглупил, отвык писать на чистом C и просто ослеп на подобные баги. Вообще, если ты, дорогой читатель, вдруг думаешь, что если у человека есть open source проект с 1000+ звездами, то этот человек какой-то по особому умный, то я тебя огорчу - это совсем не так. Я порой пишу такую дичь, что мне больше интересно понять почему в моей голове происходят такие когнитивные искажения, чем писать непосредственно код. И то насколько я хороший код пишу зависит не от количества звезд у sqlite_orm, а от простого «выспался или нет» и «устал ли я от прогулки под палящим алмаатинским солнцем сегодня».

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

    Но вернемся ко мне в то время, когда я только поправил баг. Я чувствую себя одновременно и болваном, и героем. Болваном потому что баг глупый. Героем потому что наконец-то у меня пройдут проверки пулл-реквеста на AppVeyor и я, наконец-то, волью полностью поддержку пользовательских функций. Пока юнит-тесты выполнялись я размышлял об этом баге. В частности, о том, почему он стабильно не воспроизводился на маке. На самом деле я часто с таким сталкивался. Вообще следует признать, что macOS + iOS более толерантны к неопределенному поведению, чем Windows. Ведь по сути я передавал указатель на строку, которая «умерла» уже к моменту когда надо было к ней обратиться. Однако память не только осталась у процесса, но и сохранила свое содержимое. И она сохраняла его каждый раз когда я запускал юнит-тесты на маке. То есть, мак порой превращает неопределенное поведение в определенное.

    И вот вы представьте насколько я не мог поверить своим глазам, когда увидел, что проверки вновь не прошли. Я всегда был убежден, что выражение «не верю своим глазам» имеет переносный смысл, но в ту минуту я буквально не верил своим глазам. Я думал, что глаза у меня тоже глючат, как глючил мой мозг когда я сотворил тот глупый баг, что на самом деле проверки прошли и ПР можно смело вливать, просто что-то не так с моими глазами. Однако, проверки не прошли, это был факт. «Ну, наверное это какая-то сторонняя ошибка» - подумал я, - «наверное, коммит последний не подтянулся, сеть глючит у AppVeyor, на билд-агент упал метеорит, но мой код точно не имеет багов». Как же я ошибался в тот момент.

    Я пошел на страницу деталей прогона ПР в AppVeyor. И увидел в этот раз знакомую картину: все 8 прогонов снова красные. Как если бы я ничего не коммитил! Но я коммитил! Я снова прокрутил в голове те моменты, когда я делал коммит. Я точно его делал, я не сошел с ума пока. Ок, пойдем в логи. Вот что показывают логи:

    Логи говорят, что сломались тесты фичи custom collations. Фичи никак не связанной с пользовательскими функциями. Custom collations это похожая на пользовательские функции фича, однако не имеющая ничего общего в реализации с ними. Custom collations позволяет добавлять свои collating последовательности, которые вызываются как колбэки для функций на чистом C, которые используются для сравнения строк. Повторюсь: никакой связи эти две фичи не имеют. Я могу смело выкинуть любую и вторая останется работоспособной. Контейнеры, в которых хранится информация о пользовательских функциях и коллэйшонов тоже разные (типы разные), а значит я не мог по ошибке отдать итератор на один контейнер в другой.

    Я думаю «ок, вот тут точно мой ПР про функции не причем». Значит, основная ветка разработки `dev` тоже должна показывать эту ошибку. Однако в ветке `dev` все было прекрасно - 8 зеленых прогонов. Значит дело в пользовательских функциях. Но как могут функции аффектить коллэйшоны? И почему только на венде, а не на маке? Я готов был рвать волосы на своей голове, но у меня их и так там нет. Я готов был признать свою профнепригодность, и пойти работать кем-либо еще. Может это снова толерантность к неопределенному поведению случилась? Дважды за день! Но почему тогда в ветке `dev` на Windows все работает? Фича custom collations была влита три года назад по просьбе пользователей библиотеки. Не может быть такого, что никто из этих людей не заметил, что коллэйшоны не работают.

    Ок, я немного успокоился и пошел смотреть в код. Я вам его тоже покажу. Попробуйте найти в нем ошибку.

    Если вы догадались, то вы супер. А я не догадался. Точнее, догадался только вчера. А до этого три ДОЛБАННЫХ года (!) этот код работал неправильно.

    В чем именно косяк в данном коде:

    На строке 323 второй аргумент функции, который называется f отдается полностью в местный контейнер при помощи std::move, а значит после этого f будет пустым (f это std::function), а значит на строке 335 в тернарном операторе всегда будет возвращаться альтернативный результат, а не основной. В таком случае вместо создания коллэйшона с указанным именем будет вызвано его удаление. 

    Ок, вот мы и нашли причину ошибки «no such collating sequence». Давайте править код. Я все еще немного офигевая от того, что у меня баг вылез из-за новой несвязанной фичи, влил исправление чтобы наконец-то тесты на венде прошли успешно. Я запушил и в ожидании сборки юнит-тестов на AppVeyor стал обдумывать ситуацию. Все же раньше собиралось! И на венде тоже! И другие разработчики пользовались всем этим и никто не жаловался. Что ж, у меня есть где-то час пока дойдет очередь до сборки моего ПР. Ждем.

    Вы, наверное, думаете «Женя, ну и что? Это самый эпичный баг?». Но вы подождите, еще не конец!

    Когда сборка закончилось как вы думаете какой был результат на AppVeyor? Правильно, красный. А знаете какие тесты не прошли? Вот вам картинка:

    «Выглядит как какая-то ерунда» - подумал я сразу же. Ну тут точно что-то произошло на билд-агентах. Если серьезно, то бывает, что SQLite не смог скачаться - wget не сумел отработать, и из-за этого не проходит сборка. Но (спойлер) ничего такого не случилось. И в дальнейших моих коммитах, где я добавлял выводы логов результат был идентичным: эти же три из восьми конфигураций не проходили. «Что же там не так?» спросите вы. Вот:

    Снова тот же тест, но уже с другой ошибкой. Если раньше SQLite не мог найти collating sequence, то теперь он находит эту collating sequence, но она не отрабатывает, из-за этого в контейнере `rows` пусто, хотя должна быть одна запись.
    Фак, фак фак! Как так?! Почему опять я делаю пользовательские функции, но ломаются коллэйшоны? Почему такая логика? Почему в `dev` все работает? Почему только венда? Почему так много «почему»? Я, конечно, люблю детективы, но тут очевидно, что закон подлости насмехается надо мной.

    Ладно, хватит переживать, надо скорее искать косяк. Не буду рассказывать как долго я искал косяк, просто расскажу где он оказался. Вот код:

    Это код теста. Лямбда должна осуществлять сравнение строк и возвращать индекс первого несовпадающего символа по аналогии с функцией strcmp. И я проигнорировал первый аргумент, который имеет тип int. Это длина данных для сравнения. И SQLite не дает гарантию, что второй и третий аргументы имеют после себя нуль-терминаторы. Просто почему-то раньше эти нуль-терминаторы там были. Целых три года! Но с появлением пользовательских функций три из восьми конфигураций на Windows вдруг перестали проявлять толерантность к неопределенному поведению. К такому жизнь меня точно не готовила.

    Заменив код на вот такой я добился, что все тесты стали проходить как надо:

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

    Всем спасибо за прочтение, и желаю всем зеленых сборок!

    Средняя зарплата в IT

    135 000 ₽/мес.
    — средняя зарплата во всех IT-специализациях по данным из 5 317 анкет, за 2-ое пол. 2021 года. Проверьте «в рынке» ли ваша зарплата или нет! Проверить свою зарплату
    Реклама
    AdBlock похитил этот баннер, но баннеры не зубы — отрастут

    Подробнее

    Комментарии 35

      +5

      Возможно, ваш код де-факто опирался на какое-то поведение менеджера памяти, которое в последних виндах из-за безопасности многопоточных операций или какого-нибудь userspace sandbox было поправлено, затем были исправлены некоторые места в ОС, зависящие от измененного поведения, но столь же "толерантные к UB", и наконец поломали вас. А вообще, попросите PVS-Studio на погонять вашу библиотеку, есть вероятность, что найдете много интересного.

        0

        Проставьте знак препинания в заголовке, обособив причастный оборот после определяемого слова.

          +2

          поставил, спасибо

          +2

          Заменив код на такой у меня все тесты стали проходить как надо.

          Может быть я, конечно, чего-то не понимаю (никогда не использовал sqlite C++ API), но я так понимаю, что оба этих int'а - это длины соответствующих строк. Т.е. если lhs у вас будет указывать, скажем, на строку "AAA" (length = 3), а rhs - на строку "AAAB" (анонимный параметр длины = 4), то результат strncmp будет 0 и collation будет работать неправильно.

            0

            согласен, тут не совсем очевидный API. Подробно о нем можно почитать тут https://www.sqlite.org/c3ref/create_collation.html

              +3

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

              +5

              А вам не кажется, что вы тут берете указатель на элемент коллекции collatingFunctions, который может быть перемещен при операциях с коллекцией в будущем?

                0

                хм, вы правы. Поправлю, спасибо

                  +2

                  Блин, ну лютая жесть же.
                  Причем я не спец в C++, я пишу на "C с классами и type safety".
                  Поэтому перепроверьте мои слова, я могу ошибаться.
                  Но ошибка типичная.

                    0

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

                      0

                      map не будет перемещать свои элементы. Проблема будет, если вдруг поменяете на unordered.

                      Формально некорректно, что код считает, что может быть ошибка в обращении к sqlite, но при этом collatingFunctions может уже изменить свое состояние.

                        0

                        А есть чего интересного почитать про опасности таких вещей?
                        Я лично в юности брал указатели на элементы std::vector и ловил креши, но тут все очевидно.
                        А с std::map не нагуглил ничего хорошего, только это:
                        https://stackoverflow.com/questions/516007/stdmap-pointer-to-map-key-value-is-this-possible

                          +3

                          На cppreference в разделе Modifiers в описаниях соответствующих контейнеров есть описания гарантий по ссылкам и итераторам при добавлении или удалении элементов контейнера.

                            0

                            В том, что вы нагуглили, есть ссылка на стандарт - вроде не так плохо.

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

                            Но простор для реализации всегда остается. Самый классический пример: list::size vs list::splice - что из этого делать за константу. Тут надо и стандарт знать и версию компилятора.

                    +4

                    я придумал фичу: хабраревью. Закидываешь ПР в отдельную секцию на хабре под названием "хабраревью", и туда набегает кто хочет сделать ревью по аналогии как тут некоторые пишут отзывы на мой код. Чтобы мотивировать людей можно в карму добавлять плюсы за комменты в ревью

                      0

                      Это очевидно не так для node-based контейнеров типа std::map|set, где ссылка (и указатель тоже) на элемент в контейнере будет всегда действительна до удаления элемента.

                      Но недавно я узнал, что ссылка на элемент в std::unordered_map|set также действительна до удаления. Я был уверен, что они могут стать недействительными при вставке, если происходит rehashing. Однако, в документации говорится, что недействительными при вставке могут стать только итераторы:

                      If an insertion occurs and results in a rehashing of the container, all iterators are invalidated. Otherwise iterators are not affected. References are not invalidated.

                      https://en.cppreference.com/w/cpp/container/unordered_map/operator_at

                      Так что внимательное чтение документации иногда может быть полезно.

                        0

                        А вам не кажется, что вы тут берете указатель на элемент коллекции collatingFunctions, который может быть перемещен при операциях с коллекцией в будущем?

                        нет, итераторы (и, соответственно, ссылки/указатели) на элементы большинства стандартных контейнеров не инвалидируются move'ом, и это гарантируется стандартом

                        +4

                        История интересная, но у вас осталось еще одно когнитивное искажение судя по:

                        мак порой превращает неопределенное поведение в определенное.

                        эту особенность называю толерантностью к неопределенному поведению.

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

                          0

                          Да и само утверждение, что Мак более "определенный" - это какая-то ересь.
                          Без глубокого анализа работы аллокаторов это просто утверждение на уровне "Мак более безопасен потому что... потому что".
                          Может просто так фазы луны совпали.

                            0

                            да, но какое определение у "определенного поведения"? Так вот: на практике в моем случае то, что происходило у меня три года вписывается не только в определение неопределенного поведения, но и также в определение определенного поведения (простите за тавтологию). Я точно не считаю неопределенные поведение неправильным поведением

                              0

                              Попробую дать определение определенному поведению для C++20: это поведение, закрепленное в стандарте International Standard ISO/IEC 14882:2020(E) – Programming Language C++.

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

                            +7
                            Чтобы сразу ловить такие баги, тесты надо запускать со включенным санитайзером памяти. Санитайзер старается преобразовать неопределённое поведение в определённое. Большинство обращений к неинициализированной памяти со включенным санитайзером приводит к аварийному останову с возможностью сохранить дамп или сразу в отладке посмотреть источник проблемы.
                              0

                              спасибо. Попробую как будет время

                                +2
                                С такими языками как C и C++, которые допускают UB на любой чих, их надо не пробовать, а очень активно использовать при тестировании (а если очень прижмёт, ещё и продакшене).
                                Если вкратце, memory sanitizer работает как плагин в компиляторе и перед каждым обращением к памяти ставит проверку, является ли память разрешённой к доступу или нет. Соответственно при выделении/освобождении памяти как в куче, так и на стэке, участки памяти помечается нужным образом, чтобы было понятно, можно к ней обращаться.
                                Санизайзеры могут замедлять приложение, но это ничто по сравнению с замедлением от valgring или drmemory (которые, кстати, я тоже рекомендую)
                              +1

                              бросай это дело

                                0

                                почему?

                                0

                                А вы проверяете свой код на утечки памяти?

                                  0

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

                                    0

                                    Я тоже словил как-то баг с выходом за границу массива при работе со строками. Где-то месяца 3 баг жил в проде, пока не упал на "удачной" длине строки. Ранее падения не было из-за выравнивания памяти в структуре.

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

                                    0

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

                                      +2

                                      Ну как-же... В первом случае, хотя прямо и не говорится что это UB. Но... "argument is in a valid but unspecified state after the call." Т.е. никто не гарантирует что будет работать определенным образом.

                                      Во втором, это 100% UB т.к. идет обращение к неинициализированной памяти, потому что спека как раз таки 0 после конца строки не обещала.

                                      +3

                                      PVS-Studio @Andrey2008 что скажете? :)

                                        0

                                        Первую и последнюю проблемы на раз поймал бы ASAN.

                                          +1

                                          Есть есть настроенные автотесты - можно же добавить прогоны санитайзеров и/или valgrind. Не обязательно по каждому коммиту, можно по расписанию. Просто мне кажется, что практически все упомянутые дефекты тот же валгринд нашёл бы практически сразу! Это небольшая инвестиция времени (в настройку CI), но вклад в качество кода и в процесс копания в багах зачастую неоценим (особенно когда речь о программах на классических C/C++).

                                          А потом можно попробовать и встроенные в компилятор санитайзеры (прямо "сразу" довольно непросто, покуда программа будет завершаться на первом же дефекте, и это довольно быстро удручает. valgrind в этом плане чуть практичнее, потому что он просто скрупулёзно соберёт лог всего, не приводя к аварийной остановке. Поэтому с помощью него можно сперва исправить все очевидные косяки, а потом переключиться на санитайзеры)

                                            +1

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

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

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