PVS-Studio で OpenCV をチェックする

OpenCV は、コンピューター ビジョン アルゴリズム、画像処理アルゴリズム、および汎用数値アルゴリズムのライブラリです。このライブラリは C/C++ で書かれており、BSD ライセンスの下で配布されているため、学術および商用の両方で無料で使用できます。このライブラリを PVS-Studio コード アナライザーでチェックする時が来ました。

OpenCV は大きなライブラリです。 2500 を超える最適化されたアルゴリズムが含まれており、100 万を超えるコード行で構成されています。最も複雑な関数 cv::cvtColor() の循環的複雑度は 415 です。そのコードに非常に多くのエラーや疑わしいフラグメントが見つかったのも不思議ではありません。ただし、ソース コードのサイズを考慮に入れると、このライブラリは高品質のライブラリと呼ぶことができます。

古いエラー

しかし、ここで最初にちょっとした注意事項があります。 PVS-Studio によって検出されたエラーのサンプルを調査するとき、プログラマーはこれらのエラーが本物であると信じたくありません。おそらく彼らは、自分や他人のプログラムが信頼できないかもしれないという事実を知りたくないのでしょう。彼らは次のように主張します:「わかりました。プログラムにいくつかの実際のエラーが見つかりましたが、実際にはプログラムの動作に影響を与えていません。このコードは使用されていないようです。問題はありません。」.

残念ながら、それらはもちろん間違っています。今こそ、それを証明する絶好の機会です。 1 つのプロジェクトを分析しながら、それに統合された OpenCV ライブラリもチェックしました。プロジェクトに含まれていたのは古いバージョンのライブラリだったので、見つかったエラーを調査しましたが、レポートには記載しませんでした。 OpenCV ライブラリの新しいバージョンをチェックして、それについて投稿するのは理にかなっています。それが今私たちが行ったことです。

結果は予想どおりです。古いライブラリ バージョンの多くのエラーは、新しいバージョンで修正されています。ここにいくつかあります。

最初の修正エラー:

CV_IMPL CvGLCM* cvCreateGLCM(....)
{
  CvGLCM* newGLCM = 0;
  ....
  memset( newGLCM, 0, sizeof(newGLCM) );
  ....
}

V512 「memset」関数を呼び出すと、バッファ「newGLCM」のアンダーフローが発生します。 cvtexture.cpp 138

2 番目の修正済みエラー:

CvDTreeSplit* CvDTree::find_split_cat_reg(....)
{
  ....
  double** sum_ptr = 0;
  
  .... // sum_ptr not in use
    for( i = 0; i < mi; i++ )
    {
        R += counts[i];
        rsum += sum[i];
        sum[i] /= MAX(counts[i],1);
        sum_ptr[i] = sum + i;
    }
  ....
}

V522 null ポインター「sum_ptr」の逆参照が発生する可能性があります。 mltree.cpp 2001

他にもいくつか例がありますが、すでに修正されたバグを説明するのは面白くありません。重要な点は、この事実から容赦ない結論を引き出すことができるということです:

1. PVS-Studio アナライザーによって検出されたエラーは完全に本物です。彼らは、ユーザーと開発者の両方を拷問し、血を吸っています。それらを見つけて修正する必要があります。このプロセスは悲しくて時間がかかり、ユーザーがバグを発見した後にのみ開始されます。

2. これらおよびその他の多くのエラーは、コーディング段階で PVS-Studio アナライザーによって検出できるため、開発コストが大幅に削減されます。インクリメンタル分析モードは特に便利に見えるかもしれません。

新しいバグ

注意 プロジェクトをチェックするとき、バグがプロジェクト自体を参照しているのか、プロジェクトで使用されているサードパーティ ライブラリの 1 つを参照しているのかを区別しません。それぞれの小さなライブラリを個別に説明するのは面白くありません.

この記事を、PVS-Studio が OpenCV ライブラリで発見したバグの完全なリストと見なすべきではないことにも注意してください。この記事では、アナライザーによって生成されたメッセージをスキャンしたときに最も疑わしいと判明したコード フラグメントのみを引用しています。参加する場合 OpenCV プロジェクトの開発中は、このツールのデモ バージョンを使用して、アナライザーが生成した警告のリストをより徹底的に調べることをお勧めします。

