👑 💽 👳🏿 Firebird 3.0を確認してください 🖕🏿 😧 👊🏻

少し前に、Firebird DBMSの新しいバージョンがリリースされました。このリリースは、プロジェクトの歴史上最大のものの1つになりました。アーキテクチャが大幅に再設計され、マルチスレッドのサポートが追加され、生産性が向上しました。このような重要な更新が、PVS-Studio静的コードアナライザーを使用してFirebirdを再チェックする理由でした。

はじめに

Firebirdは、クロスプラットフォームの無料のデータベース管理システムです。このプロジェクトはC ++で記述されており、Microsoft Windows、Linux、Mac OS X、および多くのUnixライクなオペレーティングシステムで実行されます。 DBMSは完全に無料で使用および配布できます。 Firebirdの詳細については、公式Webサイトをご覧ください。

Firebirdは、以前にアナライザーによってテストされていました。前のチェックに関するレポートは、記事「担保結果：PVS-Studioを使用してFirebirdをチェックする」で見つけることができます。検証のため、マスターブランチのGitHubからコードを取得しました。アセンブリの詳細については、プロジェクトWebサイトの対応する記事を参照してください。ソースファイルの分析は、コンパイラモニタリング（コンパイラモニタリング）のインターセプトを使用して、 PVS-Studioスタンドアロンバージョン6.03で実行されました。このテクノロジーにより、アセンブリシステムに統合せずにプロジェクトを検証できます。受信したレポートは、スタンドアロンバージョンとVisual Studioの両方で表示できます。

タイプミス

void advance_to_start() { .... if (!isalpha(c) && c != '_' && c != '.' && c != '_') syntax_error(lineno, line, cptr); .... }

PVS-Studio警告： V501同一のサブ式があります 'c！=' _ '' '&&'演算子の左右に。 reader.c 1203

アナライザーは、論理操作に2つの同一の部分式c！= '_'が存在することを検出しました。最後の条件では、タイプミスが許可されており、変数cは別の文字と比較する必要があります。他の関数は '$'記号の付いたチェックを使用します。おそらくここで使用する必要があります。

 if (!isalpha(c) && c != '_' && c != '.' && c != '$')

不注意なエラーの別の例：

 int put_message(....) { if (newlen <= MAX_UCHAR) { put(tdgbl, attribute); put(tdgbl, (UCHAR) newlen); } else if (newlen <= MAX_USHORT) { if (!attribute2) BURP_error(314, ""); .... } else BURP_error(315, ""); .... }

PVS-Studioの警告：

V601文字列リテラルは暗黙的にブール型にキャストされます。 2番目の引数を調べます。 backup.cpp 6113
V601文字列リテラルは暗黙的にブール型にキャストされます。 2番目の引数を調べます。 backup.cpp 6120

ここでは、BURP_error関数が誤って使用されています。次のように宣言されます。

 void BURP_error(USHORT errcode, bool abort, const MsgFormat::SafeArg& arg = MsgFormat::SafeArg()); void BURP_error(USHORT errcode, bool abort, const char* str);

2番目の引数はブール値で、文字列は3番目の引数です。その結果、文字列リテラルはtrueにキャストされます。 関数呼び出しは、BURP_error（315、true、 ""）またはBURP_error（315、false、 ""）に書き換える必要があります。

ただし、プロジェクト開発者のみがエラーを認識できる場合があります。

 void IDX_create_index(....) { .... index_fast_load ifl_data; .... if (!ifl_data.ifl_duplicates) scb->sort(tdbb); if (!ifl_data.ifl_duplicates) BTR_create(tdbb, creation, selectivity); .... }

PVS-Studio警告： V581互いに並んでいる「if」演算子の条件式は同一です。行を確認してください：506、509。idx.cpp 509

同じ条件の2つのブロックがコードの行に続きます。そのうちの1つがタイプミスをしたのかもしれませんが、この状況は個々のセクションのコピーまたは削除が原因で発生した可能性があります。いずれにしても、このようなコードは奇妙に見えます。

次の例では、ポインターのある状況を考えます。

 static void string_to_datetime(....) { .... const char* p = NULL; const USHORT length = CVT_make_string(desc, ttype_ascii, &p, &buffer, sizeof(buffer), err); const char* const end = p + length; .... while (p < end) { if (*p != ' ' && *p != '\t' && p != 0) { CVT_conversion_error(desc, err); return; } ++p; } .... }

PVS-Studio警告： V713ポインターpは、同じ論理式のnullptrに対して検証される前に、論理式で使用されました。 cvt.cpp 702

