пятница, 21 апреля 2023 г.

[prog.c++.wow] clang++-16 внезапно удивил! Неприятно удивил

Пропробовал собрать SObjectizer clang-ом 16-ой версии. Получил неожиданную ошибку:

clang нашел "ошибку" в коде, которому уже лет семь, если не восемь:

auto e = begin(path) + static_cast< state_t::path_t::difference_type >(
   m_current_state_ptr->nested_level() ) + 1;

где path -- это std::array<const state_t *, max_deep>

Поставить адресную арифметику в C++ под запрет, пусть даже при использовании -Weverything и -Werror, это мощно. Внушаить.

Даже не знаю как к этому относиться.

Конкретно этот кусочек я переписал так:

const auto past_the_end = [&path, this]() {
      auto r = begin(path);
      std::advance( r, m_current_state_ptr->nested_level() + 1u );
      return r;
   }();

Можно было бы и двумя строчками обойтись, как-то вот так:

auto past_the_end = begin(path);
std::advance( past_the_end, m_current_state_ptr->nested_level() + 1u );

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

Но хуже всего то, что это оказалась только первая ласточка. Затем clang++-16 еще "подозрительные" места нашел. С которыми так же нужно будет что-то сделать.

А самое худшее, что это все вылезло при работе над новой версией SObjectizer. Но ведь и текущую версию 5.7, затем нужно будет под clang++-16 адаптировать...

Не было печали, что называется :(

И, что самое ироничное: новый вариант кода же не стал хоть сколько-нибудь безопаснее.

Так за что боролись, спрашивается?


Upd. Ну а вот от этого я вообще выпадаю в осадок (желающие могут сами проверить на godbolt):

#include <array>

int main() {
#if 1
    char buffer[3];
    buffer[1] = 0;
#else
    std::array<char, 3> buffer;
    buffer[1] = 0;
#endif
}

Вариант с Си-ными массивами, типа, небезопасный. Поэтому ошибка компиляции. А вариант с std::array, надо полагать, безопасный-безопасный. Ага.

Ну ахринеть. Ну уж теперь-то заживем.

7 комментариев:

Pavel комментирует...

Интересно, спасибо за информацию. Это только пока с -Weverything срабатывает?

Судя по нагугленному "RFC: C++ Buffer Hardening" https://discourse.llvm.org/t/rfc-c-buffer-hardening/65734#c-safe-buffers-3
это добавили, чтобы использовать проверки границ в библиотечном `std::array::operator[]`, которые невозможно форсировать в сишных массивах.

У вас наверно предупреждение выстрелило из-за того, что результат `size_t nested_level()` может вернуть значение большее max_deep, так что теоретически возможен выход за границы. А библиотечный `std::advance()` может проверять границы в runtime, так что стало "безопаснее"

eao197 комментирует...

@Pavel

Насколько я понял, только с Weverything.

> чтобы использовать проверки границ в библиотечном `std::array::operator[]`, которые невозможно форсировать в сишных массивах

Если operator[] для array начнет в release проверять индексы, то это может многих неприятно удивить. А если это только в debug делается, то для меня лично в этом нет никакой выгоды.

Ну и я не понял, как разрабы clang-а предлагают обрабатывать аргументы командной строки если у main прототип выглядит как int main(int, const char **), т.е. от работы с голыми указателями никуда не денешься.

Pavel комментирует...

Если это предупреждение включается только с -Weverything, то тогда дальше жить пока можно.

Оно экспериментальное и по большому счету, -Weverything нужно включать только разработчикам Clang, о чем они предупреждают.

Since -Weverything enables every diagnostic, we generally don’t recommend using it. -Wall -Wextra are a better choice for most projects. Using -Weverything means that updating your compiler is more difficult because you’re exposed to experimental diagnostics which might be of lower quality than the default ones. If you do use -Weverything then we advise that you address all new compiler diagnostics as they get added to Clang, either by fixing everything they find or explicitly disabling that diagnostic with its corresponding Wno- option.

eao197 комментирует...

@Pavel

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

Вот на что я не отважился, так это на включение -W4 в MSVC++, т.к. там какие-то такие замороченные и непонятные для меня предупреждения выдавались, что я просто сдался и отказался от попыток с ними разобраться.

Pavel комментирует...

Я бы не тратил силы на -Weverything, а те кто включает его явно - сам себе злобные буратины. Лучшие C++-веды предупреждают об этом: https://quuxplusone.github.io/blog/2018/12/06/dont-use-weverything/

Насколько я помню, c -W4 MSVC++ плевался кучей предупреждений даже из SDK заголовков, типа windows.h но они легко изолируются внутри pragma warning push/pop блока.

На что недавно натолкнулся - сгенерированный гугловским protoc код для протобуфов и GRPC-сервисов выдает предупреждения при включенных -Wpedantic -Wconversion -Wsign-conversion флагах, которые вроде-бы must have для современных проектов. Но ребята в гугле изначально имеют свое видение C++, так что я не сильно удивился.

eao197 комментирует...

@Pavel

Я уже не вспомню когда для clang-а стал использовать -Weverything и почему именно его. Но вроде как в этом режиме выдавались какие-то полезные предупреждения, которых не было с -Wextra, поэтому и оставил. Ну а для подавления тех предупреждений, которые не несут пользы применяются либо pragma в коде, либо ключики -Wno- для компилятора.

> Насколько я помню, c -W4 MSVC++ плевался кучей предупреждений

Да, и я задолбался с этим бороться :)

Pavel комментирует...

Угу, -Wall и -Wextra не включают некоторые реально полезные предупреждения.

Я на нынешнем проекте использую набор из "-Werror -Wall -Wextra -pedantic -pedantic-errors -Wconversion -Wsign-conversion" и для GCC, и для Clang. Однако в чем различие между -pedantic и -pedantic-errors уже тоже не помню :)

В целом, этот набор ловит иногда опечатки и тупые ошибки, но -Wsign-conversion иногда подбешивает из-за необходимости приведения, например, из difference_type в size_t (Зачем std::count возвращает diiference_type для меня загадка)