Re: [Texmacs-dev] [PATCH] solves GCC 3.2 auto_save segfault for me...

[Igor V. Kovalenko]

Igor V. Kovalenko wrote:
> Joris van der Hoeven wrote:
>
> > But now I am again confused, because the old routine
> > destroy_tree_rep should precisely do the same thing
> > as the virtual destructor. Apparently, a string is sometimes
> > not a string or a compound not a compound...
>
>
> Indeed, this is rather clever idea :)
>
> I'm currently investigating the effect of GCC/c++ PR 8287
> "GCC3.2: Destructor called for non-constructed local object"
>
> I'll follow up on this shortly.

Well, It takes time to rebuild GCC :)

The file attached contains the verification code. It does introduce an
extra unsigned long field to tree and string classes. Their constructors
do initialize this field to magic value 0xF9CC8287LU and destructors
check for this value before proceeding. If the object is indeed constructed
the value is set to 0xDEADBEEFLU.

A test run is as follows. Compile with GCC-3.2 using all default optimizations.
Then execute and press F1. Try to select some large region of text.

You should see:

--------------------------------------
.....
TeXmacs] Loading ecbx7 at 600 dpi
TeXmacs] Loading rtcxr10 at 600 dpi
TeXmacs] Loading rtcxr0.tfm
TeXmacs] Loading rtcxr10.tfm
TeXmacs] Loading rtcxr10.600pk
TeXmacs] Loading ecrm8 at 600 dpi
TeXmacs] Loading ecrm6 at 600 dpi
TeXmacs] Loading cmr6 at 600 dpi
TeXmacs] Running ghostscript /usr/share/TeXmacs-1.0.0.21/misc/images/tm_gnu1.ps
TeXmacs] Running ghostscript /usr/share/TeXmacs-1.0.0.21/misc/images/tm_gnu2.ps
dtor_tree magic: deadbeef
[aborted, core dumped]
--------------------------------------

If you don't see this, you should wait for auto-save and repeat F1 and 
selection action.

Now I'm in a little confusion :)

There are at least two possibilities:
   1. TeXmacs is affected by GCC/c++ PR 8287. I wrote this test exactly to show 
this :)
   2. There does exist some double-deletion of tree object.

