GNU / Linux上のPVS-StudioでVimを検証する

読者は、これはフリーソフトウェアの世界の別のプロジェクトをチェックすることに関する別の記事であると考えるかもしれませんが、実際には、この記事はチェックについてではなく、完全なGNU / Linux環境でPVS-Studioアナライザーを使用する方法についてです。 Vimが検証のためのプロジェクトの選択になったのは偶然ではありません。彼もこの問題に関与したからです。

まず、Vimについて少し

Vim（ http://vim.org ）は、30年の歴史を持つクロスプラットフォームのフリーテキストエディターです。これはviの後継であり、Unixシステムの世界から来ています。

Vimは管理および開発で広く使用されており、多くのGNU / Linuxディストリビューションではデフォルトのエディターです。 Vimは、Vimスクリプトで記述されたプラグインのシステムを介して、キーボード、テキストインターフェイス、および豊富な拡張オプションのみを使用するという点で、他のテキストエディターとは異なります。

テスト自体について

Linuxのプロジェクトをチェックするためのオプションの1つは、GNU Makeなどのビルドシステムへの統合です。 vimをチェックするために、この方法が選択されました。 メイクファイル内の各コンパイラー呼び出しに対して、アナライザー呼び出しが追加されました。 便宜上、この呼び出しは同様の方法でMake変数にラップされました。

#PVS Studio vars PVS_CFLAGS = $(ALL_CFLAGS) PVS_INCFLAGS = -I$(srcdir) PVS_STUDIO = ~/PVS-Studio/PVS-Studio –cfg \ ~/PVS-Studio/PVS-Studio_vim.cfg --source-file \ $< --cl-params $(PVS_CFLAGS) -c $(PVS_INCFLAGS) $< 

次に、makeコマンドを使用して、プロジェクトを通常モードでアセンブルします（必要に応じて、「。analysis」などの分析用に別のターゲットを追加できます）。 結果は、アセンブルされたプロジェクトに加えて、生のアナライザーログです。

ご注意 アナライザーは、プロジェクトの並列アセンブリと並行して実行できます。 アナライザーの実行中の各インスタンスは、メッセージの一部をログに追加します。 したがって、アナライザーは生のログファイルをクリアしないことに注意してください。 したがって、チェックを再開する前に、以前のチェックからログを自分で削除する必要があります。

生ログで作業することは不可能です。 このログには多くの重複メッセージがあります（単一の.hファイルが複数の.cppファイルに含まれている場合）。 分析パラメーターを変更するには、構成ファイルを編集した後、テストを再度実行する必要があります。これは、大規模プロジェクトの時間に深刻な影響を及ぼします。 たとえば、特定のディレクトリのファイルに関連する警告をオフにするだけでもかまいません。 この問題を解決するため、生のPVS-Studioログを処理し、重複するメッセージを削除し、オプションファイルからメッセージにフィルターを適用し、サポートされている形式のいずれかでアナライザーの警告を表示するログパーサーユーティリティがC ++で記述されました。 このユーティリティは非常に高速に動作します（非常に大きなプロジェクトの検証ログでも、その動作時間は2〜3秒を超えません）。これにより、いくつかの分析パラメーターを簡単かつ迅速に変更し、新しいメッセージリストを取得できます。

必要に応じて、新しい出力形式を簡単に追加できます。 現時点では、xmlといわゆるerrorfileの2つがあります。 私の知る限り、正式な名前はありません。これは、Linux用の多くのプログラムがメッセージを出力する形式です。たとえば、grep、gccコンパイルエラーなどです。 便利になったのは彼でした。

大多数の開発者がVisual Studioを使用しているWindowsとは異なり、GNU / Linuxの世界には多くのIDE、テキストエディターなどがあり、ユーザーの割合も多くなっています。 当事者のコンセンサスや優位性はなく、誰もが好みに合った楽器を選択します。 それでも、プロジェクトをチェックするとき、PVS-StudioとVisual Studioの統合がこの機会を提供するため、分析の結果を取得するだけでなく、それらを便利に操作できることも重要です。 上記のエラーメッセージ形式はLinuxプログラムの一種の標準であり、ほとんどのエディターおよび開発環境はこれをサポートしていますが、残念なことに、ほとんどの場合、このサポートはアセンブリ中のstderrからのコンパイラメッセージの読み取りに対してのみ存在します。同じ場合、事前に準備したファイルからアナライザーメッセージを取得する方がはるかに便利です。

これがVimエディターの役に立ちました。 もちろん、他のツールに適したプラグインを開発することもできますが、Vimには最初にこの機能が備わっていました。


図1-分析結果を使用したvimの実行。

