Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ASAN] global-buffer-overflow in std::regex construction #40902

Closed
makortel opened this issue Feb 28, 2023 · 23 comments
Closed

[ASAN] global-buffer-overflow in std::regex construction #40902

makortel opened this issue Feb 28, 2023 · 23 comments
Labels
core-approved fully-signed

Comments

@makortel
Copy link
Contributor

After enabling LTO globally in 13_1_X many workflows in ASAN report

==18737==ERROR: AddressSanitizer: global-buffer-overflow on address 0x2b4d17189b79 at pc 0x2b4d1709db4c bp 0x2b4cfa2c61f0 sp 0x2b4cfa2c61e8
READ of size 1 at 0x2b4d17189b79 thread T2
    #0 0x2b4d1709db4b in std::__detail::_Scanner<char>::_M_advance() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xb6b4b)
    #1 0x2b4d170fd3c8 in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_match_token(std::__detail::_ScannerBase::_TokenT) [clone .part.0] [clone .lto_priv.0] (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0x1163c8)
    #2 0x2b4d170b047f in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_alternative() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xc947f)
    #3 0x2b4d170afbef in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_alternative() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xc8bef)
    #4 0x2b4d170afbef in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_alternative() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xc8bef)
    #5 0x2b4d170afbef in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_alternative() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xc8bef)
    #6 0x2b4d170afbef in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_alternative() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xc8bef)
    #7 0x2b4d170bc72c in std::__detail::_Compiler<std::__cxx11::regex_traits<char> >::_M_disjunction() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xd572c)
    #8 0x2b4d171397bb in std::enable_if<std::__detail::__is_contiguous_iter<char const*>::value, std::shared_ptr<std::__detail::_NFA<std::__cxx11::regex_traits<char> > const> >::type std::__detail::__compile_nfa<std::__cxx11::regex_traits<char>, char const*>(char const*, char const*, std::__cxx11::regex_traits<char>::locale_type const&, std::regex_constants::syntax_option_type) [clone .constprop.0] (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0x1527bb)
    #9 0x2b4d1708f3d4 in isNumber(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xa83d4)
    #10 0x2b4d17090a65 in CandIsolatorFromDeposits::SingleDeposit::SingleDeposit(edm::ParameterSet const&, edm::ConsumesCollector&&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xa9a65)
    #11 0x2b4d17092743 in CandIsolatorFromDeposits::CandIsolatorFromDeposits(edm::ParameterSet const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xab743)
    #12 0x2b4d17102bed in edm::stream::ProducingModuleAdaptor<CandIsolatorFromDeposits, edm::stream::EDProducerBase, edm::stream::EDProducerAdaptorBase>::setupStreamModules() (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0x11bbed)
    #13 0x2b4c92984728 in edm::stream::ProducingModuleAdaptorBase<edm::stream::EDProducerBase>::doPreallocate(edm::PreallocationConfiguration const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x7da728)
    #14 0x2b4c928d4e06 in edm::Maker::makeModule(edm::MakeModuleParams const&, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&) const (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x72ae06)
    #15 0x2b4c9262ee99 in edm::Factory::makeModule(edm::MakeModuleParams const&, edm::ModuleTypeResolverMaker const*, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&) const (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x484e99)
    #16 0x2b4c9264f798 in edm::ModuleRegistry::getModule(edm::MakeModuleParams const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&, edm::signalslot::Signal<void (edm::ModuleDescription const&)>&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x4a5798)
    #17 0x2b4c928c781d in edm::WorkerRegistry::getWorker(edm::WorkerParams const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x71d81d)
    #18 0x2b4c928c8a2d in edm::WorkerManager::getWorker(edm::ParameterSet&, edm::ProductRegistry&, edm::PreallocationConfiguration const*, std::shared_ptr<edm::ProcessConfiguration const>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x71ea2d)
    #19 0x2b4c928c9239 in edm::WorkerManager::addToUnscheduledWorkers(edm::ParameterSet&, edm::ProductRegistry&, edm::PreallocationConfiguration const*, std::shared_ptr<edm::ProcessConfiguration const>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x71f239)
    #20 0x2b4c9282820c in edm::StreamSchedule::StreamSchedule(std::shared_ptr<edm::TriggerResultInserter>, std::vector<edm::propagate_const<std::shared_ptr<edm::PathStatusInserter> >, std::allocator<edm::propagate_const<std::shared_ptr<edm::PathStatusInserter> > > >&, std::vector<edm::propagate_const<std::shared_ptr<edm::EndPathStatusInserter> >, std::allocator<edm::propagate_const<std::shared_ptr<edm::EndPathStatusInserter> > > >&, std::shared_ptr<edm::ModuleRegistry>, edm::ParameterSet&, edm::service::TriggerNamesService const&, edm::PreallocationConfiguration const&, edm::ProductRegistry&, edm::ExceptionToActionTable const&, std::shared_ptr<edm::ActivityRegistry>, std::shared_ptr<edm::ProcessConfiguration const>, edm::StreamID, edm::ProcessContext const*) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x67e20c)
    #21 0x2b4c927acee7 in edm::Schedule::Schedule(edm::ParameterSet&, edm::service::TriggerNamesService const&, edm::ProductRegistry&, edm::ExceptionToActionTable const&, std::shared_ptr<edm::ActivityRegistry>, std::shared_ptr<edm::ProcessConfiguration const>, edm::PreallocationConfiguration const&, edm::ProcessContext const*, edm::ModuleTypeResolverMaker const*) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x602ee7)
    #22 0x2b4c9281aec2 in edm::ScheduleItems::initModules(edm::ParameterSet&, edm::service::TriggerNamesService const&, edm::PreallocationConfiguration const&, edm::ProcessContext const*, edm::ModuleTypeResolverMaker const*) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x670ec2)
    #23 0x2b4c9255ce31 in tbb::detail::d1::function_task<edm::EventProcessor::init(std::shared_ptr<edm::ProcessDesc>&, edm::ServiceToken const&, edm::serviceregistry::ServiceLegacy)::{lambda()#1}>::execute(tbb::detail::d1::execution_data&) [clone .lto_priv.0] (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/libFWCoreFramework.so+0x3b2e31)
    #24 0x2b4c949c5303 in tbb::detail::d1::task* tbb::detail::r1::task_dispatcher::local_wait_for_all<false, tbb::detail::r1::outermost_worker_waiter>(tbb::detail::d1::task*, tbb::detail::r1::outermost_worker_waiter&) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/task_dispatcher.h:322
    #25 0x2b4c949c5303 in tbb::detail::d1::task* tbb::detail::r1::task_dispatcher::local_wait_for_all<tbb::detail::r1::outermost_worker_waiter>(tbb::detail::d1::task*, tbb::detail::r1::outermost_worker_waiter&) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/task_dispatcher.h:458
    #26 0x2b4c949c5303 in tbb::detail::r1::arena::process(tbb::detail::r1::thread_data&) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/arena.cpp:137
    #27 0x2b4c949c5303 in tbb::detail::r1::market::process(rml::job&) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/market.cpp:599
    #28 0x2b4c949c74c5 in tbb::detail::r1::rml::private_worker::run() /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/private_server.cpp:271
    #29 0x2b4c949c74c5 in tbb::detail::r1::rml::private_worker::thread_routine(void*) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/private_server.cpp:221
    #30 0x2b4c956721ce in start_thread (/lib64/libpthread.so.0+0x81ce)
    #31 0x2b4c958c3e72 in clone (/lib64/libc.so.6+0x39e72)

