Как стать автором
Поиск
Написать публикацию
Обновить

Комментарии 63

Интересно, у кого рог больше?
Всего голосов 4: ↑3 и ↓1+2
Ваш проект плох тем, что делает лучше виндовые программы, а не линуксовые =)
Я шучу и желаю проекту успеха и коммерческой успешности. Приятно читать ваши статьи, даже когда уже умер как программист давным давно.
Всего голосов 36: ↑32 и ↓4+28
Спасибо.
Всего голосов 2: ↑1 и ↓10
Для linux недавно facebook открыл flint
Всего голосов 3: ↑3 и ↓0+3
Спасибо. Не знал про такое. Но по всей видимости какая-то фитюлька. Объем исходного кода проекта 340 килобайт. Для сравнения, объем исходного кода консольной версии PVS-Studio — около 6 мегабайт.
Всего голосов 9: ↑5 и ↓4+1
Там в зависимостях google test на 4.5 мб, возможно поэтому
Всего голосов 3: ↑2 и ↓1+1
Нет конечно :-). Это библиотека для юнит-тестов, а не мозг для статического анализатора.
Всего голосов 4: ↑4 и ↓0+4
Как мне кажется, это просто для тестирования себя.
Комментарий пока не оценивали0
Это больше style-checker, а не статический анализатор кода. Просто посмотрите на заданные правила и сообщения об «ошибках»:

          lintWarning(*it, "Protected inheritance is sometimes not a good "
              "idea. Read "
              "http://stackoverflow.com/questions/6484306/effective-c-discouraging-protected-inheritance "
              "for more information.\n");


      lintError(*it, "Open Source Software may not include files from "
                     "other fbcode projects (except folly). "
                     "If this is not an fbcode include, please use "
                     "'#include <...>' instead of '#include \"...\"'. "
                     "You may suppress this warning by including the "
                     "comment 'nolint' after the #include \"...\".\n");
Всего голосов 1: ↑1 и ↓0+1
Вообще, если без шуток порассуждать о данном моменте то получается, что PVS действительно плох отсутствием кроссплатформенности. Однако с учётом размера команды проект развивается очень даже замечательно. А дальше тоже не понятно что делать, ведь если начать сильно увеличивать штат, то можно легко потонуть в бюрократии, и испортить проект. В общем ребята правда молодцы.

