インテル® IPP サンプル – エラーの修正

この記事は、インテル® ソフトウェア・ネットワークに掲載されている「Intel IPP Samples for Windows – error correction」(http://software.intel.com/en-us/articles/intel-ipp-samples-for-windows-error-correction/) の日本語参考訳です。

この記事は、PVS-Studio を使用することでプログラムがどのように安全になるかを説明した記事の 1 つで、エラーが検出される場所とエラーの種類について詳しく述べています。ここでは、インテル® インテグレーテッド・パフォーマンス・プリミティブ (インテル® IPP) 7.0 ライブラリーを使用したサンプルを紹介します。

インテル® Parallel Studio 2011 には、インテル® IPP ライブラリーが含まれています。このライブラリーには、効率的なビデオ/オーディオコーデック、信号処理ソフトウェア、イメージ・レンダリング、アーカイバーなどを作成できる多くのプリミティブが含まれています。

もちろん、このようなライブラリーを制御するのは容易なことではありません。そのため、インテルではこのライブラリーを使用したサンプルプログラムを多数用意しています。サンプルは以下からダウンロードできます。

インテル® インテグレーテッド・パフォーマンス・プリミティブ (インテル® IPP) のコードサンプル (http://www.viva64.com/go.php?url=449 (英語))
インテル® Parallel Studio 2011 用インテル® インテグレーテッド・パフォーマンス・プリミティブ 7.0 ライブラリーのサンプル (http://www.viva64.com/go.php?url=450 (英語))

ここでは、後者のサンプルを使用します。サンプルは 4 つのグループに分かれています。

IPP サンプル
IPP UIC デモ
IPP DMIP サンプル
IPP 暗号化サンプル

各セットには多くのプロジェクトが含まれているため、ここでは最初のセットである IPP サンプルを使って確認しました。サンプルの解析には、PVS-Studio 4.10 を使用しました。

この記事では、プログラマーのスキルや開発中のソリューションのレベルに関係なく、スタティック解析が有効であることを説明します。「エキスパートを使ってエラーのないコードを記述する」という考えは、ほとんどの場合うまくいきません。非常に熟練した開発者であっても、コードの記述中に誤ったコーディングをしたり入力ミスしてしまう可能性はゼロではないからです。IPP サンプルには、そういったエラーが含まれています。

IPP サンプルは非常に質の高いコードですが、そのコード行数は実に 160 万行にも及ぶものであり、いくつかのエラーが含まれています。では、その一部を具体的に見ていきましょう。

配列のインデックスの誤った指定

このサンプルは、こちらの記事の説明にも使用できるでしょう。

struct AVS_MB_INFO
{
  ...
  Ipp8u refIdx[AVS_DIRECTIONS][4];
  ...
};

void AVSCompressor::GetRefIndiciesBSlice(void){
  ...
  if (m_pMbInfo->predType[0] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][0];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[1] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][1];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[2] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][2];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[3] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][30];
    iRefNum += 1;
  }
  ...
}

PVS-Studio の診断メッセージ: V557 Array overrun is possible. The ’30’ index is pointing beyond array bound. avs_enc umc_avs_enc_compressor_enc_b.cpp 495 (V557 配列の範囲を超えている可能性があります。インデックス ’30’ は配列の境界外です。avs_enc umc_avs_enc_compressor_enc_b.cpp 495)

プログラマーはコードの一部をコピーして配列のインデックスを変更したようですが、最後のところで数字の 3 を入力した後に 0 を削除することを忘れています。その結果、インデックス 30 が指定され、配列の境界をはるかに越えてしまったのです。

同一のコード分岐

コードのコピーについて触れたついでに、似たような例を説明しておきましょう。

AACStatus aacencGetFrame(...)
{
  ...
  if (maxEn[0] > maxEn[1]) {
    ics[1].num_window_groups = ics[0].num_window_groups;
    for (g = 0; g < ics[0].num_window_groups; g++) {
      ics[1].len_window_group[g] = ics[0].len_window_group[g];
    }
  } else {
    ics[1].num_window_groups = ics[0].num_window_groups;
    for (g = 0; g < ics[0].num_window_groups; g++) {
      ics[1].len_window_group[g] = ics[0].len_window_group[g];
    }
  }
  ...
}