0x2b4d17189b79 is located 0 bytes to the right of global variable '*.LC34' defined in 'tmp/el8_amd64_gcc11/src/PhysicsTools/IsolationAlgos/plugins/PhysicsToolsIsolationAlgos_plugins/cc86QMok.ltrans6.o' (0x2b4d17189b60) of size 25
  '*.LC34' is ascii string '^[+-]?(\d+\.?|\d*\.\d*)$'
0x2b4d17189b79 is located 39 bytes to the left of global variable '*.LC35' defined in 'tmp/el8_amd64_gcc11/src/PhysicsTools/IsolationAlgos/plugins/PhysicsToolsIsolationAlgos_plugins/cc86QMok.ltrans6.o' (0x2b4d17189ba0) of size 15
  '*.LC35' is ascii string '^$\.*+?()[]{}|'
SUMMARY: AddressSanitizer: global-buffer-overflow (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02773/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_ASAN_X_2023-02-24-2300/lib/el8_amd64_gcc11/pluginPhysicsToolsIsolationAlgos_plugins.so+0xb6b4b) in std::__detail::_Scanner<char>::_M_advance()
Shadow bytes around the buggy address:
  0x056a22e29310: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x056a22e29320: 00 00 00 00 00 00 00 00 00 00 01 f9 f9 f9 f9 f9
  0x056a22e29330: 05 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 02 f9 f9 f9
  0x056a22e29340: f9 f9 f9 f9 00 00 00 00 00 00 00 02 f9 f9 f9 f9
  0x056a22e29350: 00 f9 f9 f9 f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9