Мне в PVS нравится то, что у него довольно мало воды в предупреждениях, и это очень ценно поскольку многие, даже опытные разработчики, обязательно следуют рекомендациям анализатора, не пытаясь понять зачем они это делают, и не понимая всех последствий. Для примера могу привести свой опыт, когда в одном, довольно большом и старом проекте, человек, позиционирующий себя мантейнером и руководителем проекта засрал все конструкторы инициализациями цифровых членов в 0 или какими то другими «безопасными» значениями, только потому что так ему рекомендовал CppCheck, в результате данной «подушки безопасности» провалилась производительность во многих местах поскольку была поломана отложенная инициализация. которую вносили в проект годами. Помимо этого данный руководитель не понимает и не хочет понимать как работает move-semantics и старательно выпиливает её из классов, которые уже были переписана на C++11. Вот такой маразм, уж простите за «крик души», просто наболело :)
Всего голосов 1: ↑1 и ↓0+1
Спасибо за комментарий, хочу обратить внимание (прежде всего других людей), что «мало воды в предупреждениях» — это результат очень долгой и сложной доводки анализатора под конкретный компилятор (VisualC++) и конкретную платформу. Как только анализатор запускается без такой доводки, скажем в гипотетическом линуксе — из него вода льется как из дырявого ведра.
Всего голосов 3: ↑3 и ↓0+3
Именно так, и за это вам всем огромная благодарность!
Всего голосов 1: ↑1 и ↓0+1
* Доп про конструкторы: теперь вообще не возможно узнать отработал ли какой либо функционал или нет, поскольку раньше программа сразу же падала на ошибках в новом коде, а теперь из-за 0 (nullptr) в конструкторах на указателях, и проверок на nullptr везде где только можно программа не падает, но и не пойми что делает. Этот момент заметен по жалобам пользователей у которых постоянно что то начинает глючить, и появляется всё больше и больше гейзенбагов в новых версиях. Вот такая печаль, а проект был хорошим :(
Комментарий пока не оценивали0
Здорово, благодарю!
Комментарий пока не оценивали0
С другой стороны, на glibc можно неплохо тренироваться по устранению ложноположительных срабатываний из гвинпинного мира.
Всего голосов 9: ↑8 и ↓1+7
Я бы лучше попробовал поковырять gcc, про который пишут, что кроме пары бородатых программистов никто не понимает как он работает. Ну и заодно что-нибудь из кодовой базы X11, вот там точно код тот еще. Ну и напоследок наверное что-нибудь типа Mesa явно не лишним было бы проверить.
Всего голосов 17: ↑17 и ↓0+17
Мы ещё год назад собирали проверяли gcc. Подтверждаю, что разобраться в проекте очень сложно. Там сплошные макросы, которые раскрываются в макросы, которые раскрываются в макросы, которые раскрываются в макросы…
Я сдался и бросил анализ проекта, хотя очень хотелось найти ошибки.
Всего голосов 14: ↑13 и ↓1+12
К сожалению не помню, делали ли вы этоо уже или нет, но могу посоветовать проверить LLVM. Код вполне читаемый и атавизмов в нем практически нет. А вот багов может быть много, поскольку многие модули, судя по всему, писались в спешке и очень разными людьми.
Всего голосов 1: ↑1 и ↓0+1
они проверяли clang, который является модулем (могу ошибаться в ихней архитектуре). было бы не разумно играться с модулем не посмотрев на ядро
Всего голосов 1: ↑1 и ↓0+1
If E1 has a signed type and a negative value, the resulting value is implementation-defined.

implementation-defined это не совсем то же самое, что и undefined. В данном случае «все понимают» (серьезно) что на 2's complement machine будет «правильная» реализация, и это так во всех разумных компиляторах.

Еще есть такой момент, что glibc — это стандартная библиотека языка C, ее интерфейсы описаны в стандарте C. По сути, она частично реализует язык C. Соответственно, она написана не на C, а на чем-то слегка его напоминающем, с кучей нестандартных расширений, платформо-зависимой магии и т.д.
Всего голосов 17: ↑17 и ↓0+17
Вообще между undefined behavior и implementation-defined behavior лежит пропасть. Если вы используете implementation-defined behavior — то вы, всё-таки всё ещё в рамках C и во многих случаях всё ещё в безопасности (скажем если вы использете какой-нибудь int8_t, то вы сразу попадаете в рамки 2's complement machine и можете с лёгостью делать подобные сдвиги).

А вот разъменование указателя, который позже проверяется на NULL — это не просто ошибка, это катастрофа. Дело в том, что компилятор может после этого проверку на NULL просто выкинуть. Именно потому что undefined behavior с точки зрения компилятора «не бывает» и уж если ты указатель разименовал, то ты тем самым «пообещал», что соответствующий указатель — не NULL.

А если после этого он ещё и немного код «пооптимизирует», то всё может кончится чем-нибудь ешё похлеще. Например переменная, которая инициализировалась через этот указатель может оказаться и вообще ненужной и будет выкинута. Получится просто программа, которая игнорирует тот факт, что какой-то указатель может быть равен NULLу.
Всего голосов 5: ↑5 и ↓0+5
XDR *xdrs = &clp->xdr_stream;

Это не совсем разыменование указателя, а взятие адреса, по этому код имеет потенциальное право на жизнь. Существуют реализации макроса offsetof точно таким-же образом:
en.wikipedia.org/wiki/Offsetof
habrahabr.ru/company/pvs-studio/blog/198060/#comment_6871226

У меня нет уверенности на счет того, что это точно не UB, но известные мне компиляторы понимают этот код правильно, так как сами используют такое.
Всего голосов 3: ↑2 и ↓1+1
Согласен. Я собственно и говорил, что особенных вещей не нашлось. Но надо попридираться то. А то вроде как зря мучился. :)
Всего голосов 4: ↑3 и ↓1+2
Согласно стандарту — это UB.

При этом компилятор вправе реализовать offsetof таким образом, потому что реализация стандартной библиотеки привязана к конкретному компилятору, и может спокойно полагаться на его особенности вроде вот этой.
Комментарий пока не оценивали0
С точки зрения компилятора код 
XDR *xdrs = &clp->xdr_stream;

эквивалентен коду
XDR *xdrs = &((*clp).xdr_stream);


Т.е. в этом коде разыменование — это вот это:
clp->

А взятие адреса там уже после разыменования.
Всего голосов 2: ↑2 и ↓0+2
По поводу «memset (temp_result, '\0', sizeof (temp_result));»

Можно ли заменить это на конструкцию типа?
void * (*volatile vol_memset)(void *, int, size_t) = memset;
void safe_memclear(void *mem, size_t len) { vol_memset(mem, 0, len); }

И еще вот нашел ссылочку по этой теме
Комментарий пока не оценивали0
Я не знаю. Возможно. В winnt.h сделано так:

FORCEINLINE
PVOID
RtlSecureZeroMemory(
    _Out_writes_bytes_all_(cnt) PVOID ptr,
    _In_ SIZE_T cnt
    )
{
    volatile char *vptr = (volatile char *)ptr;

#if defined(_M_AMD64)

    __stosb((PBYTE )((DWORD64)vptr), 0, cnt);

#else

    while (cnt) {

#if !defined(_M_CEE) && defined(_M_ARM)

        __iso_volatile_store8(vptr, 0);

#else

        *vptr = 0;

#endif

        vptr++;
        cnt--;
    }

#endif // _M_AMD64

    return ptr;
}
Всего голосов 1: ↑0 и ↓1-1
Кстати в новом C11 [ISO/IEC 9899:2011] должна быть функция memset_s() как раз для таких слачаев. Только не нашел какие компиляторы ее поддерживают.
Всего голосов 2: ↑2 и ↓0+2
__builtin_strcmp

Иногда гнать весь код через препроцессор компилятора не самое удачное решение. Именно как раз потому, что это implementation-defined, зависит от используемого компилятора/ОС и не позволяет вычислять ошибки совместимости. Но есть хорошие новости: можно задать параметр оптимизации -O0, когда никаких «левых» inline-подстановок компилятора не возникнет.

Для меня стало открытием наличие такого бреда в препроцессоре. В ассемблерном варианте окажется что-то вроде этого:

	movq	%rdi, %rax
	movq	%rsi, %rdi
	movq	%rax, %rsi
	jmp	strcmp
Всего голосов 1: ↑1 и ↓0+1
Удачное/не удачное — выбора нет. Препроцессинг нужен. И нужно уметь работать, в независимости, от ключей. Как проект собирается, с теми ключами и надо его проверять, подменив собой компилятор. Спасибо за информацию про -O0. Но к сожалению, жизнь разработчикам анализаторов это не упрощает. :)
Комментарий пока не оценивали0
А если учесть что GLibC с -O0 вообще собрать нельзя…
Всего голосов 2: ↑2 и ↓0+2
Насколько я знаю, этот анализатор проект не собирает — надо вручную прогнать через препроцессор и потом уже скормить ему полученные i-файлы.
Всего голосов 1: ↑0 и ↓1-1
«надо вручную прогнать через препроцессор» — Не надо сбивать с толку людей. А то останется в голове, что это велосипед с ручным приводом. :)

