PVS-Studio は CERN を支援するためにあります:Geant4 プロジェクトの分析

Geant4 プロジェクトは開発を続けているので、PVS-Studio 静的コードアナライザーで再確認するのは非常に興味深いことです。今回はバージョン 10.2 のチェックを行います (以前は 10.0 ベータ版をチェックしました)。

はじめに

Geant4 ツールキットは CERN で開発されており、モンテカルロ法を使用して物質を通過する際の粒子の挙動のシミュレーションと調査を行います。プロジェクトの初期のバージョンは Fortran で書かれており、バージョン 4 以降、プロジェクトはオブジェクト指向言語 C++ に完全に変換されました。

このプロジェクトの詳細については、プロジェクトの公式サイトをご覧ください:http://geant4.org.

このプロジェクトはすでに数回チェックされています。他の記事で結果を見つけることができます。バージョン 9.4 の解析は記事「コピペとミュオン」で説明され、バージョン 10.0-beta のチェックは記事「Geant4 のチェックを続ける」で説明されています

前回プロジェクトを確認したときから、Geant 4 はバージョン 10.02 にアップグレードされました。 PVS-Studio もバージョン 6.05 に更新されたので、このバージョンを使用しました。

プロジェクトでは、条件と比較の使用に関連して、かなりの数のエラーが発生しました。論理エラーは通常、将来の開発のためにコードを残すか、分岐ステートメントを含むコードの以前の部分を削除して不正確な変更を行ったときに発生します。同時に、単純なタイプミスや式の推論の欠如が、エラーや冗長なコードにつながる可能性があります。

状況の辛さ

Geant4 のこのチェックには多少の熱意がありました。私の知る限り、開発チームはすでに静的コードアナライザーである Coverity を定期的に使用しているからです。さまざまなリリースノートと、次のようなコード内のコメントを見て、この結論を導き出しました。

// Private copy constructor and assigment operator - copying and
// assignment not allowed. Keeps Coverity happy.

Coverity アナライザーはコードアナライザー市場のリーダーと見なされているため、Coverity 解析後に何かを見つけることはすでに大きな成果です。それにもかかわらず、PVS-Studio は多くの興味深いバグを発見しました。これは、PVS-Studio が強力で成熟した製品になったことも示しています。

'else' がありません

G4double G4EmBiasingManager::ApplySecondaryBiasing(....)
{
  ....
  if(0 == nsplit) { 
    ....
  } if(1 == nsplit) { // <=
    ....
  } else {
    ....
  }
  ....
}

V646 アプリケーションのロジックを調べることを検討してください。「else」キーワードが欠落している可能性があります。 g4embiasingmanager.cc 299

これは、if を使用して 1 つの変数の複数の値をチェックする際に発生する最も一般的なエラーの 1 つです。 .もちろん、単にフォーマットが正しくない可能性もありますが、この例では、アナライザーが実際のバグを指摘している可能性が最も高いです。

コピーの結果、else 単語が忘れられたままになっているため、この場合、過剰なコードの実行につながります。たとえば、値がゼロになり、対応するブロックからコードが実行されますが、エラーのため、else のコードが 1 との比較後にブロックします。この問題を解決するには、不足している else を追加する必要があります条件 if(1 ==nsplit) の前 .

潜在的なエラーの不適切な処理

void G4GenericPolycone::Create( .... )
{
  ....
  G4double rzArea = rz->Area();
  if (rzArea < -kCarTolerance)
    rz->ReverseOrder();

  else if (rzArea < -kCarTolerance)   // <=
  {
    ....
    G4Exception("G4GenericPolycone::Create()", 
                "GeomSolids0002",
                FatalErrorInArgument, message);
  }
  ....
}

