Находим баги в LLVM 8 с помощью анализатора PVS-Studio

[KanuTaH]

В начале содержимое объекта перемещается, а затем он используется как ни в чём не бывало

Это не всегда плохо. В случае конкретно «умных указателей» — да, использование после перемещения может свидетельствовать о проблеме, но в общем случае это не так. Перемещенный объект вполне можно переиспользовать заново (это ведь просто default constructed объект как правило), это обычная оптимизация и далеко не всегда ошибка.

[Andrey2008]

> Это не всегда плохо.
Анализтор предупреждает только об аномальном использовании.

[KanuTaH]

Там, кстати, довольно забавный баг, такой, я бы сказал, механический. В LLVM 7 setNewProgram() принимал просто указатель, и внутри просто присваивал его переменной-члену класса, при этом никак не меняя сам указатель, то есть использовал его наподобие shared_ptr. В LLVM 8 глобально перешли с голых указателей на умные, и это место как-то «механически» переделали через unique_ptr, а логика осталась вся старая :) Какой-то «баг массового механического переписывания».

[Livid]

Извините, но ЕМНИП по стандарту использование объекта после std::move небезопасно (кроме операций, не зависящих от состояния объекта). Потому что состояние объекта после перемещения стандартом определено только как "valid but unspecified", i.e. там может быть всё что угодно, и сказать, что именно там будет, не зная деталей реализации компилятора и стандартной библиотеки, нельзя. Так что такая "оптимизация" — непереносимый код. Сюрпризы теоретически могут быть даже при смене версии компилятора.

[mayorovp]

Поведение перемещения полностью задаётся кодом на С++, так что от компилятора оно не зависит. А вот от версии стандартной библиотеки — зависит.

[Livid]

Да, с компилятором это я загнул, как-то с утра пораньше у меня в голове "компилятор" и "стандартная библиотека" смешались.

[KanuTaH]

std::move() сам по себе ничего не делает. Это по сути всего лишь приведение типа для того, чтобы вызвался нужный метод (принимающий в качестве аргумента rvalue-ссылку). Если вы точно знаете, что делает вызываемый метод (например, он написан вами), то ничего "небезопасного" или "непереносимого" в этом нет.

[mayorovp]

Однако, использование объекта после перемещения все же повышает метрику wtf/мин, даже если не делает ничего небезопасного.

[KanuTaH]

Ну понятное дело, что типичный reuse перемещенного объекта должен выглядеть примерно так: прочли сериализованный объект из потока — проделали с ним какие-то дополнительные манипуляции — переместили, прочли в ту же переменную следующий объект — манипуляции — переместили, и так далее. Просто комментарий выше выглядит так, будто сам по себе std::move() что-то такое проделывает с объектом, после чего его состояние как-то меняется, причем зависимым от компилятора/stdlib образом, а это не так. Вот я товарища и поправил.

[Livid]

Ну понятно что не от самого std::move, а от конструктора перемещения или там присваивания с перемещением. Строгие формулировки мне с утра не удаются. Но что STL никаких гарантий касательно состояния объекта после перемещения не даёт кроме того что оно "valid" — факт. И сторонние библиотеки обычно не дают. А в своём коде конечно можно вообще семантику перемещения поменять как угодно, да вот только стоит ли?

[technic93]

Они же проверяют свой ког clang анализатором? Т.е это наглядно показывает что pvs находит больше или как минимум что то новое я правильно понял посыл статьи?)

[Andrey2008]

Да.

[Andrey2008]

Начались правки кода: bugs.llvm.org/show_bug.cgi?id=30996

[mayorovp]

Наверное, вот эта ссылка будет более понятной: https://bugs.llvm.org/showdependencytree.cgi?id=41655

[Occama]

Это весьма типовая ситуация, когда ошибка прячется в обработчике ошибок, так как их никто не тестирует.

Сильное утверждение =) Обработчики ошибок могут не тестироваться максимум в смоуке. Если при тестировании проверяется только корректное функционирование, то отдел тестирования не отрабатывает свою зарплату.

[tagir_valeev]

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

[datacompboy]

Что-то как-то мне всё меньше нравятся недавние единорожки… Сменили дизайнера? Что-то тут он как-то «Хопэ, парниша» напоминает позой :(

А вот кофейный расчудесен, не хватает его в календарике..!

«Подозрительный брейк» напоминает недавниее обсуждение тут с
switch (x) {
break; case Y: ...
break; case Z: ...
}

вероятно подобный код (сам по себе имхо хороший) был реформатирован, получилось уж как получилось :D

[Cerberuser]

Можно пояснить для не особо продвинутого кодера, а как оно должно работать?..

[datacompboy]

оно защищает от забытого бряка.
когда все кейсы пишутся в форме «break; case X:» потерять бряк становится тяжело.
в свежих плюсах это починили через [[fallthrough]] аннотацию.

[Alexufo]

помоему это отсылка к «так, блэт»

[Andrey2008]

Да.

[VasRedDoor]

По поводу возможных ошибок при исключении, llvm/clang компилируется с флагами fno-exceptions и fno-rtti

[vamireh]

Неопределённое поведение

FeaturesMap[Op] = FeaturesMap.size();

Здесь нет неопределённого поведения. Здесь неспецифицированный порядок вычисления.