Да, есть и вариант с *.i файлами. А также масса других методов, с которыми можно познакомиться в документации. Например, это интеграция в makefile. Ну а если у Вас Visual C++, так вообще просто надо кнопку нажать и всё проверится.
Всего голосов 2: ↑2 и ↓0+2
Ну а если у Вас Visual C++

Особенно легко и непринуждённо VS запускается в RHEL.

Если статья об анализе ключевой библиотеки Linux, вы привлекли внимание всех читателей хаба Linux — извольте приоткрыть тайну анализа линуксовых библиотек. Сейчас из поста получается «раз! — и результат» — это-то и сбивает с толку людей. А процесс копания в настройках компилятора/препроцессора, а также ковыряние километрового лога остался за кадром. Было бы интересно читать как вы решали эти проблемы, а не как тяжело писать код под Linux.
Всего голосов 1: ↑1 и ↓0+1
Эти проблемы не решены. Сейчас есть только демонстрация возможностей анализатора на проекте из мира Linux.
Комментарий пока не оценивали0
Как на счёт проверить Transmission?
Всего голосов 3: ↑1 и ↓2-1
По поводу
while (categoryvalue[0] != '\0' && categoryvalue[0] == ':')

Это не могло быть странной попыткой оптимизации? Например, такая логика автора. Проверка на '\0', если не проходит, сразу вываливаемся из цикла, без проверки на ':'.
Всего голосов 2: ↑1 и ↓10
И где оптимизация? :)
Комментарий пока не оценивали0
Вообще, судя по коду ниже (ветке else после while) похоже действительно просто лишняя проверка, а никакая не ошибка…
Комментарий пока не оценивали0
в коде все четко и однозначно. сей кусок для людей предназначен, а не анализаторов — опенсорс же. ну да, в статье это представлено в духе «скандалы-интриги-расследования», видимо чтобы читать веселее было. :)
Комментарий пока не оценивали0
> 581 /* Make CATEGORYVALUE point to the next element of the list. */
> 582 while (categoryvalue[0] != '\0' && categoryvalue[0] == ':')
> 583 ++categoryvalue;
И что же такого этот код поясняет, что не пояснил бы
> 582 while (categoryvalue[0] == ':')
> 583 ++categoryvalue;
? Как по мне, наоборот, запутывает.
Всего голосов 4: ↑4 и ↓0+4
НЛО прилетело и опубликовало эту надпись здесь
sunrpc/clnt_raw.c это пропиетарное наследие от уже вымерших мамонтов. по умолчанию не компилируется. тут анализатор занимается археологией — в самом деле, поучительно.

