Pull to refresh

Comments 7

У меня сложилось впечатление, что в Sphinx отсутствие проверок на ошибочные данные было/стало скорее фичёй. Это следует из оборачивания проверок в assert() — в случае реально возникающей проблемы (а не гипотетической), запускается дебажная версия, и баг фиксится, а в продакшене ресурсы процессора на бесполезные проверки не расходуются. Я, правда, столкнулся с ситуацией, когда дебажная версия всегда падает на определенном запросе к определенному набору данных, а обычная — работает. Да и не выставишь дебажную версию в продакшен — она не справляется с нагрузкой (к вопросу об оверхеде), а некоторые проблемы вылезают только там (видмо есть race condition). Еще можно отметить трудноуловимые глюки с тредами под FreeBSD, из-за чего пришлось перенести его на линукс, где эта скульптура из подпорок находится в более сбалансированном состоянии ;) Но в целом альтернатив Sphinx по производительности нет.
отсутствие проверок на ошибочные данные было/стало скорее фичёй

Нет, это не совсем так.


Концепция всегда была (и есть) следующая:


  • Если можно "почти бесплатно" не падать на кривом входе, следует не падать.
  • Если это дорого, делаем утилиты для проверок (это и assert, и indextool, и т.д.), но сохраняем производительность.

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


Заодно замечу, что единственный опубликованный тут "уникальный" пример "ошибки" вообще не про это. Там честное кажущееся разыменование NULL, никак не связано ни с какими оптимизациями. При этом, насколько я помню, фактически словить именно в этой точке NULL как бы невозможно, тк. а) наличие элемента в хеше проверяется ранее в функции, и б) по уставу с хешем в этот момент должен работать только текущий тред, вот ровно эта функция, внезапно стереть оттуда элемент некому. Но все эти предположения могли сломаться за годы мутации кода, конечно, и в идеале лучше подложить соломки, хотя бы и ассертом. Подложу, если этот код вообще останется в живых по итогам :-)

Интересно, а вот такой фрагмент:


int sphCollateLibcCS ( const BYTE...


            ...
            // big strings on heap
            char * pBuf1 = new char [ iLen ];
            char * pBuf2 = new char [ iLen ];

            memcpy ( pBuf1, pStr1, iLen );
            memcpy ( pBuf2, pStr2, iLen );
            pBuf1[iLen] = pBuf2[iLen] = '\0';
            ...

нашёлся? Тут то явный buffer overflow, правда в реальной жизни на него достаточно сложно "наступить" (ну, всего-то "байтик лишний" в конце записали...)

Красивая ошибка. Нет, пока такую взаимосвязь видеть ещё не научили. Но работаем и в этом направлении.
У нас явно разные понятия «красоты» — это не красивая в моём личном понимании — это скучный и тупой off-by-one классический.

Последнюю именно «красивую» ошибку даже и не вспомню навскидку. Первое и последнее, что моментально приходит в голову — в итоге оказалось вообще не в C++ коде совсем :)
Хм, очень странно и неожиданно что PVS еще не ловит такое.

В целом ощущение что Manticore один раз (когда-то давно) прогнали через Coverity, но пофиксили только часть, а Аксенову все еще лень(?) прикрутить такой CI через travis.org
Sign up to leave a comment.