[UBSAN] array subscript ... is partly outside array bounds in HeavyFlavorAnalysis/SpecificDecay

[iarspider]

In UBSAN IBs, the following warnings are emitted:

>> Compiling edm plugin src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc
(...)
In file included from src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHDecayToResTrkBuilderBase.h:16,
                 from src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHDecayToResTrkBuilder.h:16,
                 from src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHBuToJPsiKBuilder.h:15,
                 from src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:15:
In constructor 'BPHDecaySpecificBuilder<ProdType>::BPHDecaySpecificBuilder() [with ProdType = BPHRecoCandidate]',
    inlined from 'BPHDecayToResResBuilder<ProdType, Res1Type, Res2Type>::BPHDecayToResResBuilder(const std::vector<typename ResType::const_pointer>&, const std::string&, const std::vector<typename Res2Type::const_pointer>&) [with ProdType = BPHRecoCandidate; Res1Type = BPHPlusMinusCandidate; Res2Type = BPHPlusMinusCandidate]' at src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHDecayToResResBuilder.h:76:36,
    inlined from 'BPHBsToJPsiPhiBuilder::BPHBsToJPsiPhiBuilder(const BPHEventSetupWrapper&, const std::vector<std::shared_ptr<const BPHPlusMinusCandidate> >&, const std::vector<std::shared_ptr<const BPHPlusMinusCandidate> >&)' at src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHBsToJPsiPhiBuilder.h:51:69,
    inlined from 'virtual void TestBPHSpecificDecay::analyze(const edm::Event&, const edm::EventSetup&)' at src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:399:74:
  src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHDecaySpecificBuilder.h:49:29: warning: array subscript 'BPHDecayGenericBuilder<BPHRecoCandidate>[3]' is partly outside array bounds of 'unsigned char [304]' [-Warray-bounds]
    49 |   BPHDecaySpecificBuilder() {}
      |                             ^
src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc: In member function 'virtual void TestBPHSpecificDecay::analyze(const edm::Event&, const edm::EventSetup&)':
src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:399:74: note: at offset 272 into object of size 304 allocated by 'operator new'
  399 |     BPHBsToJPsiPhiBuilder* bs = new BPHBsToJPsiPhiBuilder(ew, lJPsi, lPkk);
      |                                                                          ^
In file included from src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHBsToJPsiPhiBuilder.h:15,
                 from src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:16:
In constructor 'BPHDecayToResResBuilder<ProdType, Res1Type, Res2Type>::BPHDecayToResResBuilder(const std::vector<typename ResType::const_pointer>&, const std::string&, const std::vector<typename Res2Type::const_pointer>&) [with ProdType = BPHRecoCandidate; Res1Type = BPHPlusMinusCandidate; Res2Type = BPHPlusMinusCandidate]',
    inlined from 'BPHBsToJPsiPhiBuilder::BPHBsToJPsiPhiBuilder(const BPHEventSetupWrapper&, const std::vector<std::shared_ptr<const BPHPlusMinusCandidate> >&, const std::vector<std::shared_ptr<const BPHPlusMinusCandidate> >&)' at src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHBsToJPsiPhiBuilder.h:51:69,
    inlined from 'virtual void TestBPHSpecificDecay::analyze(const edm::Event&, const edm::EventSetup&)' at src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:399:74:
  src/HeavyFlavorAnalysis/SpecificDecay/interface/BPHDecayToResResBuilder.h:76:36: warning: array subscript 'BPHDecayGenericBuilder<BPHRecoCandidate>[3]' is partly outside array bounds of 'unsigned char [304]' [-Warray-bounds]
    76 |         sCollection(&res2Collection) {}
      |                                    ^
src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc: In member function 'virtual void TestBPHSpecificDecay::analyze(const edm::Event&, const edm::EventSetup&)':
src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc:399:74: note: at offset 272 into object of size 304 allocated by 'operator new'
  399 |     BPHBsToJPsiPhiBuilder* bs = new BPHBsToJPsiPhiBuilder(ew, lJPsi, lPkk);
      |                                                           

[iarspider]

assign HeavyFlavorAnalysis/SpecificDecay

[cmsbuild]

New categories assigned: analysis

@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @iarspider.

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

cms-bot commands are listed here

[iarspider]

This instance of -Warray-bounds (with the text "partly outside the bounds") is often issued for aliasing violations where an object of one type is being access by an lvalue of a larger struct.
( from gcc bug tracker )

[iarspider]

@cms-sw/analysis-l2 gentle ping

[iarspider]

Most recent occurance: https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc12/CMSSW_14_1_UBSAN_X_2024-08-14-2300/HeavyFlavorAnalysis/SpecificDecay

[tvami]

@cms-sw/analysis-l2 gentle ping

I will have a look at this now.

[tvami]

Hi All, I've spent some time on this now... I think the issue is coming from not setting the proper defaults for lJPsi and lPkk objects. I noticed that the same code is actually included in
https://github.com/cms-sw/cmssw/blob/master/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHWriteSpecificDecay.cc
specifically the problematic line in that plugin can be found here
https://github.com/cms-sw/cmssw/blob/master/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHWriteSpecificDecay.cc#L911
and see the whole block about setting defaults
https://github.com/cms-sw/cmssw/blob/master/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHWriteSpecificDecay.cc#L866-L896

Furthermore this plugin is actually run in DQM
https://github.com/cms-sw/cmssw/blob/master/DQM/Physics/python/heavyFlavorDQMFirstStep_cff.py#L6

Which then makes me conclude that this test file is not useful anymore given that the content of it is tested in DQM (and so in some RelVals too).

My proposal is simply to delete this file, see PR #45709

[iarspider]

Thanks @tvami

[tvami]

+1

[cmsbuild]

This issue is fully signed and ready to be closed.