там история коммитов выглядит интереснее, чем сам код:
2012-05-10 Andreas Jaeger Make sunrpc code usable again
2011-04-17 Ulrich Drepper Obsolete RPC implementation in libc.
2010-08-19 Ulrich Drepper Once again change RPC copyright notices.
2010-06-28 Ulrich Drepper Revert «Sun agreed to a change of the license for the…
2009-05-21 Ulrich Drepper Sun agreed to a change of the license for the RPC code…
…
1995-02-18 Roland McGrath initial import

кстати, в текущей ревизии в копирайтах значится Oracle.
Всего голосов 1: ↑1 и ↓0+1
Как насчёт проверки KPHP HHVM?
Комментарий пока не оценивали0
для nss/getnssent_r.c анализатор написал ерунду, и у вас код отквочен не правильно:

смотрим
 127 int
 128 __nss_getent_r (const char *getent_func_name,
 129                 const char *setent_func_name,
 130                 db_lookup_function lookup_fct,
 131                 service_user **nip, service_user **startp,
 132                 service_user **last_nip, int *stayopen_tmp, int res,
 133                 void *resbuf, char *buffer, size_t buflen,
 134                 void **result, int *h_errnop)
....
 144   if (res && __res_maybe_init (&_res, 0) == -1)
 145     {
 146       *h_errnop = NETDB_INTERNAL;
 147       *result = NULL;
 148       return errno;
 149     }
 150 
...
 159   while (! no_more)
 160     {
 161       int is_last_nip = *nip == *last_nip;
 162 
 163       status = DL_CALL_FCT (fct.f,
 164                             (resbuf, buffer, buflen, &errno, &h_errno));
 165 
 166       /* The status is NSS_STATUS_TRYAGAIN and errno is ERANGE the
 167          provided buffer is too small.  In this case we should give
 168          the user the possibility to enlarge the buffer and we should
 169          not simply go on with the next service (even if the TRYAGAIN
 170          action tells us so).  */
 171       if (status == NSS_STATUS_TRYAGAIN
 172           && (h_errnop == NULL || *h_errnop == NETDB_INTERNAL)
 173           && errno == ERANGE)
 174         break;

тут видно, что в первом случае (145) присваивается значение, в указатель, пришедший в функцию в качестве параметра. он не может быть NULL по соглашению. в 163 этот указатель передается по ссылке функции DL_CALL_FCT, и в 172 проверяется уже новое значение, возвращаемое этой функцией.
Всего голосов 1: ↑1 и ↓0+1
Вот оно как. Ну и хорошо, что ошибки нет.
Всего голосов 3: ↑2 и ↓1+1
XDR *xdrs = &clp->xdr_stream;
…
if (clp == NULL)


Ложное срабатывание. Разыменовывание не происходит, просто один указатель (xdrs) получает значение другого указателя (cpl) плюс смещение поля xdr_stream.

UPD. А. уже написали выше… habrahabr.ru/company/pvs-studio/blog/213969/#comment_7356737
Всего голосов 3: ↑2 и ↓1+1
Есть такой крутой открытый графический движок — Ogre3D. У него очень хороший код, было бы интересно посмотреть на результаты проверки. Ну, вдруг, будет время…
Комментарий пока не оценивали0
К сожалению поделиться будет нечем. Команда Ogre3D давно и успешно применяет PVS-Studio, так что извините, «срыва покровов» не будет.
Всего голосов 7: ↑7 и ↓0+7
Круто!

А bitcoin не проверяли?
Комментарий пока не оценивали0
Ещё нет.
Комментарий пока не оценивали0
А можно у вас заказать проверку других опенсорсных библиотек? В частности, интересует ffmpeg
Комментарий пока не оценивали0
А это интересный способ монетизации для такого продукта )
Всего голосов 3: ↑3 и ↓0+3
Вы еще линукс-ядро не смотрели… Там иногда такое встречается-сомневаешься на С ли это написано
Всего голосов 2: ↑1 и ↓10
Мне кажется, там всё тоже будет очень хорошо, как и в glibc. Быть может, когда-то проверим.
Комментарий пока не оценивали0
Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Ваш аккаунт

Разделы

Информация

Услуги

Техническая поддержка
© 2006–2024, Habr