Как мы искали и нашли ошибку в Visual Studio C++ +92

- такой же как Forbes, только лучше.

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

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

Предыстория


Компания у нас существует относительно давно, и основной продукт уже старше некоторых сотрудников компании, так что древнего кода хватает. Тем не менее, мы стараемся держаться в современном русле, 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 за помощь в поиске этой ошибки.


Комментарии (36):

  1. FlamyXD
    /#10314646

    У меня наблюдалось подобное при рекурсивной сортировке массивов.
    В дебаге работало, в релизе ломалось. VS 2017.

    • hdfan2
      /#10314686 / +7

      В подавляющем большинстве случаев причина такого поведения ("в дебаге работало, в релизе ломалось") — неинициализированная переменная.

      • FlamyXD
        /#10314752

        А как же компилятор? Он же сообщает о использовании неинициализированной переменной.
        (по моему мнению)

        • khim
          /#10314758 / +1

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

          • olegchir
            /#10314764

            поэтому переменные нужно инициализировать всегда :)

            • khim
              /#10314788 / +5

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

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

              • olegchir
                /#10314808 / +2

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

                • khim
                  /#10314832

                  в PROD может оказаться что лучше выделить фиксированную железяку под фиксированную нагрузку, и не заигрывать с переподпиской по аппаратным ресурсам
                  Если вам нужно именно «железяку» заточить, то можно выключить overcommit. К сожалению это только на уровне всей системы делается…

                  P.S. Время выделения памяти под 100-мегабайтный массив всё равно будет мгновенным, если вы его инициализировать не будете, и заметно не мгновенным, если будете…

              • DistortNeo
                /#10315800

                Но если инициализировать нулями, то такой проблемы нет, потому что можно просто ссылаться на заполненный нулями кусок памяти с copy-on-write.

                • khim
                  /#10316090

                  Так calloc делает, но это как раз надо просить специально. По умолчанию C++ «заказывает» участок памяти и явно прописывает его нулями. Не знаю ни одного компилятора, который бы делал по другому…

                  • DistortNeo
                    /#10316218 / +1

                    А вот C# делает именно так, как написал я. Выделите массив на гиг, потом начинаете его заполнять и видите, как постепенно растёт потребление памяти. А всё потому, что в .NET любое выделение памяти инициализируется нулями, а для value-типов нельзя переопределить конструктор по умолчанию.

                    • PsyHaSTe
                      /#10318142

                      Хотел тут у себя воспроизвести, сделать скрин и разоблачить, что дотнет под массив память сразу выделяет всю и с нулями! Начал проверять — и что-то как-то резко передумал это делать...

                      • PsyHaSTe
                        /#10318152

                        С другой стороны такое ощущение, что это винда молодец, а не дотнет. Дотнет коммитит сразу весь гигабайт, а потребление постепенно растет (Private bytes):


                        using System;
                        
                        namespace ConsoleApp17
                        {
                            class Program
                            {
                                static void Main(string[] args)
                                {
                                    int[] bytes = new int[int.MaxValue/8];
                                    Console.ReadKey();
                                    for (int i = 0; i < bytes.Length; i+=1000)
                                    {
                                        bytes[i] = i;
                                        Console.WriteLine(i);
                                    }
                                    Console.WriteLine(bytes);
                                }
                            }
                        }

                        image

                        • DistortNeo
                          /#10318170

                          > С другой стороны такое ощущение, что это винда молодец, а не дотнет. Дотнет коммитит сразу весь гигабайт, а потребление постепенно растет (Private bytes):

                          Просто .NET вызывает функцию Windows для выделения памяти с одновременным заполнением нулями, а не заполняет нулями сам.

                          • PsyHaSTe
                            /#10318204 / +1

                            Не понимаю, при чем тут .Net, если это стандартный сишный calloc. Не знаю ни одного примера ни одного рантайма, которое бы делалло malloc + ручную инициализацию вместо этого.

                            • khim
                              /#10318242

                              Это стандартное поведение calloc'а на многих системах, не только на Windows. А вот C++ — так не умеет, как ни удивительно. Может в каком-нибудь C++23 добавят…

                            • DistortNeo
                              /#10318288

                              > Не понимаю, при чем тут .Net, если это стандартный сишный calloc.

                              Просто приведён в качестве примера фреймворка, где это стандартный механизм выделения памяти в противовес выделению памяти в C++.

          • mayorovp
            /#10314778

            Существует неплохая эвристика, реализованная в языке C#, которая несмотря на свою простоту закрывает большинство случаев.

            • PsyHaSTe
              /#10318158

              В C# просто не решает проблему остановки, потому что анализ флоу производится локально. Он не позволяет передать неинициализированную переменную по ref например, чтобы её внутри кто-то проинициализировал. В большинстве случаев оно и не нужно, но собственно это единственный способ решать нерешаемые задачи — на уровне семантики обрубать случаи, когда анализ слишком усложняется, и реализовать тривиальную проверку "до первого использования идентификатора в качестве rvalue он используется в качестве lvalue"

      • Antervis
        /#10315100 / +1

        значение неинициализированной переменной — всего лишь частный случай UB. В gcc, например, была подобная проблема: компилятор мог выкинуть два подряд идущих условия if (condition) {...}, if (!condition) {...} если condition зависел от ub.

        Разве что тут, как я понял, никаким ub и не пахло

  2. zRrr
    /#10314648 / +3

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

    • zRrr
      /#10314660

      Хотя иногда не стоит так сильно давить на ручку.(да, я король ассоциаций)

  3. Ivan_83
    /#10314664 / +2

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

    Тем временем в LLVM: https://bugs.llvm.org//show_bug.cgi?id=27594 и оно уже давно в базе. И никто (кроме лени) не мешает иметь это приватным патчем сразу.
    Правда бывает и хуже: https://bugs.llvm.org//show_bug.cgi?id=27737 но тут оно хотя бы работало, пусть и медленно, и был воркароунд.

    Хуже когда железо глючит да ещё так что воспроизвести трудно.

    • dmitry_dvm
      /#10315074 / -2

      Большинство современных технологий мс уже в опенсорсе.

  4. Temtaime
    /#10314942

    Несколько раз натыкался на internal compiler error со студией, как 2015, так и 2017. Код слишком объёмный, чтобы пытаться уменьшить и найти причину.
    Обычно всё сводилось к какой-нибудь замене строки на аналогичную, но «немного другую».

  5. m08pvv
    /#10315140 / +1

    поскольку на программу, собранную в Update 3, внезапно стал ругаться антивирус, причём именно тот, который мы устанавливаем клиентам

    Вот так ругаться? (собирал CoreCLR)

    • MattHash
      /#10316028 / +1

      Как говорится,

      был бы код...

  6. lieff
    /#10315730

    Тоже находил баг в 2010 студии:

    #include <stdio.h>
    
    #define TRIGGER_BUG
    // Tested with:
    // cl.exe ver 16.00.40219.01 for 80x86
    // cl.exe ver 16.00.40219.01 for x64
    // compile: cl /O2 /Ob2 vc2010_bug.cpp
    // correct ouptut: 001001000
    // actual  ouptut: 001001100
    
    volatile int val = 36;
    
    static unsigned char *mkbits(unsigned int Inp, unsigned char *buf, unsigned int BitNum)
    {
        unsigned int i;
        for (i = 0; i < BitNum; i++)
        {
    #ifndef TRIGGER_BUG
            buf[i] = (Inp & (1 << i)) ? 1 : 0;
    #else
            buf[i] = (Inp & 1);
            Inp >>= 1;
    #endif
        }
        return buf + BitNum;
    }
    
    void Pack()
    {
        unsigned char buf[10];
        unsigned char *pbuf = buf;
    
        pbuf = mkbits(val, pbuf, 7);
        pbuf = mkbits(0, pbuf, 2);
    
        for (int i = 0; i < 9; i++)
            printf("%c", buf[i] ? '1' : '0');
        printf("\n");
    }
    
    int main(int argc, char** argv)
    {
        Pack();
        return 0;
    }
    

    Работает только на указанной версии, в сервиспаках исправлено.

  7. GenadiBabenko
    /#10316252 / -1

    Столкнулся с похожей проблемой во время перехода с gcc 4.4 на 4.8 и 4.9.
    Код, который уже много лет работал в продакшн и давно не менялся, вдруг начал выдавать ошибки. Отладочная версия, естественно, работала как надо. Отладка с помощью принтов не помогла, все условия в цикле выполнялись, но он почему-то не завершался. Пришлось смотреть на сгенерированный код. Компилятор вместо условного перехода поставил безусловный, вот и получился бесконечный цикл. В компиляторе 4.9 это удалось исправить с помощью флага no-agressive-loop-optimization, а в 4.8 и это не помогло. Я так понял, что виноват не компилятор, а кривой код, который компилятор трактует как код, который может вызвать undefined behavior. Вот ссылка на эту тему: http://en.cppreference.com/w/cpp/language/ub.

  8. jia3ep
    /#10316804

    Очень похоже на ошибку, которую мы обнаружили еще в VS2008 и она проявлялась во всех последующих версиях компилятора. На текущий момент она все еще в статусе Active.

  9. andreytata
    /#10318084 / -1

    Вся линейка компиляторов от Microsoft содержала баги. Всегда. Народ регулярно их находил. Лет 15 назад народ понял что править эти баги никто не собирается. Никогда. И молча ушел в сторону GCC. Но уже и там не всё благополучно. Новая мода: — Давайте модернизировать С++, каждый год! А почему не два раза в месяц? Дальше только хуже будет. IMHO.

    • sumanai
      /#10318130 / +4

      Вся линейка компиляторов от Microsoft содержала баги. Всегда.

      Все программы сложнее HelloWorld содержали, содержат и будут содержать баги.

      • khim
        /#10318246 / -1

        Это неправда. Есть программы, которые багов не содержат, а есть такие, где последние баги были найдены многие годы назад: qmail, TeX, etc.

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

        • PsyHaSTe
          /#10318686 / -1

          Баг — понятие растяжимое. По определению это поведение, не соответствующее спецификации. Но если мы сделаем спецификацию максимально неопределенную, то багов как бы и не будет. UB != баг.

    • DistortNeo
      /#10318176 / +3

      Все компиляторы содержат баги, это не прерогатива Microsoft.

      Первый раз баг я в Borland C++ Builder поймал, когда заполнял значениями многомерный массив типа float. В Debug всё работало корректно, а в Release компилятор генерил код, который в ячейку типа float почему-то писал double.

      Второй раз бодался с Intel C++ Compiler, который тупо падал при компиляции кода, богатого новейшими фишками C++.

  10. AlexDnp
    /#10320070

    Обнаружил подобную ошибку оптимизатора, отослал в Microsoft получил такой ответ:
    «VS 2015 is not going to have any more bug fixes released, the hotfix you tried was a last effort to release fixes for some of the more ugly bugs exposed mostly by the new SSA Optimizer. I advise to move to VS 2017, especially since it’s binary compatible with 2015. There will also be some big improvements to the optimizer this year in the following VS 2017 updates.»

    Правда, ошибка до сих пор воспроизводится и в VS 2017.