Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

Математикам доверяй, но проверяй

Anna | 24.06.2014 | нет комментариев
Я подчас бываю озадачен, рассматривая ошибки в очередном программном плане. Многие из этих ошибок живут в планах годами. Глядишь на сотню ляпов в коде и изумляешься, как программа вообще работает. И чай как-то работает. Ей даже пользуются. Причем, я говорю не о коде, рисующем покемона в игре. А, скажем, о математических библиотеках. Да, вы правильно додумались. В этой статье пойдет речь о проверке кода математической библиотеки Scilab.

Scilab

Сегодня мы будем рассматривать подозрительные фрагменты кода в математическом пакете Scilab. Обзора исполнен с поддержкой инструмента PVS-Studio.

Scilab — пакет прикладных математических программ, предоставляющий сильное открытое окружение для инженерных (технических) и научных расчётов [Wikipedia].

Формальный сайт: http://www.scilab.org/

В системе доступно уйма инструментов:

  • 2D и 3D графики, анимация;
  • линейная алгебра, разреженные матрицы (sparse matrices);
  • полиномиальные и разумные функции;
  • интерполяция, аппроксимация;
  • симуляция: решение ОДУ и ДУ;
  • Scicos: гибрид системы моделирования динамических систем и симуляции;
  • дифференциальные и не дифференциальные оптимизации;
  • обработка сигналов;
  • параллельная работа;
  • статистика;
  • работа с компьютерной алгеброй;
  • интерфейс к Fortran, Tcl/Tk, C, C , Java, LabVIEW.

Приготовьтесь. Статья будет длинная. Я чай не повинен, что здесь так много различных пакостей. И хочется показать различные классы ошибок.

Безусловно, обнаруженные недочеты не имеют отношения к математике. Допустимо, все алгорифмы в библиотеке положительные и результативные. Впрочем, раз для написания выбран язык Си , нужно не забывать, что помимо ошибок в алгорифмах, бывают ещё и опечатки, разыменование нулевых указателей и прочие ляпы. Собственно, пользователю не значимо, встретится ему логическая оплошность в численном алгорифме либо он станет жертвой неинициализированной переменной.

Да, статический обзор находит только некоторые виды ошибок. Но раз их легко найти, для чего себе отказывать в таком удовольствии. Отменнее исправить ещё 10% ошибок, чем совсем ничего не исправить.

Выходит, давайте посмотрим, что рассказал мне анализатор PVS-Studio о плане Scilab.

Буфер, которого нет

Буфер, которого нет

int sci_champ_G(....)
{
  ....
  char * strf = NULL ;
  ....
  if ( isDefStrf( strf ) )
  {
    char strfl[4];
    strcpy(strfl,DEFSTRFN);
    strf = strfl;
    if ( !isDefRect( rect ) )
    {
      strf[1]='5';
    }
  }

  (*func)(stk(l1), stk(l2), stk(l3), stk(l4),
    &m3, &n3, strf, rect, arfact, 4L);
  ....  
}

Сообщение PVS-Studio: V507 Pointer to local array ‘strfl’ is stored outside the scope of this array. Such a pointer will become invalid. sci_champ.c 103

Ссылка на непостоянный массив ‘strfl’ сохраняется в переменной ‘strf’. При выходе из блока «if () {… }» данный массив перестаёт существовать. Впрочем, в программе работают с указателем ‘strf’.

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

Схожие задачи:

  • Array ‘strfl’. sci_fec.c 111
  • Array ‘strfl’. sci_grayplot.c 94
  • Array ‘strfl’. sci_matplot.c 84

Что-то не то посчитали

int C2F(pmatj)
  (char *fname, int *lw, int *j, unsigned long fname_len)
{
  ....
  ix1 = il2   4;
  m2 = Max(m, 1);
  ix1 = il   9   m * n;
  ....
}

Предупреждение PVS-Studio: V519 The ‘ix1′ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2387, 2389. stack1.c 2389