=>0x056a22e29360: 00 00 00 00 00 03 f9 f9 f9 f9 f9 f9 00 00 00[01]
  0x056a22e29370: f9 f9 f9 f9 00 07 f9 f9 f9 f9 f9 f9 07 f9 f9 f9
  0x056a22e29380: f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
  0x056a22e29390: f9 f9 f9 f9 00 06 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x056a22e293a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 f9
  0x056a22e293b0: f9 f9 f9 f9 00 00 00 02 f9 f9 f9 f9 00 00 00 04
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
Thread T2 created by T0 here:
    #0 0x2b4c9182f716 in __interceptor_pthread_create ../../../../libsanitizer/asan/asan_interceptors.cpp:216
    #1 0x2b4c949c6b9a in tbb::detail::r1::rml::internal::thread_monitor::launch(void* (*)(void*), void*, unsigned long) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/rml_thread_monitor.h:195
    #2 0x2b4c949c6b9a in tbb::detail::r1::rml::private_worker::wake_or_launch() /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/private_server.cpp:305
    #3 0x2b4c949c6b9a in tbb::detail::r1::rml::private_server::wake_some(int) /data/cmsbld/jenkins/workspace/build-any-ib/w/BUILD/el8_amd64_gcc11/external/tbb/v2021.8.0-791ebc4967ab49af60ca9ad2aa021259/tbb-v2021.8.0/src/tbb/private_server.cpp:412
    #4 0x60c00004f5ff  (<unknown module>)

https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc11/CMSSW_13_1_ASAN_X_2023-02-24-2300/pyRelValMatrixLogs/run/1.0_ProdMinBias/step3_ProdMinBias.log#/
@makortel
Copy link
Contributor Author

assign core

@dan131riley

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Issue was created by @makortel Matti Kortelainen.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

In the core software meeting today it was left unclear if these warnings are a sign of LTO and ASAN being incompatible, or a real problem in the LTO build that ASAN reports.

The regex object in question is in

cmssw/PhysicsTools/IsolationAlgos/plugins/CandIsolatorFromDeposits.cc

Lines 63 to 66 in 3d761d8

bool isNumber(const std::string &str) {
static const std::regex re("^[+-]?(\\d+\\.?|\\d*\\.\\d*)$");
return regex_match(str.c_str(), re);
}

@makortel
Copy link
Contributor Author

A quick web search with the stack trace parts led me to this GCC/libstdc++ bug report (data race in std::ctype<char> that shows up in the construction of std::regex)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77704
According to the bug report itself it has not been fixed yet, but comment 5 shows a possible workaround.

@smuzaffar
Copy link
Contributor

smuzaffar commented Mar 1, 2023 •
edited
Loading

thanks @makortel for tracking it down? how about using std::stoll(str); for this function? with change [a] I waqs able to run wf 1.0 in ASAN IB

-  static const std::regex re("^[+-]?(\\d+\\.?|\\d*\\.\\d*)$");
-  return regex_match(str.c_str(), re);
+  try 
+  {
+    std::stold(str);
+    return true;
+  }
+  catch(...) {return false; }

@smuzaffar smuzaffar mentioned this issue Mar 1, 2023
Closed
@smuzaffar
Copy link
Contributor

#40915 might fix the ASAN errors

@Dr15Jones
Copy link
Contributor

It would probably be better to use std::from_char which returns an error indicator instead of throwing.

https://en.cppreference.com/w/cpp/utility/from_chars

@makortel
Copy link
Contributor Author

makortel commented Mar 1, 2023

There is also other case of the same pattern in

