0024931: Stack overflow when writing large shapes to XML

Added QA command OCC24931 and test case bugs/fclasses/bug24931

src/LDOM/LDOM_OSStream.cxx

@@ -14,28 +14,31 @@
 // commercial license or contractual agreement.
 
 #include <LDOM_OSStream.hxx>
+#include <NCollection_DefineAlloc.hxx>
+#include <NCollection_IncAllocator.hxx>
 #include <string.h>
+#include <Standard.hxx>
 #include <Standard_Integer.hxx>
 
 // One element of sequence
+/* Can only be allocated by the allocator and assumes it is IncAllocator, so 
+   no destructor is required.
+*/
 class LDOM_StringElem
 {
   char* buf;             // pointer on data string
   int len;               // quantity of really written data
   LDOM_StringElem* next; // pointer on the next element of a sequence
 
-  LDOM_StringElem (int aLen)
+  DEFINE_NCOLLECTION_ALLOC
+  
+  LDOM_StringElem (int aLen, const Handle_NCollection_BaseAllocator& theAlloc) :
+    buf (reinterpret_cast<char*> (theAlloc->Allocate (aLen))),
+    len (0),
+    next (0)
   {
-    buf = new char[aLen];
-    len = 0;
-    next = 0;
   }
 
-  ~LDOM_StringElem ()
-  {
-    delete [] buf;
-    if (next) delete next;
-  }
 friend class LDOM_SBuffer;
 };
 
@@ -44,10 +47,11 @@ friend class LDOM_SBuffer;
 //purpose  : 
 //=======================================================================
 LDOM_SBuffer::LDOM_SBuffer (const Standard_Integer theMaxBuf)
-     : myMaxBuf (theMaxBuf), myLength(0)
+     : myMaxBuf (theMaxBuf), myLength(0),
+       myAlloc (new NCollection_IncAllocator)
 {
-  myFirstString = new LDOM_StringElem (theMaxBuf);
+  myFirstString = new (myAlloc) LDOM_StringElem (theMaxBuf, myAlloc);
-  myCurString = myFirstString;
+  myCurString   = myFirstString;
 }
 
 //=======================================================================
@@ -56,7 +60,7 @@ LDOM_SBuffer::LDOM_SBuffer (const Standard_Integer theMaxBuf)
 //=======================================================================
 LDOM_SBuffer::~LDOM_SBuffer ()
 {
-  if (myFirstString) delete myFirstString;
+  //no destruction is required as IncAllocator is used
 }
 
 //=======================================================================
@@ -65,11 +69,10 @@ LDOM_SBuffer::~LDOM_SBuffer ()
 //=======================================================================
 void LDOM_SBuffer::Clear ()
 {
-  if (myFirstString->next) delete myFirstString->next;
+  myAlloc       = new NCollection_IncAllocator;
-  myFirstString->next = 0;
+  myFirstString = new (myAlloc) LDOM_StringElem (myMaxBuf, myAlloc);
-  myFirstString->len = 0;
+  myLength      = 0;
-  myLength = 0;
+  myCurString   = myFirstString;
-  myCurString = myFirstString;
 }
 
 //=======================================================================
@@ -130,7 +133,7 @@ int LDOM_SBuffer::xsputn(const char* aStr, int n)
   }
   else if (freeLen <= 0)
   {
-    LDOM_StringElem* aNextElem = new LDOM_StringElem(Max(aLen, myMaxBuf));
+    LDOM_StringElem* aNextElem = new (myAlloc) LDOM_StringElem(Max(aLen, myMaxBuf), myAlloc);
     myCurString->next = aNextElem;
     myCurString = aNextElem;
     strncpy(myCurString->buf + myCurString->len, aStr, aLen);
@@ -142,7 +145,7 @@ int LDOM_SBuffer::xsputn(const char* aStr, int n)
     myCurString->len += freeLen;
     *(myCurString->buf + myCurString->len) = '\0';
     aLen -= freeLen;
-    LDOM_StringElem* aNextElem = new LDOM_StringElem(Max(aLen, myMaxBuf));
+    LDOM_StringElem* aNextElem = new (myAlloc) LDOM_StringElem(Max(aLen, myMaxBuf), myAlloc);
     myCurString->next = aNextElem;
     myCurString = aNextElem;
     strncpy(myCurString->buf + myCurString->len, aStr + freeLen, aLen);

src/LDOM/LDOM_OSStream.hxx

@@ -32,6 +32,7 @@
 //          and current element of sequence,
 //          also it has methods for the sequence management.
 
+#include <NCollection_BaseAllocator.hxx>
 #include <Standard_OStream.hxx>
 #include <Standard_Boolean.hxx>
 
@@ -74,10 +75,12 @@ class LDOM_SBuffer : public streambuf
   // Destructor
 
  private:
+ 
   Standard_Integer      myMaxBuf; // default length of one element
   Standard_Integer      myLength; // full length of contained data
   LDOM_StringElem* myFirstString; // the head of the sequence
   LDOM_StringElem* myCurString;   // current element of the sequence
+  Handle(NCollection_BaseAllocator) myAlloc; //allocator for chunks
 };
 
 class LDOM_OSStream : public Standard_OStream