The point of confusion is that I rebuilt TeXmacs, GCC-3.2 (20021021) and 
libstdc++ using
a patch described in PR 8287 FIX 
(http://gcc.gnu.org/ml/gcc-patches/2002-10/msg01817.html)
and still I'm able to reproduce the problem.

Anyway, there was a fix re: PR 8287 implemented in GCC-3.2 CVS on 20021029.
It's a pity some major distributions shipped a compiler with such a bug.
It's even described as 'lethal' within PR 8287 :) I agree, this usage pattern 
is quite common.

I'm now rebuilding with GCC-3.2 snapshot 20021104 (the latest available) and
will come up with the result.

--
Regards,
Igor V. Kovalenko    mailto: iko at crec dot mipt dot ru

diff -udBbr TeXmacs-1.0.0.21-src-orig/src/Basic/Data/tree.cc 
TeXmacs-1.0.0.21-src/src/Basic/Data/tree.cc
--- TeXmacs-1.0.0.21-src-orig/src/Basic/Data/tree.cc    2002-11-05 
19:26:37.000000000 +0300
+++ TeXmacs-1.0.0.21-src/src/Basic/Data/tree.cc 2002-11-08 18:13:16.000000000 
+0300
@@ -17,32 +17,53 @@
 ******************************************************************************/
 
 void
-destroy_tree_rep (tree_rep* rep) {
+destroy_tree_rep (tree_rep* rep)
+{
+/*
+    if (((concrete_struct*)rep)->magic_check_number != CTOR_MAGIC_NUMBER) {
+        cout << "rep magic mismatch, check="
+             << ((concrete_struct*)rep)->magic_check_number
+             << std::endl;
+    }
+*/
+    
   if (rep->op == STRING) delete ((atomic_rep*) ((void*) rep));
   else delete ((compound_rep*) ((void*) rep));
-  rep= NULL;
+//  rep= NULL;
 }
 
 tree::tree (tree_label l, tree t1):
-  rep (new compound_rep (l, array<tree> (1))) {
+  rep (new compound_rep (l, array<tree> (1)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
 }
 
 tree::tree (tree_label l, tree t1, tree t2):
-  rep (new compound_rep (l, array<tree> (2))) {
+  rep (new compound_rep (l, array<tree> (2)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
   ((compound_rep*) rep)->a[1]=t2;
 }
 
 tree::tree (tree_label l, tree t1, tree t2, tree t3):
-  rep (new compound_rep (l, array<tree> (3))) {
+  rep (new compound_rep (l, array<tree> (3)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
   ((compound_rep*) rep)->a[1]=t2;
   ((compound_rep*) rep)->a[2]=t3;
 }
 
 tree::tree (tree_label l, tree t1, tree t2, tree t3, tree t4):
-  rep (new compound_rep (l, array<tree> (4))) {
+  rep (new compound_rep (l, array<tree> (4)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
   ((compound_rep*) rep)->a[1]=t2;
   ((compound_rep*) rep)->a[2]=t3;
@@ -50,7 +71,10 @@
 }
 
 tree::tree (tree_label l, tree t1, tree t2, tree t3, tree t4, tree t5):
-  rep (new compound_rep (l, array<tree> (5))) {
+  rep (new compound_rep (l, array<tree> (5)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
   ((compound_rep*) rep)->a[1]=t2;
   ((compound_rep*) rep)->a[2]=t3;
@@ -62,6 +86,8 @@
            tree t1, tree t2, tree t3, tree t4, tree t5, tree t6):
   rep (new compound_rep (l, array<tree> (6)))
 {
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
   ((compound_rep*) rep)->a[0]=t1;
   ((compound_rep*) rep)->a[1]=t2;
   ((compound_rep*) rep)->a[2]=t3;
diff -udBbr TeXmacs-1.0.0.21-src-orig/src/Basic/Data/tree.hh 
TeXmacs-1.0.0.21-src/src/Basic/Data/tree.hh
--- TeXmacs-1.0.0.21-src-orig/src/Basic/Data/tree.hh    2002-11-05 
19:25:58.000000000 +0300
+++ TeXmacs-1.0.0.21-src/src/Basic/Data/tree.hh 2002-11-08 18:36:23.000000000 
+0300
@@ -69,6 +69,7 @@
 class compound_rep;
 class tree {
   tree_rep* rep; // can be atomic or compound
+  unsigned long magic_check_number;
 public:
   static const tree_label init = UNINIT; // used by hashmap<tree>() constructor
 
@@ -114,8 +115,12 @@
 
 class tree_rep: concrete_struct {
 public:
-  tree_label op;
+  const tree_label op;
   inline tree_rep (tree_label op2): op (op2) {}
+    inline ~tree_rep() 
+    {
+    }
+    
   friend class tree;
 };
 
@@ -154,29 +159,70 @@
 #endif
 
 void destroy_tree_rep (tree_rep* rep);
-inline tree::tree (const tree& x): rep (x.rep) { rep->ref_count++; }
-inline tree::~tree () {
-  if ((--rep->ref_count)==0) destroy_tree_rep (rep); }
+inline tree::tree (const tree& x): rep (x.rep)
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    
+    rep->ref_count++;
+}
+inline tree::~tree ()
+{
+    if (magic_check_number != CTOR_MAGIC_NUMBER) {
+        
+        cout << "dtor_tree magic: "
+             << std::hex << magic_check_number << std::dec
+             << std::endl;
+        abort();
+    }
+    
+    magic_check_number = DTOR_MAGIC_NUMBER;
+    
+  if ((--rep->ref_count)==0) {
+      destroy_tree_rep (rep);
+//      rep = NULL;
+      rep = (tree_rep*)0xbadf00d1;
+  }
+}
 inline atomic_rep* tree::operator -> () {
   CHECK_ATOMIC (*this, "tree::operator ->");
   return (atomic_rep*) rep; }
 inline tree& tree::operator = (tree x) {
   x.rep->ref_count++;
-  if ((--rep->ref_count)==0) destroy_tree_rep (rep);
+  if ((--rep->ref_count)==0) {
+      destroy_tree_rep (rep);
+//      rep = NULL;
+      rep = (tree_rep*)0xbadf00d2;
+  }
+  
   rep= x.rep;
   return *this; }
 
 inline tree::tree ():
-  rep (new atomic_rep (string ())) {}
+  rep (new atomic_rep (string ()))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+}
 inline tree::tree (char *s):
-  rep (new atomic_rep (s)) {}
+  rep (new atomic_rep (s))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+}
 inline tree::tree (string s):
-  rep (new atomic_rep (s)) {}
+  rep (new atomic_rep (s))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+}
 inline tree::tree (tree_label l, int n):
-  rep (new compound_rep (l, array<tree> (n))) {}
+  rep (new compound_rep (l, array<tree> (n)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+}
 inline tree::tree (tree t, int n):
-  rep (new compound_rep (t.rep->op, array<tree> (n))) {
-    CHECK_COMPOUND (t, "tree::tree (tree, int)"); }
+  rep (new compound_rep (t.rep->op, array<tree> (n)))
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    CHECK_COMPOUND (t, "tree::tree (tree, int)");
+}
 inline tree& tree::operator [] (int i) {
   CHECK_COMPOUND (*this, "tree::operator []");
   return ((compound_rep*) rep)->a[i]; }
diff -udBbr TeXmacs-1.0.0.21-src-orig/src/Basic/Types/basic.hh 
TeXmacs-1.0.0.21-src/src/Basic/Types/basic.hh
--- TeXmacs-1.0.0.21-src-orig/src/Basic/Types/basic.hh  2002-11-05 
18:15:24.000000000 +0300
+++ TeXmacs-1.0.0.21-src/src/Basic/Types/basic.hh       2002-11-08 
13:29:35.000000000 +0300
@@ -26,6 +26,9 @@
 typedef int SI;
 typedef void* pointer;
 
+#define CTOR_MAGIC_NUMBER 0xF9CC8287LU
+#define DTOR_MAGIC_NUMBER 0xDEADBEEFLU
+
 void* operator new (register size_t s);
 void  operator delete (register void* ptr);
 void* operator new[] (register size_t s);
diff -udBbr TeXmacs-1.0.0.21-src-orig/src/Basic/Types/string.cc 
TeXmacs-1.0.0.21-src/src/Basic/Types/string.cc
--- TeXmacs-1.0.0.21-src-orig/src/Basic/Types/string.cc 2002-08-10 
10:50:50.000000000 +0400
+++ TeXmacs-1.0.0.21-src/src/Basic/Types/string.cc      2002-11-08 
18:49:13.000000000 +0300
@@ -46,11 +46,13 @@
 }
 
 string::string (char c) {
+    magic_check_number = CTOR_MAGIC_NUMBER;
   rep= new string_rep(1);
   rep->a[0]=c;
 }
 
 string::string (char *a) {
+    magic_check_number = CTOR_MAGIC_NUMBER;
   register int i, n=strlen(a);
   rep= new string_rep(n);
   for (i=0; i<n; i++)
@@ -58,6 +60,7 @@
 }
 
 string::string (char* a, int n) {
+    magic_check_number = CTOR_MAGIC_NUMBER;
   register int i;
   rep= new string_rep(n);
   for (i=0; i<n; i++)
diff -udBbr TeXmacs-1.0.0.21-src-orig/src/Basic/Types/string.hh 
TeXmacs-1.0.0.21-src/src/Basic/Types/string.hh
--- TeXmacs-1.0.0.21-src-orig/src/Basic/Types/string.hh 2002-08-10 
10:50:50.000000000 +0400
+++ TeXmacs-1.0.0.21-src/src/Basic/Types/string.hh      2002-11-08 
18:51:05.000000000 +0300
@@ -23,13 +23,16 @@
 public:
   inline string_rep (): n(0), a(NULL) {}
          string_rep (int n);
-  inline ~string_rep () { if (n!=0) delete[] a; }
+  inline ~string_rep () {
+      if (n!=0) delete[] a;
+  }
   void resize (int n);
 
   friend class string;
   friend inline int N (string a);
 };
 
+/*
 class string {
   CONCRETE(string);
   inline string (): rep (new string_rep()) {}
@@ -45,7 +48,64 @@
   string operator () (int start, int end);
 };
 CONCRETE_CODE(string);
+*/
+
+class string {
+  string_rep *rep;
+  unsigned long magic_check_number;
+public:
+  inline string (const string&);
+  inline ~string ();
+  inline string_rep* operator -> ();
+  inline string& operator = (string x);
+
+  inline string (): rep (new string_rep())
+    {
+        magic_check_number = CTOR_MAGIC_NUMBER;
+    }
+  inline string (int n): rep (new string_rep (n))
+    {
+        magic_check_number = CTOR_MAGIC_NUMBER;
+    }
+  string (char c);
+  string (char *s);
+  string (char *s, int n);
+  inline char& operator [] (int i) { return rep->a[i]; }
+  bool operator == (char* s);
+  bool operator != (char* s);
+  bool operator == (string s);
+  bool operator != (string s);
+  string operator () (int start, int end);
+};
+inline string::string (const string& x)
+        :rep(x.rep)
+{
+    magic_check_number = CTOR_MAGIC_NUMBER;
+    INC_COUNT (this->rep);
+}
+inline string::~string ()
+{
+    if (magic_check_number != CTOR_MAGIC_NUMBER) {
+        
+        cout << "dtor_string magic: "
+             << std::hex << magic_check_number << std::dec
+             << std::endl;
+        abort();
+    }
+    
+    magic_check_number = DTOR_MAGIC_NUMBER;
+    DEC_COUNT (this->rep);
+}
+inline string_rep* string::operator -> ()
+{
+    return rep;
+}
+inline string& string::operator = (string x)
+{
+    INC_COUNT (x.rep); DEC_COUNT (this->rep);
+    this->rep=x.rep; return *this;
+}
 
 extern inline int N (string a) { return a->n; }
 string   copy (string a);