PVS-Studio の診断メッセージ: V523 The ‘then’ statement is equivalent to the ‘else’ statement. aac_enc aac_enc_api_fp.c 1379 (V523 ‘then’ 文と ‘else’ 文が同じです。aac_enc aac_enc_api_fp.c 1379)

この例では、プログラマーがコピーしたコードを編集し忘れたため、条件演算子 “if” の分岐はどちらも同じ動作を行っています。

デクリメント演算子 “–” とポインターの逆参照 “*” によるエラー

static void
sbrencConflictResolution (..., Ipp32s *nLeftBord)
{
  ...
  *nLeftBord = nBordNext - 1;
  ...
  if (*lenBordNext > 1) {
    ...
    *nLeftBord--;
  }
  ...
}

PVS-Studio の診断メッセージ: V532 Consider inspecting the statement of ‘*pointer–‘ pattern. Probably meant: ‘(*pointer)–‘. aac_enc sbr_enc_frame_gen.c 428 (V532 ‘*pointer–‘ 形式の文を確認してください。'(*pointer)–‘ の間違いである可能性があります。aac_enc sbr_enc_frame_gen.c 428)

“nLeftBord” ポインターは “sbrencConflictResolution” 関数の値を返します。最初の値 “nBordNext – 1” は指定したアドレスで記述されます。特定の条件では、この値を 1 つデクリメントする必要があります。値をデクリメントするために、プログラマーは次のコードを使用しています。

*nLeftBord--;

しかし、これでは値の代わりにポインター自身がデクリメントされてしまいます。正しいコードは次のようになります。

(*nLeftBord)--;

インクリメント演算子 “++” とポインターの逆参照 “*” によるエラー

次のコードは全く理解不能です。修正方法も分かりません。おそらく、何かが不足しています。

static IppStatus mp2_HuffmanTableInitAlloc(Ipp32s *tbl, ...)
{
  ...
  for (i = 0; i < num_tbl; i++) {
    *tbl++;
  }
  ...
}

PVS-Studio の診断メッセージ: V532 Consider inspecting the statement of ‘*pointer++’ pattern. Probably meant: ‘(*pointer)++’. mpeg2_dec umc_mpeg2_dec.cpp 59 (V532 ‘*pointer++’ 形式の文を確認してください。'(*pointer)++’ の間違いである可能性があります。mpeg2_dec umc_mpeg2_dec.cpp 59)

ここで、上記のサンプルのループは次のコードと等価です。

tbl += num_tbl;

PVS-Studio アナライザーは、括弧が不足していてコードは “(*tbl)++;” のようになると仮定しています。しかし、このコードも意味がありません。この場合、ループは次のコードと等価です。

*tbl += num_tbl;

このループは奇妙なループです。おそらくエラーが含まれていますが、修正方法はこのコードを記述した人にしか分からないでしょう。

エラーフラグの消失

コードにはエラーが発生したときに “-1” を返す “GetTrackByPidOrCreateNew” 関数が含まれています。

typedef signed int     Ipp32s;
typedef unsigned int   Ipp32u;

Ipp32s StreamParser::GetTrackByPidOrCreateNew(
  Ipp32s iPid, bool *pIsNew)
{
  ...
  else if (!pIsNew || m_uiTracks >= MAX_TRACK)
    return -1;
  ...
}

“GetTrackByPidOrCreateNew” 関数自体に問題はありません。しかし、この関数を使用するとエラーが発生します。

Status StreamParser::GetNextData(MediaData *pData, Ipp32u *pTrack)
{
  ...
  *pTrack = GetTrackByPidOrCreateNew(m_pPacket->iPid, NULL);

  if (*pTrack >= 0 && TRACK_LPCM == m_pInfo[*pTrack]->m_Type)
    ippsSwapBytes_16u_I((Ipp16u *)pData->GetDataPointer(),
                        m_pPacket->uiSize / 2);
  ...
}

PVS-Studio の診断メッセージ: V547 Expression ‘* pTrack >= 0’ is always true. Unsigned type value is always >= 0. demuxer umc_stream_parser.cpp 179 (V547 式 ‘* pTrack >= 0’ は常に真です。符号なし型の値は、常に >= 0 になります。demuxer umc_stream_parser.cpp 179)

この例では、”GetTrackByPidOrCreateNew” 関数によって返された値が符号なし整数型として保存されています。つまり、”-1″ が “4294967295” になるため、 “*pTrack >= 0” 条件は常に真になります。

その結果、”GetTrackByPidOrCreateNew” 関数が “-1″ を返した場合、”m_pInfo[*pTrack]->m_Type” の実行中にアクセス違反が発生します。

コピーペースト後のコードの追加漏れ

void H264SegmentDecoder::ResetDeblockingVariablesMBAFF()
{
  ...
  if (GetMBFieldDecodingFlag(m_gmbinfo->mbs[m_CurMBAddr
                                            - mb_width * 2]))
    m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
      m_CurMBAddr - mb_width * 2;
  else
    m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
      m_CurMBAddr - mb_width * 2;
  ...
}

PVS-Studio の診断メッセージ: V523 The ‘then’ statement is equivalent to the ‘else’ statement. h264_dec umc_h264_segment_decoder_deblocking_mbaff.cpp 340 (V523 ‘then’ 文と ‘else’ 文が同じです。h264_dec umc_h264_segment_decoder_deblocking_mbaff.cpp 340)

コードをよく見ると、プログラマーがコピーした行に + 1 を追加するのを忘れていることが分かります。正しいコードは次のようになります。

if (GetMBFieldDecodingFlag(m_gmbinfo->mbs[m_CurMBAddr
                                          - mb_width * 2]))
  m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
    m_CurMBAddr - mb_width * 2;
else
  m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
    m_CurMBAddr - mb_width * 2 + 1;

この場所からそれほど離れていない H264CoreEncoder_ResetDeblockingVariablesMBAFF 関数でも、”+ 1″ の追加漏れによる同じエラーが発生しています。

PVS-Studio の診断メッセージ: V523 The ‘then’ statement is equivalent to the ‘else’ statement. h264_enc umc_h264_deblocking_mbaff_tmpl.cpp.h 366 (V523 ‘then’ 文と ‘else’ 文が同じです。h264_enc umc_h264_deblocking_mbaff_tmpl.cpp.h 366)

何も削除しない削除

void H264ThreadGroup::RemoveThread(H264Thread * thread)
{
    AutomaticUMCMutex guard(m_mGuard);
    std::remove(m_threads.begin(), m_threads.end(), thread);
}

PVS-Studio の診断メッセージ: V530 The return value of function ‘remove’ is required to be utilized. h264_dec umc_h264_thread.cpp 226 (V530 関数 ‘remove’ の戻り値を使用する必要があります。h264_dec umc_h264_thread.cpp 226)

これは面白い組み合わせです。コードの個々の部分に問題はありません。マルチスレッド・アプリケーションで項目を正しく削除できるように mutex が使用されていますが、 std::remove 関数は配列から項目を削除しないで再配置するだけです。開発者は、このことをすっかり忘れているようです。正しいコードは次のようになります。

m_threads .erase(
  std::remove(m_threads.begin(), m_threads.end(), thread),
  m_threads.end());

構造体のフィールドと自身の比較

エラーを確認しているときに、H264 ビデオ圧縮標準の実装が不完全であることに気づきました。見つかったエラーの多くは、このプロジェクトに関連していました。例えば、プログラマーは急いでいたために 2 つの誤った変数名を同時に使用していました。

bool H264_AU_Stream::IsPictureSame(H264SliceHeaderParse & p_newHeader)
{
  if ((p_newHeader.frame_num != m_lastSlice.frame_num) ||
      (p_newHeader.pic_parameter_set_id !=
       p_newHeader.pic_parameter_set_id) ||
      (p_newHeader.field_pic_flag != p_newHeader.field_pic_flag) ||
      (p_newHeader.bottom_field_flag != m_lastSlice.bottom_field_flag)
      ){
      return false;
  }
  ...
}

PVS-Studio の診断メッセージ:

V501 There are identical sub-expressions ‘p_newHeader.pic_parameter_set_id’ to the left and to the right of the ‘!=’ operator. h264_spl umc_h264_au_stream.cpp 478 (V501 ‘!=’ 演算子の左辺と右辺に同じ部分式 ‘p_newHeader.pic_parameter_set_id’ があります。h264_spl umc_h264_au_stream.cpp 478)

V501 There are identical sub-expressions ‘p_newHeader.field_pic_flag’ to the left and to the right of the ‘!=’ operator. h264_spl umc_h264_au_stream.cpp 479 (V501 ‘!=’ 演算子の左辺と右辺に同じ部分式 ‘p_newHeader.field_pic_flag’ があります。h264_spl umc_h264_au_stream.cpp 479)

構造体の一部のメンバーが自身と比較されているため、比較関数は動作しません。正しいコードは次のようになります。

(p_newHeader.pic_parameter_set_id != m_lastSlice.pic_parameter_set_id)
(p_newHeader.field_pic_flag != m_lastSlice.field_pic_flag)

間違ったデータコピー

間違ったオブジェクトの使用に関連するエラーは、比較だけではなく、オブジェクトの状態をコピーする場合にも発生します。

Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  VOL.sprite_width = par->sprite_width;
  VOL.sprite_height = par->sprite_height;
  VOL.sprite_left_coordinate = par->sprite_left_coordinate;
  VOL.sprite_top_coordinate = par->sprite_left_coordinate;
  ...
}

PVS-Studio の診断メッセージ: V537 Consider reviewing the correctness of ‘sprite_left_coordinate’ item’s usage. mpeg4_enc mp4_enc_misc.cpp 387 (V537 ‘sprite_left_coordinate’ の使用法を確認してください。mpeg4_enc mp4_enc_misc.cpp 387) 間違った値が “VOL.sprite_top_coordinate” に保存されています。正しいコードは次のようになります。

VOL.sprite_top_coordinate = par->sprite_top_coordinate;

1 変数による 2 つのループ

JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void)
{
  ...
  for(c = 0; c < m_scan_ncomps; c++)
  {
    block = m_block_buffer + (DCTSIZE2*m_nblock*(j+(i*m_numxMCU)));

    // skip any relevant components
    for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++)
    {
      block += (DCTSIZE2*m_ccomp[c].m_nblocks);
     }
   ...
 }

