Thread-safe determination of TObject::IsOnHeap

core/base/inc/TStorage.h

@@ -34,14 +34,14 @@ typedef char *(*ReAllocCharFun_t)(char*, size_t, size_t);
 class TStorage {
 
 private:
-   static ULong_t        fgHeapBegin;          // begin address of heap
-   static ULong_t        fgHeapEnd;            // end address of heap
    static size_t         fgMaxBlockSize;       // largest block allocated
    static FreeHookFun_t  fgFreeHook;           // function called on free
    static void          *fgFreeHookData;       // data used by this function
    static ReAllocFun_t   fgReAllocHook;        // custom ReAlloc
    static ReAllocCFun_t  fgReAllocCHook;       // custom ReAlloc with length check
    static Bool_t         fgHasCustomNewDelete; // true if using ROOT's new/delete
+   static const UInt_t   kObjectAllocMemValue = 0x99999999;
+                                               // magic number for ObjectAlloc
 
 public:
    virtual ~TStorage() { }
@@ -77,16 +77,13 @@ class TStorage {
    static void   AddToHeap(ULong_t begin, ULong_t end);
    static Bool_t IsOnHeap(void *p);
 
+   static Bool_t FilledByObjectAlloc(UInt_t* member);
+
    ClassDef(TStorage,0)  //Storage manager class
 };
 
 #ifndef WIN32
-inline void TStorage::AddToHeap(ULong_t begin, ULong_t end)
-   { if (begin < fgHeapBegin) fgHeapBegin = begin;
-     if (end   > fgHeapEnd)   fgHeapEnd   = end; }
-
-inline Bool_t TStorage::IsOnHeap(void *p)
-   { return (ULong_t)p >= fgHeapBegin && (ULong_t)p < fgHeapEnd; }
+inline Bool_t TStorage::FilledByObjectAlloc(UInt_t *member) { return *member == kObjectAllocMemValue; }
 
 inline size_t TStorage::GetMaxBlockSize() { return fgMaxBlockSize; }
 

core/base/src/TObject.cxx

@@ -62,7 +62,7 @@ Bool_t TObject::fgObjectStat = kTRUE;
 ClassImp(TObject)
 
 //______________________________________________________________________________
-TObject::TObject() : fUniqueID(0), fBits(kNotDeleted)
+TObject::TObject() : fBits(kNotDeleted) //Need to leave FUniqueID unset
 {
    // TObject constructor. It sets the two data words of TObject to their
    // initial values. The unique ID is set to 0 and the status word is
@@ -71,8 +71,10 @@ TObject::TObject() : fUniqueID(0), fBits(kNotDeleted)
    // (see TEnv) the object is added to the global TObjectTable for
    // bookkeeping.
 
-   if (TStorage::IsOnHeap(this))
+   if (TStorage::FilledByObjectAlloc(&fUniqueID)) 
       fBits |= kIsOnHeap;
+
+   fUniqueID = 0;
 
    if (fgObjectStat) TObjectTable::AddObj(this);
 }
@@ -82,17 +84,19 @@ TObject::TObject(const TObject &obj)
 {
    // TObject copy ctor.
 
-   fUniqueID = obj.fUniqueID;  // when really unique don't copy
-   fBits     = obj.fBits;
+   fBits = obj.fBits;
 
-   if (TStorage::IsOnHeap(this))
+   if (TStorage::FilledByObjectAlloc(&fUniqueID))
       fBits |= kIsOnHeap;
    else
       fBits &= ~kIsOnHeap;
 
    fBits &= ~kIsReferenced;
    fBits &= ~kCanDelete;
 
+   //Set only after used in above call
+   fUniqueID = obj.fUniqueID;  // when really unique don't copy
+
    if (fgObjectStat) TObjectTable::AddObj(this);
 }
 

core/base/src/TStorage.cxx

@@ -62,8 +62,6 @@
 
 #define PVOID (-1)
 
-ULong_t       TStorage::fgHeapBegin = (ULong_t)-1L;
-ULong_t       TStorage::fgHeapEnd;
 size_t        TStorage::fgMaxBlockSize;
 FreeHookFun_t TStorage::fgFreeHook;
 void         *TStorage::fgFreeHookData;
@@ -323,15 +321,12 @@ void *TStorage::ObjectAlloc(size_t sz)
 {
    // Used to allocate a TObject on the heap (via TObject::operator new()).
    // Directly after this routine one can call (in the TObject ctor)
-   // TStorage::IsOnHeap() to find out if the just created object is on
+   // TStorage::FilledByObjectAlloc() to find out if the just created object is on
    // the heap.
 
-   // Needs to be protected by global mutex
-   R__LOCKGUARD(gGlobalMutex);
-
-   ULong_t space = (ULong_t) ::operator new(sz);
-   AddToHeap(space, space+sz);
-   return (void*) space;
+   void* space =  ::operator new(sz);
+   memset(space, kObjectAllocMemValue, sz);
+   return space;
 }
 
 //______________________________________________________________________________
@@ -450,15 +445,17 @@ void TStorage::EnableStatistics(int size, int ix)
 //______________________________________________________________________________
 ULong_t TStorage::GetHeapBegin()
 {
+   ::Obsolete("GetHeapBegin()", "v5-34-00", "v6-02-00");
    //return begin of heap
-   return fgHeapBegin;
+   return 0;
 }
 
 //______________________________________________________________________________
 ULong_t TStorage::GetHeapEnd()
 {
+   ::Obsolete("GetHeapBegin()", "v5-34-00", "v6-02-00");
    //return end of heap
-   return fgHeapEnd;
+   return 0;
 }
 
 //______________________________________________________________________________
@@ -482,21 +479,28 @@ void TStorage::SetCustomNewDelete()
    fgHasCustomNewDelete = kTRUE;
 }
 