条件では、参照解除直後に変数pの nullptr がチェックされます。これは、検査場所に別の条件があったはずであるか、検証が不要であったことを意味する場合があります。

コードの上を見ると、同様のフラグメントを見つけることができます。

 while (++p < end) { if (*p != ' ' && *p != '\t' && *p != 0) CVT_conversion_error(desc, err); }

このようなエラーを回避するために、ゼロとの比較には、対応するリテラルを使用することをお勧めします。タイプcharには「\ 0」 、数値には0、ポインターにはnullptrです。これをルールとして採用すれば、このような愚かな間違いの多くを避けることができます。

memcmpの危険な使用

 SSHORT TextType::compare(ULONG len1, const UCHAR* str1, ULONG len2, const UCHAR* str2) { .... SSHORT cmp = memcmp(str1, str2, MIN(len1, len2)); if (cmp == 0) cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0)); return cmp; }

PVS-Studio警告： V642 「memcmp」関数の結果を「short」型変数内に保存することは不適切です。プログラムのロジックを壊して、重要なビットが失われる可能性があります。 texttype.cpp 3

memcmp関数は次の値を返します。

str1が str2より小さい場合は <0
str1がstr2と等しい場合は0
> 0（ str1が str2より大きい場合）

文字列の不等式を持つ関数の正確な値は保証されません。したがって、 intより小さい変数に結果を保存すると、重要なビットが失われ、アプリケーションロジックに違反する可能性があります。

追加のチェック

 void Trigger::compile(thread_db* tdbb) { SET_TDBB(tdbb); Database* dbb = tdbb->getDatabase(); Jrd::Attachment* const att = tdbb->getAttachment(); if (extTrigger) return; if (!statement /*&& !compile_in_progress*/) { if (statement) return; .... } }

警告PVS-Studio： V637 2つの正反対の条件が発生しました。 2番目の条件は常にfalseです。行を確認：778、780。jrd.cpp 778

アナライザーは、2つの反対の条件のチェックを見つけました。ほとんどの場合、最初の条件を変更した結果、2番目の条件は不要になり、削除できますが、作成者はここで決定する必要があります。

次のコードスニペットは、奇妙なブランチの別の例です。

 static void asgn_from( ref* reference, int column) { TEXT variable[MAX_REF_SIZE]; TEXT temp[MAX_REF_SIZE]; for (; reference; reference = reference->ref_next) { const gpre_fld* field = reference->ref_field; .... if (!field || field->fld_dtype == dtype_text) .... else if (!field || field->fld_dtype == dtype_cstring) .... else .... } }

警告PVS-Studio： V560条件式の一部は常にfalse ：！フィールドです。 int_cxx.cpp 217

フィールドが NULLポインターでない場合、コードはelse ifの条件に到達しません。このチェックは不要であるか、別の比較が代わりに行われているはずです。この条件がアプリケーションロジックと矛盾するかどうかは不明です。

さらに、論理式のいくつかの不要なチェックが見つかりました。

 bool XnetServerEndPoint::server_init(USHORT flag) { .... xnet_connect_mutex = CreateMutex(ISC_get_security_desc(), FALSE, name_buffer); if (!xnet_connect_mutex || (xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS)) { system_error::raise(ERR_STR("CreateMutex")); } .... }

PVS-Studio警告： V728過剰なチェックを簡素化できます。「||」演算子は、反対の式「！xnet_connect_mutex」と「xnet_connect_mutex」に囲まれています。 xnet.cpp 2231

if（！Xnet_connect_mutex ||（xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS））チェックは、 if（！Xnet_connect_mutex || ERRNO == ERROR_ALREADY_EXISTS）に簡略化できます。これは、真理値表を使用して簡単に証明できます。

符号なし変数の危険な比較

 static bool write_page(thread_db* tdbb, BufferDesc* bdb, ....) { .... if (bdb->bdb_page.getPageNum() >= 0) .... }

PVS-Studioの警告： V547式 'bdb-> bdb_page.getPageNum（）> = 0'は常にtrueです。符号なしの型の値は常に> = 0です。cch.cpp 4827

条件bdb-> bdb_page.getPageNum（）> = 0は、関数が符号なしの型の値を返すため、常に真になります。プログラマーが値を誤ってチェックした可能性があります。プロジェクトでの同様の比較を考慮すると、コードは次のようになります。

 if (bdb->bdb_page.getPageNum() != 0)

