[tree] prevent nullptr access in ttreereader

[ferdymercury]

This Pull request:

Changes or fixes:

nullptr access when branch is not found

Fixes https://its.cern.ch/jira/browse/ROOT-8842 by @jpivarski

Checklist:

• [x] tested changes locally
• [ ] updated the docs (if necessary)

[ferdymercury]

For the second crash, valgrind reports:

Invalid read of size 4
  in frombuf(char*&, float*) in /opt/root_src/core/base/inc/Bytes.h:384
  1: frombuf(char*&, float*) in /opt/root_src/core/base/inc/Bytes.h:384
  2: ROOT::Experimental::TTreeReaderValueFast<float>::Deserialize(char*) in /home/user/builds/build-root_src-Desktop-Debug/include/ROOT/TTreeReaderValueFast.hxx:171
  3: ROOT::Experimental::TTreeReaderValueFast<float>::Get() in /home/user/builds/build-root_src-Desktop-Debug/include/ROOT/TTreeReaderValueFast.hxx:162
  4: ROOT::Experimental::TTreeReaderValueFast<float>::operator*() in /home/user/builds/build-root_src-Desktop-Debug/include/ROOT/TTreeReaderValueFast.hxx:165
  5: reader(TString, TString, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) in /tmp/reader.C:31
  6: 0x4ffb0b6
  7: cling::IncrementalExecutor::executeWrapper(llvm::StringRef, cling::Value*) const in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  8: cling::Interpreter::RunFunction(clang::FunctionDecl const*, cling::Value*) in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  9: 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/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  10: cling::Interpreter::process(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cling::Value*, cling::Transaction**, bool) in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  11: cling::MetaProcessor::process(llvm::StringRef, cling::Interpreter::CompilationResult&, cling::Value*, bool) in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  12: HandleInterpreterException(cling::MetaProcessor*, char const*, cling::Interpreter::CompilationResult&, cling::Value*) in /opt/root_src/core/metacling/src/TCling.cxx:2438
Address 0x2a004f98 is 0 bytes after a block of size 32,776 alloc'd  1: operator new[](unsigned long) in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so
  2: TBuffer::TBuffer(TBuffer::EMode, int) in /opt/root_src/core/base/src/TBuffer.cxx:85
  3: TBufferIO::TBufferIO(TBuffer::EMode, int) in /opt/root_src/io/io/src/TBufferIO.cxx:51
  4: TBufferFile::TBufferFile(TBuffer::EMode, int) in /opt/root_src/io/io/src/TBufferFile.cxx:90
  5: ROOT::Experimental::Internal::TTreeReaderValueFastBase::TTreeReaderValueFastBase(ROOT::Experimental::TTreeReaderFast*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) in /home/user/builds/build-root_src-Desktop-Debug/include/ROOT/TTreeReaderValueFast.hxx:53
  6: ROOT::Experimental::TTreeReaderValueFast<float>::TTreeReaderValueFast(ROOT::Experimental::TTreeReaderFast&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) in /home/user/builds/build-root_src-Desktop-Debug/include/ROOT/TTreeReaderValueFast.hxx:159
  7: reader(TString, TString, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) in /tmp/reader.C:24
  8: 0x4ffb0b6
  9: cling::IncrementalExecutor::executeWrapper(llvm::StringRef, cling::Value*) const in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  10: cling::Interpreter::RunFunction(clang::FunctionDecl const*, cling::Value*) in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  11: 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/user/builds/build-root_src-Desktop-Debug/lib/libCling.so
  12: cling::Interpreter::process(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cling::Value*, cling::Transaction**, bool) in /home/user/builds/build-root_src-Desktop-Debug/lib/libCling.so

The crash disappears if I call reader.SetEntry(0) before the for loop.

[github-actions[bot]]

Test Results

    19 files      19 suites   3d 8h 3m 3s ⏱️  3 066 tests  3 065 ✅ 0 💤  1 ❌ 56 670 runs  56 653 ✅ 0 💤 17 ❌

For more details on these failures, see this check.

Results for commit de6fd052.

:recycle: This comment has been updated with latest results.

[dpiparo]

Thanks for this improvement. I propose the following way forward, if you agree:

1. A small unit test is added for this fix
2. We wait for @pcanal to approve (with a test, I would be ready to approve and to pick up the rest of the fixing, but I prefer to have his opinion on this)
3. We promote the old JIRA item as a new GH issue specific to the residual issue

Does it make sense?

[ferdymercury]

Thanks for this improvement.

You are welcome.

1. A small unit test is added for this fix

Done.

3. We promote the old JIRA item as a new GH issue specific to the residual issue

Ok for me, but I'd say the residual issue and the full one are more or less the same. If I call reader.SetEntry(0), then the first fix is not needed either.