PVS-Studio の診断メッセージ: V535 The variable ‘c’ is being used for this loop and for the outer loop. jpegcodec jpegdec.cpp 4652 (V535 変数 ‘c’ が内側のループと外側のループの両方で使用されています。jpegcodec jpegdec.cpp 4652)

1 つの変数 ‘c’ が互いに入れ子になっている 2 つのループで使用されています。このようなデコード関数は予測できない結果を引き起こすことがあります。

冗長な多重代入

H264EncoderFrameType*
H264ENC_MAKE_NAME(H264EncoderFrameList_findOldestToEncode)(...)
{
  ...
  MaxBrefPOC =
    H264ENC_MAKE_NAME(H264EncoderFrame_PicOrderCnt)(pCurr, 0, 3);
  MaxBrefPOC =
    H264ENC_MAKE_NAME(H264EncoderFrame_PicOrderCnt)(pCurr, 0, 3);
  ...
}

PVS-Studio の診断メッセージ: V519 The ‘MaxBrefPOC’ object is assigned values twice successively. Perhaps this is a mistake. h264_enc umc_h264_enc_cpb_tmpl.cpp.h 784 (V519 ‘MaxBrefPOC’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。h264_enc umc_h264_enc_cpb_tmpl.cpp.h 784)

このコードを見たとき、あるプログラマーの笑い話を思い出しました。

