пятница, 2 сентября 2011 г.

прогнал сорцы wincheck

на PVS Studio
Нашло 2253 possible errors
Начну с плохих новостей
  • оно работает дико медленно - на 2.5 метрах ~13 минут
  • находит в основном всякий треш типа вот такого:
  • Dangerous magic number 4 used: UCHAR Tag[4]
    Дико опасная ошибка, да. Я заметил что оно вообще всегда делает стойку на константы 4 и 32
  • On 64-bit platform, structure size can be reduced from 48 to 40 bytes by rearranging the fields according to their sizes in decreasing order.
    Ну круто, чо. А как делать rearranging the fields то ? Хоть бы варианты какие предлагала
  • И самое угарное - большинство таких структур находятся например в ntdll.h, бгг
  • отчего-то вот такой вполне легальный кусок кода sizeof(PBYTE) * _countof(m_index_table) считается
    It is odd that a sizeof() operator is multiplied by sizeof()
  • также раздражает что if ( memcmp(hash, old_hash, HASH_SIZE) ) считается как
    The 'memcmp' function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes
    ну и чо - проверил я хэши и если не совпали - мне нужно предпринять какие-то действия например
Хорошие новости
  • нашла одно присваивание в if. Даже странно что visual studio ни слова не сказала
  • нашлась пара printf с меньшим числом аргументов, чем указано в format string. 
  • один printf соотв-но с большим числом аргументов, чем указано в format string.

    13 комментариев:

    1. >> находит в основном всякий треш типа вот такого
      насколько помню этот треш отключабелен. Не помню где.

      В любом случае оно полезное, даже несмотря на треш, раз смогло найти ошибки.

      ОтветитьУдалить
    2. Кстати, указанные тобой ошибки вылавливаются ворнингами clang-а, как бы ты сраный опенсурс ни ненавидел ;-)

      ОтветитьУдалить
    3. а чо - clang давно научился виндовый sdk парзить ?

      ОтветитьУдалить
    4. > В любом случае оно полезное
      кпд как у паровоза примерно только

      ОтветитьУдалить
    5. А хрен его знает, под виндой не пробовал, увы.

      ОтветитьУдалить
    6. linuxоиды меня всегда умиляли - мало того что их проги не работают под 85% компов - они еще считают себя в праве давать непроверенные советы

      линуксофашизм, как и было сказано

      ОтветитьУдалить
    7. Этот комментарий был удален автором.

      ОтветитьУдалить
    8. Здравствуйте. Спасибо за отзыв. Решил прокомментировать некоторые пункты.

      1. Диагностика типа "Dangerous magic number 4 used" относится к 64-битности и, к сожалению, действительно часто мусорна. Но, это единственный способ программистам найти и просмотреть все места, которые могут быть потенциально опасны при переносе кода на 64-битную систему. Если диагностика 64-битных ошибок не интересна, или программа уже корректно работает на 64-битной системе, то все эти диагностики легко отключить. Достаточно отжать кнопку «64». Количество сообщений уменьшится на порядок. Также есть возможность отключить отдельные предупреждения или пометить строки кода как безопасные.

      2. «Я заметил что оно вообще всегда делает стойку на константы 4 и 32.» А еще на 0x7FFFFFFF, 0x80000000, 0xFFFFFFFF. И это "жжж…" не спроста - http://www.viva64.com/ru/l/0009/

      3. "А как делать rearranging the fields то?". К сожалению, в сообщении невозможно привести упорядоченную структуру. Сообщение станет слишком длинным и нечитабельным. Делать rearranging очень просто и это описано в документации ( http://www.viva64.com/ru/d/0150/ ). Достаточно расположить поля по убыванию их размера.

      4. "И самое угарное - большинство таких структур находятся например в ntdll.h". Как я понимаю, ntdll.h не является частью стандартных инклудов VisualC++, а входит в состав SDK. Я прав? Если это так, то достаточно добавить путь до SDK в список исключений и тогда эти ошибки пропадут. Я имею в виду вот этот раздел настроек: http://www.viva64.com/ru/d/0017/

      5. "ну и чо - проверил я хэши и если не совпали - мне нужно предпринять какие-то действия например". Предупреждение о другом. Прошу посмотреть описание: http://www.viva64.com/ru/d/0115/ Кстати, это предупреждение вполне можно отключить.

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

      С уважением, Андрей Карпов.

      ОтветитьУдалить
    9. привет
      ntdll.h - плод многолетних усилий по реверсингу native api - оно выглядит ужасно

      лучше расскажите почему pvs studio ругается на выражения вида sizeof * _countof ?

      ОтветитьУдалить
    10. sizeof(PBYTE) * _countof(m_index_table)

      Макрос _countof раскрывается в (sizeof(m_index_table) / sizeof(m_index_table[0])). Соответственно получается, что один sizeof() умножается на другой sizeof():

      sizeof(PBYTE) * sizeof(m_index_table) / sizeof(m_index_table[0])

      В 90% случаев перемножение двух sizeof() свидетельствует о наличии в коде ошибки. Это и приводит к предупреждению V531 ( http://www.viva64.com/ru/d/0120/ ). Здесь конечно ложное срабатывание. И я записал себе этот случай, чтоб в дальнейшем реализовать соответствующее исключение из правила.

      Для большей наглядности покажу пример, для выявления каких ошибок служит V531.

      Проект XuiFramework:

      CPPString CPPHtmlDrawer::GetStringFromDll(...)
      {
      ...
      TCHAR szTemp[256];

      DWORD dwLen = ::LoadString(hInstDll, dwID,
      szTemp, (sizeof(szTemp) * sizeof(TCHAR)));
      ...
      }

      Должно быть:

      szTemp, sizeof(szTemp) / sizeof(TCHAR));

      ОтветитьУдалить
    11. Хочу предложить Вам ключ на 3 месяца для работы с PVS-Studio. Хочется узнать Ваше мнение о режиме инкрементального анализа. Также будем благодарны за новые замечания и пожелания. Например, благодаря Вам, следующая версия PVS-Studio уже не будет ругаться на «sizeof(PBYTE) * _countof(m_index_table)». Спасибо. (Пишите мне на karpov[@]viva64.com).

      ОтветитьУдалить
    12. привет
      спасибо большое, но боюсь что поскольку пишу я медленно, то все свои сорцы за пару лет уже на PVS Studio проверил :-)

      А если серъезно то, как показывает опыт портирования wincheck на w8 developer preview, большинство багов там от интеграции со всяким недокументированным и никакими code coverage инструментами это не отловить

      ОтветитьУдалить
    13. вот еще вспомнил одну потенциально полезную штуку - например в проекте есть некоторое количество автогенеренных файлов (например stubы от midl). Было бы неплохо чтобы такие файлы как-нть автоматически распознавались как autogenerated (например анализом комментариев в начале файла) и исключались из множества файлов для проверки

      ОтветитьУдалить