コピペのバグ

PVS-Studio アナライザーは、ミスプリントやコピー アンド ペーストによるエラーの検出に優れています。コードのコピー アンド ペーストの典型的な例を次に示します。 augAssignAnd、augAssignOr、augAssignXor、augAssignDivide などの一連の関数があります。これらの機能は、1 つの演算子でのみ異なります。確かに、ボディ関数をコピーして、それが何をしなければならないかを担当するオペレーターを修正したいという大きな誘惑を感じずにはいられません。問題は、間違いを犯す可能性が高いことです.

void MatOp::augAssignAnd(const MatExpr& expr, Mat& m) const
{
    Mat temp;
    expr.op->assign(expr, temp);
    m &= temp;
}

void MatOp::augAssignOr(const MatExpr& expr, Mat& m) const
{
    Mat temp;
    expr.op->assign(expr, temp);
    m |= temp;
}

void MatOp::augAssignDivide(const MatExpr& expr, Mat& m) const
{
    Mat temp;
    expr.op->assign(expr, temp);
    m /= temp;
}

void MatOp::augAssignXor(const MatExpr& expr, Mat& m) const
{
    Mat temp;
    expr.op->assign(expr, temp);
    m /= temp;
}

V524 「augAssignXor」関数の本体が「augAssignDivide」関数の本体と完全に同等であることは奇妙です (matop.cpp、294 行目)。 matop.cpp 318

augAssignXor() 関数は 'augAssignDivide() 関数と同じことを行うことに注意してください。それは確かに正しくありません。 augAssignXor() 関数には、「m ^=temp;」というテキストが含まれている必要があります。

コード ロジックと矛盾するコード フォーマット

これは、コピーと貼り付けに関連するもう 1 つのエラーです。調査する必要があるプログラム行が長すぎます。記事のテキストに合わせてフォーマットすると、エラーがどこにあるのかわかりません。そのため、写真を使用して表示する必要があります。

図 1. プログラム ロジックがフォーマットに対応していない。写真をクリックして拡大してください。

V640 コードの操作ロジックがフォーマットに対応していません。 2 番目のステートメントは常に実行されます。中括弧が欠落している可能性があります。 test_stereomatching.cpp 464

ご覧のとおり、長い行がコピーされ、'if' 演算子の後に配置されています。その結果、プログラムのフォーマットが実行ロジックと矛盾します。

ミスプリント

次のエラーは、コードのコピーではなく、ミスプリントが原因である必要があります。おそらく、変数名を書くときにプログラマーが失敗したのはオートコンプリートでした.

static jpc_enc_cp_t *cp_create(....)
{
  ....
  ccp->sampgrdsubstepx = 0;
  ccp->sampgrdsubstepx = 0;
  ....
}

V519 'ccp->sampgrdsubstepx' 変数には、連続して 2 回値が割り当てられます。おそらくこれは間違いです。チェック行:414, 415.jpc_enc.c 415

2 行目は次のようになります:ccp->sampgrdsubstepy =0;.

無意味なループ

typedef struct CvStereoCamera
{
 ....
 float fundMatr[9]; /* fundamental matrix */
 ....
};
CvStereoCamera stereo;

void CvCalibFilter::Stop( bool calibrate )
{
  ....
  for( i = 0; i < 9; i++ )
  {
    stereo.fundMatr[i] = stereo.fundMatr[i];
  }
  .... 
}

V570 'stereo.fundMatr[i]' 変数がそれ自体に割り当てられています。 calibfilter.cpp 339

ここにあるループは無意味です。配列項目に対して他の操作を実行する必要があるようです。

本体が 1 回だけ実行されるループは次のとおりです。

virtual CvBlob* Process(....)
{
  ....
  while(!m_Collision && m_FGWeight>0)
  {
    ....
    break;
  }
  ....
}

V612 ループ内の無条件の「中断」。 blobtrackingmsfg.cpp 600

ループ本体には「continue」演算子が含まれておらず、最後に「break」演算子があります。これはすべて非常に奇妙で、関数が正しくないに違いありません.