– なぜ全く同じ GOTO を 2 回続けて書いているんだい?
– もし最初の GOTO が動かなかったら困るじゃないか!

もちろん致命的なエラーではありませんが、エラーであることに変わりはありません。

コードの警告

AACStatus sbrencResampler_v2_32f(Ipp32f* pSrc, Ipp32f* pDst)
{
  ...
  k = nCoef-1;
  k = nCoef;
  ...
}

PVS-Studio の診断メッセージ: V519 The ‘k’ object is assigned values twice successively. Perhaps this is a mistake. aac_enc sbr_enc_resampler_fp.c 90 (V519 ‘k’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。aac_enc sbr_enc_resampler_fp.c 90)

この多重代入は、前のサンプルとは違った意味での警告でした。プログラマーはコーディングに自信がないようです。あたかも、最初に “nCoef-1″ に決定した後、”nCoef” に決定し直したように見えます。このようなコードは “実験プログラミング” と呼ばれることもあります。とにかく、こういった場合は、落ち着いてコードを眺めてみるべきです。

最小でない最小値

void MeBase::MakeVlcTableDecision()
{
  ...
  Ipp32s BestMV= IPP_MIN(IPP_MIN(m_cur.MvRate[0],m_cur.MvRate[1]),
                         IPP_MIN(m_cur.MvRate[2],m_cur.MvRate[3]));
  Ipp32s BestAC= IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                         IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2]));
  ...
}