アナライザーとログ処理ユーティリティが動作した後にvim -q <errorfile>を実行し、開いたエディターでコマンドを実行してエラーのあるバッファーを作成するだけで十分です。 例：cw20。そして、アナライザーメッセージとコードナビゲーションを操作するための快適な環境をすでに取得しています。 もちろん、Vim自体を研究したことがないため、Vim自体を研究するのに数時間費やさなければなりませんでした。また、Vimの使用原理は、おなじみのテキストエディターとは大きく異なります。 しかし、結局、私はその使用の利便性に非常に染み込んでおり、エイリアンのためのあいまいなギズモのカテゴリーから、仕事のための強力なツールのカテゴリーに移動したと言えます。 したがって、長い間、検証用のプロジェクトを選択することを心配する必要はありませんでした-もちろん、Vim自体です。 vimコードは非常に高品質であることが判明し、明らかなバグはありませんでした（コードの記述スタイルは一部の場所ではかなり議論の余地がありますが、プロジェクトの時代を割り引くことができると思います。 。

追加チェック

  if (ptr == NULL) { if (compl_leader != NULL) ptr = compl_leader; else return; /* nothing to do */ } if (compl_orig_text != NULL) { p = compl_orig_text; for (len = 0; p[len] != NUL && p[len] == ptr[len]; ++len) ; #ifdef FEAT_MBYTE if (len > 0) len -= (*mb_head_off)(p, p + len); #endif for (p += len; *p != NUL; mb_ptr_adv(p)) AppendCharToRedobuff(K_BS); } else len = 0; if (ptr != NULL) AppendToRedobuffLit(ptr + len, -1); 

V595アナライザーの警告 （1）nullptrに対して検証される前に、「ptr」ポインターが使用されました。 行を確認してください：3922、3933。

ptrポインターは既にNULLがチェックされています;この条件が真の場合、comp_leaderポインターが割り当てられていますが、これは間違いなくnullではありません。 2回目のチェックは不要です。

奇妙なmemset

 /* * If requested, store and reset the global values controlling * the exception handling (used when debugging). Otherwise avoid * clear it to a bogus compiler warning when the optimizer * uses inline functions... */ if (flags & DOCMD_EXCRESET) save_dbg_stuff(&debug_saved); else vim_memset(&debug_saved, 0, 1); 

ここで、debug_savedは構造体オブジェクトです

 struct dbg_stuff { int trylevel; int force_abort; except_T *caught_stack; char_u *vv_exception; char_u *vv_throwpoint; int did_emsg; int got_int; int did_throw; int need_rethrow; int check_cstack; except_T *current_exception; }; 

アナライザーメッセージ： V512 （1） 'memset'関数を呼び出すと、バッファー '＆debug_saved'がアンダーフローします。

構造の最初のバイトのみをリセットする必要があった理由を説明するのは困難です。 これを何らかのフラグとして使用する場合、構造体の別のフィールドとして定義することをお勧めします（ユニオンも可能です）。

疑わしいサイクル

 /* check for out-of-memory */ for (i = 0; i < num_names; ++i) { if (names[i] == NULL) { for (i = 0; i < num_names; ++i) vim_free(names[i]); num_names = 0; } } 

アナライザーメッセージ： V535 （1）変数 'i'は、このループと外部ループに使用されています。 行を確認してください：1893、1897。

外側と内側のループでは、同じカウンターiが同じ配列を通過します。 （names [i] == NULL）が最初に条件がトリガーされると、外部ループの次のステップは実行されませんが、サードパーティの人はこのコードを正しく理解するために緊張する必要があり、その記述の独創性は行動を示唆しています著者を意味した。 つまり、エラーはありませんが、コードは少し臭いがします。 おそらく、「break」ステートメントの方がループを停止するのに適しています。

スコープ

 char_u *p, *old; //... { char_u buffer[BUFLEN + 1]; //... for (p = buffer; p < buffer + len; p += l) //... 

アナライザーの警告： V507 （2）ローカル配列「バッファー」へのポインターは、この配列のスコープ外に格納されます。 そのようなポインターは無効になります。

Vimのコードには、似たような場所がたくさんあります（スタイルの問題）。 関数の一番最初に宣言されたポインターp（一般的にグローバルスコープを持ついくつかの場所で）は、より小さなスコープにのみ存在し、コードブロックを離れるときに削除される配列へのポインターを格納します。 簡単な検査中に、バッファーのスコープを出た後、ポインターpは新しい値を割り当てた後にのみ使用されるように思えましたが、これはどこかでスキップできます。 バッファスコープ内で別の変数を宣言するのではなく（実際、スタックのスペースを節約するために）、どのような目的のために、私は理解していません。 そのようなコードはサポートが不十分で、読むのが不快です。

1つの式の符号付き型と符号なし型のエラー

 for (cu = 1; cu <= 255; cu++) if (VIM_ISDIGIT(cu)) regc(cu); 

どこで

 #define VIM_ISDIGIT(c) ((unsigned)(c) - '0' < 10) 

アナライザーの警告： V658 （2）符号なし変数から値が減算されています。 これにより、オーバーフローが発生する可能性があります。 このような場合、「<」比較演算は潜在的に予期しない動作をする可能性があります。 '（unsigned）（cu）-' 0 '<10'式の検査を検討してください。

このコードは、汚いハッキングに似ています。 式を評価するとき（（unsigned）-'0' <10）、減算の結果はunsigned型の値になり、式の両方の部分を比較するときも符号なしに変換されます。 したがって、変数cuが数値0より小さい場合、オーバーフローが発生します。 この場合、コードは正しく動作し、目的を果たします（文字が数字であるかどうかをチェックします）が、コードでこのようなトリックを不必要に使用する理由はないと思います。 ループは「0」から作成し、符号なしで済ますことができます。

ポインターはNULLに初期化され、どこでも変更されず、さらに使用されます

 char_u *retval = NULL; //... if (round == 2) vim_strncpy(retval, s, len); //  retval //... if (retval == NULL) { 

アナライザーの警告： V595 （1）nullptrに対して検証される前に、「retval」ポインターが使用されました。 行を確認してください：7903、7907。

これはすでに間違いのようです。 アナライザーは不必要なチェックについて話しますが、実際には、問題は主に別のものにあります。 retvalポインターは0に初期化され、この関数で値が変わる単一の場所は見つかりませんでした。 同時に、strncpyは何度も実行されます。 その後、彼は突然NULLをチェックすることにしました。

reallocの安全でない使用

 /* TODO: check for vim_realloc() returning NULL. */ l->t = vim_realloc(l->t, newlen * sizeof(nfa_thread_T)); 

V701 （2）アナライザーの警告 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインター 'l-> t'が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。

多くのプロジェクトでよくある間違いであるこのエラーの本質は、アナライザーメッセージで詳しく説明されています。 幸いなことに、コメントから判断すると、これはすぐに修正されます。 Vimコードの他の場所では、reallocが正しく使用されています。

誤検知のいくつかの例

 if (ireg_icombine && len == 0) { /* If \Z was present, then ignore composing characters. * When ignoring the base character this always matches. */ if (len == 0 && sta->c != curc) result = FAIL; 

V560 （2）条件式の一部は常に真です：len == 0。

V571 （2）定期的なチェック。 「len == 0」条件は、6032行目で既に検証されています。

 if (VIsual_active) { if (VIsual_active && (VIsual_mode != wp->w_old_visual_mode || type == INVERTED_ALL)) 

V571 （2）定期的なチェック。 「VIsual_active」条件は、1515行目ですでに検証されています。

同様のチェックを行う場所がさらにいくつかあります。 彼らは議論にあまり関心を示さず、ほとんどの場合害を与えませんが、場合によっては論理的な間違いが隠される可能性があるため、そのような場所に注意を払う必要があります。

ちょうど悪いコード、最初のバイトだけが構造に記入されています

 #ifdef FEAT_TAG_BINS /* This is only to avoid a compiler warning for using search_info * uninitialised. */ vim_memset(&search_info, 0, (size_t)1); #endif 

V512 （1） 'memset'関数を呼び出すと、バッファー '＆search_info'のアンダーフローが発生します。

解説はなぜこれが行われたのかを説明していますが、方法はかなり奇妙です。 コンパイラの警告を回避するためのより多くの美しい方法があります。

短い名前を使用する悪い習慣

 extern char *UP, *BC, PC; 

アナライザーの警告： V707 （2）グローバル変数に短い名前を付けることは悪い習慣と見なされます。 「UP」、「BC」、「PC」変数の名前を変更することをお勧めします。

これは、Vimコードでは珍しいことではありません。 多くの変数には1文字と2文字の名前が付いており、多くの場合スコープが広く、この場合は一般的にグローバルです。 500行以上の長さの関数と組み合わせると、非常に読みにくいコードになります。

状態の奇妙な割り当てi

 int i = 2; /* index in s[] just after <Esc>[ or CSI */ //... if (n >= 8 && t_colors >= 16 && ((s[0] == ESC && s[1] == '[') || (s[0] == CSI && (i = 1) == 1)) && s[i] != NUL && (STRCMP(s + i + 1, "%p1%dm") == 0 || STRCMP(s + i + 1, "%dm") == 0) && (s[i] == '3' || s[i] == '4')) 

アナライザーの警告： V560 （2）条件式の一部は常に真です：（i = 1）== 1。

これが間違いなのか、iを1に割り当てる奇妙な方法なのかを判断するのは困難です。 そのように明確に書くことは価値がありません。

おわりに

結論として、Windowsマシンを使用せずにGNU LinuxでPVS-Studioを使用してプロジェクトをチェックすることは、非常に現実的で非常に便利です。 Vimもこの数を手伝いました。最初に同様の検証テストを受けました。

この記事は英語です。

英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：アントントカレフ。 GNU / LinuxでのPVS-StudioによるVimの分析 。