Pull to refresh

Comments 66

Предлагаю развлечение: исправляем все найденные ошибки и проверяем, а работает ли вообще программа после их исправления.
Тот, кто сможет отвечать (про абстрактную программу) на вопрос: «А работает ли она вообще», получит Нобелевскую премию сразу же.
UFO just landed and posted this here
Это верно для бесконечных (открытых) систем. Если вычислительная система конечна (имеет фиксированный объем памяти) и не взаимодействует с внешним миром иначе чем через начальную передачу агрументов и конечное получение результата (нет ввода/вывода), то очевидно, на более мощной системе можно сгенерировать все ее возможные состояния (образы оперативной памяти) и построить полный граф переходов между ними (для каждого состояния делаем один шаг в эмуляторе, получаем новое состояние и ищем его в массиве всех состояний). Для простоты, совокупность всех бит оперативки можно считать номером состояния вычислительной системы.
Ну а дальше очевидно: какое-то состояние мы принимаем за «конечное» (алгоритм закончил работу). Дальше достаточно проанализировать граф — если из начального состояния цепочка переходов ведет в одно из пройденных состояний — там бесконечный цикл. Поскольку число состояний конечно, то определить это можно за конечное время. Если же мы приходим в конечное состояние — алгоритм останавливается и дает результат.
UFO just landed and posted this here
Дополню, надо не только писать, применяя особые принципы, но и железо надо будет менять. Причина тому — очень большой разрыв между логикой железа и логикой программы, из этой же области баги с «плавающим» результатом вычислений при пересборке, а то и при перезапуске программы, и т.п. радости. На самом деле современные языки программирования, и современное далеки от оптимальных реализаций.
Проблема останова неразрешима для машин с потенциально неограниченным объемом памяти, который нужен для хранения неизвестно каких больших чисел, возникающих в процессе расчетов.

Какую-нибудь минимальную машину с маленьким объемом памяти можно смоделировать и проанализировать уже на существующих мощностях.
Только это никому не интересно. Интересны не ограниченные маленькие машины (+алгоритмы), останавливающиеся при переполнении памяти, а большие машины. Полезны машины в той мере, в какой они не вносят доп. ограничений в полезные алгоритмы на них исполняемые. Маленький объем памяти = маленькая полезность.
эм. неразрешима задача о завершении _любого_ алгоритма.
а вот для конкретных — вполне себе даже практическое направление. я им даже занимался в дипломные времена :) вычислительных способностей современных систем, для этого, пока, явно недостаточно, но с точки зрения математики проблем почти нет.
ru.wikipedia.org/wiki/Логика_Хоара
Предупреждение PVS-Studio: V519 The 'maxint' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 154, 157. taucs_scilab.c 157

А зачем было определять maxint в коде? Чем им INT_MAX не угодил?
Ну и если уж хочется вычислить, то может лучше maxint = (unsigned int)-1 использовать.

Всё бы хорошо, если бы не оператор 'break'. Будет заменён только один пробел. Впрочем, если убрать 'break' это не поможет. Функция strlen() вернёт ноль, и цикл всё равно остановится.

Насколько я помню strlen() вычисляется один раз перед запуском цикла и без break цикл прокрутится strlen(line) раз.
> Насколько я помню strlen() вычисляется один раз перед запуском цикла и без break цикл прокрутится strlen(line) раз.

Нет. Это фантазии. Язык Си/Си++ работает не так.

P.S. Кажется я нашёл одного человека, после которого в коде можно встретить такое :)

static void tell_str(FILE * stream, char *str)
{
    unsigned int i;
    for (i = 0; i < strlen(str); ++i)
      tell_char(stream, str[i]);
}

PVS-Studio: V814 Decreased performance. Calls to the 'strlen' function have being made multiple times when a condition for the loop's continuation was calculated. pscp.c 82
Здесь конечно не критично, но зато коротко и показательно. А вообще это очень частый ляп, который иногда может существенно снизить производительность на пустом месте при обработке большого текста.
И этим человеком был Альберт Эйнштейн
Ну, строго говоря, вы тоже не совсем правду написали. strlen(line) не вернет 0 в общем случае, даже после замены первого попавшегося пробела на \0, но цикл, тем не менее, прервется :)
далеко не всегда INT_MAX определен, и полагаться на это неправильно,
но вот что гораздо более неправильно — полагаться на то, что отрицательные числа представлены в виде дополнительного кода.