С переменной ‘ix1′ что-то неладно. Мне кажется, тут какая-то опечатка.

В начале проверка, после этого инициализация

В начале проверка, затем инициализация

Увлекательный фрагмент кода. Необходимо получить некоторые значения, а после этого проверить их. Но получилось напротив.

int sci_Playsound (char *fname,unsigned long fname_len)
{
  ....
  int m1 = 0, n1 = 0;
  ....
  if ( (m1 != n1) && (n1 != 1) ) 
  {
    Scierror(999,_("%s: Wrong size for input argument #%d: ")
                 _("A string expected.n"),fname,1);
    return 0;
  }
  sciErr = getMatrixOfWideString(pvApiCtx, piAddressVarOne,
             &m1,&n1,&lenStVarOne, NULL);
  ....
}

Предупреждения PVS-Studio: V560 A part of conditional expression is always false: (m1 != n1). sci_playsound.c 66; V560 A part of conditional expression is always true: (n1 != 1). sci_playsound.c 66

Переменные m1 и n1 обязаны получить значения при вызове функции getMatrixOfWideString(). Потом эти переменные обязаны быть проверены. Вот только получилось так, что проверка осуществляется до вызова функции getMatrixOfWideString().

В момент проверки переменные m1 и n1 равны 0. Условие «if ( (m1 != n1) && (n1 != 1) )» не выполняется. В итоге, поверка никак не влияет на работу программы.

Итого. Не осуществляется проверка переменных m1 и n1.

Волшебные числа

Воображаемый друг

void CreCommon(f,var)
     FILE *f;
     VARPTR var;
{
  ....
  if ( strncmp(var->fexternal, "cintf", 4)==0 )
  ....
}

Предупреждение PVS-Studio: V666 Consider inspecting third argument of the function ‘strncmp’. It is possible that the value does not correspond with the length of a string which was passed with the second argument. crerhs.c 119

Применяется магическое число 4. И это число неправильное. В строке «cintf» пять символов, а не четыре. Не используйте такие волшебные числа.

Я бы сделал особый макрос для вычисления длины строковых литералов и применял бы его так:

if ( strncmp(var->fexternal, "cintf", litlen("cintf"))==0 )

Как изготовить макрос ‘litlen’, обговаривать не будем. Есть масса методов на всякий вкус. Основное, избавиться от числа.

Другие неправильные размеры строк:

  • crerhs.c 121
  • crerhs.c 123
  • crerhs.c 125
  • crerhs.c 127

1, 2, 3, 4, 4, 6

Массив, дырка.

int C2F(run)(void)
{
  ....
  static int *Lpt = C2F(iop).lpt - 1;
  ....
  Lpt[1] = Lin[1   k];
  Lpt[2] = Lin[2   k];
  Lpt[3] = Lin[3   k];
  Lpt[4] = Lin[4   k];
  Lct[4] = Lin[6   k ];
  Lpt[6] = k;
  ....
}

Предупреждение PVS-Studio: V525 The code containing the collection of similar blocks. Check items ’1′, ’2′, ’3′, ’4′, ’4′ in lines 1005, 1006, 1007, 1008, 1009. run.c 1005

Опечатка в последовательности чисел. В итоге один элемент массива останется неинициализированным. Дозволено массу увлекательных математических итогов получить.

Эволюция кода

Эволюция

