👨🏾‍🤝‍👨🏽 👋🏼 ⚗️ 数学ソフトウェアの使用による頭痛 🐇 📄 🤫

たまたま、インターネット上で、一見異なるトピック、つまり大学と学生向けの無料のMatlabの代替案、および静的コード分析を使用したアルゴリズムのエラーの発見について話していました。これらの議論はすべて、現代のプログラムのコードのひどい品質を組み合わせています。特に、数学者や科学者向けのソフトウェアの品質。問題は、そのようなプログラムの助けを借りて実行された計算と研究への信頼から即座に生じます。このトピックを振り返り、エラーを探してみましょう。

はじめに

「アルゴリズム」という用語を定義することから始めたいと思います。アルゴリズムは、特定の結果を達成するためのエグゼキューターのアクションの順序を説明する一連の命令です（ウィキペディア）。したがって、ソースコードをアルゴリズムと残りのコードに分割しないでください。たとえば、ソートアルゴリズムは、ファイルを開く、文字列内の文字を検索するなどのコードとまったく同じです。コードにはエラーが含まれている可能性があり、幸いなことに、静的コード分析のツールを使用して、初期段階で多くのエラーを検出できます。

ただし、いわゆる「アルゴリズム」エラーを検索するために、いくつかの数学的パッケージのコードを分析することにしました。このようなコードには、いくつかの数式が単純にプログラムされる多くの機能があります。コードについてそう考えていない人もいることがわかります。そして、それに応じて、どのようなエラーが発生する可能性があります。

この記事に記載されているすべてのコードの欠陥を特定するために、C / C ++ / C＃プログラミング言語のWindows / Linuxで実行されるPVS-Studio静的アナライザーバージョン6.15を使用しました。

サードパーティからのエラー

この話は、 Point Cloud Library （PCL、 GitHub ）プロジェクトでバグを見つけることから始まりました。多くの間違いを見つけて記事を書くという目標を自分自身に設定せずに、レポートを調べたところ、非常に興味深いエラーが見つかりました。

V533 「for」演算子内で誤った変数がインクリメントされている可能性があります。「i」の検討を検討してください。 sparsematrix.inl 212

template<class T> SparseMatrix<T>& SparseMatrix<T>::operator *= (const T& V) { for( int i=0 ; i<rows ; i++ ) for( int ii=0 ; ii<rowSizes[i] ; i++ ) m_ppElements[i][ii].Value *= V; return *this; }

オーバーロードされた演算子 "* ="では、すべての行列要素に特定のV値を乗算します。コードの作成者は、このアルゴリズムに対して非常に重大な誤りを犯しました。その理由は、行列の最初の列のみが変化し、配列の境界を超える無限ループも発生する可能性があるためです。

このコードは、 Poisson Surface Reconstruction数学ライブラリからのものです。エラーがコードの最新バージョンに存在することを確認しました。このライブラリがさらに多くのプロジェクトに含まれることを考えるのは怖いです。

次に、もう1つの奇妙なコードを示します。