а про strlen — уж совсем смешно. вычисляется не strlen. вычисляется условие. условие продолжения цикла. и strlen — лишь часть выражения в этим условии. так с чего-бы в выражении «i<(int)strlen(line)» правую часть не перевычислять, а левую перевычислять?
С того, что правая часть может быть инвариантом цикла — не изменяться во время его исполнения. Достаточно сообразительный компилятор™ может вычислить её только один раз.

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

Если бы там было:
static void tell_str2(FILE *stream, char *str, float value)
{
    for (size_t i = 0; i < 300 * sin(value); i++)
    {
        tell_char(stream, str[i]);
    }
}
то gcc, к примеру, вполне успешно вычисляет условие правую часть условия только один раз в самом начале.
>>Доказывать неизменность памяти в общем случае в сишечке сложно, поэтому компилятор включает режим параноика и вызвает strlen() каждый раз заново.

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

interrupt_has_occured = 0;
while(!interrupt_has_occured);

дык вот пока ты не расскажешь gcc, что эта переменная-то у тебя volatile, он будет включать режим «недопараноика», и делать бесконечный цикл.
Потому что цикл пустой, и gcc успешно доказывает сам себе, что переменная не меняется. Поставь внутрь вызов любой внешней функции — и результат совсем другой.
эм. а зачем? вот не нужна мне тут внешняя функция. я просто жду когда прерывание придет.
gcc обмануть?
дык для того специальное слово есть — volatile
Я просто поясняю, кто, что и кому доказывает.

gcc пытается доказать, что все в программе неизменяемое и константное. Когда получается — применяет оптимизации.

Вам же ничего доказывать не надо. volatile — это не доказательство, а команда «не трож!».
то, что он пытается доказать — это хорошо.
то, что он, на основе этих доказательств, пытается сделать код короче/быстрее — это просто превосходно.
но вот когда я, ваяя свой код, должен задумываться не о своих алгоритмах, а о том, что взбредет в голову gcc — это, простите, п… ц.
А не надо задумываться об этом. Надо просто запомнить, где и когда ставится volatile.
Нет, вы должны задумываться над тем, почему в вашем коде значение переменной магическим образом изменяется где-то в другом месте, что совершенно нельзя понять только по приведённым двум строчкам. gcc это тоже непонятно. Он не осознаёт названия переменных.
магическим???
вроде создатели gcc что-то слышали о прерываниях. и даже включили термин «interrupt» в свой язык.

ну да бог с ним. нонешнее поколение не очень любит микроконтроллеры и программирование драйверов.

но есть всеми любимая многопоточность. неужели и тут мне придется рассказывать gcc, что глобальную переменную может поменять не только он сам?
Локальная переменная точно будет оптимизирована, что касается глобальный — режим параноика как правило касается указателей. В общем случае, указатели даже могут пересекать одну и ту же область памяти и компилятор вынужден предполагает наихудшие сценарии. А вообще см. коммент ниже — я давно не видел хороших статей про оптимизирующие компиляторы — всё анализаторы да анализаторы… Понятно, что все пишут плохой код, вопрос в том как писать хороший:)
Вот есть у тебя глобальная переменная interrupt_has_occured.
ну и цикл:

interrupt_has_occured = 0;
while(!interrupt_has_occured);


Ну так для глобальной переменной, которую могут менять из разных потоков естественно надо ставить volatile, вот только тут сравнение не совсем корректное, ибо тут нет вызова функции, и по логике компилятора, который без volatile положит interrupt_has_occured в кеш процессора, т.е. сделает всё верно, и тем самым на порядки оптимизирует скорость доступа к этой переменной, это всего лишь локальный цикл… а если компилятор совсем умный, и видит, что interrupt_has_occured в цикле не меняется он вообще сделает бесконечный цикл и проверку выкинет совсем :) Ну т.е. тут всё правильно, ведь C++ довольно низкоуровневый язык, и программисту, самостоятельно пишущему, конструкции синхронизации потоков не знать про volatile нельзя.