int write_xml_states(
  int nvar, const char * xmlfile, char **ids, double *x)
{
  ....
  FILE *fd = NULL;
  ....
  wcfopen(fd, (char*)xmlfile, "wb");
  if (fd < 0)
  {
    sciprint(_("Error: cannot write to  '%s'  n"), xmlfile);
    ....
}

Предупреждение PVS-Studio: V503 This is a nonsensical comparison: pointer < 0. scicos.c 5826

Я примерно уверен, что когда-то в этом коде для открытия файла применялась функция open. После этого, код переписали и стали применять функцию на _wfopen. Её вызов спрятан в макрос ‘wcfopen’.

А вот проверку, что файл удачно открыт, исправить позабыли. Функция open() возвращает в случае ошибки значение -1. Проверять же, что указатель поменьше нуля, не имеет никакого утилитарного смысла.

Ещё одно место, где прослеживается история.

void taucs_ccs_genmmd(taucs_ccs_matrix* m,
  int** perm, int** invperm)
{
  int  n, maxint, delta, nofsub;
  ....
  maxint = 32000;
  assert(sizeof(int) == 4);
  maxint = 2147483647; /* 2**31-1, for 32-bit only! */
  ....
}

Предупреждение 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 = 32000;». После этого ниже возникло:

assert(sizeof(int) == 4);
maxint = 2147483647; /* 2**31-1, for 32-bit only! */

Сортируем один элемент

Сортируем один элемент

char *getCommonPart(char **dictionary, int sizeDictionary)
{
  ....
  char *currentstr = dictionary[0];
  qsort(dictionary, sizeof dictionary / sizeof dictionary[0],
        sizeof dictionary[0], cmp);
  ....
}

Предупреждение PVS-Studio: V514 Dividing sizeof a pointer ‘sizeof dictionary’ by another value. There is a probability of logical error presence. getcommonpart.c 76

2-й довод функции qsort() — это число элементов в массиве. Из-за ошибки, число элементов неизменно равно одному.

Разглядим выражение «sizeof dictionary / sizeof dictionary[0]». Тут размер указателя делится на размер указателя. Итог равен единице.

Вероятно, верный код должен был быть таким:

qsort(dictionary, sizeDictionary, sizeof dictionary[0], cmp);

Аналогичная оплошность тут: getfilesdictionary.c 105

Упрямые строки

void GetenvB(char *name, char *env, int len)
{
  int ierr = 0, one = 1;
  C2F(getenvc)(&ierr,name,env,&len,&one);
  if (ierr == 0) 
  {
    char *last = &env[len-1];
    while ( *last == ' ' ) { last = '' ; } 
    last--;
  }
  ....
}  

V527 It is odd that the ” value is assigned to ‘char’ type pointer. Probably meant: *last = ”. getenvb.c 24

Эта строка страшна. Либо очаровательна, если мы говорим об увлекательных ошибках.

while ( *last == ' ' ) { last = '' ; }

Если 1-й символ в строке пробел, то указатель станет равен нулю. Дальше возникнет обращение по нулевому указателю.

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

while ( *last == ' ' ) { *last   = '' ; }

Комично, что есть ещё одно место в коде, где тоже хотят менять пробелы на нули. И его тоже не удалось написать верно.

static int msg_101(int *n, int *ierr)
{
  ....
  for (i=0;i<(int)strlen(line);i  )
  {
    if (line[i]==' ') line[i]='';
    break;
  }
  ....
}

Предупреждение PVS-Studio: V612 An unconditional ‘break’ within a loop. msgs.c 1293

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

Схожие «одноразовые» циклы:

  • V612 An unconditional ‘break’ within a loop. msgs.c 1313
  • V612 An unconditional ‘break’ within a loop. api_common.cpp 1407

Разыменовывание нулевого указателя

Разыменовывание нулевого указателя

char **splitLineCSV(....)
{
  ....
  if (retstr[curr_str] == NULL)
  {
    *toks = 0;
    FREE(substitutedstring);
    substitutedstring = NULL;
    freeArrayOfString(retstr, strlen(substitutedstring));
    return NULL;
  }
  ....
}

Предупреждение PVS-Studio: V575 The null pointer is passed into ‘strlen’ function. Inspect the first argument. splitline.c 107

Необычный код. В начале очевидно обнулили указатель ‘substitutedstring’. После этого, отдали его на растерзание в функцию strlen().

Скорее каждого, вызов функции freeArrayOfString() должен быть размещен выше, чем вызов FREE().

Это была разминка. Сейчас разглядим больше трудный случай.

inline static void create(void * pvApiCtx, const int position,
  const int rows, const int cols, long long * ptr)
{
  int * dataPtr = 0;
  alloc(pvApiCtx, position, rows, cols, dataPtr);
  for (int i = 0; i < rows * cols; i  )
  {
    dataPtr[i] = static_cast<int>(ptr[i]);
  }
}

V522 Dereferencing of the null pointer ‘dataPtr’ might take place. scilababstractmemoryallocator.hxx 222

В этой функции хотят выделить память, применяя функцию alloc(). Может показаться, что функция возвращает значение по ссылке. Последним доводом является указатель ‘dataPtr’. Кажется, что в него и будет записан указатель на выделенный буфер памяти.

Но это не так. Указатель останется равен нулю. Давайте посмотрим, как объявлена функция alloc():

inline static int *alloc(
  void * pvApiCtx, const int position, const int rows,
  const int cols, int * ptr)

Видите, конечный довод не является ссылкой. Кстати, вообще не ясно, для чего он необходим. Заглянем вовнутрь функции alloc():

inline static int *alloc(
  void * pvApiCtx, const int position, const int rows,
  const int cols, int * ptr)
{
  int * _ptr = 0;
  SciErr err = allocMatrixOfInteger32(
    pvApiCtx, position, rows, cols, &_ptr);
  checkError(err);
  return _ptr;
}

Конечный довод ‘ptr’ вообще не применяется.

В любом случае, код выделения памяти неверен. Код должен быть таким:

inline static void create(void * pvApiCtx, const int position,
  const int rows, const int cols, long long * ptr)
{
  int *dataPtr = alloc(pvApiCtx, position, rows, cols, 0);
  for (int i = 0; i < rows * cols; i  )
  {
    dataPtr[i] = static_cast<int>(ptr[i]);
  }
}

Схожие обстановки:

  • scilababstractmemoryallocator.hxx 237
  • scilababstractmemoryallocator.hxx 401

Неправильные сообщения об ошибках

Анализатор PVS-Studio Зачастую находит опечатки в обработчиках ошибок. Данный код редко выполняется, и ошибки подолгу остаются незамеченными. Я думаю, из-за таких ошибок мы Зачастую не можем осознать, что же с программой не так. Выданное программой диагностическое сообщение не соответствует реальности.

Пример неправильного образования сообщения об ошибке:

static SciErr fillCommonSparseMatrixInList(....)
{
  ....
  addErrorMessage(&sciErr, API_ERROR_FILL_SPARSE_IN_LIST,
   _("%s: Unable to create list item #%d in Scilab memory"),
   _iComplex ? "createComplexSparseMatrixInList" :
               "createComplexSparseMatrixInList",
   _iItemPos   1);
  ....
}

Сообщение PVS-Studio: V583 The ‘?:’ operator, regardless of its conditional expression, always returns one and the same value: «createComplexSparseMatrixInList». api_list.cpp 2398

В автономности от значения переменной ‘_iComplex’, неизменно будет распечатано «createComplexSparseMatrixInList».

Подобно:

  • api_list.cpp 2411
  • api_list.cpp 2418
  • api_list.cpp 2464
  • api_list.cpp 2471

Сейчас разглядим обработчик ошибки, тот, что никогда не получит управление:

#define __GO_FIGURE__ 9
#define __GO_UIMENU__ 21
int sci_uimenu(char *fname, unsigned long fname_len)
{
  ....
  if (iParentType == __GO_FIGURE__ &&
      iParentType == __GO_UIMENU__)
  {
    Scierror(999, _("%s: Wrong type for input argument #%d: ")
             _("A '%s' or '%s' handle expected.n"), 
             fname, 1, "Figure", "Uimenu");
    return FALSE;
  }
  ....
}

Предупреждение PVS-Studio: V547 Expression ‘iParentType == 9 && iParentType == 21′ is always false. Probably the ‘||’ operator should be used here. sci_uimenu.c 99

Условие (iParentType == __GO_FIGURE__ && iParentType == __GO_UIMENU__) никогда не выполняется. Переменная не может быть единовременно равна числу 9 и числу 21. Я думаю, тут хотели написать так:

if (iParentType != __GO_FIGURE__ &&
    iParentType != __GO_UIMENU__)

Ещё один, исключительно приторный пример.

int set_view_property(....)
{
  BOOL status = FALSE;
  ....
  status = setGraphicObjectProperty(
    pobjUID, __GO_VIEW__, &viewType, jni_int, 1);

  if (status = TRUE)
  {
    return SET_PROPERTY_SUCCEED;
  }
  else
  {
    Scierror(999, _("'%s' property does not exist ")
      _("for this handle.n"), "view");
    return  SET_PROPERTY_ERROR ;
  }
  ....
}

Предупреждение PVS-Studio: V559 Suspicious assignment inside the condition expression of ‘if’ operator: status = 1. set_view_property.c 61

Оплошность тут: «if (status = TRUE)». Взамен сопоставления, происходит присваивание.

Неимение выбора

Отсутствие выбора

Функция, которую дозволено очевидно сократить. Видимо, она написана с поддержкой Copy-Paste, и в скопированном коде что-то позабыли исправить.

static int uf_union  (int* uf, int s, int t) {
  if (uf_find(uf,s) < uf_find(uf,t)) 
  {
    uf[uf_find(uf,s)] = uf_find(uf,t); 
    return (uf_find(uf,t)); 
  }
  else
  {
    uf[uf_find(uf,s)] = uf_find(uf,t); 
    return (uf_find(uf,t)); 
  }
}

Предупреждение PVS-Studio: V523 The ‘then’ statement is equivalent to the ‘else’ statement. taucs_scilab.c 700

Самостоятельно от данные выполняются одинаковые действия.

Сейчас иная обстановка. Тут совпадают данные:

int sci_xset( char *fname, unsigned long fname_len )
{
  ....
  else if ( strcmp(cstk(l1), "mark size") == 0)
  ....
  else if ( strcmp(cstk(l1), "mark") == 0)  
  ....
  else if ( strcmp(cstk(l1), "mark") == 0)
  ....
  else if ( strcmp(cstk(l1), "colormap") == 0)
  ....
}

Предупреждение PVS-Studio: V517 The use of ‘if (A) {…} else if (A) {…}’ pattern was detected. There is a probability of logical error presence. Check lines: 175, 398. sci_xset.c 175

Есть ещё несколько неправильных условий:

  • sci_xset.c 159
  • h5_readdatafromfile_v1.c 1148
  • h5_readdatafromfile.c 1010

Классика

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

static void appendData(....)
{
  ....
  sco_data *sco = (sco_data *) * (block->work);
  int maxNumberOfPoints = sco->internal.maxNumberOfPoints;
  int numberOfPoints = sco->internal.numberOfPoints;

  if (sco != NULL && numberOfPoints >= maxNumberOfPoints)
  ....
}  

Предупреждение PVS-Studio: V595 The ‘sco’ pointer was utilized before it was verified against nullptr. Check lines: 305, 311. canimxy3d.c 305

В начале, осуществляли доступ к членам, применяя указатель ‘sco’:

int maxNumberOfPoints = sco->internal.maxNumberOfPoints;
int numberOfPoints = sco->internal.numberOfPoints;

А потом внезапно припомнили, что данный указатель нужно проверить:

if (sco != NULL .....

Анализатор выдал ещё 61 одно предупреждение V595. Перечислять их в статье не вижу смысла. Привожу их отдельным списком: scilab-v595.txt.

Ещё одна распространённая обстановка — применение неправильных спецификаторов формата (format specifiers) при работе с функцией sprintf() и схожих ей. Примерно всё, что нашлось, не увлекательно. Печатаем беззнаковые значения, как знаковые. Следственно привожу все эти предупреждения тоже списком:scilab-v576.txt.

Из увлекательного дозволено подметить только вот это:

#define FORMAT_SESSION "%s%s%s"
char *getCommentDateSession(BOOL longFormat)
{
  ....
  sprintf(line, FORMAT_SESSION, SESSION_PRAGMA_BEGIN,
          STRING_BEGIN_SESSION, time_str, SESSION_PRAGMA_END);
  ....
}

Предупреждение PVS-Studio: V576 Incorrect format. A different number of actual arguments is expected while calling ‘sprintf’ function. Expected: 5. Present: 6. getcommentdatesession.c 68

Не будет распечатана строка SESSION_PRAGMA_END.

Осмотрительно, неопределённое поведение

Осторожно, неопределённое поведение. Это С  !

short ezxml_internal_dtd(ezxml_root_t root, char *s, size_t len)
{
  ....
  while (*(n =   s   strspn(s, EZXML_WS)) && *n != '>') {
  ....
}

Предупреждение PVS-Studio: V567 Undefined behavior. The ‘s’ variable is modified while being used twice between sequence points. ezxml.c 385

Неведомо, будет вычислено в начале выражение ” s’ либо выражение «strspn(s, EZXML_WS)». Соответственно, итог может отличаться на различных компиляторах, платформах и так дальше.

Иной, больше увлекательный случай. Тут неопределённое поведение появляется из-за опечатки.

static char **replaceStrings(....)
{
  ....
  int i = 0;
  ....
  for (i = 0; i < nr; i = i  )
  ....
}

V567 Undefined behavior. The ‘i’ variable is modified while being used twice between sequence points. csvread.c 620

Напасть тут: i = i .

По каждой видимости, хотел написать так:

for (i = 0; i < nr; i  )

Ещё о строках

char *PLD_strtok(....)
{
  ....
  if ((st->start)&&(st->start != ''))
  ....
}

Предупреждение PVS-Studio: V528 It is odd that pointer to ‘char’ type is compared with the ” value. Probably meant: *st->start != ”. pldstr.c 303

Хотели проверить, что строка не пустая. Но на самом деле указатель два раза сравнивается с NULL. Верный код:

if ((st->start)&&(st->start[0] != ''))

Подобный ляп:

V528 It is odd that pointer to ‘char’ type is compared with the ” value. Probably meant: ** category == ”. sci_xcospalload.cpp 57

Дальнейший код по каждой видимости недописан:

int sci_displaytree(char *fname, unsigned long fname_len)
{
  ....
  string szCurLevel = "";
  ....
  //Add node level
  if (szCurLevel != "")
  {
    szCurLevel   ".";
  }
  ....
}

Предупреждение PVS-Studio: V655 The strings was concatenated but are not utilized. Consider inspecting the ‘szCurLevel “.”‘ expression. sci_displaytree.cpp 80

Код, тот, что работает вследствие везению

Везение

static int sci_toprint_two_rhs(void* _pvCtx,
                               const char *fname)
{
  ....
  sprintf(lines, "%s%sn", lines, pStVarOne[i]);
  ....
}

Предупреждение PVS-Studio: V541 It is dangerous to print the string ‘lines’ into itself. sci_toprint.cpp 314

Функция sprintf() сберегает итог своей работы в буфер ‘lines’. При этом, данный же буфер является одной из входных строк. Так делать не отлично. Код абсолютно может трудиться. Но это небезопасно. При смене компилятора дозволено получить непредвиденный и малоприятный итог.

Аналогичная обстановка: sci_coserror.c 94

Пример кода, тот, что работает, правда и не правилен:

typedef struct JavaVMOption {
    char *optionString;
    void *extraInfo;
} JavaVMOption;

JavaVMOption *options;

BOOL startJVM(char *SCI_PATH)
{
  ....
  fprintf(stderr, "%d: %sn", j, vm_args.options[j]);
  ....
}

Предупреждение PVS-Studio: V510 The ‘fprintf’ function is not expected to receive class-type variable as fourth actual argument. jvm.c 247

Тут хотели распечатать строку, на которую ссылается указатель ‘optionString’. Прав

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB