?

Log in

No account? Create an account
баг (программистское) - По делам сюда приплыл, а не за этим — ЖЖ [entries|archive|friends|userinfo]
Anatoly Vorobey

[ website | Website ]
[ userinfo | livejournal userinfo ]
[ archive | journal archive ]

Links
[Links:| English-language weblog ]

баг (программистское) [май. 3, 2018|02:39 pm]
Anatoly Vorobey
[Tags|]

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

Missing red channel in Chrome Remote Desktop

Если вы разбираетесь в C++, почитайте сами, а если нет, то вот краткий пересказ:

1. В Хроме, когда он используется для удаленного захода на другой компьютер (Chrome Remote Desktop), внезапно пропал красный цвет, в том числе из комбинаций цветов (так что белый фон стал сине-зеленым, например).

2. Идем по истории изменений, находим, что это произошло в изменении, которое не меняло исходники, а вместо этого перешло к новой версии компилятора ("roll clang"). Вечер перестает быть томным.

3. Ищем и находим способ "дешево" воссоздать проблему для данного набора исходников (в данном случае - как сделать "удаленный" вход внутри одного компьютера).

4. Берем исходники в этот момент, строим две версии компилятора, старую и новую. Компилируем все дерево исходников старой - бага нет, новой - баг есть. Компилируем все дерево старой, и начинаем искать, какие исходники приводят к проблеме, если только их построить новой.

4. Постепенно сужаемся до большой компоненты дерева...

5. ...до конкретной директории...

6. ...до нескольких файлов...

7. ...до одного файла...

8. Находим массивы констант, которые используются для перевода цветов из режима YUV в режим RGB. Массивы определены внутри функции и внутри блока кода, но не помечены static. При этом используются уже после выхода из этого блока кода, через указатель, естественно.

9. Помечаем массивы static, проблема решена. Проблема была вызвана тем, что новая версия компилятора более агрессивно использовала локальное место на стеке, когда она знала, что ей позволено (потому что блок кода завершился и эти массивы должны использоваться только в нем).

И так каждый день! (примечание: не всегда так цветисто).
СсылкаОтветить

Comments:
[User Picture]From: pargentum
2018-05-03 12:16 pm

Вообще-то статья расстрельная

А на неделе хозяйка велела вернуть буфер из служебной функции, а я объявил локальный массив внутри функции и на него указатель вернул. А она взяла стэк с мусором и начала моей харей по ему водить...
(Ответить) (Thread)
[User Picture]From: avva
2018-05-03 03:21 pm

Re: Вообще-то статья расстрельная

Смешно, но это не тот случай.
(Ответить) (Parent) (Thread)
[User Picture]From: pargentum
2018-05-03 03:27 pm

Re: Вообще-то статья расстрельная

Почему не тот? У обычной висячей ссылки тоже содержимое далеко не всегда сразу перезаписывается, поэтому их и тяжело обнаруживать. И это тоже может зависеть от настроек оптимизатора.
(Ответить) (Parent) (Thread)
[User Picture]From: avva
2018-05-03 03:47 pm

Re: Вообще-то статья расстрельная

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

Тут, во-первых, многие не знают, что компилятор может решить реюзнуть эту память, а из-за того, что это был массив констант-литералов, подозреваю, автор мог еще даже и не сообразить, что оно вообще может быть на стеке.
(Ответить) (Parent) (Thread)
[User Picture]From: scaredy_cat_333
2018-05-04 04:59 am
1. Судя по тому, что в переписке нет жалоб на пропуск бага средствами статического анализа - средства статического анализа не применяются. Это удивительно для сложного промышленного софта.

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

Remoting folks: if this code had any coverage, asan should've told us about the use after scope, with a nice stack and all. And ideally the test would've tested some color roundtripping, and that would've caught the problem before the compiler roll. Do you want to use this bug for adding test coverage for this code, or do you want to use a new bug for that?

И замечанию, и сарказму в нем - аплодирую.
(Ответить) (Parent) (Thread)
[User Picture]From: netp_npokon
2018-05-04 07:25 pm
1. У систем статического анализа слишком велик процент ложных срабатываний. К сожалению, это очень большая проблема для проектов масштаба хрома: дерево будет постоянно закрываться, разработчики будут ругаться и в конце концов перестанут обращать внимание на ошибки.
(Ответить) (Parent) (Thread)
[User Picture]From: scaredy_cat_333
2018-05-05 12:38 am
>1. У систем статического анализа слишком велик процент ложных срабатываний.

Велик, факт.

>К сожалению, это очень большая проблема для проектов масштаба хрома:

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

>дерево будет постоянно закрываться, разработчики будут ругаться и в конце концов перестанут обращать внимание на ошибки.

А? На срабатывание открывается тикет, если анализ показывает, что оно ложное - в конфигурацию чекера вставляется команда заткнуть впредь такой-то стек. Не ложное срабатывание должно быть исправлено перед чекином в мастер. Будут ругаться и перестанут - это с какой-то другой планеты.

3. Кусок кода, приведённый кем-то в комментариях производит впечатление наскоро сляпанного, а потом так и оставленного, прототипа. Такое ловится на ура ещё на code review. Или у них и code review нету? Как это все летает-то тогда.
(Ответить) (Parent) (Thread)
From: (Anonymous)
2018-05-06 06:32 am
В хроме двадцать миллионов строк кода, в вашем - миллиард?

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

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

По такому принципу был устроен valgrind workflow в хроме (проекте на два порядка меньше вашего). Этот ад был побежден ценой больших усилий и потребовал, в частности, замены валгринда на более робастные инструменты, которые могут сразу закрывать дерево.

> Не ложное срабатывание должно быть исправлено перед чекином в мастер.

В свете всего вышесказанного - good luck setting up a precommit bot.
(Ответить) (Parent) (Thread)
From: (Anonymous)
2018-05-06 05:44 pm
> Не ложное срабатывание должно быть исправлено перед чекином в мастер

Ложное тоже, подавлением стека или еще как, иначе смысла никакого нет.
(Ответить) (Parent) (Thread)