From 87db0efb5de88341006e2d1e2414dc5bb0c88d2d Mon Sep 17 00:00:00 2001
From: scott snyder <snyder@bnl.gov>
Date: Thu, 20 Apr 2017 16:46:49 +0200
Subject: [PATCH] InDetEventAthenaPool: Avoid undefined behavior.

Code in this package was taking advantage of undefined behavior to change
the name used by the MsgStreams of converters (a protected data member)
to something more meaningful.  This results in warnings from the
undefined behavior sanitizer.

cnvname.AthenaPoolCnvSvc added an optional argument to the converter
constructors to allow specifying this name.  So switch to using this.



Former-commit-id: 746f7f53292714b7ea77c54d94b233cd43c92946
---
 .../src/InDetLowBetaCandidateCnv.cxx          |  9 +--------
 .../src/InDetLowBetaContainerCnv.cxx          |  9 +--------
 .../src/InDetSimDataCnv_p1.cxx                |  3 ++-
 .../InDetEventAthenaPool/src/MsgUtil.h        | 20 -------------------
 .../src/PixelClusterContainerCnv.cxx          |  5 +----
 .../src/PixelGangedClusterAmbiguitiesCnv.cxx  |  8 --------
 .../src/PixelGangedClusterAmbiguitiesCnv.h    |  2 +-
 .../src/PixelRDO_ContainerCnv.cxx             |  2 --
 .../src/PixelRDO_ContainerCnv.h               |  2 +-
 .../src/SCT_ClusterContainerCnv.cxx           |  4 +---
 .../src/SCT_RDO_ContainerCnv.cxx              |  5 +----
 .../src/TRT_DriftCircleContainerCnv.cxx       |  5 +----
 .../src/TRT_RDO_ContainerCnv.cxx              |  3 ---
 .../src/TRT_RDO_ContainerCnv.h                |  2 +-
 14 files changed, 11 insertions(+), 68 deletions(-)

diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaCandidateCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaCandidateCnv.cxx
index b677a849bb0..fa83294187d 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaCandidateCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaCandidateCnv.cxx
@@ -4,10 +4,9 @@
 
 #include "InDetLowBetaCandidateCnv.h"
 #include "InDetEventTPCnv/InDetLowBetaInfo/InDetLowBetaCandidateCnv_tlp1.h"
-#include "MsgUtil.h"
 
 InDetLowBetaCandidateCnv::InDetLowBetaCandidateCnv(ISvcLocator* svcloc) :
-  T_AthenaPoolCustomCnv<InDet::InDetLowBetaCandidate, InDetLowBetaCandidate_PERS >( svcloc ),
+  T_AthenaPoolCustomCnv<InDet::InDetLowBetaCandidate, InDetLowBetaCandidate_PERS >( svcloc, "InDetLowBetaCandidateConverter" ),
   m_TPConverter (new InDetLowBetaCandidateCnv_tlp1)
 { }
 InDetLowBetaCandidateCnv::~InDetLowBetaCandidateCnv()
