From c89332d9da0cd7d1c9508c6de359fa6dde3aac44 Mon Sep 17 00:00:00 2001
From: Scott Snyder <scott.snyder@cern.ch>
Date: Wed, 3 Jul 2019 09:35:11 +0000
Subject: [PATCH] MuonHoughPatternTools: Avoid use-after-free error.
A MuonLayerHough::Maximum object has a pointer to a MuonLayerHough object.
The latter lives in a temporary State object.
The Maximum objects, however, can end up being entered in a vector
and recorded in SG. In such a case, the State objects can then be deleted.
leaving dangling pointers in the Maximum objects.
This has not been observed to cause crashes; instead, it just occasionally
gives wrong results.
---
.../MuonHoughPatternTools/HoughDataPerSec.h | 10 ++++++++--
.../MuonHoughPatternTools/MuonHoughPatternFinderTool.h | 2 +-
.../MuonHoughPatternTools/MuonLayerHoughTool.h | 5 +++--
.../src/MuonHoughPatternFinderTool.cxx | 2 +-
.../IMuonHoughPatternFinderTool.h | 2 +-
.../src/MooSegmentFinderAlg.cxx | 2 +-
.../src/MooSegmentFinderAlg.h | 2 +-
.../IMooSegmentCombinationFinder.h | 2 +-
8 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/HoughDataPerSec.h b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/HoughDataPerSec.h
index 3dcd46c589b..a18caf8424c 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/HoughDataPerSec.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/HoughDataPerSec.h
@@ -8,6 +8,7 @@
#include "AthenaKernel/CLASS_DEF.h"
#include "MuonLayerHough/MuonLayerHough.h"
#include "MuonLayerHough/MuonPhiLayerHough.h"
+#include "MuonLayerHough/MuonRegionHough.h"
#include <map>
#include <set>
#include <vector>
@@ -54,9 +55,14 @@ namespace Muon {
return std::max( nmaxHitsInRegion[0], std::max( nmaxHitsInRegion[1], nmaxHitsInRegion[2] ) );
}
};
+
+ struct HoughDataPerSectorVec : public std::vector<HoughDataPerSec>
+ {
+ MuonHough::MuonDetectorHough detectorHoughTransforms;
+ };
}
CLASS_DEF(Muon::HoughDataPerSec, 163257499, 1)
-CLASS_DEF(std::vector<Muon::HoughDataPerSec>, 118215228, 1)
+CLASS_DEF(Muon::HoughDataPerSectorVec, 61014906, 1)
-#endif
\ No newline at end of file
+#endif
diff --git a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonHoughPatternFinderTool.h b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonHoughPatternFinderTool.h
index 4c7bcd39c64..56aa17845ba 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonHoughPatternFinderTool.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonHoughPatternFinderTool.h
@@ -58,7 +58,7 @@ namespace Muon {
virtual StatusCode finalize();
/** find patterns for a give set of MuonPrepData collections + optionally CSC segment combinations */
- std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<std::vector<Muon::HoughDataPerSec>>>
+ std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<Muon::HoughDataPerSectorVec> >
find( const std::vector<const MdtPrepDataCollection*>& mdtCols,
const std::vector<const CscPrepDataCollection*>& cscCols,
const std::vector<const TgcPrepDataCollection*>& tgcCols,
diff --git a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonLayerHoughTool.h b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonLayerHoughTool.h
index d8afe07d870..536176ffa21 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonLayerHoughTool.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/MuonHoughPatternTools/MuonLayerHoughTool.h
@@ -86,7 +86,8 @@ namespace Muon {
typedef HoughDataPerSec::RegionPhiMaximumVec RegionPhiMaximumVec;
typedef HoughDataPerSec HoughDataPerSector;
- typedef std::vector<HoughDataPerSector> HoughDataPerSectorVec;
+
+ typedef Muon::HoughDataPerSectorVec HoughDataPerSectorVec;
typedef HoughDataPerSectorVec::const_iterator HoughDataPerSectorCit;
@@ -163,8 +164,8 @@ namespace Muon {
struct State {
MaximumVec seedMaxima;
- MuonHough::MuonDetectorHough detectorHoughTransforms;
std::unique_ptr<HoughDataPerSectorVec> houghDataPerSectorVec { std::make_unique<HoughDataPerSectorVec>() };
+ MuonHough::MuonDetectorHough& detectorHoughTransforms { houghDataPerSectorVec->detectorHoughTransforms };
std::set<Identifier> truthHits;
std::set<Identifier> foundTruthHits;
std::set<Identifier> outputTruthHits;
diff --git a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/src/MuonHoughPatternFinderTool.cxx b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/src/MuonHoughPatternFinderTool.cxx
index 40eba039d28..8cab349ae6a 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/src/MuonHoughPatternFinderTool.cxx
+++ b/MuonSpectrometer/MuonReconstruction/MuonPatternFinders/MuonPatternFinderTools/MuonHoughPatternTools/src/MuonHoughPatternFinderTool.cxx
@@ -156,7 +156,7 @@ namespace Muon {
return StatusCode::SUCCESS;
}
- std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<std::vector<HoughDataPerSec>>>
+std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<Muon::HoughDataPerSectorVec>>
MuonHoughPatternFinderTool::find( const std::vector<const MdtPrepDataCollection*>& mdtCols,
const std::vector<const CscPrepDataCollection*>& cscCols,
const std::vector<const TgcPrepDataCollection*>& tgcCols,
diff --git a/MuonSpectrometer/MuonReconstruction/MuonRecTools/MuonRecToolInterfaces/MuonRecToolInterfaces/IMuonHoughPatternFinderTool.h b/MuonSpectrometer/MuonReconstruction/MuonRecTools/MuonRecToolInterfaces/MuonRecToolInterfaces/IMuonHoughPatternFinderTool.h
index 5eb0e9821c6..95460bcb801 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonRecTools/MuonRecToolInterfaces/MuonRecToolInterfaces/IMuonHoughPatternFinderTool.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonRecTools/MuonRecToolInterfaces/MuonRecToolInterfaces/IMuonHoughPatternFinderTool.h
@@ -35,7 +35,7 @@ namespace Muon {
static const InterfaceID& interfaceID();
/** find patterns for a give set of MuonPrepData collections + optionally CSC segment combinations */
- virtual std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<std::vector<Muon::HoughDataPerSec>>>
+ virtual std::pair<std::unique_ptr<MuonPatternCombinationCollection>, std::unique_ptr<Muon::HoughDataPerSectorVec>>
find( const std::vector<const MdtPrepDataCollection*>& mdtCols,
const std::vector<const CscPrepDataCollection*>& cscCols,
const std::vector<const TgcPrepDataCollection*>& tgcCols,
diff --git a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.cxx b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.cxx
index c1e0b81a2c5..8cbb6bd2c86 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.cxx
+++ b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.cxx
@@ -129,7 +129,7 @@ StatusCode MooSegmentFinderAlg::execute()
// write hough data to SG
if (output.houghDataPerSectorVec) {
- SG::WriteHandle<std::vector<Muon::HoughDataPerSec>> handle {m_houghDataPerSectorVecKey};
+ SG::WriteHandle<Muon::HoughDataPerSectorVec> handle {m_houghDataPerSectorVecKey};
ATH_CHECK(handle.record(std::move(output.houghDataPerSectorVec)));
} else {
ATH_MSG_VERBOSE("HoughDataPerSectorVec was empty, key: " << m_houghDataPerSectorVecKey.key());
diff --git a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.h b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.h
index 4aaf2a3a4f1..2a50b6043d1 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MooSegmentCombinationFinder/src/MooSegmentFinderAlg.h
@@ -74,7 +74,7 @@ class MooSegmentFinderAlg : public AthAlgorithm
SG::WriteHandleKey<MuonPatternCombinationCollection> m_patternCombiLocation;
SG::WriteHandleKey<Trk::SegmentCollection> m_segmentLocation;
- SG::WriteHandleKey<std::vector<Muon::HoughDataPerSec>> m_houghDataPerSectorVecKey {this,
+ SG::WriteHandleKey<Muon::HoughDataPerSectorVec> m_houghDataPerSectorVecKey {this,
"Key_MuonLayerHoughToolHoughDataPerSectorVec", "HoughDataPerSectorVec", "HoughDataPerSectorVec key"};
ToolHandle<Muon::IMooSegmentCombinationFinder> m_segmentFinder; //<! pointer to the segment finder
diff --git a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MuonSegmentCombinerToolInterfaces/MuonSegmentCombinerToolInterfaces/IMooSegmentCombinationFinder.h b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MuonSegmentCombinerToolInterfaces/MuonSegmentCombinerToolInterfaces/IMooSegmentCombinationFinder.h
index e2e59b91e51..2a169bff371 100644
--- a/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MuonSegmentCombinerToolInterfaces/MuonSegmentCombinerToolInterfaces/IMooSegmentCombinationFinder.h
+++ b/MuonSpectrometer/MuonReconstruction/MuonSegmentCombiners/MuonSegmentCombinerTools/MuonSegmentCombinerToolInterfaces/MuonSegmentCombinerToolInterfaces/IMooSegmentCombinationFinder.h
@@ -31,7 +31,7 @@ namespace Muon
struct Output {
MuonPatternCombinationCollection* patternCombinations;
Trk::SegmentCollection* segmentCollection;
- std::unique_ptr<std::vector<HoughDataPerSec>> houghDataPerSectorVec;
+ std::unique_ptr<Muon::HoughDataPerSectorVec> houghDataPerSectorVec;
Output() : patternCombinations(0),segmentCollection(0) {}
};
--
GitLab