V517 「if (A) {...} else if (A) {...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。行を確認してください:102, 105. g4genericpolycone.cc 102

このコードが何を意図していたのかを推測することしかできません。このフラグメントは、エラーメッセージをキャッチして形成することを目的としている可能性が非常に高いですが、誤った条件のキャストでは、エラーメッセージは表示されません。プログラムが後でどのように動作するかは不明です。ハンドラーが別の場所でバグをキャッチする可能性がありますが、プログラムがエラーなしで動作し続ける可能性がありますが、誤った結果が出力されます。この問題の原因を正確に言うのは非常に困難です。条件式の 1 つと過剰な else の両方にある可能性があるためです。キーワード。しかし、フォーマットから判断すると、両方の条件付きブロックが正しいと安全に想定でき、 else を削除する必要があります。 2 番目の条件付きブロックの前。

コピー＆ペーストのおかげで、このエラーは複製され、さらに 3 つのフラグメントで見つかりました:

V517 「if (A) {...} else if (A) {...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。行を確認してください:193, 196. g4polycone.cc 193
V517 「if (A) {...} else if (A) {...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。チェック行:219, 222. g4polyhedra.cc 219
V517 「if (A) {...} else if (A) {...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。チェック行:207, 211. g4persistencycentermessenger.cc 207

ヌルポインター逆参照

G4double * theShells;
G4double * theGammas;

void G4ParticleHPPhotonDist::InitAngular(....)
{
 ....
 if ( theGammas != NULL ) 
 {
   for ( i = 0 ; i < nDiscrete ; i++ )
   {
     vct_gammas_par.push_back( theGammas[ i ] );
     vct_shells_par.push_back( theShells[ i ] );
     ....
   }
 }
 if ( theGammas == NULL ) theGammas = new G4double[nDiscrete2];
 if ( theShells == NULL ) theShells = new G4double[nDiscrete2];
 .... 
}

V595 'theShells' ポインターは、nullptr に対して検証される前に使用されました。チェック行:147, 156. g4particlehpphotondist.cc 147

プログラムでは、ポインター処理に関連するエラーがよく見られます。この場合、2 つのオブジェクトが同時に処理されますが、正しさをチェックするのは 1 つのみです。このエラーは長い間気付かれないままになる可能性がありますが、theShells へのポインターが null であることが判明すると、未定義のプログラム動作が発生します。これを修正するには、次のように条件を変更する必要があります:

if ( theGammas != NULL && theShells != NULL) ....

ポインターのチェックが欠落しているもう 1 つのフラグメント。

V595 'fCurrentProcess' ポインターは、nullptr に対して検証される前に使用されました。チェック行:303, 307. g4steppingmanager2.cc 303

ヌルポインタの使用法

G4hhElastic::G4hhElastic(....) 
  : G4HadronElastic("HadrHadrElastic")
{
  ....
  fTarget = target; // later vmg
  fProjectile = projectile;
  ....
  fTarget  = G4Proton::Proton(); // later vmg
  fProjectile  = 0;                        // <=
  fMassTarg   = fTarget->GetPDGMass();
  fMassProj   = fProjectile->GetPDGMass(); // <=
  ....
}

V522 null ポインター「fProjectile」の逆参照が発生する可能性があります。 g4hhelastic.cc 184

このフラグメントは前のものと似ています。しかし、ここではゼロ値が明示的に割り当てられたポインターがあり、その後、変数は他の変数の初期化に使用されます。プログラマーは、最初の代入の変数値を使用するつもりだった可能性があるため、2 番目の代入は不要です。おそらく、0 は別の変数に割り当てられるはずでした。この割り当ての本当の理由は、プロジェクトの開発者だけが知っています。いずれにせよ、このような初期化は正しくないため、このコードフラグメントは確認する価値があります。

無効なビット演算

#define dependentAxis 1
#define allowByRegion 2

static enum xDataTOM_interpolationFlag 
  xDataTOM_interpolation_getFromString( .... ) {
    ....
    if( flag | allowByRegion ) {....}  // <=
    if( flag | dependentAxis ) {....}  // <=
    ....
}

V617 状態の検査を検討してください。「|」の「2」引数ビット演算にゼロ以外の値が含まれています。 xdatatom_interpolation.cc 85
V617 状態の検査を検討してください。 '|' の '1' 引数ビット演算にゼロ以外の値が含まれています。 xdatatom_interpolation.cc 88

アナライザーは、関数の 2 つの隣接する文字列に対して警告を発行しました。条件内にゼロ以外の定数を持つビット単位の OR があります。このような式の結果は常にゼロ以外になり、プログラムのロジックが正しくなくなります。このようなエラーは、タイプミスが原因で発生することがよくあります。また、条件では、ビットごとの OR の代わりに、別のビットごとの演算を使用する必要があります。この場合、作成者はビットごとの AND を使用するつもりだったので、次のようになるはずです:

if( flag & allowByRegion ) {....}
if( flag & dependentAxis ) {....}

追加課題

G4ThreeVector G4GenericTrap::SurfaceNormal(....) const
{
  ....
  if ( noSurfaces == 0 )
  {
    ....
    sumnorm=apprnorm;
  }
  else if ( noSurfaces == 1 )  { sumnorm = sumnorm; } // <=
  else                         { sumnorm = sumnorm.unit(); }
  ....
}

V570 「sumnorm」変数がそれ自体に割り当てられています。 g4generictrap.cc 515

このコードフラグメントでは、冗長な条件文に論理エラーがあります。ここにあることを意図したものの 1 つ:1 つに対する検証中に、変数は別の変数に割り当てられ、その名前も sumnorm. と似ています。しかし、コードのチェックされた部分にそのような変数は見られなかったので、これは単なる冗長なチェックであると推測する危険があります。これを修正するには、次のように条件を単純化しましょう:

if ( noSurfaces == 0 )
{
  ....
  sumnorm=apprnorm; 
}
else if ( noSurfaces != 1 ) { sumnorm = sumnorm.unit(); }

別の疑わしいフラグメント:

void G4UImanager::StoreHistory(G4bool historySwitch,....)
{
  if(historySwitch)
  {
    if(saveHistory)
    { historyFile.close(); }
    historyFile.open((char*)fileName);
    saveHistory = true;
  }
  else
  {
    historyFile.close();
    saveHistory = false;
  }
  saveHistory = historySwitch;
}

V519 'saveHistory' 変数には、連続して 2 回値が割り当てられます。おそらくこれは間違いです。チェック行:541, 543. g4uimanager.cc 543

ここでも論理エラーが見られます。 historySwitch、 の値に応じて、関数内のコード saveHistory を変更しますフラグを付け、ファイルで操作を実行します。その結果はフラグによって報告されます。しかし、すべての操作の後、変数 saveHistory 値 historySwitch が割り当てられているだけです .条件の値がすでに設定されていて、それを台無しにしてしまったので、これは奇妙です。ほとんどの場合、これは冗長な割り当てであり、削除する必要があります。

別のフラグメントにも同様のエラーがあります:

V519 'lvl' 変数には、連続して 2 回値が割り当てられます。おそらくこれは間違いです。チェック行:277, 283. g4iontable.cc 283

単一式の複数チェック

bool parse(....) 
{
 ....           
 if( (word0=="line_pattern") ||
     (word0=="line_pattern") ) { .... } 
 ....
}

V501 '||' の左右に同一のサブ式 '(word0 =="line_pattern")' がありますオペレーター。 style_parser 1172

ほとんどの場合、これは、同じ条件内で同じタイプの複数の変数をテストし、その構成にコピーと貼り付けを使用する場合に発生します。

この例には、エラーを明確に確認できる非常に小さなコードフラグメントがあります。この場合、それは単なるタイプミスであり、コピーされたコードが原因である可能性が最も高いです。しかし、これは単純なチェックを行うだけで簡単に検出できるという意味ではありません。この条件は、さまざまなチェックの長いツリーから取得されました。アナライザーは、このような構造の検出に特に役立ち、コードのリファクタリング中のエラーを防ぎます。

エラーではない場合でも、コードを修正する必要があるため、このコードを保守する担当者が二重チェックによって混乱することはありません。

プロジェクトの他の部分でも同様のフラグメントが見つかりました。

V501 '||' の左右に同一のサブ式があります。演算子:ITTU->size() !=np || ITTU->size() !=np g4peneloperayleighmodel.cc 11563
V501 '||' の左右に同一の部分式 '(ptwXY1->interpolation ==ptwXY_interpolationFlat)' があります。オペレーター。 ptwxy_binaryoperators.cc 301

リファクタリングの問題

G4ReactionProduct * G4ParticleHPLabAngularEnergy::Sample(....)
{
  ....
  //if ( it == 0 || it == nEnergies-1 ) 
  if ( it == 0 )
  {
    if(it==0) ....
     ....
  }
  ....
}

V571 定期チェック。「if (it ==0)」条件は、123 行目で既に検証されています。 g4particlehplabangularenergy.cc 125

リファクタリングのプロセス中に、フラグメントが変更されないことがあります。これはまさにこの例で起こったことです。古いメッセージはコメント化され、新しいメッセージは内部の追加チェックと同じになりました。これを修正するには、コードブロックの修正をより慎重に検討するか、条件内の余分なチェックを削除する必要があります。

同様の問題のあるフラグメント:

V571 定期チェック。「if (proj_momentum>=10.)」条件は、809 行目ですでに検証されています。g4componentgghadronnucleusxsc.cc 815
V571 定期チェック。「if (proj_momentum>=10.)」条件は、869 行目ですでに検証されています。g4componentgghadronnucleusxsc.cc 875
V571 定期チェック。「if (proj_momentum>=10.)」条件は、568 行目で既に検証されています。g4componentggnuclnuclxsc.cc 574
V571 定期チェック。「if (proj_momentum>=10.)」条件は、1868 行で既に検証されています。g4nuclnucldiffuseelastic.cc 1875

チェック済みの式

void GFlashHitMaker::make(....)
{
  ....
  if( gflashSensitive )
  {
    gflashSensitive->Hit(&theSpot);
  }
  else if ( (!gflashSensitive ) && 
           ( pSensitive ) && 
           (....)
          ){....}
  ....
}

V560 条件式の一部が常に真:(!gflashSensitive)。 gflashhitmaker.cc 102

指定されたブロックで、else の条件セクションは冗長です。 else ブロックに入る前提条件は、すでに gflashSensitive の false 値です変数なので、もう一度チェックする必要はありません。

別の同様のフラグメント:

void UseWorkArea( T* newOffset ) 
{
  ....
  if( offset && offset!=newOffset )
  {
    if( newOffset != offset ) {....}
    else {....}
  }
  ....
}

V571 定期チェック。「newOffset !=offset」条件は、154 行目で既に検証されています。g4geomsplitter.hh 156

同じ変数が内部条件ブロックでチェックされます。このチェックは、内部条件ブロックへのエントリの条件であったため、常に肯定的な結果が生成されます。その結果、コードは内側の else では実行されません。ブロック。

プロジェクト内の他のいくつかのフラグメントでも、同じ冗長チェックが見つかりました。ああ、このコピペ:

V571 定期チェック。「newOffset !=offset」条件は 113 行目で既に検証されています。g4pdefsplitter.hh 115
V571 定期チェック。「newOffset !=offset」条件は、141 行目で既に検証されています。g4vuplsplitter.hh 143

役に立たない状態

void G4XXXStoredViewer::DrawView() {
  ....
  if (kernelVisitWasNeeded) {
    DrawFromStore();
  } else {
    DrawFromStore();
  }
  ....
}

V523 'then' ステートメントは 'else' ステートメントと同等です。 g4xxxstoredviewer.cc 85

2 つの分岐内のコードは同一であり、条件に関係なく同じコードが実行されるため、条件が役に立たなくなります。この種のアナライザーメッセージは、適切に処理されていないコードや、類似した名前を持つさまざまな定数や関数をコピーする際のタイプミスを示している可能性があります。この場合、このブロックが何のために作成されたのかは明確ではありませんが、レビューして修正する必要があることは明らかです。

別の同様のフラグメントがありました:

V523 'then' ステートメントは 'else' ステートメントと同等です。 g4xxxsgviewer.cc 84

冗長状態

Void G4VTwistSurface::CurrentStatus::ResetfDone(....)
{
  if (validate == fLastValidate && p && *p == fLastp)
  {
     if (!v || (v && *v == fLastv)) return;
  }         
  ....
}

V728 過剰チェックを簡略化できます。「||」演算子は反対の式 '!v' と 'v' で囲まれています。 g4vtwistsurface.cc 1198

このコードフラグメントにはエラーはありませんが、次のように簡略化できます:

if (!v || *v == fLastv) return;

いくつかの同様のフラグメント:

V728 過剰チェックを簡略化できます。「||」演算子は、反対の式 '!a_cut' と 'a_cut' で囲まれています。配列 168
V728 過剰チェックを簡略化できます。「||」演算子は、反対の式 '!a_cut' と 'a_cut' で囲まれています。配列 180
V728 過剰チェックを簡略化できます。「||」演算子は、反対の式 '!a_cut' と 'a_cut' で囲まれています。配列 240
V728 過剰チェックを簡略化できます。「||」演算子は、反対の式 '!a_cut' と 'a_cut' で囲まれています。配列 287
V728 過剰チェックを簡略化できます。「||」演算子は、反対の式 'p ==0' と 'p !=0' で囲まれています。 g4emmodelactivator.cc 216

コンストラクターの呼び出しが正しくありません

class G4PhysicsModelCatalog
{
  private:  
  ....
    G4PhysicsModelCatalog();
  ....
  static modelCatalog* catalog;
  ....
};

G4PhysicsModelCatalog::G4PhysicsModelCatalog()
{ if(!catalog) { 
    static modelCatalog catal;
    catalog = &catal; 
  } 
}

G4int G4PhysicsModelCatalog::Register(const G4String& name)
{
  G4PhysicsModelCatalog();
  .... 
}

V603 オブジェクトは作成されましたが、使用されていません。コンストラクターを呼び出したい場合は、'this->G4PhysicsModelCatalog::G4PhysicsModelCatalog(....)' を使用する必要があります。 g4physicsmodelcatalog.cc 51

現在のオブジェクトにアクセスする代わりに、新しい一時オブジェクトが作成され、すぐに破棄されます。その結果、オブジェクトのフィールドは初期化されません。コンストラクターの外部でフィールドの初期化を使用する必要がある場合は、別の関数を作成してアクセスすることをお勧めします。ただし、コンストラクターを呼び出したい場合は、this. という単語を使用してコンストラクターにアクセスする必要があります。 C++11 を使用する場合、最も適切な決定はデリゲートコンストラクターを使用することです。これらのエラーの詳細と修正方法については、この本を参照してください (セクション 19「コンストラクターを別のコンストラクターから適切に呼び出す方法」を参照してください)。

初期化中のタイプミス

static const G4String name[numberOfMolecula] = {
 ....
 "(CH_3)_2S", "N_2O",       
 "C_5H_10O" "C_8H_6", "(CH_2)_N",
 ....
};

V653 アレイの初期化には、2 つの部分からなる疑わしい文字列が使用されます。カンマが欠落している可能性があります。このリテラルを調べることを検討してください:"C_5H_10O" "C_8H_6". g4hparametrisedlossmodel.cc 324

ここでは、定数を使用した配列の初期化にエラーがあります。タイプミスの結果、コンマが欠落しています。同時にいくつかのトラブルがあります:

2 つの文字列定数が 1 つに連結されます。そして、式の 1 つを "C_5H_10OC_8H_6" として取得します。前例のない種類のアルコール
インデックスで配列にアクセスすると、予期しない式が得られることがあります。
そして最後に - 範囲外の配列インデックスがある可能性があります。

投げ忘れ

class G4HadronicException : public std::exception {....}
void G4CrossSectionDataStore::ActivateFastPath( ....)
{
  ....
  if ( requests.insert( { key , min_cutoff } ).second ) {
    ....
    G4HadronicException(__FILE__,__LINE__,msg.str());
  }
}

V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4crosssectiondatastore.cc 542

関数の大部分は、メッセージを形成して例外を作成します。しかし、スローが欠落しているため、未使用の例外が作成されます。プログラムは引き続き動作しますが、未定義の動作や不正確な評価につながる可能性があります。

プロジェクトの別の部分でエラーが繰り返されました。

V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4generalphasespacedecay.hh 126
V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4particlehpthermalscattering.cc 515
V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4particlehpthermalscattering.cc 574
V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4particlehpthermalscattering.cc 585
V596 オブジェクトは作成されましたが、使用されていません。「throw」キーワードが欠落している可能性があります:throw G4HadronicException(FOO); g4particlehpthermalscattering.cc 687

出力エラー

bool G4GMocrenIO::storeData2() {
  ....
  ofile.write("GRAPE    ", 8);
  ....
}

V666 関数 'write' の 2 番目の引数を調べることを検討してください。値が、最初の引数で渡された文字列の長さと一致しない可能性があります。 g4gmocrenio.cc 1351

このエラーは、実際の文字列の長さと、関数内で長さを指定する引数の不一致が原因で発生します。この場合、スペースによって作成された特定のインデントが形成されたためにエラーが発生しました。一見すると、スペースがいくつあるかわかりません。このエラーは、プロジェクトを最後にチェックしたときからまだ存在しているため、おそらく考慮されていません。このバグは、V666 診断の例のデータベースに含まれていました。

結論

リストされているすべてのエラーが実際に危険であるとは限りませんが、多数の小さなバグが将来、より深刻な結果につながる可能性があります。そのため、プロジェクトを定期的にチェックして、重大な結果につながる前に、初期段階でエラーを検出する必要があります。アナライザーは、最も厄介なバグを見つけて修正し、バグに変わる前にプロジェクト内の危険な場所を検出するのに非常に役立ちます。プロジェクトで PVS-Studio アナライザーをダウンロードして試すことをお勧めします:http://www.viva64.com/en/pvs-studio/download/.