PVS-Studio の診断メッセージ: V501 There are identical sub-expressions to the left and to the right of the ‘<' operator: (m_cur.AcRate [2]) < (m_cur.AcRate [2]) me umc_me.cpp 898 (V501 '<' 演算子の左辺と右辺に同じ部分式があります: (m_cur.AcRate [2]) < (m_cur.AcRate [2]) me umc_me.cpp 898)

配列のインデックスを間違って入力した別の例です。最後のインデックスは、2 ではなく 3 でなければなりません。正しいコードは次のようになります。

Ipp32s BestAC= IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                       IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3]));

コードのほかの部分に問題がない場合、この種のエラーはやっかいです。ここでは、最小項目が “m_cur.AcRate[3]” に保存された場合にのみこのエラーが発生します。この種のエラーは、テスト中に発見できず、ユーザーが特定のデータを入力したときに初めて分かることもあります。

最大でない最大値

最大値に関する問題もあります。

Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  i = IPP_MAX(mBVOPsearchHorBack, mBVOPsearchHorBack);
  ...
}

PVS-Studio の診断メッセージ: V501 There are identical sub-expressions ‘(mBVOPsearchHorBack)’ to the left and to the right of the ‘>’ operator. mpeg4_enc mp4_enc_misc.cpp 547 (V501 ‘>’ 演算子の左辺と右辺に同じ部分式 ‘(mBVOPsearchHorBack)’ があります。mpeg4_enc mp4_enc_misc.cpp 547)

mBVOPsearchHorBack 変数は 2 回使用されています。実際には、プログラマーは mBVOPsearchHorBack と mBVOPsearchVerBack を使用するつもりでした。

i = IPP_MAX(mBVOPsearchHorBack, mBVOPsearchVerBack);

単純な入力ミス

typedef struct
{
  ...
  VM_ALIGN16_DECL(Ipp32f)  nb_short[2][3][__ALIGNED(MAX_PPT_SHORT)];
  ...
} mpaPsychoacousticBlock;

static void mp3encPsy_short_window(...)
{
  ...
  if (win_counter == 0) {
    nb_s = pBlock->nb_short[0][3];
  }
  ...
}

PVS-Studio の診断メッセージ: V557 Array overrun is possible. The ‘3’ index is pointing beyond array bound. mp3_enc mp3enc_psychoacoustic_fp.c 726 (V557 配列の範囲を超えている可能性があります。インデックス ‘3’ は配列の境界外です。mp3_enc mp3enc_psychoacoustic_fp.c 726)

単純な入力ミスにより、インデックス ‘3’ が ‘2’ の代わりに使用されていると想定されます。結果はご想像のとおりです。

速度低下を引き起こすエラー

void lNormalizeVector_32f_P3IM(Ipp32f *vec[3], Ipp32s* mask,
                               Ipp32s len) {
  Ipp32s  i;
  Ipp32f  norm;

  for(i=0; i<len; i++) {
    if(mask<0) continue;
    norm = 1.0f/sqrt(vec[0][i]*vec[0][i]+
           vec[1][i]*vec[1][i]+
           vec[2][i]*vec[2][i]);
           vec[0][i] *= norm; vec[1][i] *= norm; vec[2][i] *= norm;
  }
}

PVS-Studio の診断メッセージ: V503 This is a nonsensical comparison: pointer < 0. ipprsample ippr_sample.cpp 501 (V503 無意味な比較です: pointer < 0。ipprsample ippr_sample.cpp 501)

エラーにより速度が低下するサンプルコードの例です。このアルゴリズムはマスク配列で指定された項目のみを正規化するべきですが、このコードはすべての項目を正規化します。エラーは “if(mask<0)" 条件にあります。プログラマーは "i" インデックスを使用することを忘れています。"mask" ポインターはほとんどの場合ゼロ以上になるので、すべての項目が処理されてしまいます。

