habrahabr

Занимательная археология. Или PVS-Studio проверяет Microsoft Word 1.1a

  • четверг, 3 апреля 2014 г. в 03:10:35
http://habrahabr.ru/company/pvs-studio/blog/217921/

Череп единорога
Недавно компания Microsoft сделала подарок всем программистам, которые хотят покопаться в чем-то интересном. Microsoft открыли исходный код MS-DOS v 1.1, v 2.0 и Word for Windows 1.1a. Операционная система MS-DOS написана на ассемблере, и к ней анализатор не применим. А вот Word написан на языке Си. Исходным кодам Word 1.1a почти 25 лет, однако нам кое-как удалось их проверить. Конечно никакой практической ценности в этой проверке нет. Just for fun.

Где поживиться исходниками


Возможно, многим будет интересна не сколько эта статья, а сам факт, что можно скачать исходные коды MS-DOS v 1.1, v 2.0 и Word for Windows 1.1a. Тем, кому интересно самим покопаться в исходных кодах, отправляю к первоисточнику.

Пресс-релиз: Computer History Museum Makes Historic MS-DOS and Word for Windows Source Code Available to the Public.

Проверка Word 1.1a


Рисунок 1. Word for Windows 1.1a.

Рисунок 1. Word for Windows 1.1a.

Word for Windows 1.1a был выпущен в 1990 году. 25 марта 2014 код этого продукта стал доступен публике. Word был и остаётся флагманским продуктом компании Microsoft. Мне и многим другим интересно посмотреть на внутренности программного продукта, который так сильно поспособствовал коммерческим успехам компании Microsoft.

Я решил проверить код Word 1.1a с помощью нашего инструмента PVS-Studio. Это статический анализатор Си/Си++ кода. Естественно, это не так просто. Анализатор рассчитан на работу с проектами, разрабатываемыми как минимум в Visual Studio 2005. А сейчас предо мной исходники на языке Си, которым более 20 лет. Можно сказать, что это доисторические времена. По крайней мере, тогда не существовало стандарта языка Си. Каждый компилятор был сам по себе. К счастью, в исходных кодах Word 1.1a не оказалось каких-то необычных моментов и использования большого количества нестандартных расширений компилятора.

Для анализа необходимы препроцессированные файлы (*.i). Имея препроцессированные файлы, можно воспользоваться инструментом PVS-Studio Standalone. С его помощью можно выполнить анализ и изучить диагностические сообщения. Конечно, анализатор не рассчитан на анализ 16-битных программ. Но этих результатов анализа будет вполне достаточно для удовлетворения любопытства. Внимательно анализировать проект 24 летней давности нет никакого практического смысла.

Итак, основная загвоздка состояла в том, как получить препроцессированные файлы. Я попросил своего коллегу поколдовать в этом направлении. Он подошёл к решению весьма творчески. Он выполнил препроцессирование с помощью GCC 4.8.1. Вряд ли кто-то ещё так издевался над исходниками Word 1.1. Использовать GCC — надо ведь было такое придумать. Фантазёр.

Самое интересное, что вышло вполне удачно. Была написана маленькая утилита, которая запускала препроцессирование с помощью GCC 4.8.1 на каждый файл из директории, в которой он лежал. По мере вывода ошибок, связанных с включением заголовочных файлов, в параметры запуска добавлялись ключи -I с путём до нужных файлов. Парочка ненайденных заголовочных файлов были созданы пустыми. Все остальные проблемы раскрытия #include были связаны с включением ресурсов, поэтому были закомментированы. При препроцессировании определялся макрос WIN, т.к. в коде есть ветка для WIN и MAC.

Дальше в дело вступил PVS-Studio Standalone и ваш покорный слуга. Я выписал подозрительные фрагменты кода и готов вам их показать. Но вначале ещё кое что о проекте.

Разное о коде Word 1.1a



Самые сложные функции


Самая большая цикломатическая сложность у следующих функций:
  1. CursUpDown — 219;
  2. FIdle — 192;
  3. CmdDrCurs1 — 142.

#ifdef WIN23


Просматривая исходные коды и встретив "#ifdef WIN23", я заулыбался. И даже выписал это место. Я подумал, что это опечатка и должно быть написано #ifdef WIN32.

Когда я увидел WIN23 второй раз я засомневался. А потом вдруг осознал, что я смотрю исходники 24 летней давности. WIN23 означает версию Windows 2.3.

