Generated Cpp code for Modelica functions using uninitialized varaibles

[AnHeuermann]

Running

loadModel(Modelica, {"3.2.2"}); getErrorString();
setCommandLineOptions("-d=newInst --simCodeTarget=Cpp"); getErrorString();
simulate(Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGas); getErrorString();

will generate OMCppModelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGasFunctions.cpp with

//if outvars missing
void /*_omcQ_24DER_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5F_5FTXRetType */ Functions::_omcQ_24DER_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5F_5FTX(double T_, BaseArray<double>& X_, bool exclEnthForm_, int refChoice_, double h_off_, double _omcQ_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5FTX_24funDERT_, BaseArray<double>& _omcQ_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5FTX_24funDERX_, double _omcQ_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5FTX_24funDERh_5Foff_,_omcQ_24DER_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5F_5FTXRetType & output )
{
[...]
  /* varOutput varInits(_omcQ_24Modelica_24PMedia_24PExamples_24PTests_24PMediaTestModels_24PIdealGases_24PSimpleNaturalGas_24Pvolume_24PMedium_24Ph_5FTX_24funDERh_) */ 
  do
  {

    {
      [...]
      int tmp51;
      [...]
      if (!tmp51) throw ModelicaSimulationError(MODEL_ARRAY_FUNCTION,"Internal error");
      [...]
    }
    [...]
  }
  [...]
}

Please note that variable tmp51 is not used at all, so whenever tmp51 happens to be initialized with 0 the function will throw an error.

The Cpp code generation for this was a copy of the C code generation and is still very similar. When generating code for the C target the generated code looks very similar, but doesn't have this error.

[AnHeuermann]

The relevant template for this code generation is in ​daeExpReduction in CodegenCppCommonOld.tpl .

It looks like this problem was fixed for the C code generation in ​7fc85eb69d5c28a5c279b254b7b24c3403b3bd4c. So I'll try doing the same for CodegenCFunctions.tpl and CodegenCppCommonOld.tpl.

[AnHeuermann]

​https://github.com/OpenModelica/OpenModelica/pull/6978

[casella]

Wow, this kind of low-level bugs looks scary to me, good that you spotted this out!

I noticed in the ​history records that there are several Media examples in the testsuite that alternatively succeed and fail more or less at random at each Jenkins test run, e.g.

Modelica.Media.Examples.TestOnly.FlueGas
ModelicaTest.Media.TestOnly.MixIdealGasAir
ModelicaTest.Media.TestAllProperties.FlueGasSixComponents

Besides the fact that this causes spurious regressions and improvements all the time, which is annoying, I always wondered why this happened.

Could it be that the same root cause is involved? Thos models all involve gas mixtures in some way, so this is quite likely.

[AnHeuermann]

Could be, if they are consistently failing or passing for C the probability is very high.

[AnHeuermann]

We should open a ticket for all tests with C++ that are switching between failing and passing all the time, while the same issues are passing for C. I guess in most of these cases we can find a root cause for this (as long those are no timeouts) and the fix will be "do the same thing as we did for C 3 years ago".

[sjoelund.se]

This kind of error could easily be solved by adding -Werror=uninitalized to CXXFLAGS for generated code (and probably good for the C code as well)... Just need to check that the compiler supports it before adding such flags though.

[AnHeuermann]

Good idea, I'll open a PR with it.

[AnHeuermann]

This kind of error could easily be solved by adding -Werror=uninitalized to CXXFLAGS for generated code (and probably good for the C code as well)... Just need to check that the compiler supports it before adding such flags though.

Mhhh... I tried locally on a random model and we get a ton of warnings and errors with clang --analyze. Could be harder to have automated then I anticipated.

But this issue should be solved now, at east ​https://libraries.openmodelica.org/branches/history/newInst/2020-11-22%2021:29:28..2020-11-25%2020:31:47.html is looking good.

[sjoelund.se]

clang --analyze is quite different from regular clang. It performs a lot more checks. We did have ​https://test.openmodelica.org/jenkins/job/periodic/job/Jenkins.slow/job/master/ but it seems broken