正しいコードは次のようになります。

if(mask[i]<0) continue;

常に 0 になる計算

int ec_fb_GetSubbandNum(void *stat)
{
    _fbECState *state=(_fbECState *)stat;
    return (state->freq-state->freq);
}

PVS-Studio の診断メッセージ: V501 There are identical sub-expressions to the left and to the right of the ‘-‘ operator: state->freq – state->freq speech ec_fb.c 250 (V501 ‘-‘ 演算子の左辺と右辺に同じ部分式があります: state->freq – state->freq speech ec_fb.c 250)

このエラーにより、関数は常に 0 を返します。何か間違ったものを引いていると思われますが、その項目が何であるかは分かりません。

バッファー・オーバーフローの間違った処理

typedef unsigned int    Ipp32u;

UMC::Status Init(..., Ipp32u memSize, ...)
{
  ...
  memSize -= UMC::align_value<Ipp32u>(m_nFrames*sizeof(Frame));
  if(memSize < 0)
      return UMC::UMC_ERR_NOT_ENOUGH_BUFFER;
  ...
}

PVS-Studio の診断メッセージ: V547 Expression ‘memSize < 0' is always false. Unsigned type value is never < 0. vc1_enc umc_vc1_enc_planes.h 200 (V547 式 'memSize < 0' は常に偽になります。符号なし型は < 0 になることはありません。vc1_enc umc_vc1_enc_planes.h 200)

バッファーサイズが不足している場合の処理が間違って実装されています。プログラムはエラーコードを返す代わりに動作を続け、クラッシュするでしょう。問題は、”memSize” 変数が “符号なし整数” 型であることです。このため、”memSize < 0" 条件は常に偽になり、バッファーがオーバーフローした状態で動作が続けられます。

これは、ソフトウェアの脆弱性を表す一例でもあります。例えば、間違ったデータをプログラムに渡してバッファー・オーバーフローを意図的に引き起こすことができます。ところで、このコードには同様のエラーが約 10 個含まれていました。ここでは詳しい説明は省略します。

間違った判定によるオーバーラン

Ipp32u m_iCurrMBIndex;
VC1EncoderMBInfo* VC1EncoderMBs::GetPevMBInfo(Ipp32s x, Ipp32s y)
{
  Ipp32s row = (y>0)? m_iPrevRowIndex:m_iCurrRowIndex;
  return ((m_iCurrMBIndex - x <0 || row <0)? 0 :
    &m_MBInfo[row][m_iCurrMBIndex - x]);
}

PVS-Studio の診断メッセージ: V547 Expression ‘m_iCurrMBIndex – x < 0' is always false. Unsigned type value is never < 0. vc1_enc umc_vc1_enc_mb.cpp 188 (V547 式 'm_iCurrMBIndex - x < 0' は常に偽です。符号なし型は < 0 になることはありません。vc1_enc umc_vc1_enc_mb.cpp 188)

“m_iCurrMBIndex” 変数は “符号なし” 型です。このため、”m_iCurrMBIndex – x” 式も “符号なし” 型になります。つまり、”m_iCurrMBIndex – x < 0" 条件は常に偽になります。どんな結果になるか見てみましょう。

例えば、”m_iCurrMBIndex” 変数が 5、”x” 変数が 10 であるとします。

“m_iCurrMBIndex – x” 式は 5u – 10i = 0xFFFFFFFBu となります。

“m_iCurrMBIndex – x < 0" 条件は偽です。

“m_MBInfo[row][0xFFFFFFFBu]” 式が実行され、オーバーランが発生します。

‘?:’ 三項演算子の使用によるエラー

三項演算子の使用は間違いを起こしやすいため、注意が必要です。しかし、プログラマーは、できるだけ短いコードを記述しようとするものです。また、変わった言語構造を使いたがります。例えば、次のように記述することがよくあります。

