To get a trial key
fill out the form below
Team License (a basic version)
Enterprise License (an extended version)
* By clicking this button you agree to our Privacy Policy statement

Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

Free PVS-Studio license for Microsoft MVP specialists
* By clicking this button you agree to our Privacy Policy statement

To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Going On with the Check of Geant4

Going On with the Check of Geant4

Nov 14 2013
Author:

This is the correct article about the results of checking the Geant4 project, which I have written after the previous incorrect one. Let me remind you the whole story. I have recently checked an old version of the Geant4 library and reported the results in the article "Copy-Paste and Muons". Why old version? Nobody is perfect, and we finally made a mistake ourselves. To find out which exactly, see the previous article. This time I offer you a brief report about checking Geant4 of the version 10.0-beta.

Summary of the previous article

In the article "Copy-Paste and muons" I was reasoning about how useful the static analysis methodology is and about the PVS-Studio analyzer's diagnostic abilities. I checked an ancient version of the Geant4 project (version 4.9.4), found a number of supposedly incorrect code fragments and described them in the article.

Geant4 (for GEometry ANd Tracking) is a platform for "the simulation of the passage of particles through matter," using Monte Carlo methods. It is the successor of the GEANT series of software toolkits developed by CERN, and the first to use object oriented programming (in C++). Its development, maintenance and user support are taken care by the international Geant4 Collaboration. Application areas include high energy physics and nuclear experiments, medical, accelerator and space physics studies. The software is used by a number of research projects around the world.

The project website: http://geant4.org.

In the previous article, I described at least 16 suspicious code fragments. The recent check of the new version has revealed only 10 of those. The rest are either fixed or discarded together with the pieces of code. I won't discuss those defects once again in this article; if you want to see them, please refer to the previous article (there are comments below each sample telling if a particular bug is fixed in the new version or not.)

Sorry for such a strange format of my investigation, but I hope that it won't in any way prevent the developers from fixing some defects in the Geant4 project and that the PVS-Studio will attract their attention.

If I'm correct, the previous library version dates back to 2011. Much has changed since then, and it's no wonder that some new strange code fragments were found. Let's see if there is anything new or anything I missed during the previous check.

New suspicious code fragments

A complete list of all the suspicious fragments that caught my eye is saved in the file geant4_new.txt. But please don't rely solely on this list; the developers should check the project themselves and study all the warnings. We can grant a free registration key to the Geant4 developers for some time so that they could check their code: see the feedback page.

Identical functions

G4double G4CsvAnalysisManager::GetH2Xmin(G4int /*id*/) const
{
  ExceptionForHistograms("GetH2Xmin");
  return 0;
}

G4double G4CsvAnalysisManager::GetH2Xmax(G4int /*id*/) const
{
  ExceptionForHistograms("GetH2Xmin");
  return 0;
}

PVS-Studio's diagnostic message: V524 It is odd that the body of 'GetH2Xmax' function is fully equivalent to the body of 'GetH2Xmin' function. _G4analysis-archive g4csvanalysismanager.cc 933

The GetH2Xmax() function should probably call the ExceptionForHistograms() function with a different parameter:

ExceptionForHistograms("GetH2Xmax");

This bug doesn't look serious. As far as I understand, this is a construct to handle an exception. However, I decided to mention this Copy-Paste bug anyway.

Zero energy

The function CalculateTotalEnergy () sums up values in the 'Etot' variable which suddenly gets zeroed. That's a very strange thing.

G4double G4RKFieldIntegrator::CalculateTotalEnergy(const
  G4KineticTrackVector& Barions)
{
  G4double Etot = 0;
  ....
  for(G4int c2 = c1 + 1; c2 < nBarion; c2++)
  {  
    ....
    //  Esk2
    Etot += t1*std::pow(Alpha/pi, 3/2)*
            std::exp(-Alpha*r12*r12);

    // Eyuk
    Etot += ....;

    // Ecoul
    Etot += 1.44*p1->GetDefinition()->GetPDGCharge()*
            p2->GetDefinition()->GetPDGCharge()/r12*
            Erf(std::sqrt(Alpha)*r12);

    // Epaul
    Etot = 0;
    ....
  }
  ....
}

PVS-Studio's diagnostic message: V519 The 'Etot' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 80, 83. _G4processes-archive g4rkfieldintegrator.cc 83

Strange logic

G4double G4EmBiasingManager::ApplySecondaryBiasing(....)
{
  ....
  if(0 == nsplit) { 
    if(safety > fSafetyMin) ....
  } if(1 == nsplit) { 
    weight = ApplyRussianRoulette(vd, index);
  } else {
    G4double tmpEnergy = pPartChange->GetProposedKineticEnergy();
    G4ThreeVector tmpMomDir = ....
    weight = ApplySplitting(vd, track, currentModel, index, tcut);
    pPartChange->SetProposedKineticEnergy(tmpEnergy);
    pPartChange->ProposeMomentumDirection(tmpMomDir);
  }
  ....
}

PVS-Studio's diagnostic message: V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. _G4processes-archive g4embiasingmanager.cc 299

The code formatting suggests that the programmer was using the "else if" construct. But I can't see any here. If we format the code properly, we'll get the following:

if(0 == nsplit) { 
  if(safety > fSafetyMin) ....
}

if(1 == nsplit) { 
  weight = ApplyRussianRoulette(vd, index);
} else {
  G4double tmpEnergy = pPartChange->GetProposedKineticEnergy();
  G4ThreeVector tmpMomDir = ....
  weight = ApplySplitting(vd, track, currentModel, index, tcut);
  pPartChange->SetProposedKineticEnergy(tmpEnergy);
  pPartChange->ProposeMomentumDirection(tmpMomDir);
}

Notice that the block referring to the 'else' branch is executed every time whenever the "1 != nsplit" condition is true. It suspect that the programmer wanted the program logic to be somewhat different.

A similar issue can be found in the following fragment:

V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. _G4processes-archive g4embiasingmanager.cc 347

Incomplete code?

void G4MolecularDecayTable::AddExcitedState(const G4String& label)
{
  channelsMap::iterator channelsIter =
    fDecayChannelsMap.find(label);
  if(channelsIter != fDecayChannelsMap.end())
  {
    G4String errMsg = "Excited state" + label +
                      " already registered in the decay table.";
    G4Exception("G4MolecularDecayTable::AddExcitedState",
                "G4MolecularDecayTable003",
                FatalErrorInArgument, errMsg);
     return;
  }
  fDecayChannelsMap[label] ;
}

PVS-Studio's diagnostic message: V607 Ownerless expression 'fDecayChannelsMap[label]'. _G4processes-archive g4moleculardecaytable.cc 140

The end of the function is very strange:

fDecayChannelsMap[label] ;

What's it? Something is missing? What did the programmer intend to do with an array cell?

Kinematics

The next sample is rather lengthy. I shortened it as much as I could, but unfortunately it is still pretty large. Read it and note the values the 'id' variable takes.

void G4QMDCollision::CalKinematicsOfBinaryCollisions(
  G4double dt)
{
  ....
  G4int id = 0;
  ....
  if ( secs )
  {
    ....
    id++;
    ....
  }
  if ( std::abs ( eini - efin ) < fepse*10 ) 
    ....
  else
  {  
    ....             
    for ( G4int i0i = 0 ; i0i < id-1 ; i0i++ )
    {
      theSystem->DeleteParticipant( i0i+n0 );
    }
    ....
  }
  ....
}

PVS-Studio's diagnostic message: V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. _G4processes-archive g4qmdcollision.cc 228

If the "if ( secs )" condition is false, the variable 'id' will remain equal to zero. In this case we might get the following loop:

for ( G4int i0i = 0 ; i0i < -1 ; i0i++ )

And that will be a very strange loop indeed. I guess something is wrong with the CalKinematicsOfBinaryCollisions() function's logic.

Miscellaneous

There are a few more warnings I didn't describe in the previous article. Neither will I in this one. Just one sample:

class G4HadronicException : public std::exception
{
  ....
};

inline G4double G4GeneralPhaseSpaceDecay::Pmx(
  G4double e, G4double p1, G4double p2)
{
   if (e-p1-p2 < 0 )
   {  
    G4HadronicException(__FILE__, __LINE__,
      "G4GeneralPhaseSpaceDecay::Pmx "
      "energy in cms > mass1+mass2");
   }
  ....
}

PVS-Studio's diagnostic message. V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw G4HadronicException(FOO); _G4processes-archive g4generalphasespacedecay.hh 116

There are some mistakes with missing 'throw' operators. They result in an object of the 'G4HadronicException' type being created and at once destroyed, the program going on to work with incorrect data.

For other examples of such typos, see the file geant4_new.txt. There you will also find some warnings related to microoptimizations.

Conclusion

Me checking obsolete code is a nice story, huh? Yeah, I've finally slipped up myself. :)

A good opportunity to troll me, isn't it?

Popular related articles
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
PVS-Studio for Java

Date: Jan 17 2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…

Comments (0)

Next comments
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept