Разработка → Как мы искали и нашли ошибку в Visual Studio C++

alexleo 16 июля в 18:03 18,2k
Это был чудесный летний день. За окном сияли тучки, нежными голосами пели вороны, на автомойке весело пачкали шампунем чью-то машину, за стеной тихо скрёбся перфоратор — в общем, идиллия.

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

Предыстория


Компания у нас существует относительно давно, и основной продукт уже старше некоторых сотрудников компании, так что древнего кода хватает. Тем не менее, мы стараемся держаться в современном русле, Modern C++ активно используется, поэтому около года назад основной проект был переведён на VC2015. Это был отдельный цирк с конями, бубнами, блэкджеком и валерьянкой. Вспомогательный код переводится по мере того, как появляется время и желание. В данном случае, я решил перевести на VC2015 один из таких вспомогательных проектов, который очень активно используется нашей техподдержкой.

Я был уверен, что уже знаю подводные камни такого перехода, и что задача займёт не более часа работы.

Внешне программа проста: она показывает список строк, взятых из разных таблиц базы данных. Когда пользователь выделяет какую-либо строку, заголовки столбцов списка меняются на названия столбцов в соответствующей таблице.

И тут я замечаю, что этого не происходит.

In Vivo


Проверяю несколько раз, воспроизводимость стопроцентная. Собираю проект с нуля, запускаю, проверяю. Нуль на массу.

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

Какой прекрасный денёк.

Функция обновления заголовков сама по себе довольно простая. Сначала она считает некий condition, а затем работает примерно вот такой код:
int flag = !application->settings.showsize; // showsize имеет тип BYTE
int first = columnData - flag;
int last = ALL_COLUMNS - flag;
if (condition)
{
    for (int i = first; i < last; i++)
    {
        listctrl.SetColumn(i, "Какой-то текст");
    }
}
else
{
    for (int i = first; i < last; i++)
    {
        listctrl.SetColumn(i, "Какой-то другой текст");
    }
}

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

Таким образом, при любом раскладе хоть что-нибудь, но должно было происходить. Более того, отладчик, добираясь до этого кода, просто перепрыгивает на конец функции. Открываю дизассемблер и вижу, что весь этот кусок кода (и ещё десяток строк перед ним) в листинге просто отсутствует. Разумеется, оптимизированный код иногда выглядит покруче любого спагетти- и даже валенки-кода. Но в данном случае нигде с самого начала функции нет даже намёка на какие-либо переходы куда-либо ещё. В листинге виден расчёт условия, и сразу после него — выход.

Я начинаю понимать, что ошибка гораздо более интересная, чем казалось в начале, и зову нашего спеца, который в компании занимается ситуациями вида «всякая неведомая хрень», и которого обедом не корми — дай найти какой-нибудь баг в винде. Иллюзий на тему «в компиляторах ошибок не бывает» мы не питаем, однако опыт подсказывает, что 99,99% «ошибок в компиляторах» сводятся к кривым рукам разработчиков программ.

In vitro


Когда ошибка возникает только в оптимизированном коде, это может означать, что мы где-то наткнулись на неизвестное нам UB, и первым кандидатом становится строка
int flag = !application->settings.showsize;
Очень быстро выясняется, что проблема действительно где-то тут, но «не все так однозначно». Стоило заменить выражение на константу, другую переменную, поставить тернарный оператор вместо отрицания или хотя бы поместить структуру на стек, как код волшебным образом появлялся в листинге.

За неимением лучших идей, вытаскиваем эту функцию в отдельный чистый проект и безжалостно выкидываем всё лишнее. Тут же выясняем, что шаманство со структурами и указателями можно заменить на обычный volatile:
#include <stdio.h>
int main()
{
    volatile int someVar = 1;
    const int indexOffset = someVar ? 0 : 1;    // Цикл выбрасывается
    // const int indexOffset = !someVar;        // Цикл выбрасывается
    // const int indexOffset = 0;               // Всё хорошо
    // const int indexOffset = 1;               // Всё хорошо
    // const int indexOffset = someVar;         // Всё хорошо
    // const int indexOffset = someVar + 1;     // Всё хорошо
    for (int i = 1 - indexOffset; i < 2 - indexOffset; ++i)
    {
        printf("Test passed\n");
    }
    return 1;
}

Здесь мы были немало удивлены, поскольку в оригинальном коде замена отрицания на тернарный оператор в строке
int flag = !application->settings.showsize;
возвращала кусок кода на место, а для volatile это уже не работало.

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

Расследование


Тут стоит отметить, что основная программа сейчас собирается в vs2015 Update 2, поскольку на программу, собранную в Update 3, внезапно стал ругаться антивирус, причём именно тот, который мы устанавливаем клиентам, и могло получиться… некрасиво. Однако у некоторых разработчиков, включая меня, установлен Update 3. Мы проверили на нескольких разных компьютерах и версиях VS, и получилось, что ошибка присутствует только в Update 3. Позднее окажется, что правильнее было написать «начиная с Update 3».

Гугл ясно дал понять, что он не при делах, так что следующим логичным шагом было написание
вопроса на StackOverflow. Надо сказать, отправить на StackOverflow вопрос, начинающийся с фразы «Мы тут ошибку в компиляторе нашли» — это всё равно, что порезать себе руку и прыгнуть в бассейн с акулами, но в данном случае акулы оказались сытыми и доброжелательными. Буквально через несколько минут тестовый пример ещё больше упростили, подсказали инструмент, который позволял посмотреть результат трансляции этого кода разными компиляторами, и, что ещё важнее, прозвучала волшебная фраза «new SSA optimizer introduced in VS2015 Update 3». Там же упоминался волшебный ключик -d2SSAOptimizer-, отключающий новый оптимизатор.

На этот раз гугление привело нас в блог Introducing a new, advanced Visual C++ code optimizer разработчика из Visual C++ Optimizer team с его координатами и предложением отправлять ему сообщения об ошибках, чем мы и воспользовались. И буквально через 10-15 минут получили следующий ответ:
Да, это определённо ошибка в самом SSA-оптимизаторе — обычно большинство ошибок, о которых нам сообщают, как об ошибках оптимизатора, находятся в других местах и иногда проявляются спустя 20 лет только сейчас.

Ошибка в небольшой оптимизации, которая пытается удалить сравнения вида (a — Const1) CMP (a — Const2), если не происходит переполнения. Ошибка возникает из-за того, что в вашем коде есть выражение (1 — indexOffset) CMP (2 — indexOffset), и, хоть вычитание, разумеется, некоммутативно, оптимизатор этого не учитывает и обрабатывает (1 — indexOffset), как будто это (indexOffset — 1).

Исправление для этой ошибки будет опубликовано в следующем большом обновлении для VS2017. До этого времени хорошим решением может стать отключение SSA-оптимизатора для этой функции, если это не вызовет сильного замедления. Это можно сделать с помощью #pragma optimize("", off): msdn.microsoft.com/en-us/library/chh3fb0k.aspx
Оригинал
Yes, this is indeed a bug in the SSA Optimizer itself — usually most bugs reported as being in the new optimizer are in other parts, sometimes exposed now after 20 years.

It's in a small opt. that tries to remove a comparison looking like (a — Const1) CMP (a — Const2), if there is no overflow. The issue is that your code has (1 — indexOffset) CMP (2 — indexOffset) and subtraction is not commutative, of course — but the optimizer code disregards that and handles (1 — indexOffset) as if it's (indexOffset — 1).

A fix for this issue will be released in the next larger update for VS2017. Until then, disabling the SSA Optimizer would be a decent workaround. Disabling optimizations for only this function may be a better approach if it doesn't slow down things too much. This can be done with #pragma optimize("", off): msdn.microsoft.com/en-us/library/chh3fb0k.aspx

Эпилог


Как показало расследование, на текущий момент этой ошибке подвержены все компиляторы VC++, начиная с версии 2015 Update 3 и заканчивая самыми современными версиями. Пока неизвестно, когда будет выпущено исправление, так что если вы обнаружите, что из вашей программы чудесным образом пропадают куски кода, проверьте, может быть новый оптимизатор решил, что этот код ему нужнее, чем вам?

Несколько разочаровывает, что исправление будет выпущено только для VS2017, тем не менее, теперь мы знаем, что с этим делать.

Какой прекрасный денёк!

Спасибо Codeguard за помощь в поиске этой ошибки.
Проголосовать:
+92
Сохранить: