JobOptionSvc: use stringstream to read job options file

The current method of reading the input file in JopOptionSvc leads to undefined behavior errors in the ATLAS debug build on el9/gcc13/LCG_104 (see ATEAM-937). Use std::stringstream instead, which is also supposedly faster.

Also rewrite GetLastLineAndColumn to not read the file twice.

Click to show UBSAN trace

/cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/bits/basic_string.h:278:29: runtime error: execution reached an unreachable program point
    #0 0x423f2b in std::__cxx11::basic_string, std::allocator >::_M_is_local() const /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/bits/basic_string.h:278
    #1 0x423dd2 in std::__cxx11::basic_string, std::allocator >::_M_dispose() /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/bits/basic_string.h:293
    #2 0x7f3035ae0506 in void std::__cxx11::basic_string, std::allocator >::_M_construct > >(std::istreambuf_iterator >, 
std::istreambuf_iterator >, std::input_iterator_tag) /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/bits/basic_string.tcc:200
    #3 0x7f3035ad93f9 in std::__cxx11::basic_string, std::allocator >::basic_string >, void>(std::istreambuf_iterator >, 
std::istreambuf_iterator >, std::allocator const&) /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/bits/basic_string.h:764
    #4 0x7f3035ad5e99 in ParseStream > > >, Gaudi::Parsers::Skipper
Grammar > > > > > > /build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternal
s/src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/Parser.cpp:52
    #5 0x7f3035ad5350 in ParseFile > > >, Gaudi::Parsers::SkipperGr
ammar > > > > > > /build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternals/
src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/Parser.cpp:104
    #6 0x7f3035ad4e10 in Gaudi::Parsers::Parse(Gaudi::Parsers::Position const&, std::basic_string_view >, std::basic_string_view >, Gaudi::Parsers::IncludedFiles*, Gaudi::Parsers::M
essages*, Gaudi::Parsers::Node*) /build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternals/src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/Parser.cpp:123
    #7 0x7f3035ad4d7b in Gaudi::Parsers::Parse(std::basic_string_view >, std::basic_string_view >, Gaudi::Parsers::IncludedFiles*, Gaudi::Parsers::Messages*, Gaudi::Parsers::Node*) 
/build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternals/src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/Parser.cpp:117
    #8 0x7f3035a48bc3 in Gaudi::Parsers::ReadOptions(std::basic_string_view >, std::basic_string_view >, Gaudi::Parsers::Messages*, Gaudi::Parsers::Catalog*, Gaudi::Parsers::Units*,
 Gaudi::Parsers::PragmaOptions*, Gaudi::Parsers::Node*) /build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternals/src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/Analyzer.cpp:391
    #9 0x7f3035a5b4a7 in JobOptionsSvc::readOptions(std::basic_string_view >, std::basic_string_view >) /build/atnight/localbuilds/nightlies/Athena/main/build/build/AthenaExternals/
src/Gaudi/GaudiCoreSvc/src/JobOptionsSvc/JobOptionsSvc.cpp:256

changed milestone to %v37r1

changed the description

I'm all in favour as long as it fixes the ASAN issue. And I would even claim that this is more readable code than what we have in place at the moment.

added 1 commit

• 3e492d4b - JobOptionSvc: use stringstream to read job options file

Compare with previous version

added 1 commit

• be8e0c40 - JobOptionSvc: use stringstream to read job options file

Compare with previous version

changed the description

resolved all threads

added lhcb-gaudi-head label

added 1 commit

• 3bf2eb72 - JobOptionSvc: use stringstream to read job options file

Compare with previous version

resolved all threads

• [2023-10-26 00:03] Validation started with lhcb-gaudi-head#3726
• [2023-10-26 00:09] Validation started with lhcb-run2-gaudi-head#637
• [2023-10-26 00:18] Validation started with lhcb-sim10-gaudi-head#417

approved this merge request

merged

mentioned in commit 893080c8

FYI, the UBSAN error is actually a libstdc++ bug fixed in 13.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109703

This issue only surfaces when constructing a string with a range of input iterator, and the uninitialized _M_string_length happens to be greater than _S_local_capacity, i.e., 15 for the std::string specialization.

So that explains why the changes in this MR made the problem disappear (of course, the new code is better in many other respects as well).

mentioned in merge request !1521 (closed)