cmssw/CommonTools/ParticleFlow/plugins/PFCandIsolatorFromDeposit.cc

Lines 77 to 81 in 3d761d8

bool PFCandIsolatorFromDeposits::SingleDeposit::isNumber(const std::string &str) const {
static const std::regex re("^[+-]?(\\d+\\.?|\\d*\\.\\d*)$");
return regex_match(str.c_str(), re);
}
double PFCandIsolatorFromDeposits::SingleDeposit::toNumber(const std::string &str) const { return atof(str.c_str()); }

@smuzaffar
Copy link
Contributor

@Dr15Jones , std::from_char does not recognize the + sign at the start

2-4) Floating-point parsers: Expects the pattern identical to the one used by [std::strtod](https://en.cppreference.com/w/cpp/string/byte/strtof) in the default ("C") locale, except that
  - the plus sign is not recognized outside of the exponent (only the minus sign is permitted at the beginning)

if #40915 looks good then I can also take care of #40902 (comment)

@smuzaffar
Copy link
Contributor

Or shouldn't we just update

cmssw/PhysicsTools/IsolationAlgos/python/CandIsolatorFromDeposits_cfi.py

Line 8 in 46bdccd

weight = cms.string('1'),
to use weight = cms.double(1), ?

@smuzaffar
Copy link
Contributor

#40902 (comment) may be not, there is some logic to use weightExpr_ function.

@smuzaffar smuzaffar mentioned this issue Mar 2, 2023
Closed
@makortel
Copy link
Contributor Author

makortel commented Mar 3, 2023

In principle it would be straightforward to check if the first character is +, and if it is, pass the character sequence starting from the second character to std::from_chars.

(alternatively one could ask if supporting leading + is really necessary)

@smuzaffar
Copy link
Contributor

feel free to open a new PR with std::from_char :-)

@makortel makortel mentioned this issue Mar 3, 2023
Closed
@makortel
Copy link
Contributor Author

makortel commented Mar 3, 2023

feel free to open a new PR with std::from_char :-)

Challenge accepted :), see #40956. A potential downside is the larger amount of code.

@Dr15Jones
Copy link
Contributor

Is it possible this is a false positive because of a 'coincidence' of placement of storage of the static object? I.e. could the compiler have placed the string '^[+-]?(\d+.?|\d*.\d*)$' just to the left of a pre-assigned space for the static std::regex ? So it looks to ASAN that it read off the end of the array but in reality the member data of the static object just happens to be the next address over?

@dan131riley
Copy link

I believe ASAN puts guard bytes around static objects. In fact, it tells us that the access is "39 bytes to the left of global variable '*.LC35'", the next static allocation.

@Dr15Jones
Copy link
Contributor

Dr15Jones commented Mar 7, 2023 •
edited
Loading

I think this is the implementation
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/regex_scanner.tcc#L70

looking at the various scan I see lines like
https://github.com/gcc-mirror/gcc/blob/b2b841fa25513a87fcb6e8195c2ad3ea354426d4/libstdc%2B%2B-v3/include/bits/regex_scanner.tcc#L98-L108

which shows that it could have dereferenced the value at _M_end.
EDIT: scratch that. The value returned from _M_current++ is the previous address which is then dereferenced and the comparison is the address after the increment.

@Dr15Jones
Copy link
Contributor

OK, I seem to have gotten a working job. I just put isNumber and toNumber into an anonymous namespace and the jobs now run.

@makortel
Copy link
Contributor Author

makortel commented Mar 7, 2023

Chris' PR is #40984

@dan131riley
Copy link

I diffed the assembler output before and after Chris's PR, prior to LTO, and the assembler is identical except for some random stuff in the section names. Whatever is getting changed by that PR is definitely happening at the LTO stage, which is making me somewhat nervous about LTO.

@makortel
Copy link
Contributor Author

+core

Crash itself was fixed, although @dan131riley's concerns (#40902 (comment)) remain.

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

This was referenced Jun 22, 2023
Closed
Merged
@makortel makortel mentioned this issue Jun 26, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core-approved fully-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use std::from_chars instead of std::regex+std::atof makortel/cmssw
5 participants
@makortel @dan131riley @Dr15Jones @smuzaffar @cmsbuild