V607所有者のない表現「j <残り」。 allocator.h 120

 void rollBack(const AllocatorState& state){ .... if(state.index<index){ .... for(int j=0;j<remains;j++){ memory[index][j].~T(); new(&memory[index][j]) T(); } index=state.index; remains=state.remains; } else{ for(int j=0;j<state.remains;j<remains){ // <= memory[index][j].~T(); new(&memory[index][j]) T(); } remains=state.remains; } .... }

この奇妙なループはまだコードに残っているため、頻繁に実行されることはないと思います。しかし、おそらくプログラムの異常終了により、誰かが奇妙なハングアップを起こしました。コードの品質に関するいくつかのアイデアが形成されました。それでは、より大きなプロジェクトであるScilabに移りましょう。Scilabでは、頭痛の種が待っています。

サイラブ

プロジェクトについて

Scilabは、工学（技術）および科学計算のためのオープンな環境を提供する、応用数学プログラムのパッケージです。この開発環境は、さまざまな機関や研究で広く使用されているMatlabの公に利用可能な代替手段の1つです。 Matlabのもう1つの人気のある代替手段はGNU Octaveであり、以前はこれらのプロジェクトに既に注意を払っていました。

Scilabチェック（2014年3月）;
GNU Octave （2015年8月）を確認してください。

Scilabに関する新しい記事を書く前に、古い記事を読んで、2つの結論だけを出しました。

3年間、彼らはほんの2、3の場所を修正しませんでした（「すべてがそのように動作するのに、なぜ不明確な動作を修正するのですか？」-開発者は考えたようです）。
プロジェクトには多くの新しいバグがあります。読者を飽きさせないように、記事に数ダースだけ入れることにしました。

Visual StudioのプロジェクトファイルはScilabのソースにすぐに存在するので、ワンクリックでプロジェクトを開いて確認できます。

美しいタイプミス

V530関数 'back'の戻り値を使用する必要があります。 sci_mscanf.cpp 274

 types::Function::ReturnValue sci_mscanf(....) { .... std::vector<types::InternalType*> pITTemp = std::vector<...>(); .... case types::InternalType::ScilabString : { .... pITTemp.pop_back(); // <= pITTemp.push_back(pType); } break; case types::InternalType::ScilabDouble : { .... pITTemp.back(); // <= ??? pITTemp.push_back(pType); } break; .... }

コード補完がプログラマーのトリックを演じたようです。コードでは、 sci_mscanf関数は常にベクトルの最後の要素を削除してから新しい要素を追加しますが、プログラマーはpop_back（）の代わりにback（）関数を呼び出すことでミスを犯しました。このようにback（）関数を呼び出しても意味がありません。

V595 nullptrに対して検証される前に、「Block.inptr」ポインターが使用されました。行を確認してください：478、479。sci_model2blk.cpp 478

 types::Function::ReturnValue sci_model2blk(....) { .... Block.inptr[i] = MALLOC(size); if (Block.inptr == nullptr) { freeBlock(&Block); Scierror(888, _("%s : Allocation error.\n"), name.data()); return types::Function::Error; } memset(Block.inptr[i], 0x00, size); .... }

タイプミスの非常に興味深いケースで、メモリの割り当ての制御が機能しなくなったためです。ほとんどの場合、正しいコードは次のようになります。

 Block.inptr[i] = MALLOC(size); if (Block.inptr[i] == nullptr) { .... }

V595 nullptrに対して検証される前に、「pwstLines」ポインターが使用されました。行をチェック：78、79。mgetl.cpp 78

 int mgetl(int iFileID, int iLineCount, wchar_t ***pwstLines) { *pwstLines = NULL; .... *pwstLines = (wchar_t**)MALLOC(iLineCount * sizeof(wchar_t*)); if (pwstLines == NULL) { return -1; } .... }

驚くほど似たエラー。コードの作成者は星をカウントしなかったため、条件で間違ったポインターがチェックされます。

V595 nullptrに対して検証される前に、「array_size」ポインターが使用されました。行をチェック：67、68。diary_manager.cpp 67

 wchar_t **getDiaryFilenames(int *array_size) { *array_size = 0; if (SCIDIARY) { std::list<std::wstring> wstringFilenames = SCIDIARY->get.... *array_size = (int)wstringFilenames.size(); if (array_size > 0) { .... } .... }

安定性は習熟の兆候です。プログラマーは再びポインターを逆参照するのを忘れました。これは、ある配列のサイズがゼロと比較されるのではなく、この変数へのポインターが比較される理由です。

V501 「||」の左側と右側に同一の副次式「strncmp（tx、 "％pi"、3）== 0」があります。演算子。 stringtocomplex.c 276

 static int ParseNumber(const char* tx) { .... else if (strlen(tx) >= 4 && (strncmp(tx, "%eps", 4) == 0 || strncmp(tx, "+%pi", 4) == 0 || strncmp(tx, "-%pi", 4) == 0 || strncmp(tx, "+Inf", 4) == 0 || strncmp(tx, "-Inf", 4) == 0 || strncmp(tx, "+Nan", 4) == 0 || strncmp(tx, "-Nan", 4) == 0 || strncmp(tx, "%nan", 4) == 0 || strncmp(tx, "%inf", 4) == 0 )) { return 4; } else if (strlen(tx) >= 3 && (strncmp(tx, "+%e", 3) == 0 || strncmp(tx, "-%e", 3) == 0 || strncmp(tx, "%pi", 3) == 0 // <= || strncmp(tx, "Nan", 3) == 0 || strncmp(tx, "Inf", 3) == 0 || strncmp(tx, "%pi", 3) == 0)) // <= { return 3; } .... }

この関数には、数値を解析するためのコードが含まれています。アナライザーは、「％pi」の2つの同一行と疑わしい比較を見つけました。隣接するコードを見ると、重複した行の代わりに、「-％pi」または「-Inf」という行があると想定できます。これは余分なコピーされたコード行である可能性もありますが、削除する方が適切です。

操作の優先順位

V502おそらく、「?:」演算子は予想とは異なる方法で動作します。「？：」演算子の優先順位は、「==」演算子よりも低くなっています。 sci_sparse.cpp 49

 types::Function::ReturnValue sci_sparse(....) { bool isValid = true; .... for (int i = 0 ; isValid && i < in.size() ; i++) { switch (in[i]->getType()) { case types::InternalType::ScilabBool : case types::InternalType::ScilabSparseBool : { isValid = (i == (in.size() > 1) ? 1 : 0); } .... }

操作の優先順位に関するエラーは、最新のコードでは非常に一般的です。（記事「 C / C ++の論理式。専門家が間違える方法」を参照）。

上記のコードスニペットにもエラーがありますが、非常に幸運なため、エラーコードはプログラマーが期待したとおりに機能します。インデックス0と1を持つ配列要素が比較に関与し、真理と偽りの整数表現も値0と1であるという事実だけが原因で、このコードは奇跡的にまだ正しく機能しています。

コードを操作の正しい優先度に書き換える必要があります。

 isValid = (i == (in.size() > 1 ? 1 : 0));

V590 「iType！=-1 && iType == 8」式の検査を検討してください。表現が過剰であるか、誤植が含まれています。 scilabview.cpp 175

 void ScilabView::createObject(int iUID) { int iType = -1; int *piType = &iType; getGraphicObjectProperty(....); if (iType != -1 && iType == __GO_FIGURE__) { m_figureList[iUID] = -1; setCurrentFigure(iUID); } .... }

このフラグメントには、操作の優先順位に関するエラーが含まれています。これは、以前に提案された記事でも検討されています。

条件付き部分式（iType！= -1）は、条件式全体の結果には影響しません。この例の真理値表を作成することにより、エラーを検証できます。

別のそのような例：

V590「iObjectType！=-1 && iObjectType == 5」式の検査を検討してください。表現が過剰であるか、誤植が含まれています。 sci_unglue.c 90

誤ったエラーメッセージ

Scilabのエラーに関する以前の記事では、メッセージを印刷する際のエラーに関する小さなセクションもありました。また、新しいタイプのコードには、このタイプのエラーが非常に多くありました。

V517 「if（A）{...} else if（A）{...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。行を確認してください：159、163。cdfbase.c 159

 void cdf_error(char const* const fname, int status, double bound) { switch (status) { .... case 10: if (strcmp(fname, "cdfchi") == 0) // <= { Scierror(999 _("%s: cumgam returned an error\n"), fname); } else if (strcmp(fname, "cdfchi") == 0) // <= { Scierror(999, _("%s: gamma or inverse gamma routine failed\n"), fname); } break; .... }

Scilabには多数のcdf関数があります。提示されたコードフラグメントでは、これらの関数のリターンコードの解釈が実行されます。そして、ここに問題があります-関数名のタイプミスにより、何らかのエラー警告が表示されることはありません。この投稿を検索すると、 cdfgam関数につながります。この機能を使用していて、数学パッケージの作者のタイプミスのためにいくつかの問題を見つけることができなかったユーザーに同情したいと思います。

V510 'Scierror'関数は、3番目の実引数としてクラス型変数を受け取ることを期待されていません。 sci_winqueryreg.cpp 149

 const std::string fname = "winqueryreg"; types::Function::ReturnValue sci_winqueryreg(....) { .... if (rhs != 2 && rhs != 3) { Scierror(77, _("%s: Wrong number...\n"), fname.data(), 2, 3); return types::Function::Error; } .... else { Scierror(999, _("%s: Cannot open Windows regist..."), fname); return types::Function::Error; } .... }

1か所で行を印刷するときに、 data（）メソッドを呼び出すのを忘れていました。

V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 48

 int sci_scinotes(char * fname, void* pvApiCtx) { .... try { callSciNotesW(NULL, 0); } catch (GiwsException::JniCallMethodException exception) { Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str()); } catch (GiwsException::JniException exception) { Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str()); } .... }

値によって例外がキャッチされました。これは、コピーコンストラクタの助けを借りて新しいオブジェクトが構築され、例外に関する情報の一部が失われることを意味します。正しいオプションは、参照によって例外をキャッチすることです。

そのような場所がいくつかあります。

V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_builddoc.cpp 270
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_closescinotesfromscilab.cpp 45
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_closescinotesfromscilab.cpp 50
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 52
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 263
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 272
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 349
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 353
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 365
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 sci_scinotes.cpp 369
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 visitor_common.cpp 1743
V746タイプのスライス。例外は、値ではなく参照によってキャッチする必要があります。 overload.cpp 135

奇妙なコード

奇妙なコード。なぜこのように書くべきか、どうすればそれをより良く修正するかが明確ではないからです。

V523 「then」ステートメントは「else」ステートメントと同等です。 data3d.cpp 51

 void Data3D::getDataProperty(int property, void **_pvData) { if (property == UNKNOWN_DATA_PROPERTY) { *_pvData = NULL; } else { *_pvData = NULL; } }

常にポインターをゼロにする単純な関数を次に示します。

V575 「memset」機能は「0」要素を処理します。 3番目の引数を調べます。 win_mem_alloc.c 91

 void *MyHeapAlloc(size_t dwSize, char *file, int line) { LPVOID NewPointer = NULL; if (dwSize > 0) { _try { NewPointer = malloc(dwSize); NewPointer = memset (NewPointer, 0, dwSize); } _except (EXCEPTION_EXECUTE_HANDLER) { } .... } else { _try { NewPointer = malloc(dwSize); NewPointer = memset (NewPointer, 0, dwSize); } _except (EXCEPTION_EXECUTE_HANDLER) { } } return NewPointer; }

dwSize変数の値に関係なく、同じコードが常に実行されます。なぜそれを複製するのですか？

V695範囲の交差は条件式内で可能です。例：if（A <5）{...} else if（A <2）{...}。行を確認してください：438、442。sci_sorder.c 442

 int sci_sorder(char *fname, void* pvApiCtx) { .... if (iRows * iCols > 0) { dblTol1 = pdblTol[0]; } else if (iRows * iCols > 1) { dblTol2 = pdblTol[1]; } .... }

EXPR> 0の場合、 EXPR> 1のチェックは意味をなさないため、2番目の条件は常にfalseです。このコードには明らかに何らかのエラーがあります。

ヌルポインターの逆参照と未定義の動作

V522 NULLポインター「dataz」の逆参照が行われる場合があります。 polylinedata_wrap.c 373

 BOOL translatePolyline(int uid, double x, double y, double z, int flagX, int flagY, int flagZ) { double *datax = NULL; double *datay = NULL; double *dataz = NULL; // <= int i = 0; if (x != 0.0) { datax = getDataX(uid); if (datax == NULL) return FALSE; .... if (z != 0 && isZCoordSet(uid)) { if (flagZ) { for (i = 0; i < getDataSize_(uid); ++i) { dataz[i] = pow(10.,log10(dataz[i]) + z); // <= } } else { for (i = 0; i < getDataSize_(uid); ++i) { dataz[i] += z; // <= } } } return TRUE; }

datax 、 datay 、およびdatazの配列があります。後者はどこでも初期化されませんが、特定の条件下で使用されます。

V595 nullptrに対して検証される前に、「番号」ポインターが使用されました。行をチェックしてください：410、425。scilab_sscanf.cpp 410

 int scilab_sscanf(....) { .... wchar_t* number = NULL; .... number = (wchar_t*)MALLOC((nbrOfDigit + 1) * sizeof(wchar_t)); memcpy(number, wcsData, nbrOfDigit * sizeof(wchar_t)); number[nbrOfDigit] = L'\0'; iSingleData = wcstoul(number, &number, base); if ((iSingleData == 0) && (number[0] == wcsData[0])) { .... } if (number == NULL) { wcsData += nbrOfDigit; } else { wcsData += (nbrOfDigit - wcslen(number)); } .... }

メモリーはmalloc（）関数を使用して番号行の下に割り当てられましたが、ポインターをチェックする前に数回参照解除され、 memcpy（）関数に引数として渡されますが、これは受け入れられません。

V595 nullptrに対して検証される前に、「OuputStrings」ポインターが使用されました。行を確認：271、272。spawncommand.c 271

 char **CreateOuput(pipeinfo *pipe, BOOL DetachProcess) { char **OuputStrings = NULL; .... OuputStrings = (char**)MALLOC((pipe->NumberOfLines) * ....); memset(OuputStrings, 0x00,sizeof(char*) * pipe->NumberOfLines); if (OuputStrings) { char *line = strtok(buffer, LF_STR); int i = 0; while (line) { OuputStrings[i] = convertLine(line, DetachProcess); .... }

ここでは、動的メモリがOuputStrings変数に割り当てられていますが、このポインターをチェックする前に、割り当てられたメモリはmemset（）関数を使用してリセットされるため、これを行うことはできません。関数のドキュメントからの引用：「 'dest'がNULLポインターの場合の動作は未定義です 」。

メモリリークと閉じられていないリソース

V611メモリーは「new T []」演算子を使用して割り当てられましたが、「delete」演算子を使用して解放されました。このコードを調べることを検討してください。「delete [] piP;」を使用する方がおそらく良いでしょう。 sci_grand.cpp 990

V611メモリーは「new T []」演算子を使用して割り当てられましたが、「delete」演算子を使用して解放されました。このコードを調べることを検討してください。「delete [] piOut;」を使用することをお勧めします。 sci_grand.cpp 991

 types::Function::ReturnValue sci_grand(....) { .... int* piP = new int[vectpDblInput[0]->getSize()]; int* piOut = new int[pDblOut->getSize()]; .... delete piP; delete piOut; .... }

ここでは、2つの重大な間違いがありました。配列に動的メモリを割り当てた後、 delete []演算子、つまり角かっこ付き。

V773 'doc'ポインターを解放せずに関数が終了しました。メモリリークが発生する可能性があります。 sci_builddoc.cpp 263

 int sci_buildDoc(char *fname, void* pvApiCtx) { .... try { org_scilab_modules_helptools::SciDocMain * doc = new .... if (doc->setOutputDirectory((char *)outputDirectory.c_str())) { .... } else { Scierror(999, _("...."), fname, outputDirectory.c_str()); return FALSE; // <= } if (doc != NULL) { delete doc; } } catch (GiwsException::JniException ex) { Scierror(....); Scierror(....); Scierror(....); return FALSE; } .... }

状況によっては、 ドキュメントポインターをクリアせずに関数が終了します。また、 docポインターをNULLと比較することは正しくありません。 new演算子がメモリの割り当てに失敗した場合、 NULLを返すのではなく、例外をスローします。

これは、Scilabプロジェクトで見つかったメモリリークの最も明らかな例です。彼らはメモリを解放することを計画していることがわかりますが、ある場所ではそれをするのを忘れていました。

一般に、プロジェクトで多くのメモリリークが見つかりました。ポインタは単純にクリアされず、どこにも保存されません。なぜなら私はScilab開発者ではありません。そのような場合にエラーが発生する場所と発生しない場所を判断するのは困難です。しかし、私は多くのメモリリークがあると考えています。この数学パッケージのユーザーは、私の言葉を確認できます。

V773 'hProcess'ハンドルの可視性スコープは、リソースを解放せずに終了しました。リソースリークが発生する可能性があります。 killscilabprocess.c 35

 void killScilabProcess(int exitCode) { HANDLE hProcess; /* Ouverture de ce Process avec droit pour le tuer */ hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, ....); if (hProcess) { /* Tue ce Process */ TerminateProcess(hProcess, exitCode); } else { MessageBox(NULL, "....", "Warning", MB_ICONWARNING); } }

リソースリーク。ドキュメントによると、 OpenProcess関数を呼び出した後、 CloseHandle関数を呼び出す必要があります。

おわりに

現時点では、Scilab 6.0.0はScilabの公式Webサイトで安定版と見なされていますが、気づいたように、安定性はここから遠いです。アナライザーはリポジトリから最新バージョンをチェックしましたが、原則として、エラーは非常に長い時間コード内に存在し、おそらく「安定した」バージョンに分類されます。私自身もScilabユーザーでしたが、ずっと前にエラーの数を確認できました。このようなソフトウェアが、数学的な計算にこのようなツールを使用している人々の研究を遅らせないことを願っています。

多くの数学があり、さまざまな研究で需要がある次の実績のあるプロジェクトは、 OpenCVライブラリです。

同僚のアンドレイ・カルポフに注意してください。 この記事のトピックは、私が記事で述べた考えと強く重複しています。

おそらく読者は彼らに精通することに興味があるでしょう。

英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Svyatoslav Razmyslov。数学ソフトウェアの使用による頭痛

記事を読んで質問がありますか？

多くの場合、記事には同じ質問が寄せられます。ここで回答を収集しました： PVS-Studioバージョン2015に関する記事の読者からの質問への回答。リストをご覧ください。

数学ソフトウェアの使用による頭痛