ヌル文字とヌル ポインターの混乱

int jpc_atoaf(char *s, int *numvalues, double **values)
{
  char *cp;
  ....
  while ((cp = strtok(0, delim))) {
    if (cp != '\0') {
      ++n;
    }
  }
  ....
}

V528 'char' 型へのポインタが '\0' 値と比較されるのは奇妙です。おそらく次のような意味です:*cp !='\0'. jpc_util.c 105

同じ間違いがここにあります:jpc_util.c 123.

チェック if(cp !='\0') は無意味です。 strtok() 関数が null ポインターを返す場合、ループは終了します。プログラマーは、行末が見つかったかどうかを確認するつもりだったに違いありません。この場合、チェックは次のようになります:if(*cp !='\0').

コンディションのミスプリント

ミスプリントによって一部の変数の値がチェックされない場合、さまざまな種類のエラーが発生します。

dr3dr2 変数はチェックされません:

CV_IMPL void cvComposeRT(
  const CvMat* _rvec1, const CvMat* _tvec1,
  const CvMat* _rvec2, const CvMat* _tvec2,
  CvMat* _rvec3, CvMat* _tvec3,
  CvMat* dr3dr1, CvMat* dr3dt1,
  CvMat* dr3dr2, CvMat* dr3dt2,
  CvMat* dt3dr1, CvMat* dt3dt1,
  CvMat* dt3dr2, CvMat* dt3dt2)
{
  ....
  if( _rvec3 || dr3dr1 || dr3dr1 )
  ....
}

V501 '||' の左右に同一のサブ式があります演算子:_rvec3 || || dr3dr1 || dr3dr1 キャリブレーション.cpp 415

cmptlut[2] 配列項目がチェックされていません:

bool Jpeg2KDecoder::readHeader()
{
  ....
  cmptlut[0] = ....
  cmptlut[1] = ....
  cmptlut[2] = ....
  if( cmptlut[0] < 0 || cmptlut[1] < 0 || cmptlut[0] < 0 )
    result = false;
  ....
}

V501 '||' の左右に同一の部分式 'cmptlut[0] <0' があります。オペレーター。 grfmt_jpeg2000.cpp 215

dst_size.height 変数はそれ自体と比較されます:

CV_IMPL IplImage* icvCreateIsometricImage(....)
{
  ....
  if( !dst || dst->depth != desired_depth ||
      dst->nChannels != desired_num_channels ||
      dst_size.width != src_size.width ||
      dst_size.height != dst_size.height )
  ....
}

V501 '!=' 演算子の左右に同じ部分式があります:dst_size.height !=dst_size.height epilines.cpp 2118

まったく無意味な条件:

void CvDTreeTrainData::read_params(....)
{
  ....
  if( cat_var_count != cat_var_count ||
      ord_var_count != ord_var_count )
    CV_ERROR(CV_StsParseError,
    "var_type is inconsistent with cat_var_count and ord_var_count");
  ....
}

V501 「!=」演算子の左右に同一の部分式があります:cat_var_count !=cat_var_count tree.cpp 1415

V501 「!=」演算子の左右に同一の部分式があります:ord_var_count !=ord_var_count tree.cpp 1415

他の同様のエラーについては、対応する診断メッセージを引用させてください:

  • V501 「==」演算子の左右に同じ部分式があります:M.size() ==M.size() imgwarp.cpp 3672
  • V501 「&&」演算子の左右に同じ部分式があります:data &&dims>=1 &&data mat.hpp 434
  • V501 「&&」演算子の左右に同じ部分式があります:0 <=d &&_sizes &&d <=32 &&_sizes matrix.cpp 186
  • V501 「==」演算子の左右に同じ部分式があります:M.size() ==M.size() imgwarp.cpp 3685

ポインタはチェック前に使用されます

これは、ポインターが最初に使用されたときにのみ、null ポインターであるかどうかがチェックされるときに非常に頻繁に発生するエラーです。 OpenCV ライブラリも例外ではありません。これらのエラーは次のようになります:

