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

На практике далеко не всегда предупреждения компилятора одинаково полезны. Зачастую они не помогают разработчикам, а мешают им и могут провоцировать на исправление совершенно правильного кода, т.е. на нарушение правила «работает — не трогай».

Для примера будет история об одной строке кода из довольно популярной библиотеки с открытым кодом для работы с XML. Контекст этой строки такой:

    fseek( fp, 0, SEEK_END );
    const long filelength = ftell( fp );
    fseek( fp, 0, SEEK_SET );
    if( filelength == -1L ) {
        return FILE_READ_ERROR;
    }
    if( filelength >= (size_t)-1 ) {// ВОТ ЭТА СТРОКА
        return FILE_READ_ERROR;
    }
    const size_t size = filelength;
    _charBuffer = new char[size+1];
    size_t read = fread( _charBuffer, 1, size, fp );
    if( read != size ) {
        return FILE_READ_ERROR;
    }

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

Выполняется (по сути, а в коде стоит противоположная и возврат кода ошибки) проверка, что filelength строго меньше, чем максимальное значение size_t. Проверка выполняется на «строго меньше», потому что нужно выделить памяти на один элемент больше, чтобы записать завершающий нулевой символ перед дальнейшим разбором содержимого файла.

Все бы хорошо, но компилятор негодуэ — ведь с одной стороны сравнения знаковый тип long, а с другой стороны — беззнаковый size_t. ПРЕДУПРЕЖДЕНИЕ -Wsign-compare от gcc!!!

Что же делать, куда бежать??? Это же предупреждение!!!