@@ -18,9 +17,6 @@ InDetLowBetaCandidateCnv::~InDetLowBetaCandidateCnv()
 
 
 InDetLowBetaCandidate_PERS* InDetLowBetaCandidateCnv::createPersistent(InDet::InDetLowBetaCandidate* transCont) {
-  IDEvtAthPool::setMsgName(this,"InDetLowBetaCandidateConverter");//So msg() won't use name "AthenaPoolConverter" 
-  //do it in both createPersistent and createTransient since there is no initialize method (could add it of course)
-
   ATH_MSG_DEBUG("InDetLowBetaCandidateCnv::createPersistent");
 
   InDetLowBetaCandidate_PERS *persObj = m_TPConverter->createPersistent(transCont, msg());
@@ -30,9 +26,6 @@ InDetLowBetaCandidate_PERS* InDetLowBetaCandidateCnv::createPersistent(InDet::In
 
 
 InDet::InDetLowBetaCandidate* InDetLowBetaCandidateCnv::createTransient() {
-  IDEvtAthPool::setMsgName(this,"InDetLowBetaCandidateConverter");//So msg() won't use name "AthenaPoolConverter" 
-  //do it in both createPersistent and createTransient since there is no initialize method (could add it of course)
-
   ATH_MSG_DEBUG("InDetLowBetaCandidateCnv::createTransient ");
   
   static pool::Guid tlp1_guid("8C24589F-FBAA-4686-9254-B5C360A94733");
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaContainerCnv.cxx
index 1d4a2a7fb41..c731090f9c2 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetLowBetaContainerCnv.cxx
@@ -4,10 +4,9 @@
 
 #include "InDetLowBetaContainerCnv.h"
 #include "InDetEventTPCnv/InDetLowBetaInfo/InDetLowBetaContainerCnv_tlp1.h"
-#include "MsgUtil.h"
 
 InDetLowBetaContainerCnv::InDetLowBetaContainerCnv(ISvcLocator *svcloc)
-  : InDetLowBetaContainerCnvBase(svcloc),
+  : InDetLowBetaContainerCnvBase(svcloc, "InDetLowBetaContainerCnv"),
     m_TPConverter_tlp1 (new InDetLowBetaContainerCnv_tlp1)
 { }
 
@@ -19,9 +18,6 @@ InDetLowBetaContainerCnv::~InDetLowBetaContainerCnv()
 
 // createPersistent
 InDetLowBetaContainer_PERS *InDetLowBetaContainerCnv::createPersistent(InDet::InDetLowBetaContainer *transObj) {
-  //should really do it just once in ::initialize instead:
-  IDEvtAthPool::setMsgName(this,"InDetLowBetaContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
   ATH_MSG_DEBUG("InDetLowBetaContainerCnv::createPersistent called");
 
   InDetLowBetaContainer_PERS *p_InDetLowBetaCont = m_TPConverter_tlp1->createPersistent(transObj, msg());
@@ -32,9 +28,6 @@ InDetLowBetaContainer_PERS *InDetLowBetaContainerCnv::createPersistent(InDet::In
  
 // createTransient
 InDet::InDetLowBetaContainer *InDetLowBetaContainerCnv::createTransient() {
-  //should really do it just once in ::initialize instead:
-  IDEvtAthPool::setMsgName(this,"InDetLowBetaContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-  
   ATH_MSG_DEBUG("InDetLowBetaContainerCnv::createTransient called");
 
   static pool::Guid tlp1_guid("2EBE2034-8157-477B-B327-D37BE8A0317D");
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetSimDataCnv_p1.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetSimDataCnv_p1.cxx
index 8c7898190a5..df32aa4e292 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetSimDataCnv_p1.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/InDetSimDataCnv_p1.cxx
@@ -8,7 +8,8 @@
 // Persistent class and converter header file
 #include "InDetEventAthenaPool/InDetSimData_p1.h"
 #include "InDetSimDataCnv_p1.h"
-#include "AthenaKernel/IProxyDict.h"
+#include "AthenaBaseComps/AthMessaging.h"
+
 
 typedef std::vector<InDetSimData::Deposit>::const_iterator depositIterator;
 
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/MsgUtil.h b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/MsgUtil.h
index 092ddd6dc3b..ea5e23983e0 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/MsgUtil.h
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/MsgUtil.h
@@ -5,26 +5,6 @@
 #ifndef INDETEVENTATHENAPOOL_MSGFIX
 #define INDETEVENTATHENAPOOL_MSGFIX
 
-// A few utilities for using MSG service easily in converters. Here
-// temporarily until someone comes up with a more general approach.
-//
-// Thomas Kittelmann March 2011
-
-#include "AthenaBaseComps/AthMessaging.h"
-#include "GaudiKernel/MsgStream.h"
- 
-namespace IDEvtAthPool {
-  //Small slightly dirty workaround so we can set the source name on the
-  //AthMessaging message streams to something else than "AthenaPoolConverter""
-  class MsgStreamSourceSettable : public MsgStream {
-  public:
-    void setSource(const char*c) { if (m_source!=c) m_source=c; }
-  };
-  inline void setMsgName(AthMessaging*a,const char*c) {
-    if (a&&c) static_cast<MsgStreamSourceSettable*>(&(a->msg()))->setSource(c);
-  }
-}
-
 //Defines similar to ATH_MSG_DEBUG and ATH_MSG_VERBOSE which accepts the MsgStream as an argument: 
 #ifdef MSG_DEBUG
 #undef MSG_DEBUG
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelClusterContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelClusterContainerCnv.cxx
index c37638cee94..850e899e66a 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelClusterContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelClusterContainerCnv.cxx
@@ -5,14 +5,13 @@
 #include "StoreGate/StoreGateSvc.h"
 #include "PixelClusterContainerCnv.h"
 #include "InDetIdentifier/PixelID.h"
-#include "MsgUtil.h"
 
 #include <memory>
 
 #include <iostream>
 
   PixelClusterContainerCnv::PixelClusterContainerCnv (ISvcLocator* svcloc)
-    : PixelClusterContainerCnvBase(svcloc),
+    : PixelClusterContainerCnvBase(svcloc, "PixelClusterContainerCnv"),
       m_converter_p0(),
       m_storeGate(nullptr)
   {}
@@ -21,8 +20,6 @@
 
 
 StatusCode PixelClusterContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"PixelClusterContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    ATH_MSG_INFO("PixelClusterContainerCnv::initialize()");
 
    StatusCode sc = PixelClusterContainerCnvBase::initialize();
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.cxx
index c6942586aa0..12bf6d5a06a 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.cxx
@@ -11,14 +11,9 @@
 #include "PixelGangedClusterAmbiguitiesCnv.h"
 #include "InDetEventTPCnv/InDetPrepRawData/PixelGangedClusterAmbiguities_p1.h"
 #include "InDetEventTPCnv/InDetPrepRawData/PixelGangedClusterAmbiguitiesCnv_p1.h"
-#include "MsgUtil.h"
-
 static PixelGangedClusterAmbiguitiesCnv_p1   TPconverter;
 
 PixelGangedClusterAmbiguities_PERS* PixelGangedClusterAmbiguitiesCnv::createPersistent(InDet::PixelGangedClusterAmbiguities* transObj) {
-    //repeated twice, should really be in an initialize method
-    IDEvtAthPool::setMsgName(this,"PixelGangedClusterAmbiguitiesConverter");//So msg() won't use name "AthenaPoolConverter" 
-
     ATH_MSG_DEBUG("PixelGangedClusterAmbiguities write");
     PixelGangedClusterAmbiguities_PERS *persObj = TPconverter.createPersistent( transObj, msg() );
     ATH_MSG_DEBUG("Success");
@@ -26,9 +21,6 @@ PixelGangedClusterAmbiguities_PERS* PixelGangedClusterAmbiguitiesCnv::createPers
 }
     
 InDet::PixelGangedClusterAmbiguities* PixelGangedClusterAmbiguitiesCnv::createTransient() {
-    //repeated twice, should really be in an initialize method
-    IDEvtAthPool::setMsgName(this,"PixelGangedClusterAmbiguitiesConverter");//So msg() won't use name "AthenaPoolConverter" 
-
     static pool::Guid   p1_guid("FE36CE7E-EADF-481F-A55A-26DA0030DFAA");
 //     static pool::Guid   p0_guid("380D8BB9-B34F-470F-92CC-06C3D60F7BE4");
     if( compareClassGuid(p1_guid) ) {
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.h b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.h
index cf1ef6833aa..3da1affd7ff 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.h
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelGangedClusterAmbiguitiesCnv.h
@@ -22,7 +22,7 @@ typedef  T_AthenaPoolCustomCnv<InDet::PixelGangedClusterAmbiguities, PixelGanged
 class PixelGangedClusterAmbiguitiesCnv : public PixelGangedClusterAmbiguitiesCnvBase {
 friend class CnvFactory<PixelGangedClusterAmbiguitiesCnv >;
 protected:
-  PixelGangedClusterAmbiguitiesCnv (ISvcLocator* svcloc) : PixelGangedClusterAmbiguitiesCnvBase(svcloc) {}
+  PixelGangedClusterAmbiguitiesCnv (ISvcLocator* svcloc) : PixelGangedClusterAmbiguitiesCnvBase(svcloc, "PixelGangedClusterAmbiguitiesConverter") {}
   virtual PixelGangedClusterAmbiguities_PERS*   createPersistent (InDet::PixelGangedClusterAmbiguities* transObj);
   virtual InDet::PixelGangedClusterAmbiguities*        createTransient ();
 };
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.cxx
index b57e99492df..284ba17954b 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.cxx
@@ -33,8 +33,6 @@ namespace {
 }
 //================================================================
 StatusCode PixelRDO_ContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"PixelRDO_ContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    StatusCode sc = PixelRDO_ContainerCnvBase::initialize();
    if (sc.isFailure()) {
      ATH_MSG_FATAL("PixelRDO_ContainerCnvBase::initialize() returned failure !");
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.h b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.h
index 04cc3eed732..432d02de825 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.h
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/PixelRDO_ContainerCnv.h
@@ -38,7 +38,7 @@ class PixelRDO_ContainerCnv : public PixelRDO_ContainerCnvBase {
 
 protected:
   PixelRDO_ContainerCnv (ISvcLocator* svcloc)
-    : PixelRDO_ContainerCnvBase(svcloc),
+    : PixelRDO_ContainerCnvBase(svcloc, "PixelRDO_ContainerCnv"),
       m_converter_p0(),
       m_storeGate(nullptr)
   {}
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_ClusterContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_ClusterContainerCnv.cxx
index 6abcbd74945..08c6b67b882 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_ClusterContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_ClusterContainerCnv.cxx
@@ -13,7 +13,7 @@
 #include <iostream>
 
   SCT_ClusterContainerCnv::SCT_ClusterContainerCnv (ISvcLocator* svcloc)
-    : SCT_ClusterContainerCnvBase(svcloc),
+    : SCT_ClusterContainerCnvBase(svcloc, "SCT_ClusterContainerCnv"),
       m_converter_p0(),
       m_storeGate(nullptr)
   {}
@@ -22,8 +22,6 @@
 
 
 StatusCode SCT_ClusterContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"SCT_ClusterContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    ATH_MSG_INFO("SCT_ClusterContainerCnv::initialize()");
 
    StatusCode sc = SCT_ClusterContainerCnvBase::initialize();
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_RDO_ContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_RDO_ContainerCnv.cxx
index 0441369eeca..fd41f623577 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_RDO_ContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/SCT_RDO_ContainerCnv.cxx
@@ -5,7 +5,6 @@
 #include "SCT_RDO_ContainerCnv.h"
 
 #include "InDetIdentifier/SCT_ID.h"
-#include "MsgUtil.h"
 
 #include <memory>
 
@@ -37,15 +36,13 @@ namespace {
 }
 
 SCT_RDO_ContainerCnv::SCT_RDO_ContainerCnv (ISvcLocator* svcloc)
-  : SCT_RDO_ContainerCnvBase(svcloc),
+  : SCT_RDO_ContainerCnvBase(svcloc, "SCT_RDO_ContainerCnv"),
     m_converter_p0(),
     m_storeGate(nullptr)
 {}
 
 //================================================================
 StatusCode SCT_RDO_ContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"SCT_RDO_ContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    StatusCode sc = SCT_RDO_ContainerCnvBase::initialize();
    if (sc.isFailure()) {
      ATH_MSG_FATAL("SCT_RDO_ContainerCnvBase::initialize() returned failure !");
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_DriftCircleContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_DriftCircleContainerCnv.cxx
index d8ecc97d99c..dd12d213ae1 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_DriftCircleContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_DriftCircleContainerCnv.cxx
@@ -4,14 +4,13 @@
 
 #include "TRT_DriftCircleContainerCnv.h"
 #include "InDetIdentifier/TRT_ID.h"
-#include "MsgUtil.h"
 
 #include <memory>
 
 #include <iostream>
 
   TRT_DriftCircleContainerCnv::TRT_DriftCircleContainerCnv (ISvcLocator* svcloc)
-    : TRT_DriftCircleContainerCnvBase(svcloc),
+    : TRT_DriftCircleContainerCnvBase(svcloc, "TRT_DriftCircleContainerCnv"),
       m_converter_p0(),
       m_storeGate(nullptr)
   {}
@@ -20,8 +19,6 @@
 
 
 StatusCode TRT_DriftCircleContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"TRT_DriftCircleContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    ATH_MSG_INFO("TRT_DriftCircleContainerCnv::initialize()");
 
    StatusCode sc = TRT_DriftCircleContainerCnvBase::initialize();
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.cxx b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.cxx
index 161b1631d26..add6ef5a8d5 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.cxx
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.cxx
@@ -2,7 +2,6 @@
   Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
 */
 
-#include "MsgUtil.h"
 #include "TRT_RDO_ContainerCnv.h"
 
 #include "InDetIdentifier/TRT_ID.h"
@@ -34,8 +33,6 @@ namespace {
 }
 //================================================================
 StatusCode TRT_RDO_ContainerCnv::initialize() {
-   IDEvtAthPool::setMsgName(this,"TRT_RDO_ContainerCnv");//So msg() won't use name "AthenaPoolConverter" 
-
    ATH_MSG_INFO("TRT_RDO_ContainerCnv::initialize()");
 
    StatusCode sc = TRT_RDO_ContainerCnvBase::initialize();
diff --git a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.h b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.h
index 538fd194a80..b761e4d1001 100644
--- a/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.h
+++ b/InnerDetector/InDetEventCnv/InDetEventAthenaPool/src/TRT_RDO_ContainerCnv.h
@@ -41,7 +41,7 @@ class TRT_RDO_ContainerCnv : public TRT_RDO_ContainerCnvBase {
 
 protected:
   TRT_RDO_ContainerCnv (ISvcLocator* svcloc)
-    : TRT_RDO_ContainerCnvBase(svcloc),
+    : TRT_RDO_ContainerCnvBase(svcloc, "TRT_RDO_ContainerCnv"),
       m_converter_p1(),
       m_converter_p0(),
       m_storeGate(nullptr)
-- 
GitLab