NULLポインターの逆参照

 static bool initializeFastMutex(FAST_MUTEX* lpMutex, LPSECURITY_ATTRIBUTES lpAttributes, BOOL bInitialState, LPCSTR lpName) { if (pid == 0) pid = GetCurrentProcessId(); LPCSTR name = lpName; if (strlen(lpName) + strlen(FAST_MUTEX_EVT_NAME) - 2 >= MAXPATHLEN) { SetLastError(ERROR_FILENAME_EXCED_RANGE); return false; } setupMutex(lpMutex); char sz[MAXPATHLEN]; if (lpName) .... }

PVS-Studio警告： V595 nullptrに対して検証される前に、「lpName」ポインターが使用されました。行を確認してください：2814、2824。isc_sync.cpp 2814

警告V595はPVS-Studioによって検証されたプロジェクトで最も一般的であり、Firebirdも例外ではありませんでした。合計で、この警告で30以上の場所が見つかりました。

この例では、 nullptr pointerをチェックする前にstrlen（lpName）呼び出しが行われます。これにより、nullポインターを関数に渡そうとしたときに未定義の動作が発生します。ポインターの逆参照はstrlen呼び出しでここに隠されているため、静的アナライザーを使用しないとエラーを検出するのが難しくなります。

新しい後にnullptrを確認する

 rem_port* XnetServerEndPoint::get_server_port(....) { .... XCC xcc = FB_NEW struct xcc(this); try { .... } catch (const Exception&) { if (port) cleanup_port(port); else if (xcc) cleanup_comm(xcc); throw; } return port; }

PVS-Studio警告： V668メモリは「new」演算子を使用して割り当てられたため、「xcc」ポインターをnullに対してテストする意味はありません。メモリ割り当てエラーの場合、例外が生成されます。 xnet.cpp 2533

アナライザーは、new演算子がnullptrを返すことができないことを警告します。チェックするには、try-catchブロックまたはnew（std :: nothrow）を使用する必要があります。ただし、この例では、すべてがやや複雑です。メモリを割り当てるには、マクロFB_NEWが使用されます。 alloc.hファイルで宣言されています。

 #ifdef USE_SYSTEM_NEW #define OOM_EXCEPTION std::bad_alloc #else #define OOM_EXCEPTION Firebird::BadAlloc #endif #define FB_NEW new(__FILE__, __LINE__) inline void* operator new(size_t s ALLOC_PARAMS) throw (OOM_EXCEPTION) { return MemoryPool::globalAlloc(s ALLOC_PASS_ARGS); }

非標準のアロケーターが使用されているため、この特定の例が間違いであるかどうかを判断することは困難です。ただし、 スロー（std :: bad_alloc）が演算子定義で指定されているため、このようなチェックは疑わしいように見えます。

reallocの危険な使用

 int mputchar(struct mstring *s, int ch) { if (!s || !s->base) return ch; if (s->ptr == s->end) { int len = s->end - s->base; if ((s->base = realloc(s->base, len+len+TAIL))) { s->ptr = s->base + len; s->end = s->base + len+len+TAIL; } else { s->ptr = s->end = 0; return ch; } } *s->ptr++ = ch; return ch; }

PVS-Studio 警告： V701 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインター 's-> base'が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 mstring.c 42

