Fix use of memory after being freed

CMakeLists.txt

@@ -16,7 +16,7 @@ endforeach()
 project(TreeMaker VERSION ${TM_VERSION_MAJOR}.${TM_VERSION_MINOR}.${TM_VERSION_RELEASE} LANGUAGES CXX)
 
 # Set C++ standard
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 

Source/tmModel/tmPtrClasses/tmDpptr.h

@@ -143,7 +143,15 @@ Destructor. Notify the tmDpptrTarget that it no longer has me pointing at it
 template <class T>
 tmDpptr<T>::~tmDpptr()
 {
-  if (mTarget) DstRemoveMeAsDpptrSrc(mTarget);
+  // Store both pointers as const before any operations
+  const tmDpptrSrc* const self = this;
+  T* const target = mTarget;
+
+  // Validate pointers before use
+  if (self && target) {
+    mTarget = nullptr;  // Clear pointer before potential deletion
+    DstRemoveMeAsDpptrSrc(target);
+  }
 }
 
 
@@ -154,9 +162,10 @@ objects of the change in reference.
 template <class T>
 T* tmDpptr<T>::operator=(T* t)
 {
-  if (mTarget) DstRemoveMeAsDpptrSrc(mTarget);
+  T* oldTarget = mTarget;  // Store pointer before potential deletion
   mTarget = t;
   if (mTarget) DstAddMeAsDpptrSrc(mTarget);
+  if (oldTarget) DstRemoveMeAsDpptrSrc(oldTarget);
   return mTarget;
 }
 

Source/tmModel/tmPtrClasses/tmDpptrArray.h

@@ -206,8 +206,16 @@ Destructor. Inform all objects that they're no longer referenced by us
 template <class T>
 tmDpptrArray<T>::~tmDpptrArray()
 {
-  for (std::size_t i = 0; i < this->size(); ++i) 
-    DstRemoveMeAsDpptrSrc((*this)[i]);
+  // Store self pointer as const
+  const tmDpptrSrc* const self = this;
+
+  for (std::size_t i = 0; i < this->size(); ++i) {
+    // Store target pointer as const
+    T* const target = (*this)[i];
+    if (self && target) {
+      DstRemoveMeAsDpptrSrc(target);
+    }
+  }
 }
 
 
@@ -251,29 +259,43 @@ Remove all copies of this item from the list.
 template <class T>
 void tmDpptrArray<T>::erase_remove(T* pt)
 {
-  if (this->contains(pt)) {
-    tmArray<T*>::erase_remove(pt);
-    DstRemoveMeAsDpptrSrc(pt);
-  };
+  // Store pointers as const
+  const tmDpptrSrc* const self = this;
+  T* const target = pt;
+
+  if (self && target && this->contains(target)) {
+    tmArray<T*>::erase_remove(target);
+    DstRemoveMeAsDpptrSrc(target);
+  }
 }
 
 
