外は冬で、今年は終わりに近づいています。つまり、2020年にPVS-Studioアナライザーによって検出された最も興味深いエラーを検討するときです。
昨年は多数の新しい診断ルールが特徴であり、そのトリガーによってこのトップに入ることができたことに注意する必要があります。また、アナライザコアを継続的に改善し、その使用のための新しいシナリオを追加します。これについては、ブログで読むことができます 。アナライザーでサポートされている他の言語（C、C＃、Java）に興味がある場合は、同僚の記事をご覧ください。それでは、過去1年間にPVS-Studioによって発見された最も記憶に残るバグに移りましょう。
10位：モジュロを1で割る
V10631回の操作によるモジュロは無意味です。結果は常にゼロになります。llvm-stress.cpp 631
void Act() override {
....
// If the value type is a vector, and we allow vector select,
// then in 50% of the cases generate a vector select.
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
unsigned NumElem =
cast<FixedVectorType>(Val0->getType())->getNumElements();
CondTy = FixedVectorType::get(CondTy, NumElem);
}
....
}
開発者は、モジュロ除算を使用して0から1の間のランダムな値を取得したいと考えていました。ただし、X％1のような操作 は常に0を返します。この場合、条件を次のように書き直すのが正しいでしょう。
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))
このエラーは、記事「PVS-StudioでのClang11の確認」の上部に含まれていました 。
9位：4つのチェック
PVS-Studioは、コードの次のセクションに対して4つの警告を発行しました。
- V560条件式の一部は常に真です：x> =0。editor.cpp1137
- V560条件式の一部は常に真です：y> =0。editor.cpp1137
- V560条件式の一部は常に真です：x <40。editor.cpp1137
- V560条件式の一部は常に真です：y <30。editor.cpp1137
int editorclass::at( int x, int y )
{
if(x<0) return at(0,y);
if(y<0) return at(x,0);
if(x>=40) return at(39,y);
if(y>=30) return at(x,29);
if(x>=0 && y>=0 && x<40 && y<30)
{
return contents[x+(levx*40)+vmult[y+(levy*30)]];
}
return 0;
}
すべての警告は最後の ifステートメントに対するものです。問題は、そこで実行される4つのチェックすべてが常にtrueを返すこと です。これが重大な間違いだとは言えませんが、かなりおかしいことがわかりました。一般に、これらのチェックは冗長であり、削除できます。
このエラーは記事の先頭に入りました：「 VVVVVV ??? VVVVVV !!!」。
8位：削除の代わりに削除[]
V611メモリは「newT []」演算子を使用して割り当てられましたが、「delete」演算子を使用して解放されました。このコードを調べることを検討してください。'delete [] poke_data;'を使用することをお勧めします。CCDDE.CPP 410
BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
....
char *poke_data = new char [length + 2*sizeof(int)]; // <=
....
if(DDE_Class->Poke_Server( .... ) == FALSE) {
CCDebugString("C&C95 - POKE failed!\n");
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (FALSE);
}
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (TRUE);
}
アナライザは、互換性のない方法でメモリが割り当てられ、解放されたという事実に関連するエラーを検出しました。配列に割り当てられたメモリを解放するには、deleteではなくdelete []演算子 を 使用します。
このエラーは、「コマンド＆コンカーゲームコード：90年代のバグ。第2巻」の記事からトップになりました 。
7位：範囲外のバッファ
次に使用するnet_hostname_get関数を見てみましょう 。
#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
#endif
この場合、前処理中に、＃elseブランチに関連するオプションが 選択されました。つまり、前処理されたファイルでは、関数は次のように実装されます。
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
この関数は、7バイトの配列へのポインターを返します（文字列の最後の終端ゼロを考慮に入れてください）。
次に、配列のオーバーフローにつながるコードを見てみましょう。
static int do_net_init(void)
{
....
(void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
....
}
PVS-Studio警告： V512 [CWE-119]「memcpy」関数を呼び出すと、「net_hostname_get（）」バッファーが範囲外になります。log_backend_net.c 114
前処理後、 MAX_HOSTNAME_LENは次のように展開されます。
(void)memcpy(hostname, net_hostname_get(),
sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));
したがって、データをコピーすると、文字列リテラルのオーバーランが発生します。これがプログラムの実行にどのように影響するかは、未定義の動作につながるため、予測が困難です。
このバグは記事のトップになりました：「 Zephyrオペレーティングシステムコードの品質の調査」。
6位：非常に奇妙な何か
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
memcpy(cpy_mntpt, mntpt, strlen(mntpt));
}
return cpy_mntpt;
}
PVS-Studioの警告： V575 [CWE-628]「memcpy」関数は文字列全体をコピーしません。 'strcpy / strcpy_s'関数を使用して、ターミナルnullを保持します。 shell.c 427
誰かがstrdup関数のアナログを作成しようとしました が、失敗しました。
アナライザーの警告から始めましょう。memcpy関数 は行をコピーすると表示されますが、ターミナル0はコピーされないため、非常に疑わしいです。
この端末0はここにコピーされているようです：
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
しかし、違います！これは、ターミナルゼロがそれ自体にコピーされる原因となるタイプミスです！書き込みにあることに注意してくださいmntpt配列 しないように、 cpy_mntpt。その結果、mntpt_prepare関数 は不完全な端末null文字列を返します。
実際、プログラマーは次のように書きたかったのです。
((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';
しかし、なぜそんなに難しいのかはまだはっきりしていません！このコードは次のように簡略化できます。
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
strcpy(cpy_mntpt, mntpt);
}
return cpy_mntpt;
}
このバグは、前述の記事「Zephyrオペレーティングシステムコードの品質の調査」のトップになりました 。
5位：不適切なオーバーフロー保護
V547 [CWE-570]式 'rel_wait <0'は常にfalseです。符号なしタイプの値が0未満になることはありません。os_thread_windows.c359
static DWORD
get_rel_wait(const struct timespec *abstime)
{
struct __timeb64 t;
_ftime64_s(&t);
time_t now_ms = t.time * 1000 + t.millitm;
time_t ms = (time_t)(abstime->tv_sec * 1000 +
abstime->tv_nsec / 1000000);
DWORD rel_wait = (DWORD)(ms - now_ms);
return rel_wait < 0 ? 0 : rel_wait;
}
この場合、rel_wait変数 は符号なしDWORDタイプ です。これは、結果が常に真であるため、rel_wait <0の比較は意味 がないことを意味します。
エラー自体はあまり興味深いものではありません。しかし、彼らがそれをどのように修正しようとしたかは興味深いことがわかりました。変更は修正されておらず、コードが単純化されているだけであることが判明しました。この話の詳細については、同僚の記事「PVS-Studioが自動コード編集を提供しない理由」を参照してください 。
エラーは記事の冒頭に入力されました：「 IntelからのPMDKライブラリのコレクションのコードの静的分析とエラーではないエラー」。
4位：std、兄弟に書いてはいけない
V1061'std '名前空間を拡張すると、未定義の動作が発生する可能性があります。sized_iterator.hh 210
// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
util::SizedIterator second)
{
util::swap(*first, *second);
}
} // namespace std
トリガーの元となった記事：「 DeepSpeechプロジェクトのコードの分析、または名前空間stdに記述すべきでない理由」では、これを実行すべきでない理由について詳しく説明しています。
3位：失敗したスクロールバー
V501。'-'演算子の左側と右側には、同一の部分式があります。bufferHeight--bufferHeight TermControl.cpp 592
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(bufferHeight - bufferHeight);
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}
これがいわゆる「歴史の引き金」です。この場合、エラーのため、Windowsターミナルのスクロールバーは機能しませんでした。このバグに基づいて記事全体が書かれ、同僚が調査を行い、なぜこれが起こったのかを解明しました。興味ありますか？これが「 できなかったスクロールバー」です。
2位：混乱した半径と高さ
また、いくつかのアナライザーの警告について説明します。
- V764「CreateWheel」関数に渡される引数の順序が正しくない可能性があります：「height」および「radius」。StandardJoints.cpp 791
- V764「CreateWheel」関数に渡される引数の順序が正しくない可能性があります：「height」および「radius」。StandardJoints.cpp 833
- V764「CreateWheel」関数に渡される引数の順序が正しくない可能性があります：「height」および「radius」。StandardJoints.cpp 884
関数呼び出しは次のとおりです。
NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);
そして、これは彼女の広告がどのように見えるかです：
static NewtonBody* CreateWheel (DemoEntityManager* const scene,
const dVector& location, dFloat radius, dFloat height)
関数呼び出しで引数が逆になりました。
このエラーは、「PVS-Studio静的アナライザーを使用したニュートンゲームダイナミクスの再チェック」という記事からトップになりました 。
そもそも：結果の消去
V519「color_name」変数には2回連続して値が割り当てられます。おそらくこれは間違いです。チェック行：621、627。string.cpp 627
static bool parseNamedColorString(const std::string &value,
video::SColor &color)
{
std::string color_name;
std::string alpha_string;
size_t alpha_pos = value.find('#');
if (alpha_pos != std::string::npos) {
color_name = value.substr(0, alpha_pos);
alpha_string = value.substr(alpha_pos + 1);
} else {
color_name = value;
}
color_name = lowercase(value); // <=
std::map<const std::string, unsigned>::const_iterator it;
it = named_colors.colors.find(color_name);
if (it == named_colors.colors.end())
return false;
....
}
この関数は、透明度パラメーターを使用して色名を解析し、その16進コードを返す必要があります。条件をチェックした結果に応じて、行分割の結果または関数引数のコピーのいずれかがcolor_name変数に渡されます 。
ただし、小文字（）関数で は、結果の文字列自体は小文字に変換されませんが、元の関数引数に変換されます。その結果、parseNamedColorString（）が返すはずだった色が失われるだけ です。
color_name = lowercase(color_name);
このエラーは、「PVS-Studio：セルフホストエージェントを使用したAzureDevOpsでのプルリクエストの分析」という記事からトップになりました 。
結論
過去1年間で、オープンソースプロジェクトに多くのバグが見つかりました。これらは、通常のコピー＆ペーストエラー、一定のエラー、メモリリーク、およびその他の多くの問題でした。私たちのアナライザーは静止していません、そしてトップに今年書かれた新しい診断のいくつかのポジティブがあります。
収集したバグを楽しんでいただけたでしょうか。個人的にはとても面白いと思いました。しかし、もちろん、あなたのビジョンは私のものとは異なる可能性があるため、ブログの記事を読んだり、オープンソースプロジェクトでPVS-Studioによって検出されたエラーのリストを確認したりして、「トップ10 ...」を作成できます 。
：私は、過去数年のトップ10のC ++エラーであなたの注意の記事にもたらす 2016年、 2017年、 2018年、 2019年。
この記事を英語を話す聴衆と共有したい場合は、翻訳リンクを使用してください：VladislavStolyarov。 2020年にC ++プロジェクトで見つかったトップ10のバグ。