CV_IMPL CvStringHashNode* 
cvGetHashedKey( CvFileStorage* fs, .... )
{
  ....
  CvStringHash* map = fs->str_hash;
  if( !fs )
    return 0;
  ....
}

V595 'fs' ポインターは、nullptr に対して検証される前に使用されました。行を確認してください:617, 619.persistence.cpp 617

void CvBlobTrackerAuto1::Process(IplImage* pImg, IplImage* pMask)
{
  ....
  CvBlob* pBN = NewBlobList.GetBlob(i);
  pBN->ID = m_NextBlobID;

  if(pBN &&
     pBN->w >= CV_BLOB_MINW &&
     pBN->h >= CV_BLOB_MINH)
  ....
}

V595 'pBN' ポインターは、nullptr に対して検証される前に使用されました。チェック行:432, 434. blobtrackingauto.cpp 432

V595 診断メッセージが生成されるコード フラグメントをこれ以上引用する必要はないと思います。それらは多数あり、同じように見えます。その上で PVS-Studio を実行し、これらすべてのフラグメントを確認することをお勧めします。

注意。 V595 診断は、コード フラグメントが確かに正しくないことを常に示しているわけではありません。場合によっては、ポインターが理論上でもゼロに等しくならないことがあります。この場合、コードを読むときに混乱を招かないようにチェックを外すことができます。また、ポインタではなく、参照によってオブジェクトを渡すのが最善です。

サイズの混乱

完全なバッファ処理ではなく、バッファの最初のバイトのみが処理される原因となる多くのバグがあります。ほとんどの場合、ポインターのサイズがそれが指す配列のサイズと混同され、後者の代わりに前者が計算されるという問題が原因です (例)。ここも同じようです。

CAPDRIVERCAPS caps;
bool CvCaptureCAM_VFW::open( int wIndex )
{
  ....
  memset( &caps, 0, sizeof(caps));
  capDriverGetCaps( hWndC, &caps, sizeof(&caps));
  ....
}

V568 sizeof() 演算子の引数が '&caps' 式であることは奇妙です。 cap_vfw.cpp 409

CAPDRIVERCAPS 構造体のサイズの代わりにポインター サイズが capDriverGetCaps() 関数に渡されます。

別のコード片を次に示します。エラーは印刷ミスが原因だったに違いありません。ゼロで埋められるのは「latestCounts」配列ですが、代わりに計算されるのは「latestPoints」配列のサイズです。

class CV_EXPORTS CvCalibFilter
{
  ....
  enum { MAX_CAMERAS = 3 };
  int latestCounts[MAX_CAMERAS];
  CvPoint2D32f* latestPoints[MAX_CAMERAS];
  ....
};

void CvCalibFilter::SetCameraCount( int count )
{
  ....
  memset( latestCounts, 0, sizeof(latestPoints) );
  ....
}

V512 「memset」関数を呼び出すと、バッファ「latestCounts」のオーバーフローが発生します。 calibfilter.cpp 238

このコード フラグメントには、64 ビット エラーが含まれています。ポインターのサイズが 32 ビット アプリケーションの 'int' 型のサイズと一致するため、コードは 32 ビット プログラム バージョンで適切に機能します。ただし、64 ビット プログラム バージョンをコンパイルすると、バッファ オーバーフローが発生します。

奇妙ですが、これらのエラーは長期間気付かれないことがあります。まず、32 ビット プログラムは常に正しく動作します。ただし、64 ビット バージョンを使用している場合でも、配列を超えてメモリをクリアしても害はありません。これらのエラーは通常、別のコンパイラの使用を開始したり、近くのコード フラグメントのリファクタリングを実行したりすると明らかになります。

不十分なテスト

少し前に書いた投稿で、テストのエラーは TDD テクノロジの脆弱性の 1 つであることをお伝えしました。テストは多くの場合、プログラムのセキュリティを提供するだけのふりをしています。静的コード分析は、TDD 方法論を補完する非常に優れた方法です。プログラム テキストのバグを見つけるだけでなく、テストからバグの多くを排除するのにも役立ちます。

OpenCV ライブラリのテストでもエラーが見つかるのは当然です。