src/QABugs/QABugs_19.cxx

@@ -2078,6 +2078,65 @@ static Standard_Integer OCC24565 (Draw_Interpretor& di, Standard_Integer argc, c
   return 0;
 }
 
+#include <Handle_BRepTools_NurbsConvertModification.hxx>
+#include <BRepPrimAPI_MakeCylinder.hxx>
+#include <BRepBuilderAPI_Copy.hxx>
+#include <BRepTools_NurbsConvertModification.hxx>
+static TopoDS_Shape CreateTestShape (int& theShapeNb)
+{
+  TopoDS_Compound aComp;
+  BRep_Builder aBuilder;
+  aBuilder.MakeCompound (aComp);
+  //NURBS modifier is used to increase footprint of each shape
+  Handle_BRepTools_NurbsConvertModification aNurbsModif = new BRepTools_NurbsConvertModification;
+  TopoDS_Shape aRefShape = BRepPrimAPI_MakeCylinder (50., 100.).Solid();
+  BRepTools_Modifier aModifier (aRefShape, aNurbsModif);
+  if (aModifier.IsDone()) {
+    aRefShape = aModifier.ModifiedShape (aRefShape);
+  }
+  int aSiblingNb = 0;
+  for (; theShapeNb > 0; --theShapeNb) {
+    TopoDS_Shape aShape;
+    if (++aSiblingNb <= 100) { //number of siblings is limited to avoid long lists
+		aShape = BRepBuilderAPI_Copy (aRefShape, Standard_True /*CopyGeom*/).Shape();
+    } else {
+      aShape = CreateTestShape (theShapeNb);
+    }
+    aBuilder.Add (aComp, aShape);
+  }
+  return aComp;
+}
+
+#include <AppStd_Application.hxx>
+#include <TDataStd_Integer.hxx>
+#include <TNaming_Builder.hxx>
+static Standard_Integer OCC24931 (Draw_Interpretor& di, Standard_Integer argc, const char** argv)
+{
+  if (argc != 1) {
+    di << "Usage: " << argv[0] << " invalid number of arguments"<<"\n";
+    return 1;
+  }
+  TCollection_ExtendedString aFileName ("testdocument.xml");
+  PCDM_StoreStatus aSStatus  = PCDM_SS_Failure;
+
+  Handle(TDocStd_Application) anApp = new AppStd_Application;
+  {
+    Handle(TDocStd_Document) aDoc;
+    anApp->NewDocument ("XmlOcaf", aDoc);
+    TDF_Label aLab = aDoc->Main();
+    TDataStd_Integer::Set (aLab, 0);
+    int n = 10000; //must be big enough
+    TopoDS_Shape aShape = CreateTestShape (n);
+    TNaming_Builder aBuilder (aLab);
+    aBuilder.Generated (aShape);
+
+    aSStatus = anApp->SaveAs (aDoc, aFileName);
+    anApp->Close (aDoc);
+  }
+  QCOMPARE (aSStatus, PCDM_SS_OK);
+  return 0;
+}
+
 #include <AppStdL_Application.hxx>
 #include <TDocStd_Application.hxx>
 #include <TDataStd_Integer.hxx>
@@ -2218,5 +2277,6 @@ void QABugs::Commands_19(Draw_Interpretor& theCommands) {
   theCommands.Add ("OCC24565", "OCC24565 FileNameIGS FileNameSTOR", __FILE__, OCC24565, group);
   theCommands.Add ("OCC24755", "OCC24755", __FILE__, OCC24755, group);
   theCommands.Add ("OCC24834", "OCC24834", __FILE__, OCC24834, group);
+  theCommands.Add ("OCC24931", "OCC24931", __FILE__, OCC24931, group);
   return;
 }

src/XmlMNaming/XmlMNaming_NamedShapeDriver.cxx

@@ -370,7 +370,7 @@ void XmlMNaming_NamedShapeDriver::WriteShapeSection
   //  Add text to the "shapes" element
   if (myShapeSet.NbShapes() > 0) {
     myShapeSet.SetFormatNb(2);
-    LDOM_OSStream aStream (1024);
+    LDOM_OSStream aStream (16 * 1024);
 //    ostrstream aStream;
 //    aStream.rdbuf() -> setbuf (0, 16380);
     myShapeSet.Write (aStream);

tests/bugs/fclasses/bug24931

@@ -0,0 +1,17 @@
+puts "=========="
+puts "OCC24931"
+puts "=========="
+puts ""
+####################################################
+# Stack overflow when writing large shapes to XML
+####################################################
+
+pload QAcommands
+
+set info [OCC24931]
+
+if { [regexp "OK" $info] != 1 } {
+  puts "Error: Stack is overflow"
+} else {
+  puts "OK: Stack is good"
+}