p.s: ну и представьте себе какую работу должен выполнить компилятор, что бы узнать где эта переменная реально используется, что бы понять что она глобальная, код годами компилироваться будет, ибо компилятору придётся полностью эмулировать работу приложения, т.е. просчитать все варианты использования кода.
Напрашивается хороший пост про оптимизацию в компиляторах. Те же подводные камни с оптимизацией битовыми сдвигами и т.п.
if ( strncmp(var->fexternal, "cintf", litlen("cintf"))==0 )
Так лучше-то не стало. Теперь надо заново вводить строчку, в которой тоже можно запросто опечататься. Надо тогда и на это макрос навесить (а-ля litcmp)
Можно написать шаблонную функцию-прослойку, которая будет знать, какой длины строка ей передано. По аналогии с:
template <size_t size>
int swprintf(
   wchar_t (&buffer)[size],
   const wchar_t *format [,
   argument]...
);
Не нужно усложнять код только ради одной несчастной strcmp. Любой нормальный компилятор умеет вычислять strlen от константы.
Проблема не в компиляторе, а в дублировании кода. Кто поручится, что через пять лет при очередной проверке кода очередной статический анализатор не заметит следующее:
if ( strncmp(var->fexternal, "cintfx", litlen("cintf"))==0 )
А для этого вообще и придумали strcmp, функция безопасная, тем более для «внутренних данных». Я понимаю ещё если строка приходила из враждебной среды да без проверок. Тут просто явно ставилась задача сравнить первые несколько символов. Для наглядности можно даже так написать:

strncmp(var->fexternal, "cintfx", strlen("xxxx"))


Всегда приятно видеть код который говорит за себя. Например:
char date[sizeof("DD.MM.YY")];
Вообще-то стало. Операция «скопипастить строковый литерал» намного проще операции «подсчитать число символов в строковом литерале и записать его».

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

Но в целом согласен — можно сделать еще лучше.
Гораздо понятнее вообще без длины, двух литералов и странных макросов:
if (strcmp(var->fexternal, "cintf") == 0)
Это другое. Скорее всего, они хотят проверить, что строка начинается с этих символов, а не вся строка равна.
Ну сделали бы тогда
static inline
bool strprefix(const char *str, const char *prefix)
{
    return (strncmp(str, prefix, strlen(prefix)) == 0);
}
Работает для всего, проверяет типы, избавляет от «== 0», делает волосы шелковистыми.
Для литералов успешно инлайнится и упрощается до четырёх ассемблерных инструкций.
Для того, чтобы такое чудо сотворить им бы машина времени понадобилась. Двадцать лет назад strlen был гораздо менее шелковистым.
Расширения кругозора для, что такого ужасного было в strlen 20 лет назад?
Отсутствие оптимизаций. То есть были кой-какие оптимизирующие компиляторы, но вычисление strlen'а во время компиляции — это было скорее чем-то, о чём вы могли прочитать в статье о том какие классные компиляторы на CRAY используются, чем что-то с чем вы могли столкнуться в реальной жизни.

Первые более-менее прилично оптимизирующие компиляторы C/C++ появились примерно лет 10 назад. До этого всё, что было рассчитано на то, что компилятор что-то где-то там как-то подставить/раскроет (например STL) приводило к жутким тормозам.
Спасибо. То есть strlen, как таковая, с тех пор никаким гламуром не обзавелась. А то я, по скудоумию, неверно связал ваш комментарий с контекстом предыдущего, и воображение стало рисовать всякие замысловатые картины происходящего в те далекие времена :)
Нет, не стало. «Связанность» этих двух строк всё равно приходится поддерживать программисту, дублируя одну и ту же информацию. И корень проблемы именно в этом, а не в том, заметна ошибка или нет / сложно её допустить или нет.

Да и два одинаковых строковых литерала куда заметнее
Вы правда думаете, что на код кто-то смотрит? Немало ошибок, описаных в статье, «заметны для человека» (см., например, первый пример в блоке «Неправильные сообщения об ошибках»), но тем не менее, мы их наблюдаем. Не стоит надеяться, что раз что-то требует лишь пары нажатий Ctrl+C и Ctrl+V, то оно будет сделано.
Предложите свой вариант.
Смотря для чего.
Как уже выяснили дальше в комментариях, вполне возможно, что исходный код
if ( strncmp(var->fexternal, "cintf", 4)==0 )
так и задуман. Тогда он выглядит легитимно. Можно заменить литерал на константу и тогда уже использовать litlen.
Про свой вариант я писал ещё в самом начале ветки: поверх предложенного в статье подхода навесить ещё один макрос вида
#define litcmp(a, b) strncmp(a, b, litlen(b))
Используется магическое число 4. И это число неправильное