void CV_Resize_Test::resize_1d(....)
{
  ....
  for (int r = 0; r < cn; ++r)
  {
    xyD[r] = 0;
    for (int k = 0; k < ksize; ++k)
      xyD[r] += w[k] * xyS[k * cn + r];
    xyD[r] = xyD[r];
  }
  ....
}

V570 'xyD[r]' 変数がそれ自体に割り当てられています。 test_imgwarp_strict.cpp 560

「xyD[r] =xyD[r];」表情がとても怪しい。おそらく、このテストは、意図したとおりにチェックしていない可能性があります。

ここに別の行があります:"cls_map[r];"。どういう意味ですか?

void ann_get_new_responses(....)
{
  ....
  for( int si = 0; si < train_sidx->cols; si++ )
  {
    int sidx = train_sidx_ptr[si];
    int r = cvRound(responses_ptr[sidx*r_step]);
    CV_DbgAssert(fabs(responses_ptr[sidx*r_step]-r) < FLT_EPSILON);
    int cls_map_size = (int)cls_map.size();
    cls_map[r];
    if ( (int)cls_map.size() > cls_map_size )
      cls_map[r] = cls_count++;
  }
  ....
}

V607 所有者のない式 'cls_map[r]'。 test_mltests2.cpp 342

他にも奇妙なフラグメントがいくつかあります。例:

void Core_DetTest::get_test_array_types_and_sizes(....)
{
  ....
  sizes[INPUT][0].width =
  sizes[INPUT][0].height = sizes[INPUT][0].height;
  ....
}

V570 'sizes[INPUT][0].height' 変数がそれ自体に割り当てられています。 test_math.cpp 1356

不特定の動作

以下に示すコードは、プログラム内で希望どおりに機能する可能性があります。しかし、それが永遠に続くわけではないことに注意してください。負の数シフトを意味します。これらの変化の詳細については、記事「Wade not in in unknown waters. Part three」を参照してください。

CvSeq * cvFindNextContour( CvContourScanner scanner )
{
  ....
  new_mask = INT_MIN >> 1;
  ....
}

V610 未規定の動作。シフト演算子 '>>' を確認してください。左オペランド '(- 2147483647 - 1)' は負です。輪郭.cpp 1012

その他

void CvFuzzyMeanShiftTracker::SearchWindow::initDepthValues(....)
{
  unsigned int d=0, mind = 0xFFFF, maxd = 0,
           m0 = 0, m1 = 0, mc, dd;
  ....
  for (int j = 0; j < height; j++)
  {
    ....
    if (d > maxd)
      maxd = d;
    ....
  }
}

V547 式 'd> maxd' は常に false です。符号なし型の値が <0 になることはありません。fuzzymeanshifttracker.cpp 386

'd' 変数はループ内で変更されません。これは、'd> maxd' 条件が成立しないことを意味します。

void jpc_init_t2state(jpc_enc_t *enc, int raflag)
{
  ....
  for (pass = cblk->passes; pass != endpasses; ++pass) {
    pass->lyrno = -1;
    pass->lyrno = 0;
  }
  ....
}

V519 「pass->lyrno」変数には、連続して 2 回値が割り当てられます。おそらくこれは間違いです。チェック行:539, 540.jpc_t2enc.c 540


void KeyPointsFilter::retainBest(vector<KeyPoint>& keypoints, int
                                 n_points)
{
  ....
  if( n_points > 0 && keypoints.size() > (size_t)n_points )
  {
    if (n_points==0)
    {
      keypoints.clear();
      return;
    }
  ....
}

V637 2 つの反対の条件が発生しました。 2 番目の条件は常に false です。チェック行:195, 197. keypoint.cpp 195

void HOGDescriptor::detectMultiScaleROI(....) const
{
  ....
  double *linearwt = new double[totwords+1];
  ....
  delete linearwt;
  ....
}

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

結論

高度な資格を持つプログラマーでさえ間違いを犯すことはありませんが、PVS-Studio ツールを使用すると、コーディングの段階ですでに間違いの多くをなくすことができます。そうしないと、これらのエラーを検出して修正するのに 10 倍の費用がかかります。