-#ifdef WIN32
 
 //______________________________________________________________________________
-void TStorage::AddToHeap(ULong_t begin, ULong_t end)
+void TStorage::AddToHeap(ULong_t, ULong_t)
 {
    //add a range to the heap
-   if (begin < fgHeapBegin) fgHeapBegin = begin;
-   if (end   > fgHeapEnd)   fgHeapEnd   = end;
+   ::Obsolete("AddToHeap(ULong_t,ULong_t)", "v5-34-00", "v6-02-00");
 }
 
 //______________________________________________________________________________
-Bool_t TStorage::IsOnHeap(void *p)
+Bool_t TStorage::IsOnHeap(void *)
 {
    //is object at p in the heap?
-   return (ULong_t)p >= fgHeapBegin && (ULong_t)p < fgHeapEnd;
+   ::Obsolete("IsOnHeap(void*)", "v5-34-00", "v6-02-00");
+   return false;
+}
+
+#ifdef WIN32
+//______________________________________________________________________________
+Bool_t TStorage::FilledByObjectAlloc(UInt_t *member)
+{
+   //called by TObject's constructor to determine if object was created by call to new
+   return *member == kObjectAllocMemValue;
 }
 
 //______________________________________________________________________________

core/cont/src/TMap.cxx

@@ -278,16 +278,12 @@ void TMap::PrintCollectionEntry(TObject* entry, Option_t* option, Int_t recurse)
    printf("Key:   ");
    entry->Print();
    TROOT::IndentLevel();
-   if (TStorage::IsOnHeap(val)) {
-      printf("Value: ");
-      TCollection* coll = dynamic_cast<TCollection*>(val);
-      if (coll) {
-         coll->Print(option, recurse);
-      } else {
-         val->Print(option);
-      }
+   printf("Value: ");
+   TCollection* coll = dynamic_cast<TCollection*>(val);
+   if (coll) {
+     coll->Print(option, recurse);
    } else {
-      printf("Value: 0x%lx\n", (ULong_t) val);
+     val->Print(option);
    }
 }
 

etc/valgrind-root.supp

@@ -484,6 +484,29 @@
    fun:_ZL20G__cpp_setup_global0v
 }
 
+######## ROOT TObject on heap
+
+{
+   TObject::TObject() uses uninitialized value
+   Memcheck:Cond
+   fun:_ZN7TObjectC1Ev
+   ...
+}
+
+{
+   TObject::TObject(const TObject&) uses uninitialized value
+   Memcheck:Cond
+   fun:_ZN7TObjectC1ERKS_
+   ...
+}
+
+{
+   gcc optimizer confuses valgrind on TObject::~TObject()
+   Memcheck:Cond
+   fun:_ZN7TObjectD1Ev
+   ...
+}
+
 ######### Misc
 
 {