У них есть ещё вот такой код:
if ( strncmp(var->fexternal,"cintf",4)==0 ||
     strncmp(var->fexternal,"cboolf",5)==0 ||
     strncmp(var->fexternal,"cdoublef",7)==0 ||
     strncmp(var->fexternal,"cfloatf",6)==0 ||
     strncmp(var->fexternal,"ccharf",5)==0)
{
    ...
}


Они везде проверяют совпадение n-1 символов. Либо так и задумано, либо 'f' добавилось позднее, а магические числа поправить забыли.
А дальше идёт вот такой код, где проверяется полное совпадение строк:

if ( strcmp(var->fexternal,"cintf")==0 ||
     strcmp(var->fexternal,"cboolf")==0 ||
     strcmp(var->fexternal,"cdoublef")==0 ||
     strcmp(var->fexternal,"cfloatf")==0 ||
     strcmp(var->fexternal,"ccharf")==0 )


Поэтому, возможно, что ошибки нет. Ну, разве что Magic number. :)
Заплати 19$ и можешь месяц читать статью с результатами анализа?
Они, кстати, Coverity Scan используют на регулярной основе. У них было 1966 багов по состоянию на 27 сентября 2013 года, теперь исправлено 1029, осталось 937.
Хорошие новости (для PVS-Studio). Кто там хотел сравнение PVS-Studio и Coverity?
У них почему-то забагован именно код для работы и связывания с java/jvm. В остальных модулях ситуация получше.

P.S.Секретом ли является язык программирования, на котором написан PVS Studio?
Сам анализатор — Си++.
Плагин к Visual Studio — C#.
Исходный код пестрит велосипедами. Может быть у них на Си пишут студенты?

Хотя ваш анализатор не проверяет утечки памяти, прочувствуйте гордость за своего подопечного:
Скрытый текст
contrib/toolbox_skeleton/sci_gateway/c/sci_foo.c:116: error: Memory leak: newMatrixOfBoolean
modules/api_scilab/tests/unit_tests/integer_reading_api.c:212: error: Memory leak: psDataOut
modules/api_scilab/tests/unit_tests/integer_writing_api.c:212: error: Memory leak: psDataOut
modules/api_scilab/tests/unit_tests/integer_reading_api.c:212: error: Memory leak: piDataOut
modules/api_scilab/tests/unit_tests/integer_writing_api.c:212: error: Memory leak: piDataOut
modules/api_scilab/tests/unit_tests/integer_reading_api.c:212: error: Memory leak: pucDataOut
modules/api_scilab/tests/unit_tests/integer_writing_api.c:212: error: Memory leak: pucDataOut
modules/api_scilab/tests/unit_tests/integer_reading_api.c:212: error: Memory leak: pusDataOut
modules/api_scilab/tests/unit_tests/integer_writing_api.c:212: error: Memory leak: pusDataOut
modules/api_scilab/tests/unit_tests/integer_reading_api.c:212: error: Memory leak: puiDataOut
modules/api_scilab/tests/unit_tests/integer_writing_api.c:212: error: Memory leak: puiDataOut
modules/arnoldi/sci_gateway/c/sci_eigs.c:231: error: Memory leak: Bcplx
modules/arnoldi/src/c/eigs.c:341: error: Memory leak: R
modules/arnoldi/src/c/eigs.c:341: error: Memory leak: RC
modules/arnoldi/src/c/eigs.c:652: error: Memory leak: R
modules/arnoldi/src/c/eigs.c:652: error: Memory leak: RC
modules/core/src/cpp/hashtable_core.cpp:248: style: Class 'copy_name' is unsafe, 'copy_name::names' can leak by wrong usage.
modules/external_objects/src/cpp/newInstance.cpp:119: error: Deallocating a deallocated pointer: tmpvar
modules/external_objects/src/cpp/newInstance.cpp:117: error: Deallocating a deallocated pointer: args
modules/external_objects/src/cpp/newInstance.cpp:117: error: Memory pointed to by 'args' is freed twice.
modules/fileio/sci_gateway/c/sci_fprintfMat.c:215: error: Memory leak: Format
modules/fileio/sci_gateway/c/sci_fprintfMat.c:223: error: Memory leak: Format
modules/fileio/sci_gateway/c/sci_fprintfMat.c:229: error: Memory leak: Format
modules/fileio/sci_gateway/c/sci_fprintfMat.c:237: error: Memory leak: Format
modules/fileio/sci_gateway/c/sci_fprintfMat.c:243: error: Memory leak: Format
modules/fileio/sci_gateway/c/sci_fprintfMat.c:249: error: Memory leak: Format
modules/gui/src/cpp/SetUicontrolBackgroundColor.cpp:42: error: Memory leak: allColors
modules/gui/src/cpp/SetUicontrolPosition.cpp:42: error: Memory leak: position
modules/gui/src/cpp/SetUiobjectForegroundColor.cpp:40: error: Memory leak: allColors
modules/gui/src/cpp/SetUicontrolValue.cpp:76: error: Memory leak: value
modules/gui/sci_gateway/cpp/sci_toprint.cpp:314: error: Undefined behavior: Variable 'lines' is used as parameter and destination in s[n]printf().
modules/output_stream/src/c/msgstore.c:124: error: Memory leak: str
modules/scicos/src/scicos_sundials/src/nvec_ser/nvector_serial.c:371: error: Memory pointed to by 'v' is freed twice.
modules/types/src/cpp/ScilabToJava.cpp:587: error: Memory leak: d
modules/types/src/cpp/ScilabToJava.cpp:625: error: Memory leak: d

