Pull to refresh

Comments 35

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Это очевидно не так для 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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Sign up to leave a comment.

Articles