From e6d30b0edebb1c237892be60b72781a690e034ee Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 8 Sep 2022 16:51:30 -0500 Subject: [PATCH] core: Disable test for already deleted object. On some platform operator delete taints the memory, so even right after the deletion the information stored by ~TObject is already erased. On those platform we no longer rely on the kNotDelete bit hack and rely on the system (which has tainted the memory assumingly for a reason) to detect the use-after-delete problems. Introduce 2 new functions. TObject::IsDestructed (used by TClonesArray) that detects that the destructor has been run and is active in all configuration. This should be used if the code knows that the memory has not been freed/deleted. ROOT::Detail::HasBeenDeleted(TObject*) with returns true if the platform does not taint the memory and if the kNotDeleted is not set (in all other case, it returns false) This fixes #11330 --- .../python/JupyROOT/helpers/cppcompleter.py | 1 + .../JupyROOT/helpers/cppcompleter.py | 1 + core/base/inc/TObject.h | 42 +++++++++++ core/base/src/TObject.cxx | 70 +++++++++++++++++++ core/base/src/TQConnection.cxx | 4 +- core/base/src/TROOT.cxx | 2 +- core/cont/src/TClonesArray.cxx | 26 +++---- core/cont/src/TCollection.cxx | 2 +- core/cont/src/THashList.cxx | 4 +- core/cont/src/TList.cxx | 14 ++-- core/cont/src/TObjArray.cxx | 4 +- core/cont/src/TObjectTable.cxx | 2 +- core/gui/src/TContextMenu.cxx | 2 +- etc/valgrind-root.supp | 8 +++ graf2d/gpad/src/TButton.cxx | 4 +- graf2d/gpad/src/TCanvas.cxx | 2 +- graf2d/gpad/src/TPad.cxx | 16 ++--- graf2d/gpad/src/TRatioPlot.cxx | 33 +++++---- graf2d/graf/src/TPaveText.cxx | 2 +- graf3d/g3d/src/TGeometry.cxx | 2 +- graf3d/g3d/src/TNode.cxx | 2 +- gui/gui/src/TGFileBrowser.cxx | 4 +- gui/gui/src/TGFrame.cxx | 2 +- gui/gui/src/TGMdiMainFrame.cxx | 2 +- gui/gui/src/TGTextEntry.cxx | 2 +- hist/hist/src/TEfficiency.cxx | 2 +- hist/hist/src/TH1.cxx | 4 +- net/net/src/TApplicationServer.cxx | 2 +- tutorials/net/spyserv.C | 2 +- 29 files changed, 195 insertions(+), 68 deletions(-) diff --git a/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py b/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py index b9f622c3292f5..f8ec85031148b 100644 --- a/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py +++ b/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py @@ -75,6 +75,7 @@ class CppCompleter(object): ... print(suggestion) TROOT::IsA TROOT::IsBatch + TROOT::IsDestructed TROOT::IsEqual TROOT::IsEscaped TROOT::IsExecutingMacro diff --git a/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py b/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py index 5ab9d96f2df41..aa966442387f2 100644 --- a/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py +++ b/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py @@ -87,6 +87,7 @@ class CppCompleter(object): ... print(suggestion) TROOT::IsA TROOT::IsBatch + TROOT::IsDestructed TROOT::IsEqual TROOT::IsEscaped TROOT::IsExecutingMacro diff --git a/core/base/inc/TObject.h b/core/base/inc/TObject.h index 08d2f865b58c8..3942f0f5e7ddc 100644 --- a/core/base/inc/TObject.h +++ b/core/base/inc/TObject.h @@ -33,6 +33,10 @@ class TObjArray; class TMethod; class TTimer; +namespace ROOT { +namespace Internal { + bool DeleteChangesMemoryImpl(); +}} class TObject { @@ -163,6 +167,16 @@ class TObject { virtual Int_t Write(const char *name = nullptr, Int_t option = 0, Int_t bufsize = 0); virtual Int_t Write(const char *name = nullptr, Int_t option = 0, Int_t bufsize = 0) const; + /// IsDestructed + /// + /// \note This function must be non-virtual as it can be used on destructed (but + /// not yet modified) memory. This is used for example in TClonesArray to record + /// the element that have been destructed but not deleted and thus are ready for + /// re-use (by operator new with placement). + /// + /// \return true if this object's destructor has been run. + Bool_t IsDestructed() const { return !TestBit(kNotDeleted); } + //----- operators void *operator new(size_t sz) { return TStorage::ObjectAlloc(sz); } void *operator new[](size_t sz) { return TStorage::ObjectAllocArray(sz); } @@ -226,6 +240,7 @@ class TObject { static void SetObjectStat(Bool_t stat); friend class TClonesArray; // needs to reset kNotDeleted in fBits + friend bool ROOT::Internal::DeleteChangesMemoryImpl(); ClassDef(TObject,1) //Basic ROOT object }; @@ -365,4 +380,31 @@ namespace cling { std::string printValue(TObject *val); } +namespace ROOT { + +namespace Internal { + bool DeleteChangesMemory(); +} // Internal + +namespace Detail { + + +/// @brief Check if the TObject's memory has been deleted. +/// @warning This should be only used for error mitigation as the answer is only +/// sometimes correct. It actually just checks whether the object has been +/// deleted, so this will falsely return true for an object that has +/// been destructed but its memory has not been deleted. This will return an +/// undefined value if the memory is re-used between the deletion and the check. +/// i.e. This is useful to prevent a segmentation fault in case where the problem +/// can be detected when the deletion and the usage are 'close-by' +/// @warning In enviroment where delete taints (changes) the memory, this function +/// always returns false as the marker left by ~TObject will be overwritten. +/// @param obj The memory to check +/// @return true if the object has been destructed and it can be inferred that it has been deleted +R__ALWAYS_INLINE bool HasBeenDeleted(const TObject *obj) { + return !ROOT::Internal::DeleteChangesMemory() && obj->IsDestructed(); +} + +}} // ROOT::Details + #endif diff --git a/core/base/src/TObject.cxx b/core/base/src/TObject.cxx index 4496d881ca78d..523690b925cf4 100644 --- a/core/base/src/TObject.cxx +++ b/core/base/src/TObject.cxx @@ -56,6 +56,76 @@ Bool_t TObject::fgObjectStat = kTRUE; ClassImp(TObject); +namespace ROOT { +namespace Internal { + +// Return true if delete changes/poisons/taints the memory. +// +// Detect whether operator delete taints the memory. If it does, we can not rely +// on TestBit(kNotDeleted) to check if the memory has been deleted (but in case, +// like TClonesArray, where we know the destructor will be called but not operator +// delete, we can still use it to detect the cases where the destructor was called. + +bool DeleteChangesMemoryImpl() +{ + static constexpr UInt_t kGoldenUUID = 0x00000021; + static constexpr UInt_t kGoldenbits = 0x03000000; + + TObject *o = new TObject; + o->SetUniqueID(kGoldenUUID); + UInt_t *o_fuid = &(o->fUniqueID); + UInt_t *o_fbits = &(o->fBits); + + if (*o_fuid != kGoldenUUID) { + Error("CheckingDeleteSideEffects", + "fUniqueID is not as expected, we got 0x%.8x instead of 0x%.8x", + *o_fuid, kGoldenUUID); + } + if (*o_fbits != kGoldenbits) { + Error("CheckingDeleteSideEffects", + "fBits is not as expected, we got 0x%.8x instead of 0x%.8x", + *o_fbits, kGoldenbits); + } + if (gDebug >= 9) { + unsigned char *oc = reinterpret_cast(o); // for address calculations + unsigned char references[sizeof(TObject)]; + memcpy(references, oc, sizeof(TObject)); + + // The effective part of this code (the else statement is just that without + // any of the debug statement) + delete o; + + // Not using the error logger, as there routine is meant to be called + // during library initialization/loading. + fprintf(stderr, + "DEBUG: Checking before and after delete the content of a TObject with uniqueID 0x21\n"); + for(size_t i = 0; i < sizeof(TObject); i += 4) { + fprintf(stderr, "DEBUG: 0x%.8x vs 0x%.8x\n", *(int*)(references +i), *(int*)(oc + i)); + } + } else + delete o; // the 'if' part is that surrounded by the debug code. + + // Intentionally accessing the deleted memory to check whether it has been changed as + // a consequence (side effect) of executing operator delete. If there no change, we + // can guess this is always the case and we can rely on the changes to fBits made + // by ~TObject to detect use-after-delete error (and print a message rather than + // stop the program with a segmentation fault) + if ( *o_fbits != 0x01000000 ) { + // operator delete tainted the memory, we can not rely on TestBit(kNotDeleted) + return true; + } + return false; +} + +bool DeleteChangesMemory() +{ + static const bool value = DeleteChangesMemoryImpl(); + if (gDebug >= 9) + DeleteChangesMemoryImpl(); // To allow for printing the debug info + return value; +} + +}} // ROOT::Detail //////////////////////////////////////////////////////////////////////////////// /// Copy this to obj. diff --git a/core/base/src/TQConnection.cxx b/core/base/src/TQConnection.cxx index 448dbb922cbf9..16cd3c5653e39 100644 --- a/core/base/src/TQConnection.cxx +++ b/core/base/src/TQConnection.cxx @@ -284,7 +284,7 @@ CallFunc_t *TQSlot::StartExecuting() { void TQSlot::EndExecuting() { fExecuting--; - if (!TestBit(kNotDeleted) && !fExecuting) + if (ROOT::Detail::HasBeenDeleted(this) && !fExecuting) gCling->CallFunc_Delete(fFunc); } @@ -344,7 +344,7 @@ inline void TQSlot::ExecuteMethod(void *object, Longptr_t *paramArr, Int_t npara fExecuting++; gCling->CallFunc_Exec(fFunc, address); fExecuting--; - if (!TestBit(kNotDeleted) && !fExecuting) + if (ROOT::Detail::HasBeenDeleted(this) && !fExecuting) gCling->CallFunc_Delete(fFunc); } diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index a09aa34f251a9..d7e218bbd3038 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -1371,7 +1371,7 @@ TObject *TROOT::FindSpecialObject(const char *name, void *&where) } } if (!temp) return nullptr; - if (temp->TestBit(kNotDeleted)) return temp; + if (!ROOT::Detail::HasBeenDeleted(temp)) return temp; return nullptr; } diff --git a/core/cont/src/TClonesArray.cxx b/core/cont/src/TClonesArray.cxx index 5f0620880c2c6..623570b02e353 100644 --- a/core/cont/src/TClonesArray.cxx +++ b/core/cont/src/TClonesArray.cxx @@ -156,7 +156,7 @@ bool R__SetClonesArrayTFormulaUpdater(Updater_t func) { /// Internal Utility routine to correctly release the memory for an object static inline void R__ReleaseMemory(TClass *cl, TObject *obj) { - if (obj && obj->TestBit(TObject::kNotDeleted)) { + if (obj && ! obj->IsDestructed()) { // -- The TObject destructor has not been called. cl->Destructor(obj); } else { @@ -366,7 +366,7 @@ void TClonesArray::Compress() TObject *TClonesArray::ConstructedAt(Int_t idx) { TObject *obj = (*this)[idx]; - if ( obj && obj->TestBit(TObject::kNotDeleted) ) { + if ( obj && ! obj->IsDestructed() ) { return obj; } return (fClass) ? static_cast(fClass->New(obj)) : nullptr; @@ -388,7 +388,7 @@ TObject *TClonesArray::ConstructedAt(Int_t idx) TObject *TClonesArray::ConstructedAt(Int_t idx, Option_t *clear_options) { TObject *obj = (*this)[idx]; - if ( obj && obj->TestBit(TObject::kNotDeleted) ) { + if ( obj && !obj->IsDestructed() ) { obj->Clear(clear_options); return obj; } @@ -444,13 +444,13 @@ void TClonesArray::Delete(Option_t *) // In case of emulated class, we can not use the delete operator // directly, it would use the wrong destructor. for (Int_t i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fClass->Destructor(fCont[i],kTRUE); } } } else { for (Int_t i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } } @@ -516,7 +516,7 @@ void TClonesArray::ExpandCreate(Int_t n) for (i = 0; i < n; i++) { if (!fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -553,7 +553,7 @@ void TClonesArray::ExpandCreateFast(Int_t n) for (i = 0; i < n; i++) { if (i >= oldSize || !fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -575,7 +575,7 @@ TObject *TClonesArray::RemoveAt(Int_t idx) int i = idx-fLowerBound; - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } @@ -601,7 +601,7 @@ TObject *TClonesArray::Remove(TObject *obj) if (i == -1) return nullptr; - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } @@ -627,7 +627,7 @@ void TClonesArray::RemoveRange(Int_t idx1, Int_t idx2) Bool_t change = kFALSE; for (TObject **obj = fCont+idx1; obj <= fCont+idx2; obj++) { if (!*obj) continue; - if ((*obj)->TestBit(kNotDeleted)) { + if (!(*obj)->IsDestructed()) { (*obj)->~TObject(); } *obj = nullptr; @@ -801,7 +801,7 @@ void TClonesArray::Streamer(TBuffer &b) for (Int_t i = 0; i < nobjects; i++) { if (!fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -836,7 +836,7 @@ void TClonesArray::Streamer(TBuffer &b) if (nch) { if (!fKeep->fCont[i]) fKeep->fCont[i] = (TObject*)fClass->New(); - else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -922,7 +922,7 @@ TObject *&TClonesArray::operator[](Int_t idx) fKeep->fCont[idx] = (TObject*) TStorage::ObjectAlloc(fClass->Size()); // Reset the bit so that: // obj = myClonesArray[i]; - // obj->TestBit(TObject::kNotDeleted) + // ! obj->IsDestructed() // will behave correctly. // TObject::kNotDeleted is one of the higher bit that is not settable via the public // interface. But luckily we are its friend. diff --git a/core/cont/src/TCollection.cxx b/core/cont/src/TCollection.cxx index 5c5190ef148b6..ea8da3b25ed79 100644 --- a/core/cont/src/TCollection.cxx +++ b/core/cont/src/TCollection.cxx @@ -584,7 +584,7 @@ void TCollection::RecursiveRemove(TObject *obj) TObject *object; while ((object = next())) { - if (object->TestBit(kNotDeleted)) object->RecursiveRemove(obj); + if (!ROOT::Detail::HasBeenDeleted(object)) object->RecursiveRemove(obj); } } diff --git a/core/cont/src/THashList.cxx b/core/cont/src/THashList.cxx index aa2e8688a9b4e..83d139b832ed0 100644 --- a/core/cont/src/THashList.cxx +++ b/core/cont/src/THashList.cxx @@ -227,7 +227,7 @@ void THashList::Delete(Option_t *option) auto obj = tlk->GetObject(); // In case somebody else access it. tlk->SetObject(nullptr); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -350,7 +350,7 @@ void THashList::RecursiveRemove(TObject *obj) while (lnk.get()) { next = lnk->NextSP(); TObject *ob = lnk->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { ob->RecursiveRemove(obj); } lnk = next; diff --git a/core/cont/src/TList.cxx b/core/cont/src/TList.cxx index 034c7be31adaf..3ce34a2b8bef2 100644 --- a/core/cont/src/TList.cxx +++ b/core/cont/src/TList.cxx @@ -440,12 +440,12 @@ void TList::Clear(Option_t *option) // delete only heap objects marked OK to clear auto obj = tlk->GetObject(); if (!nodel && obj) { - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { Error("Clear", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); } else if (obj->IsOnHeap()) { if (obj->TestBit(kCanDelete)) { - if (obj->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(obj)) { TCollection::GarbageCollect(obj); } } @@ -501,7 +501,7 @@ void TList::Delete(Option_t *option) // delete only heap objects auto obj = tlk->GetObject(); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -530,7 +530,7 @@ void TList::Delete(Option_t *option) // delete only heap objects auto obj = tlk->GetObject(); tlk->SetObject(nullptr); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -643,7 +643,7 @@ TObjLink *TList::FindLink(const TObject *obj, Int_t &idx) const while (lnk) { object = lnk->GetObject(); if (object) { - if (object->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(object)) { if (object->IsEqual(obj)) return lnk; } } @@ -773,7 +773,7 @@ void TList::RecursiveRemove(TObject *obj) auto cached = fCache.lock(); if (cached && cached->fNext.get() == nullptr && cached->fPrev.lock().get() == nullptr) { TObject *ob = cached->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { ob->RecursiveRemove(obj); } } @@ -787,7 +787,7 @@ void TList::RecursiveRemove(TObject *obj) while (lnk.get()) { next = lnk->fNext; TObject *ob = lnk->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { if (ob->IsEqual(obj)) { lnk->SetObject(nullptr); if (lnk == fFirst) { diff --git a/core/cont/src/TObjArray.cxx b/core/cont/src/TObjArray.cxx index ac02d217fdeca..3b531dbeba36c 100644 --- a/core/cont/src/TObjArray.cxx +++ b/core/cont/src/TObjArray.cxx @@ -675,7 +675,7 @@ void TObjArray::RecursiveRemove(TObject *obj) R__COLLECTION_WRITE_LOCKGUARD(ROOT::gCoreMutex); for (int i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted) && fCont[i]->IsEqual(obj)) { + if (fCont[i] && !ROOT::Detail::HasBeenDeleted(fCont[i]) && fCont[i]->IsEqual(obj)) { fCont[i] = nullptr; // recalculate array size if (i == fLast) @@ -683,7 +683,7 @@ void TObjArray::RecursiveRemove(TObject *obj) fLast--; } while (fLast >= 0 && !fCont[fLast]); Changed(); - } else if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) + } else if (fCont[i] && !ROOT::Detail::HasBeenDeleted(fCont[i])) fCont[i]->RecursiveRemove(obj); } } diff --git a/core/cont/src/TObjectTable.cxx b/core/cont/src/TObjectTable.cxx index ed43ccb393e7a..e78f7f339cf98 100644 --- a/core/cont/src/TObjectTable.cxx +++ b/core/cont/src/TObjectTable.cxx @@ -383,7 +383,7 @@ void TObjectTable::UpdateInstCount() const for (int i = 0; i < fSize; i++) if ((op = fTable[i])) { // attention: no == - if (op->TestBit(TObject::kNotDeleted)) + if (!ROOT::Detail::HasBeenDeleted(op)) op->IsA()->AddInstance(op->IsOnHeap()); else Error("UpdateInstCount", "oops 0x%zx\n", (size_t)op); diff --git a/core/gui/src/TContextMenu.cxx b/core/gui/src/TContextMenu.cxx index dc266ee6d2ab0..cf6f17742ced7 100644 --- a/core/gui/src/TContextMenu.cxx +++ b/core/gui/src/TContextMenu.cxx @@ -126,7 +126,7 @@ void TContextMenu::Action(TClassMenuItem *menuitem) if (object) { // If object deleted, remove from popup and return - if (!(object->TestBit(kNotDeleted))) { + if (ROOT::Detail::HasBeenDeleted(object)) { menuitem->SetType(TClassMenuItem::kPopupSeparator); menuitem->SetCall(nullptr, ""); return; diff --git a/etc/valgrind-root.supp b/etc/valgrind-root.supp index 230362de38cc4..01ea89754559d 100644 --- a/etc/valgrind-root.supp +++ b/etc/valgrind-root.supp @@ -1013,6 +1013,14 @@ fun:_ZN11TBufferFile13ReadStdStringERSs } +######## ROOT use-after-delete detection attempts + +{ + Checking if delete taints memory + Memcheck:Addr4 + fun:_ZN4ROOT8Internal*DeleteChangesMemory* +} + ######## ROOT TObject on heap { diff --git a/graf2d/gpad/src/TButton.cxx b/graf2d/gpad/src/TButton.cxx index c768d018c2d26..d724956d1d386 100644 --- a/graf2d/gpad/src/TButton.cxx +++ b/graf2d/gpad/src/TButton.cxx @@ -149,7 +149,7 @@ void TButton::Draw(Option_t *option) void TButton::ExecuteEvent(Int_t event, Int_t px, Int_t py) { //check case where pressing a button deletes itself - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (IsEditable()) { TPad::ExecuteEvent(event,px,py); @@ -202,7 +202,7 @@ void TButton::ExecuteEvent(Int_t event, Int_t px, Int_t py) gROOT->ProcessLine(GetMethod()); } //check case where pressing a button deletes itself - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; SetBorderMode(1); Modified(); Update(); diff --git a/graf2d/gpad/src/TCanvas.cxx b/graf2d/gpad/src/TCanvas.cxx index 4f976b954a4c6..605e23a3926cf 100644 --- a/graf2d/gpad/src/TCanvas.cxx +++ b/graf2d/gpad/src/TCanvas.cxx @@ -687,7 +687,7 @@ void TCanvas::Destructor() if ((*gThreadXAR)("CDEL", 2, arr, 0)) return; } - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; SafeDelete(fContextMenu); if (!gPad) return; diff --git a/graf2d/gpad/src/TPad.cxx b/graf2d/gpad/src/TPad.cxx index ab8c1846e8e10..4d4db5996c93e 100644 --- a/graf2d/gpad/src/TPad.cxx +++ b/graf2d/gpad/src/TPad.cxx @@ -379,7 +379,7 @@ TPad::TPad(const char *name, const char *title, Double_t xlow, TPad::~TPad() { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; Close(); CloseToolTip(fTip); DeleteToolTip(fTip); @@ -633,7 +633,7 @@ void TPad::Clear(Option_t *option) SafeDelete(fView); if (fPrimitives) fPrimitives->Clear(option); if (fFrame) { - if (fFrame->TestBit(kNotDeleted)) delete fFrame; + if (! ROOT::Detail::HasBeenDeleted(fFrame)) delete fFrame; fFrame = nullptr; } } @@ -973,18 +973,18 @@ Int_t TPad::ClipPolygon(Int_t n, Double_t *x, Double_t *y, Int_t nn, Double_t *x void TPad::Close(Option_t *) { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (!fMother) return; - if (!fMother->TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(fMother)) return; if (fPrimitives) fPrimitives->Clear(); if (fView) { - if (fView->TestBit(kNotDeleted)) delete fView; + if (!ROOT::Detail::HasBeenDeleted(fView)) delete fView; fView = nullptr; } if (fFrame) { - if (fFrame->TestBit(kNotDeleted)) delete fFrame; + if (!ROOT::Detail::HasBeenDeleted(fFrame)) delete fFrame; fFrame = nullptr; } @@ -1274,7 +1274,7 @@ void TPad::Draw(Option_t *option) // pad cannot be in itself and it can only be in one other pad at a time if (!fPrimitives) fPrimitives = new TList; if (gPad != this) { - if (fMother && fMother->TestBit(kNotDeleted)) + if (fMother && !ROOT::Detail::HasBeenDeleted(fMother)) if (fMother->GetListOfPrimitives()) fMother->GetListOfPrimitives()->Remove(this); TPad *oldMother = fMother; fCanvas = gPad->GetCanvas(); @@ -4611,7 +4611,7 @@ TPad *TPad::Pick(Int_t px, Int_t py, TObjLink *&pickobj) void TPad::Pop() { if (!fMother) return; - if (!fMother->TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(fMother)) return; if (!fPrimitives) fPrimitives = new TList; if (this == fMother->GetListOfPrimitives()->Last()) return; diff --git a/graf2d/gpad/src/TRatioPlot.cxx b/graf2d/gpad/src/TRatioPlot.cxx index 25967b2b3f147..1f6b911cc2e72 100644 --- a/graf2d/gpad/src/TRatioPlot.cxx +++ b/graf2d/gpad/src/TRatioPlot.cxx @@ -111,26 +111,31 @@ TRatioPlot::~TRatioPlot() gROOT->GetListOfCleanups()->Remove(this); - if (fRatioGraph && fRatioGraph->TestBit(kNotDeleted)) delete fRatioGraph; - if (fConfidenceInterval1 && fConfidenceInterval1->TestBit(kNotDeleted)) delete fConfidenceInterval1; - if (fConfidenceInterval2 && fConfidenceInterval2->TestBit(kNotDeleted)) delete fConfidenceInterval2; + auto safeDelete = [](TObject *obj) { + if (obj && !ROOT::Detail::HasBeenDeleted(obj)) + delete obj; + }; + + safeDelete(fRatioGraph); + safeDelete(fConfidenceInterval1); + safeDelete(fConfidenceInterval2); for (unsigned int i=0;iTestBit(kNotDeleted)) delete fSharedXAxis; - if (fUpperGXaxis && fUpperGXaxis->TestBit(kNotDeleted)) delete fUpperGXaxis; - if (fLowerGXaxis && fLowerGXaxis->TestBit(kNotDeleted)) delete fLowerGXaxis; - if (fUpperGYaxis && fUpperGYaxis->TestBit(kNotDeleted)) delete fUpperGYaxis; - if (fLowerGYaxis && fLowerGYaxis->TestBit(kNotDeleted)) delete fLowerGYaxis; - if (fUpperGXaxisMirror && fUpperGXaxisMirror->TestBit(kNotDeleted)) delete fUpperGXaxisMirror; - if (fLowerGXaxisMirror && fLowerGXaxisMirror->TestBit(kNotDeleted)) delete fLowerGXaxisMirror; - if (fUpperGYaxisMirror && fUpperGYaxisMirror->TestBit(kNotDeleted)) delete fUpperGYaxisMirror; - if (fLowerGYaxisMirror && fLowerGYaxisMirror->TestBit(kNotDeleted)) delete fLowerGYaxisMirror; + safeDelete(fSharedXAxis); + safeDelete(fUpperGXaxis); + safeDelete(fLowerGXaxis); + safeDelete(fUpperGYaxis); + safeDelete(fLowerGYaxis); + safeDelete(fUpperGXaxisMirror); + safeDelete(fLowerGXaxisMirror); + safeDelete(fUpperGYaxisMirror); + safeDelete(fLowerGYaxisMirror); - if (fUpYaxis && fUpYaxis->TestBit(kNotDeleted)) delete fUpYaxis; - if (fLowYaxis && fLowYaxis->TestBit(kNotDeleted)) delete fLowYaxis; + safeDelete(fUpYaxis); + safeDelete(fLowYaxis); } //////////////////////////////////////////////////////////////////////////////// diff --git a/graf2d/graf/src/TPaveText.cxx b/graf2d/graf/src/TPaveText.cxx index aa9f24e71d2ba..708a7db84bf7a 100644 --- a/graf2d/graf/src/TPaveText.cxx +++ b/graf2d/graf/src/TPaveText.cxx @@ -115,7 +115,7 @@ TPaveText::TPaveText(Double_t x1, Double_t y1,Double_t x2, Double_t y2, Option_ TPaveText::~TPaveText() { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (fLines) fLines->Delete(); delete fLines; fLines = nullptr; diff --git a/graf3d/g3d/src/TGeometry.cxx b/graf3d/g3d/src/TGeometry.cxx index 5355c2b53e76c..b4b02e7adb0e9 100644 --- a/graf3d/g3d/src/TGeometry.cxx +++ b/graf3d/g3d/src/TGeometry.cxx @@ -346,7 +346,7 @@ TNode *TGeometry::GetNode(const char *name) const { TNode *node= (TNode*)GetListOfNodes()->First(); if (!node) return 0; - if (node->TestBit(kNotDeleted)) return node->GetNode(name); + if (!ROOT::Detail::HasBeenDeleted(node)) return node->GetNode(name); return 0; } diff --git a/graf3d/g3d/src/TNode.cxx b/graf3d/g3d/src/TNode.cxx index 92a72dc1f7e91..ace9ebce38aa1 100644 --- a/graf3d/g3d/src/TNode.cxx +++ b/graf3d/g3d/src/TNode.cxx @@ -382,7 +382,7 @@ TNode *TNode::GetNode(const char *name) const TObjLink *lnk = fNodes->FirstLink(); while (lnk) { node = (TNode *)lnk->GetObject(); - if (node->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(node)) { nodefound = node->GetNode(name); if (nodefound) return nodefound; } diff --git a/gui/gui/src/TGFileBrowser.cxx b/gui/gui/src/TGFileBrowser.cxx index 8f049e88e703a..225863f118be4 100644 --- a/gui/gui/src/TGFileBrowser.cxx +++ b/gui/gui/src/TGFileBrowser.cxx @@ -656,7 +656,7 @@ void TGFileBrowser::Update() TGListTreeItem *curr = fListTree->GetSelected(); // GetCurrent() ?? if (curr) { TObject *obj = (TObject *) curr->GetUserData(); - if (obj && !obj->TestBit(kNotDeleted)) { + if (obj && ROOT::Detail::HasBeenDeleted(obj)) { // if the item to be deleted has a filter, // delete its entry in the map if (CheckFiltered(curr)) @@ -665,7 +665,7 @@ void TGFileBrowser::Update() curr = 0; obj = 0; } - else if (obj && obj->TestBit(kNotDeleted) && + else if (obj && !ROOT::Detail::HasBeenDeleted(obj) && obj->InheritsFrom("TObjString") && curr->GetParent()) { fListTree->GetPathnameFromItem(curr->GetParent(), path); if (strlen(path) > 1) { diff --git a/gui/gui/src/TGFrame.cxx b/gui/gui/src/TGFrame.cxx index dadbb6d2bb2b4..1ad1940f3c6b6 100644 --- a/gui/gui/src/TGFrame.cxx +++ b/gui/gui/src/TGFrame.cxx @@ -1731,7 +1731,7 @@ Bool_t TGMainFrame::HandleClientMessage(Event_t *event) if ((event->fFormat == 32) && ((Atom_t)event->fUser[0] == gWM_DELETE_WINDOW) && (event->fHandle != gROOT_MESSAGE)) { Emit("CloseWindow()"); - if (TestBit(kNotDeleted) && !TestBit(kDontCallClose)) + if (!ROOT::Detail::HasBeenDeleted(this) && !TestBit(kDontCallClose)) CloseWindow(); } return kTRUE; diff --git a/gui/gui/src/TGMdiMainFrame.cxx b/gui/gui/src/TGMdiMainFrame.cxx index b6609603f30e4..96a6bc669846e 100644 --- a/gui/gui/src/TGMdiMainFrame.cxx +++ b/gui/gui/src/TGMdiMainFrame.cxx @@ -963,7 +963,7 @@ Int_t TGMdiMainFrame::Close(TGMdiFrame *mdiframe) TGMdiDecorFrame *frame = GetDecorFrame(mdiframe); Restore(mdiframe); mdiframe->Emit("CloseWindow()"); - if (frame && mdiframe->TestBit(kNotDeleted) && !mdiframe->TestBit(TGMdiFrame::kDontCallClose)) + if (frame && !ROOT::Detail::HasBeenDeleted(mdiframe) && !mdiframe->TestBit(TGMdiFrame::kDontCallClose)) return frame->CloseWindow(); return kTRUE; } diff --git a/gui/gui/src/TGTextEntry.cxx b/gui/gui/src/TGTextEntry.cxx index c71f713cd73eb..f51c6cd419575 100644 --- a/gui/gui/src/TGTextEntry.cxx +++ b/gui/gui/src/TGTextEntry.cxx @@ -1229,7 +1229,7 @@ Bool_t TGTextEntry::HandleKey(Event_t* event) if ((EKeySym)keysym == kKey_Enter || (EKeySym)keysym == kKey_Return) { ReturnPressed(); // emit signal - if (!TestBit(kNotDeleted)) return kTRUE; + if (ROOT::Detail::HasBeenDeleted(this)) return kTRUE; fSelectionOn = kFALSE; } else if (event->fState & kKeyShiftMask && (EKeySym)keysym == kKey_Backtab) { diff --git a/hist/hist/src/TEfficiency.cxx b/hist/hist/src/TEfficiency.cxx index e0132c8bab955..2a7002de3d69e 100644 --- a/hist/hist/src/TEfficiency.cxx +++ b/hist/hist/src/TEfficiency.cxx @@ -1033,7 +1033,7 @@ TEfficiency::~TEfficiency() TObject* obj = 0; while ((obj = fFunctions->First())) { while(fFunctions->Remove(obj)) { } - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { break; } delete obj; diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index 8257ceccd9798..dfa759d744ba2 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -623,7 +623,7 @@ TH1::TH1(): TNamed(), TAttLine(), TAttFill(), TAttMarker() TH1::~TH1() { - if (!TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(this)) { return; } delete[] fIntegral; @@ -644,7 +644,7 @@ TH1::~TH1() //and may have been already deleted. while ((obj = fFunctions->First())) { while(fFunctions->Remove(obj)) { } - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { break; } delete obj; diff --git a/net/net/src/TApplicationServer.cxx b/net/net/src/TApplicationServer.cxx index ea59b3fc80079..bcd8c9693c105 100644 --- a/net/net/src/TApplicationServer.cxx +++ b/net/net/src/TApplicationServer.cxx @@ -896,7 +896,7 @@ Int_t TApplicationServer::SendCanvases() while (lnk) { TObject *sc = lnk->GetObject(); lnk = lnk->Next(); - if ((sc->TestBit(kNotDeleted)) && sc == o) + if ((!ROOT::Detail::HasBeenDeleted(sc)) && sc == o) sentalready = kTRUE; } if (!sentalready) { diff --git a/tutorials/net/spyserv.C b/tutorials/net/spyserv.C index 53e8a2ea68eb6..587f1f41a4757 100644 --- a/tutorials/net/spyserv.C +++ b/tutorials/net/spyserv.C @@ -133,7 +133,7 @@ SpyServ::SpyServ() TSocket *s; if ((s = fMon->Select(20)) != (TSocket*)-1) HandleSocket(s); - if (!fCanvas->TestBit(TObject::kNotDeleted)) + if (ROOT::Detail::HasBeenDeleted(fCanvas)) break; if (gROOT->IsInterrupted()) break;