Меня уже давно мучает вопрос, за который я скорее всего буду заминусован, но не спросить все равно не могу:

В чем смысл проверять разные проекты c помощью одного и того же набора правил, разумеется, кроме пиара вашего продукта?

У PVS-Studio, если я правильно понимаю, несколько сотен различных предупреждений, однако из статьи в статью повторяются одни и те же: V507, V519, V560 и множество других. Какая польза читателям статьи от десятка примеров одной и той же ошибки, или от осознания того, что помимо всех прочих проектов ее допустили еще в проекте X?
Ошибка одна, но контекст её возникновения обычно неоднозначен. Та же утечка памяти — банальная и распространённая ошибка. Тем не менее условия, в которых она возникает — безграничны.

Пиар, конечно, это не отменяет, но с другой стороны, очередному проекту Х придётся призадуматься о качестве кода, что хоть немного сделает мир лучше.
вот я тут как сильно третье лицо скажу: мне очень не нравится их формат чисто рекламных публикаций. Но не нравится мне он именно в части рекламы. А вот наполнение очень даже интересное обычно.

Поэтому я вам предлагаю читать эти статьи немного в другом виде. Заменять шапку вида «Мы прогнали наш продукт на исходниках...», на что-то типа «Мы заставили 1000 индусов просмотреть код ...».
А дальше-то прикольные вещи идут. И не важно что 1000 индусов смотрели не сам исходный код, а логи их продукта. Прикольные вещи они и правда находят.

А вот оценивать что эффективнее просматривать исходники или логи — это не программерское занятие.
Меня еще знаете что смущает. Что продукт вроде как «С++ статический анализатор», но вот в таких статьях, что-то связанное с плюсами именно редко видишь. Все какие-то неверные вызовы libc функций да кривые макросы и побитовые операции.
UFO just landed and posted this here
А 1992 — это из какого фильма?
А вы код PVS-Studio тоже через него же прогоняете?
«Всё бы хорошо, если бы не оператор 'break'. Будет заменён только один пробел. Впрочем, если убрать 'break' это не поможет. Функция strlen() вернёт ноль, и цикл всё равно остановится.» — великолепно! Ошибка второго уровня — требует двух исправлений, чтобы все заработало! Реально адская вещь! Кстати, есил убрать break, анализатор обнаружит второй уровень проблемы?
Нет. Такое он не сообразит. Тем более, что это имеет какой-то смысл. Я сейчас подумал, что возможно тут хотели иное. Возможно следует обрезать строку по пробелу. Тогда забыты скобки и надо было так:
static int msg_101(int *n, int *ierr)
{
  ....
  for (i=0;i<(int)strlen(line);i++)
  {
    if (line[i]==' ')
    {
      line[i]='\0';
      break;
    }
  }
  ....
}
Да уж, впечатляет!
Жаль, нет линуксовой версии, чтобы попробовать в действии.
Sign up to leave a comment.