root-project
Faulty logic in TMVA::DataLoader::AddDataset
Check duplicate issues.
- [X] Checked for duplicates
Description
One of the overloads of the function abuses the logic of the class:
https://github.com/root-project/root/blob/224551cf3fc5bdc80b2693578d082eb9cb0e32a3/tmva/tmva/src/DataLoader.cxx#L126-L133
In this overload a DatasetInfo object is created on the heap (as a temporary). Then DatasetManager::AddDatasetInfo takes the input argument as l-value ref and finally stores the DatasetInfo object as a shallow-reference in the internal TList data member fDataSetInfoCollection. This data member is not owning, thus at destruction time it will not take care of deleting its items.
https://github.com/root-project/root/blob/224551cf3fc5bdc80b2693578d082eb9cb0e32a3/tmva/tmva/src/DataSetManager.cxx#L105-L113
At the same time, the DataSetInfo object was created with new but never deleted. This logic is faulty and is related to the following report by valgrind:
==290973== Conditional jump or move depends on uninitialised value(s)
==290973== at 0x2C720F3A: TMVA::DataLoader::AddTree(TTree*, TString const&, double, TCut const&, TMVA::Types::ETreeType) (DataLoader.cxx:360)
==290973== by 0x2C72113C: TMVA::DataLoader::AddSignalTree(TTree*, double, TMVA::Types::ETreeType) (DataLoader.cxx:373)
==290973== by 0x4F9573B: ???
==290973== by 0x4F9107E: ???
==290973== by 0x6EC8BCD: cling::IncrementalExecutor::executeWrapper(llvm::StringRef, cling::Value*) const (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6E4EA4E: cling::Interpreter::RunFunction(clang::FunctionDecl const*, cling::Value*) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6E4F13C: cling::Interpreter::EvaluateInternal(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cling::CompilationOptions, cling::Value*, cling::Transaction**, unsigned long) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6F36428: cling::MetaSema::actOnxCommand(llvm::StringRef, llvm::StringRef, cling::Value*) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6F45003: cling::MetaParser::isXCommand(cling::MetaSema::ActionResult&, cling::Value*) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6F468D4: cling::MetaParser::isCommand(cling::MetaSema::ActionResult&, cling::Value*) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6F2EF9F: cling::MetaProcessor::process(llvm::StringRef, cling::Interpreter::CompilationResult&, cling::Value*, bool) (in /home/vpadulan/Programs/rootproject/rootbuild/fix-tutorial-tmva-cnn-classification-testing/lib/libCling.so)
==290973== by 0x6C20B67: HandleInterpreterException(cling::MetaProcessor*, char const*, cling::Interpreter::CompilationResult&, cling::Value*) (TCling.cxx:2438)
==290973== by 0x6C215B6: TCling::ProcessLine(char const*, TInterpreter::EErrorCode*) (TCling.cxx:2582)
==290973== by 0x6C24E6E: TCling::ProcessLineSynch(char const*, TInterpreter::EErrorCode*) (TCling.cxx:3545)
==290973== by 0x4C06042: TApplication::ExecuteFile(char const*, int*, bool) (TApplication.cxx:1865)
==290973== by 0x4C0586B: TApplication::ProcessFile(char const*, int*, bool) (TApplication.cxx:1737)
==290973== by 0x4C05680: TApplication::ProcessLine(char const*, bool, int*) (TApplication.cxx:1710)
==290973== by 0x487A58F: TRint::ProcessLineNr(char const*, char const*, int*) (TRint.cxx:820)
==290973== by 0x4878CD2: TRint::Run(bool) (TRint.cxx:461)
==290973== by 0x401446: main (rmain.cxx:84)
==290973== Uninitialised value was created by a stack allocation
==290973== at 0x4F950B4: ???
==290973==
Where AddTree ultimately calls into AddDataset
https://github.com/root-project/root/blob/224551cf3fc5bdc80b2693578d082eb9cb0e32a3/tmva/tmva/src/DataLoader.cxx#L360
Reproducer
valgrind --track-origin=yes --num-callers=30 --suppressions=$ROOTSYS/etc/valgrind-root.supp --suppressions=$ROOTSYS/etc/valgrind-root-python.supp root.exe -l -b -q $ROOTSYS/tutorials/tmva/TMVA_CNN_Classification.C
ROOT version
any
Installation method
any
Operating system
any
Additional context
No response
After more investigation, it is less clear to me how the valgrind report and the AddDataset function are related. Still, the logic of the function seems faulty any way.