Суровые времена


В коде мне попалась вот такая интересная строка.
Assert((1 > 0) == 1);

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

Конечно, если считать K&R стандартом, то по идее условие ((1 > 0) == 1) всегда выполняется. Но K&R это был лишь стандарт де-факто и не более. Это проверка на адекватность компилятора.

Результаты проверки


Теперь поговорим о подозрительных местах, найденных мною в коде. Думаю, ради этого вы и читаете эту статью. Приступим.

Бесконечный цикл


void GetNameElk(elk, stOut)
ELK elk;
unsigned char *stOut;
{
  unsigned char *stElk = &rgchElkNames[mpelkichName[elk]];
  unsigned cch = stElk[0] + 1;

  while (--cch >= 0)
    *stOut++ = *stElk++;
}

Предупреждение PVS-Studio: V547 Expression '-- cch >= 0' is always true. Unsigned type value is always >= 0. mergeelx.c 1188

Цикл «while (--cch >= 0)» никогда не остановится. Переменная 'cch' имеет тип unsigned. Значит, сколько не уменьшай эту переменную, она всегда останется >= 0.

Выход за границу массива из-за опечатки


uns rgwSpare0 [5];

DumpHeader()
{
  ....
  printUns ("rgwSpare0[0]   = ", Fib.rgwSpare0[5], 0, 0, fTrue);
  printUns ("rgwSpare0[1]   = ", Fib.rgwSpare0[1], 1, 1, fTrue);
  printUns ("rgwSpare0[2]   = ", Fib.rgwSpare0[2], 0, 0, fTrue);
  printUns ("rgwSpare0[3]   = ", Fib.rgwSpare0[3], 1, 1, fTrue);
  printUns ("rgwSpare0[4]   = ", Fib.rgwSpare0[4], 2, 2, fTrue);
  ....
}

Предупреждение PVS-Studio: V557 Array overrun is possible. The '5' index is pointing beyond array bound. dnatfile.c 444

Как-то так получилось, что в первой строке написано: Fib.rgwSpare0[5]. Это неправильно. В массиве всего 5 элементов, а значит максимальный индекс должен быть равен 4. Значение '5' это результат опечатки. По всей видимости в первой строке должен был использоваться нулевой индекс:
printUns ("rgwSpare0[0]   = ", Fib.rgwSpare0[0], 0, 0, fTrue);

Неинициализированная переменная


FPrintSummaryInfo(doc, cpFirst, cpLim)
int doc;
CP cpFirst, cpLim;
{
  int fRet = fFalse;
  int pgnFirst = vpgnFirst;
  int pgnLast = vpgnLast;
  int sectFirst = vsectFirst;
  int sectLast = sectLast;
  ....
}

Предупреждение PVS-Studio: V573 Uninitialized variable 'sectLast' was used. The variable was used to initialize itself. print2.c 599

Переменная 'sectLast' присваивается сама себе:
int sectLast = sectLast;

Кажется, для инициализации должна была быть использована переменная 'vsectLast':
int sectLast = vsectLast;

Нашлось ещё одна идентичная ошибка. Видимо последствие Copy-Paste:

V573 Uninitialized variable 'sectLast' was used. The variable was used to initialize itself. print2.c 719

Неопределённое поведение


CmdBitmap()
{
  static int  iBitmap = 0;
  ....
  iBitmap = ++iBitmap % MAXBITMAP;
}

Предупреждение PVS-Studio: V567 Undefined behavior. The 'iBitmap' variable is modified while being used twice between sequence points. ddedit.c 107

Не знаю, как к такому коду относились 20 лет назад. Но сейчас это считается хулиганством, так как приводит к неопределённому поведению.

Аналогично:
  • V567 Undefined behavior. The 'iIcon' variable is modified while being used twice between sequence points. ddedit.c 132
  • V567 Undefined behavior. The 'iCursor' variable is modified while being used twice between sequence points. ddedit.c 150

Неудачный вызов функции printf()


ReadAndDumpLargeSttb(cb,err)
  int     cb;
  int     err;
{
  ....
  printf("\n - %d strings were read, "
         "%d were expected (decimal numbers) -\n");
  ....
}

Предупреждение PVS-Studio: V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 3. Present: 1. dini.c 498

Функция printf(), это функция с переменным количеством аргументов. Ей можно передать аргументы, а можно и не передать. Вот здесь про аргументы забыли, в результате чего будет распечатан мусор.

Неинициализированные указатели


В одной из вспомогательных утилит, которая имеется в составе исходников Word, можно встретить что-то вообще непонятное.
main(argc, argv)
int argc;
char * argv [];
{
  FILE * pfl;
  ....
  for (argi = 1; argi < argc; ++argi)
  {
    if (FWild(argv[argi]))
    {
      FEnumWild(argv[argi], FEWild, 0);
    }
    else
    {
      FEWild(argv[argi], 0);
    }

    fclose(pfl);
  }
  ....
}

Предупреждение PVS-Studio: V614 Uninitialized pointer 'pfl' used. Consider checking the first actual argument of the 'fclose' function. eldes.c 87

Переменная 'pfl' не инициализируется до цикла и в самом цикле. Зато много раз вызывается функция fclose(pfl). Впрочем, всё это вполне могло успешно работать. Функция вернёт статус ошибки, и программа продолжит свою работу.

А вот ещё одна опасная функция. Скорее всего, её вызов приведёт к аварийному завершению программы.
FPathSpawn( rgsz )
char *rgsz[];
{ /* puts the correct path at the beginning of rgsz[0]
     and calls FSpawnRgsz */
  char *rgsz0;

  strcpy(rgsz0, szToolsDir);
  strcat(rgsz0, "\\");
  strcat(rgsz0, rgsz[0]);
  return FSpawnRgsz(rgsz0, rgsz);
}

Предупреждение PVS-Studio: V614 Uninitialized pointer 'rgsz0' used. Consider checking the first actual argument of the 'strcpy' function. makeopus.c 961

Указатель ' rgsz0' ничем не инициализируется. Это не мешает начать копировать в него строку.

Опечатка в условии


....
#define wkHdr    0x4000
#define wkFtn    0x2000
#define wkAtn    0x0008
....
#define wkSDoc    (wkAtn+wkFtn+wkHdr)

CMD CmdGoto (pcmb)
CMB * pcmb;
{
  ....
  int wk = PwwdWw(wwCur)->wk;
    if (wk | wkSDoc)
      NewCurWw((*hmwdCur)->wwUpper, fTrue);
  ....
}

Предупреждение PVS-Studio: V617 Consider inspecting the condition. The '(0x0008 + 0x2000 + 0x4000)' argument of the '|' bitwise operation contains a non-zero value. dlgmisc.c 409

Условие (wk | wkSDoc) всегда истинно. На самом деле, здесь, скорее всего, хотели написать:
if (wk & wkSDoc)

В общем, перепутали оператор | и &.

И под конец длинный, но простой пример


int TmcCharacterLooks(pcmb)
CMB * pcmb;
{
  ....
  if (qps < 0)
  {
    pcab->wCharQpsSpacing = -qps;
    pcab->iCharIS = 2;
  }
  else  if (qps > 0)
  {
    pcab->iCharIS = 1;
  }
  else
  {
    pcab->iCharIS = 0;
  }
  ....
  if (hps < 0)
  {
    pcab->wCharHpsPos = -hps;
    pcab->iCharPos = 2;
  }
  else  if (hps > 0)
  {
    pcab->iCharPos = 1;
  }
  else
  {
    pcab->iCharPos = 1;
  }
  ....
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. dlglook1.c 873

Когда работают с переменной 'qps', то записывают в 'pcab->iCharIS' следующие значения: 2, 1, 0.

Аналогично работают с переменной 'hps'. Но при этом в переменную 'pcab->iCharPos' помещаются подозрительные числа: 2, 1, 1.

Скорее всего, это опечатка. В самом конце, наверное, следовало использовать ноль.

Заключение


Найдено совсем немного странных мест. Причины две. Во-первых, код мне показался написан качественно и весьма понятно. Во-вторых, анализ был все-таки неполноценным. Учить же анализатор особенностям старого Си нет практической надобности.

Надеюсь, я подарил вам несколько минут интересного чтения. Спасибо за внимание. И попробуйте анализатор PVS-Studio на своём коде.

Эта статья на английском


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Archeology for Entertainment, or Checking Microsoft Word 1.1a with PVS-Studio.

Прочитали статью и есть вопрос?
Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio и CppCat, версия 2014. Пожалуйста, ознакомьтесь со списком.