Один из разработчиков исправляет проверку:

    if( (size_t)filelength >= (size_t)-1 ) {

… и компилятор УЗБАГОИЛСЯ доволен. Успех? Увы, изначально правильный код теперь работает уже не так, как было задумано. Размеры long и size_t не связаны друг с другом. Размер любого из этих типов может быть больше размера второго на конкретной платформе. Первоначальный код требовал в таком случае преобразования обоих значений к одному и тому же типу — достаточного для обоих размера. В «исправленном» коде произойдет потеря части значащих разрядов в случае, если размер long больше размера size_t.

Так для избавления от предупреждения в изначально правильном коде этот код исправили и сломали.

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

     if( (unsigned long)filelength >= (size_t)-1 ) {

В исправленном коде сравниваемые значения оба беззнаковых типов, поэтому ужасная проблема(тм), о которой gcc сообщал предупреждением -Wsign-compare, здесь отсутствует. Приведение long к unsigned long здесь не приводит к неверной интерпретации данных, потому что единственно возможное отрицательное значение обработано ранее.

Успех? Нет, не так быстро.

Правку вливают в основную ветвь, после чего начинают поступать сообщения о новом предупреждении. При компиляции под платформы, где размер unsigned long меньше размера size_t, clang выдает предупреждение -Wtautological-constant-out-of-range-compare «ПРЕДУПРЕЖДЕНИЕ!!! Диапазон значений unsigned long так уныл и безнадежно ограничен, что сравнение всегда дает один и тот же результат и не имеет решительно никакого смысла».

Полностью переносимый код для любой платформы, говорите? Нет, когда явное приведение long к size_t могло повлечь потерю разрядов, это всех устраивало. А теперь clang выдает бесполезное предупреждение и пользователи библиотеки оставляют комментарии к правке и сообщения о дефектах. Это же предупреждение!!!

Что же делать… ведь нужно сделать так, чтобы на платформах, где размер unsigned long меньше размера size_t, сравнения не было, а на остальных платформах — было.

А, вот решение «проблемы»:

template <bool = (sizeof(long) >= sizeof(size_t))>
struct LongFitsIntoSizeTMinusOne {
    static bool Fits( long value )
    {
        return (unsigned long)value < (size_t)-1;
    }
};
template <> bool LongFitsIntoSizeTMinusOne<false>::Fits( long /*value*/ )
{
    return true;
}

— Что это, Бэрримор???
— Это шаблон Баскервилей, сэр!!!

Здесь можно было бы применить шаблон функции, но значения параметров шаблона по умолчанию для шаблонов функций поддерживаются только начиная с C++11, а библиотека — чтобы расширить аудиторию — старается без него обходиться. struct вместо class используется, чтобы не указывать видимость public явно.

Применяется так:

    if ( !LongFitsIntoSizeTMinusOne<>::Fits( filelength ) ) {

В зависимости от соотношения между размерами long и size_t компилятор выбирает либо специализацию, либо реализацию по умочанию. В первом случае бессмысленной проверки не будет, и компилятору не о чем будет ПРЕДУПРЕЖДАТЬ.

В основную ветвь вливают проверку с использованием этого шаблона — и «проблема» решена.

Краткое содержание предыдущего текста… Совершенно правильный код с одним сравнением целочисленной переменной и целочисленной константы правили трижды, чтобы сделать счастливыми два очень популярных компилятора. Первая же из правок этот совершенно правильный код сломала, зато сделала счастливым компилятор, ради которого вносили правку. В итоге вместо простого сравнения получили что-то ближе к FizzBuzz Enterprise. Зато нет предупреждений.

Теперь давайте порассуждаем…

Насколько популярным должен быть сферический кусок открытого кода в вакууме, чтобы он имел шансы пройти весь путь от первой правки до последней, и насколько более вероятно, что менее популярный код так и остался бы сломанным первой правкой? Понадобилось в общей сложности явно больше одного часа работы разработчиков с не самой низкой квалификацией, чтобы найти показанное выше решение, которое даже не исправляет ошибку, а превращает просто правильный код в правильный код без предупреждений.

Почему так произошло? Причин ровно две.

Первая причина — разработчики почти безоговорочно доверяют компилятору. Рядом с такой сложной программой многие разработчики ощущают себя как рядом со 107-метровым монументом «Покорителям космоса» на проспекте Мира в Москве. Современный компилятор — это очень сложная программа, и многие считают, что она не может ошибаться. Увы, это мнение неверно — чем программа сложнее, тем больше у нее возможностей ошибиться. Компилятор может выдать предупреждение на совершенно правильный код.

Вторая причина — разработчики статических анализаторов (особенно входящих в состав компиляторов) обычно считают, что ложное срабатывание — это не проблема и ничего страшного, главное ПРЕДУПРЕДИТЬ, а «разработчик должен разобраться». Разберется непременно, но далеко не факт, что в результате код станет лучше.

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

Насколько вдумчивый анализ нужен в приведенном примере? Проверяемое значение получено от выполнения функции ftell(), ее поведение задокументировано. gcc, например, уже умеет оптимизировать — и «доламывать» не соответствующий Стандарту — код, используя требования Стандарта передавать в «строковые функции» — например, memcpy() — только ненулевые указатели, подробности есть в этой публикации. Как он это делает? Разработчики gcc добавили в него такую возможность — опознавать ряд функций как «строковые» и делать некоторые предположения о значениях передаваемых в эти функции указателей.

Точно так же разработчики компилятора могут добавить в него возможность использовать данные о поведении других функций стандартной библиотеки — вряд ли тривиально, но определенно выполнимо. Сторонние статические анализаторы также «знают» о функциях стандартной библиотеки и популярных сред, это позволяет им сообщать, например, о возможной логической ошибке в приведенном выше коде в случае, когда результат ftell() не проверяется на сообщающее об ошибке значение -1L.

Техническая возможность для значительных улучшений есть. Это вопрос приоритетов и отношения к проблеме.

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

Что на выходе? Многие средства статического анализа пока работают по принципам «главное предупредить» и «разработчик должен», этим они провоцируют разработчиков вносить в код ненужные правки, попутно иногда ломать работающий код. Можно, конечно, объявить этих разработчиков недостойными и криворукими, но не стоит забывать, что от их работы зависит судьба миллионов строк кода, используемых каждый день сотнями миллионов человек.

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

Каждый раз как средство статического анализа выдает предупреждение на правильный код, где-то грустит один котик(тм) и, возможно, в совершенно правильный код вносится ненужная правка, провоцирующая очередную ошибку с последствиями как у Heartbleed.

Дмитрий Мещеряков,
департамент продуктов для разработчиков