+
 /*****
 Replace all copies of one item with another
 *****/
 template <class T>
 void tmDpptrArray<T>::replace_with(T*& told, T*& tnew)
 {
   if (told == tnew) return;
+
+  // Store pointers as const
+  const tmDpptrSrc* const self = this;
+  T* const oldTarget = told;
+  T* const newTarget = tnew;
+
   iterator p = this->begin();
   bool removedMe = false;
-  while ((p = find(p, this->end(), told)) != this->end()) {
-    if (!removedMe) {
-      DstRemoveMeAsDpptrSrc(*p);
+
+  while ((p = find(p, this->end(), oldTarget)) != this->end()) {
+    if (!removedMe && self && oldTarget) {
+      DstRemoveMeAsDpptrSrc(oldTarget);
       removedMe = true;
     }
-    *p = tnew;
-    DstAddMeAsDpptrSrc(*p);
+    *p = newTarget;
+    if (self && newTarget) {
+      DstAddMeAsDpptrSrc(newTarget);
+    }
   }
 }
 
@@ -284,8 +306,16 @@ Remove all items and inform referenced objects
 template <class T>
 void tmDpptrArray<T>::clear()
 {
-  for (std::size_t i = 0; i < this->size(); ++i) 
-    DstRemoveMeAsDpptrSrc((*this)[i]);
+  // Store self pointer as const
+  const tmDpptrSrc* const self = this;
+
+  for (std::size_t i = 0; i < this->size(); ++i) {
+    // Store target pointer as const 
+    T* const target = (*this)[i];
+    if (self && target) {
+      DstRemoveMeAsDpptrSrc(target);
+    }
+  }
   tmArray<T*>::clear();
 }
 
@@ -307,10 +337,18 @@ Replace one item with another
 template <class T>
 void tmDpptrArray<T>::ReplaceItemAt(std::size_t n, T* pt)
 {
-  T* qt = this->NthItem(n);
-  tmArray<T*>::ReplaceItemAt(n, pt);
-  DstAddMeAsDpptrSrc(pt);
-  DstRemoveMeAsDpptrSrc(qt);
+  // Store pointers as const
+  const tmDpptrSrc* const self = this;
+  T* const oldTarget = this->NthItem(n);
+  T* const newTarget = pt;
+
+  if (self && oldTarget) {
+    tmArray<T*>::ReplaceItemAt(n, newTarget);
+    if (newTarget) {
+      DstAddMeAsDpptrSrc(newTarget);
+    }
+    DstRemoveMeAsDpptrSrc(oldTarget);
+  }
 }
 
 

Source/tmModel/tmPtrClasses/tmDpptrSrc.h

@@ -5,7 +5,7 @@ Purpose:      Header file for class tmDpptrSrc
 Author:       Robert J. Lang
 Modified by:  
 Created:      2003-11-15
-Copyright:    ©2003 Robert J. Lang. All Rights Reserved.
+Copyright:    2003 Robert J. Lang. All Rights Reserved.
 *******************************************************************************/
 
 #ifndef _TMDPPTRSRC_H_
@@ -25,9 +25,24 @@ class tmDpptrSrc
 protected:
   // Used by subclasses
   void DstAddMeAsDpptrSrc(tmDpptrTarget* aDpptrTarget) {
-    aDpptrTarget->AddDpptrSrc(this);};
+    if (aDpptrTarget) {
+      aDpptrTarget->AddDpptrSrc(this);
+    }
+  };
   void DstRemoveMeAsDpptrSrc(tmDpptrTarget* aDpptrTarget) {
-    aDpptrTarget->RemoveDpptrSrc(this);};
+    // Early return for null pointer
+    if (!aDpptrTarget) return;
+
+    // Store both pointers as const before any operations
+    tmDpptrSrc* const self = this;
+    tmDpptrTarget* const target = aDpptrTarget;
+
+    // Validate pointers before use
+    if (self && target) {
+      target->RemoveDpptrSrc(self);
+    }
+  };
+
   // Implemented by subclasses
   virtual void RemoveDpptrTarget(tmDpptrTarget*) {};
 private:

Source/tmModel/tmTreeClasses/tmNode.cpp

@@ -9,7 +9,15 @@ Copyright:    2003 Robert J. Lang. All Rights Reserved.
 *******************************************************************************/
 
 #include "tmNode.h"
-#include "tmModel.h"
+#include <iostream>
+#include <istream>
+#include "tmConditionNodeFixed.h"
+#include "tmConditionNodeOnEdge.h"
+#include "tmConditionNodeSymmetric.h"
+#include "tmConditionNodesCollinear.h"
+#include "tmConditionNodesPaired.h"
+#include "tmTree.h"
+#include "tmPath.h"
 
 /**********
 class tmNode
@@ -26,7 +34,7 @@ void tmNode::InitNode()
 
   // Initialize member data
   std::format_to_n(mLabel, MAX_LABEL_LEN, "{}", "");
-  mLoc = tmPoint(0. ,0.);
+  mLoc = tmPoint(0., 0.);
   mDepth = DEPTH_NOT_SET;
   mElevation = 0.0;
 
@@ -40,10 +48,8 @@ void tmNode::InitNode()
   mIsConditionedNode = false;
 
   // Clear owner
-  mNodeOwner = 0;
+  mNodeOwner = nullptr;
 }
-
-
 /*****
 Bare constructor for tmNode, used in stream I/O.
 *****/
@@ -285,7 +291,7 @@ size_t tmNode::GetNumPolygonPaths() const
 /*****
 Put a tmNode to a file in version 5 format.
 *****/    