vm_file* vm_file_fopen(...)
{
  ...
  mds[3] = FILE_ATTRIBUTE_NORMAL |
           (islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING;
  ...
}

PVS-Studio の診断メッセージ: V502 Perhaps the ‘?:’ operator works in a different way than it was expected. The ‘?:’ operator has a lower priority than the ‘|’ operator. vm vm_file_win.c 393 (V502 ‘?:’ 演算子の動作が期待とは異なる可能性があります。’?:’ 演算子よりも ‘|’ 演算子のほうが優先されます。vm vm_file_win.c 393)

“mds[3]” 項目には、フラグ FILE_ATTRIBUTE_NORMAL と FILE_FLAG_NO_BUFFERING を組み合わせた値が代入されるはずです。しかし、”mds[3]” 項目には常に 0 が代入されます。

プログラマーは “|” 演算子の優先順位が “?:” 演算子よりも高いことを忘れていたのです。このため、コードの式は次のように処理されます (括弧に注意)。

(FILE_ATTRIBUTE_NORMAL | (islog == 0)) ?

0 : FILE_FLAG_NO_BUFFERING;

“FILE_ATTRIBUTE_NORMAL | (islog == 0)” 条件は常に真となり、”mds[3]” 項目に 0 が代入されます。
正しいコードは次のようになります (括弧に再度注意)。

FILE_ATTRIBUTE_NORMAL |
  ((islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING);

配列制御のエラー

AACStatus alsdecGetFrame(...)
{
  ...
  for (i = 0; i < num; i++) {
    ...
    *tmpPtr = (Ipp32s)((tmp << 24) + ((tmp & 0xff00) << 8) +
                      ((tmp >> 8) & 0xff00) + (tmp >> 24));
    *tmpPtr = *srcPrt;
    ...
  }
  ...
}

PVS-Studio の診断メッセージ: V519 The ‘* tmpPtr’ object is assigned values twice successively. Perhaps this is a mistake. aac_dec als_dec_api.c 928 (V519 ‘* tmpPtr’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。aac_dec als_dec_api.c 928)

読者の方は、このコードを自分で調べて結論を出してみてください。このコードは “特殊である” とだけ述べておきましょう。

不自然な代入

static
IPLStatus ownRemap8u_Pixel(...) {
  ...
  saveXMask    = xMap->maskROI;
  saveXMask    = NULL;
  saveYMask    = yMap->maskROI;
  saveYMask    = NULL;
  ...
}

PVS-Studio の診断メッセージ:

V519 The ‘saveXMask’ object is assigned values twice successively. Perhaps this is a mistake. ipl iplremap.c 36 (V519 ‘saveXMask’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。 ipl iplremap.c 36)

V519 The ‘saveYMask’ object is assigned values twice successively. Perhaps this is a mistake. ipl iplremap.c 38 (V519 ‘saveYMask’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。ipl iplremap.c 38)

このような奇妙なコードがなぜ記述されたのか理解できません。このブロックは異なる関数で 8 回も繰り返されているのです!

この他にも 1 つの変数で不自然な代入がありました。

Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  mNumOfFrames = par->NumOfFrames;
  mNumOfFrames = -1;
  ...
}

PVS-Studio の診断メッセージ: V519 The ‘mNumOfFrames’ object is assigned values twice successively. Perhaps this is a mistake. mpeg4_enc mp4_enc_misc.cpp 276 (V519 ‘mNumOfFrames’ オブジェクトに続けて 2 回値が代入されています。間違いの可能性があります。mpeg4_enc mp4_enc_misc.cpp 276)

まとめ

この記事では、IPP サンプルで検出されるエラーの一部について説明しました。すべてのエラーを説明すると冗長になるため、この記事ではすでに説明したエラーと同様のエラーはリストしていません。また、入力ミスにより常に真になる assert() のように、ここで取り上げる必要がないと思われるエラーについても説明していません。エラーが含まれるかどうか分からなかった多くのコードは省略しました。しかし、熟練した開発者であっても、大規模なプロジェクトを記述することがいかに困難であるかをご理解いただけたと思います。

この記事の最初に述べた考えを再度述べることにしましょう。優れたプログラマーであっても、入力ミス、うっかりミス、コピーペーストによるミス、論理的なミスを起こす可能性はゼロではありません。この記事が、”正しいコードを記述する” ことですべてのエラーを防げると信じている人々への答えとなることを期待しています。

皆さんの C/C++/C++0x プロジェクトがうまくいくことをお祈りしています。私の大好きなスタティック解析手法を使用して皆さんができるだけ多くのエラーを見つけることができますように！

インテル® ソフトウェア製品のパフォーマンス/最適化に関する詳細は、最適化に関する注意事項 (英語) を参照してください。