ptr = realloc（ptr、size）の形式の式は、 reallocがnullptrを返すときにメモリポインターが失われるという点で不適切です。これを回避するには、 reallocの結果を一時変数に保存し、nullptrを確認した後、 ptrに値を割り当てます。

 temp_ptr = realloc(ptr, new_size); if (temp_ptr == nullptr) { //handle exception } else { ptr = temp_ptr; }

スイッチで未使用の列挙値

 template <typename CharType> LikeEvaluator<CharType>::LikeEvaluator(....) { .... PatternItem *item = patternItems.begin(); .... switch (item->type) { case piSkipFixed: case piSkipMore: patternItems.grow(patternItems.getCount() + 1); item = patternItems.end() - 1; // Note: fall into case piNone: item->type = piEscapedString; item->str.data = const_cast<CharType*> (pattern_str + pattern_pos - 2); item->str.length = 1; break; case piSearch: item->type = piEscapedString; // Note: fall into case piEscapedString: item->str.length++; break; } .... }

警告PVS-Studio： V719 switchステートメントは、 'PatternItemType'列挙型のすべての値をカバーするわけではありません：piDirectMatch。 evl_string.h 324

すべての列挙値がスイッチ構成で使用されたわけではなく、デフォルトのブロックがありません。ここでpiDirectMatch処理を追加するのを忘れた可能性があります。同様の警告がある場所のリスト：

V719 switchステートメントは、 'PatternItemType'列挙型のすべての値をカバーするわけではありません：piDirectMatch、piSkipMore。 evl_string.h 351
V719 switchステートメントは、 'PatternItemType'列挙型のすべての値をカバーするわけではありません：piDirectMatch。 evl_string.h 368
V719 switchステートメントは、 'PatternItemType'列挙型のすべての値をカバーするわけではありません：piDirectMatch。 evl_string.h 387

バッファオーバーフロー

 const int GDS_NAME_LEN = 32; .... bool get_function(BurpGlobals* tdgbl) { .... struct isc_844_struct { .... short isc_870; /* gds__null_flag */ .... char isc_874 [125]; /* RDB$PACKAGE_NAME */ .... } isc_844; att_type attribute; TEXT temp[GDS_NAME_LEN * 2]; .... SSHORT prefixLen = 0; if (!/*X.RDB$PACKAGE_NAME.NULL*/ isc_844.isc_870) { prefixLen = static_cast<SSHORT>(strlen(/*X.RDB$PACKAGE_NAME*/ isc_844.isc_874)); memcpy(temp, /*X.RDB$PACKAGE_NAME*/ isc_844.isc_874, prefixLen); temp[prefixLen++] = '.'; } .... }

PVS-Studio警告： V557アレイがオーバーランする可能性があります。 'prefixLen ++'インデックスの値は124に達する可能性があります。restore.cpp 10040

バッファーisc_844.isc_874のサイズはそれぞれ125であり、 strlen（isc_844.isc_874）の可能な最大値は124です。tempのサイズは64で、この値よりも小さくなっています。このようなインデックスで書き込むと、バッファオーバーフローが発生する可能性があります。一時変数により多くのメモリを割り当てる方が安全です。

負のシフト

 static ISC_STATUS stuff_literal(gen_t* gen, SLONG literal) { .... if (literal >= -32768 && literal <= 32767) return stuff_args(gen, 3, isc_sdl_short_integer, literal, literal >> 8); .... }

PVS-Studio警告： V610不特定の動作。シフト演算子「>>」を確認してください。左のオペランドは負です（ 'literal' = [-32768..32767]）。 array.cpp 848

コードには、負の数の右シフトが含まれています。 C ++標準では、このようなアクションには不特定の動作があります。つまり、コンパイラとプラットフォームが異なると結果が異なる可能性があります。次のように書き直してください：

 if (literal >= -32768 && literal <= 32767) return stuff_args(gen, 3, isc_sdl_short_integer, literal, (ULONG)literal >> 8);

警告が消えた別の場所：

V610不特定の動作。シフト演算子「>>」を確認してください。左のオペランドが負です（ 'i64value' = [-2147483648..2147483647]）。 exprnodes.cpp 6382

変数オーバーライド

 THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg) { int exit_code = -1; try { Service* svc = (Service*)arg; RefPtr<SvcMutex> ref(svc->svc_existence); int exit_code = svc->svc_service_run->serv_thd(svc); svc->started(); svc->svc_sem_full.release(); svc->finish(SVC_finished); } catch (const Exception& ex) { // Not much we can do here iscLogException("Exception in Service::run():", ex); } return (THREAD_ENTRY_RETURN)(IPTR) exit_code; }

PVS-Studio警告： V561新たに宣言するよりも、 'exit_code'変数に値を割り当てる方がおそらく良いでしょう。前の宣言：svc.cpp、行1893。svc.cpp1898

この例では、割り当てる代わりに、変数exit_codeがオーバーライドされます。これにより、スコープから以前の変数が非表示になり、その結果、関数は常に-1に等しい無効な値を返します。

正しいコードは次のとおりです。

 THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg) { int exit_code = -1; try { Service* svc = (Service*)arg; RefPtr<SvcMutex> ref(svc->svc_existence); exit_code = svc->svc_service_run->serv_thd(svc); svc->started(); svc->svc_sem_full.release(); svc->finish(SVC_finished); } catch (const Exception& ex) { // Not much we can do here iscLogException("Exception in Service::run():", ex); } return (THREAD_ENTRY_RETURN)(IPTR) exit_code; }

おわりに

前のテストで見つかった問題のほとんどは、プロジェクトの開発者によって修正され、現在はコード内にありません。つまり、アナライザーは素晴らしい仕事をしました。ただし、定期的に使用することで最良の結果を得ることができます。これにより、早い段階でエラーをキャッチできます。インクリメンタル分析とビルドシステムとの互換性により、静的アナライザーをプロジェクトに簡単に統合できます。静的アナライザーを使用すると、膨大な時間を節約でき、デバッグや動的分析では検出が困難なエラーを見つけることができます。

この記事を英語を話す聴衆と共有したい場合は、翻訳へのリンクを使用してください：Pavel Belikov。 Firebird 3.0の分析。

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

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

Firebird 3.0を確認してください