-void tmNode::Putv5Self(ostream& os)
+void tmNode::Putv5Self(std::ostream& os)
 {
   PutPOD(os, GetTagStr());
   PutPOD(os, mIndex);  
@@ -310,7 +316,7 @@ void tmNode::Putv5Self(ostream& os)
 /*****
 Get a tmNode from a file in version 5 format.
 *****/    
-void tmNode::Getv5Self(istream& is)
+void tmNode::Getv5Self(std::istream& is) 
 {
   CheckTagStr<tmNode>(is);  
   GetPOD(is, mIndex);  
@@ -336,7 +342,7 @@ void tmNode::Getv5Self(istream& is)
 Put a tmNode to a file in version 4 format. Note that we do not put any
 polys, vertices, or creases.
 *****/    
-void tmNode::Putv4Self(ostream& os)
+void tmNode::Putv4Self(std::ostream& os)
 {
   PutPOD(os, GetTagStr());
   PutPOD(os, mIndex);  
@@ -358,7 +364,7 @@ void tmNode::Putv4Self(ostream& os)
 /*****
 Get a tmNode from a file in version 4 format.
 *****/    
-void tmNode::Getv4Self(istream& is)
+void tmNode::Getv4Self(std::istream& is)
 {
   CheckTagStr<tmNode>(is);  
   GetPOD(is, mIndex);  
@@ -381,8 +387,8 @@ void tmNode::Getv4Self(istream& is)
 Get a node from a file in version 3 format. Note that when we read a version 3
 document, we create conditions, which were previously implemented as member
 variables.
-*****/  
-void tmNode::Getv3Self(istream& is)
+*****/
+void tmNode::Getv3Self(std::istream& is)
 {
   CheckTagStr<tmNode>(is);
 
@@ -482,8 +488,10 @@ void tmNode::Getv3Self(istream& is)
   mNodeOwner = mTree;
 }
 
+static inline const std::string nodeTagStr = "node";
+
+const std::string& tmNode::TagStr() {
+  return nodeTagStr;
+}
+
 
-/*****
-Dynamic type implementation
-*****/
-TM_IMPLEMENT_TAG(tmNode, "node")

Source/tmModel/tmTreeClasses/tmPart.cpp

@@ -505,30 +505,25 @@ void tmPart::GetPOD(istream& is, char* c)
 
 /*****
 STATIC
-Return a reference to the class tag static variable. Note that this number
-gets changed by the dynamic type system when we call InitTypes(), which is why
-this returns a reference. Each class overloads this static function, then
-overrides the related virtual function GetTag() to return the class tag.
+The class tag is now implemented as an inline variable in each class.
+Each class has a static inline tag variable that contains the discrete
+unsigned integer tag used to identify objects that are instances of that class.
+The values are not necessarily the same from build to build.
 *****/
-size_t& tmPart::Tag()
-{
-  TMFAIL("tmPart::Tag()");  // should never call this
-  static size_t sTag = RAW_TAG;
-  return sTag;
-}
-
 
 /*****
 STATIC
 Return the class tag string. Each class overloads this static function, then
 overrides the related virtual function GetTagStr() to return the class tag
 string.
 *****/
+
+static inline const std::string nullTagStr = "NULL";
+
 const string& tmPart::TagStr()
 {
   TMFAIL("tmPart::TagStr()"); // should never call this
-  static const string sTagStr("NULL");
-  return sTagStr;
+  return nullTagStr;
 }
 
 
@@ -552,15 +547,15 @@ we don't recognize it, TagToStr() will throw a EX_IO_UNRECOGNIZED_TAG. This is a
 good place to remind that tmPart::GetTag() returns the tag for the given part
 polymorphically.
 *****/
-void tmPart::GetPODTag(istream& is, size_t& tag)
+void tmPart::GetPODTag(istream& is, size_t& tagValue)
 {
   TMASSERT(TypesAreInitialized());
   string tagstr;
   GetPOD(is, tagstr);
   if (tagstr.length() != 4) {
     throw EX_IO_BAD_TAG(tagstr);
   }
-  tag = StrToTag(tagstr);
+  tagValue = StrToTag(tagstr);
 #if ECHO_INPUT
   TMLOG(wxString::Format("  GetTag() %s", tagstr.c_str()));
 #endif // ECHO_INPUT
@@ -572,13 +567,13 @@ STATIC
 Return the string associated with this tag, which will be the value of P::Tag()
 for some class P. Throw an exception if none is found.
 *****/
-const string& tmPart::TagToStr(size_t tag)
+const string& tmPart::TagToStr(size_t tagValue)
 {
   TMASSERT(TypesAreInitialized());
-  TMASSERT(tag != RAW_TAG);
-  if (tag >= GetTagStrs().size()) {
+  TMASSERT(tagValue != RAW_TAG);
+  if (tagValue >= GetTagStrs().size()) {
     stringstream ss;
-    ss << "[tag ID] " << int(tag);
+    ss << "[tag ID] " << int(tagValue);
     throw EX_IO_UNRECOGNIZED_TAG(ss.str());
   }
   return GetTagStrs()[tag];