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

[Igor V. Kovalenko]

Hi, Joris!

I've had done an extensive debugging session :) and here is the result.

It seems that destroy_tree_rep is scribbling over wrong data. I propose the 
following way to
eliminate this. Let it use virtual destructors. Sorry this was discussed before,
I'm only a few weeks on-list.

I tested TeXmacs with the patch included and it seems to be stable now.
I used the following compiler options:
  1. Full optimization, eliminate inline expansions,with debug info 
(un-inlining helps to debug)
     -g3 -O3 -fexpensive-optimizations -fno-exceptions -fno-inline

  2. The default for full optimization, with debug info
     -g3 -O3 -fexpensive-optimizations -fno-exceptions

and tried to turn off TeXmacs memory allocator (in basic.cc) as was discussed 
before, which gives
  a. Eliminate (#ifdef out) new/delete/new[]/delete[] operators from 
Basic/Types/basic.cc
  b. Leave TeXmacs memory allocators intact.

In total of 4 tests where mentioned options were permuted I used my test file 
to crash TeXmacs.
Nope, attempt failed :). All tests resulted in stable behaviour. I'm using this 
now,
and will notify if I still encounter the segfault.

I've not tested with GCC 2.96 as I try to move off it :)

Comments?


[-------------------------PATCH starts 
here-----------------------------------------]
diff -udBbHr TeXmacs-1.0.0.20-src/src/Basic/Types/basic.hh 
TeXmacs-1.0.0.20-src-20021029/src/Basic/Types/basic.hh
--- TeXmacs-1.0.0.20-src/src/Basic/Types/basic.hh       2002-10-28 
10:38:21.000000000 +0300
+++ TeXmacs-1.0.0.20-src-20021029/src/Basic/Types/basic.hh      2002-11-03 
22:24:11.000000000 +0300
@@ -46,7 +46,7 @@
 struct concrete_struct {
   int ref_count;
   inline concrete_struct (): ref_count (1) { DEBUG(concrete_count++); }
-  inline ~concrete_struct () { DEBUG(concrete_count--); }
+  inline virtual ~concrete_struct () { DEBUG(concrete_count--); }
 };

 extern int abstract_count;
diff -udBbHr TeXmacs-1.0.0.20-src/src/Basic/Data/tree.hh 
TeXmacs-1.0.0.20-src-20021029/src/Basic/Data/tree.hh
--- TeXmacs-1.0.0.20-src/src/Basic/Data/tree.hh 2002-10-24 12:18:09.000000000 
+0400
+++ TeXmacs-1.0.0.20-src-20021029/src/Basic/Data/tree.hh        2002-11-03 
22:27:49.000000000 +0300
@@ -153,18 +153,33 @@
 #define CHECK_COMPOUND(t,s)
 #endif

-void destroy_tree_rep (tree_rep* rep);
+//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 ()
+{
+    if ((--rep->ref_count)==0) {
+        //destroy_tree_rep (rep);
+        delete rep;
+        rep = NULL;
+    }
+}
+
 inline atomic_rep* tree::operator -> () {
   CHECK_ATOMIC (*this, "tree::operator ->");
   return (atomic_rep*) rep; }
-inline tree& tree::operator = (tree x) {
+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);
+      delete rep;
+  }
+
   rep= x.rep;
-  return *this; }
+  return *this;
+}

 inline tree::tree ():
   rep (new atomic_rep (string ())) {}
[-------------------------PATCH ends 
here-------------------------------------------]


Here is an excerpt from valgrind log which corroborates my debug sessions.
The access in string.hh:47 is hidden by CONCRETE_CODE

This is the read part of erroneous behaviour:
------------------------------------------
==26615== Invalid read of size 4
==26615==    at 0x804F10B: ??? (Basic/Types/string.hh:47)
==26615==    by 0x82BC5E1: ??? (Basic/Data/tree.cc:22)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x804FCD3: ??? (Basic/Data/tree.hh:163)
==26615==    by 0x8050433: ??? (Basic/Types/array.hh:28)
==26615==    by 0x8050101: ??? (Basic/Types/array.hh:42)
==26615==    by 0x82BC569: ??? (Basic/Data/tree.cc:27)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x804FCD3: ??? (Basic/Data/tree.hh:163)
==26615==    by 0x8050433: ??? (Basic/Types/array.hh:28)
==26615==    by 0x8050101: ??? (Basic/Types/array.hh:42)
==26615==    by 0x82BC569: ??? (Basic/Data/tree.cc:27)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x8331D73: tm_data_rep::save(string, tm_view_rep*) 
(Texmacs/Data/tm_file.cc:267)
==26615==    by 0x8333C77: tm_data_rep::auto_save() 
(Texmacs/Data/tm_file.cc:379)
==26615==    by 0x833C4A7: 
tm_widget_rep::handle_alarm(event_ptr<alarm_event_rep>) 
(Texmacs/Layout/tm_widget.cc:340)
==26615==    by 0x805E80D: basic_widget_rep::handle(event) 
(Window/Widget/Basic/basic_widget.cc:330)
==26615==    by 0x833C4E0: tm_widget_rep::handle(event) 
(Texmacs/Layout/tm_widget.cc:354)
==26615==    by 0x82F4EBA: operator<<(widget, event) 
(Window/Widget/Basic/widget.cc:123)
==26615==    by 0x8310B76: x_display_rep::event_loop() (Window/event.hh:55)
==26615==    by 0x834904B: TeXmacs_main(int, char**) 
(Texmacs/Texmacs/texmacs.cc:179)
==26615==    by 0x40353CBD: gh_launch_pad (gh_init.c:61)
==26615==    by 0x403570DD: invoke_main_func (init.c:629)
==26615==    by 0x40379114: scm_internal_lazy_catch (throw.c:285)
==26615==    by 0x40357092: scm_boot_guile_1 (init.c:602)
==26615==    by 0x40356DA4: scm_boot_guile (init.c:444)
==26615==    by 0x40353CF2: gh_enter (gh_init.c:72)
==26615==    by 0x8349F57: main (/usr/include/c++/3.2/iostream:62)
==26615==    by 0x42015A54: (within /lib/i686/libc-2.3.1.so)
==26615==    by 0x804BA45: XMapRaised (in 
/usr/libexec/TeXmacs-1.0.0.20/bin/texmacs.bin)
==26615==    Address 0x466332E4 is 4 bytes inside a block of size 16 free'd
==26615==    at 0x4003D0FE: __builtin_delete (vg_clientfuncs.c:194)
==26615==    by 0x4003D123: operator delete(void*) (vg_clientfuncs.c:204)
==26615==    by 0x82B3CBD: tm_writer::write(tree) (Basic/Data/tree.hh:161)
==26615==    by 0x82B3EB4: tm_writer::write(tree) (Basic/Data/tree.hh:162)
==26615==    by 0x82B4661: tree_to_texmacs(tree) (Basic/Data/tree.hh:162)
==26615==    by 0x82B4AE8: tree_to_texmacs_document(tree) 
(Basic/Types/string.hh:47)
==26615==    by 0x81741A3: save_tree(string, tree, bool) 
(Basic/Data/tree.hh:162)
==26615==    by 0x8331B8B: tm_data_rep::save(string, tm_view_rep*) 
(Basic/Data/tree.hh:162)
==26615==    by 0x8333C77: tm_data_rep::auto_save() 
(Texmacs/Data/tm_file.cc:379)
==26615==    by 0x833C4A7: 
tm_widget_rep::handle_alarm(event_ptr<alarm_event_rep>) 
(Texmacs/Layout/tm_widget.cc:340)
------------------------------------------

This is the write part of erroneous behaviour:
------------------------------------------
==26615==
==26615== Invalid write of size 4
==26615==    at 0x804F111: ??? (Basic/Types/string.hh:47)
==26615==    by 0x82BC5E1: ??? (Basic/Data/tree.cc:22)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x804FCD3: ??? (Basic/Data/tree.hh:163)
==26615==    by 0x8050433: ??? (Basic/Types/array.hh:28)
==26615==    by 0x8050101: ??? (Basic/Types/array.hh:42)
==26615==    by 0x82BC569: ??? (Basic/Data/tree.cc:27)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x804FCD3: ??? (Basic/Data/tree.hh:163)
==26615==    by 0x8050433: ??? (Basic/Types/array.hh:28)
==26615==    by 0x8050101: ??? (Basic/Types/array.hh:42)
==26615==    by 0x82BC569: ??? (Basic/Data/tree.cc:27)
==26615==    by 0x82B83D6: destroy_tree_rep(tree_rep*) (Basic/Data/tree.cc:35)
==26615==    by 0x8331D73: tm_data_rep::save(string, tm_view_rep*) 
(Texmacs/Data/tm_file.cc:267)
==26615==    by 0x8333C77: tm_data_rep::auto_save() 
(Texmacs/Data/tm_file.cc:379)
==26615==    by 0x833C4A7: 
tm_widget_rep::handle_alarm(event_ptr<alarm_event_rep>) 
(Texmacs/Layout/tm_widget.cc:340)
==26615==    by 0x805E80D: basic_widget_rep::handle(event) 
(Window/Widget/Basic/basic_widget.cc:330)
==26615==    by 0x833C4E0: tm_widget_rep::handle(event) 
(Texmacs/Layout/tm_widget.cc:354)
==26615==    by 0x82F4EBA: operator<<(widget, event) 
(Window/Widget/Basic/widget.cc:123)
==26615==    by 0x8310B76: x_display_rep::event_loop() (Window/event.hh:55)
==26615==    by 0x834904B: TeXmacs_main(int, char**) 
(Texmacs/Texmacs/texmacs.cc:179)
==26615==    by 0x40353CBD: gh_launch_pad (gh_init.c:61)
==26615==    by 0x403570DD: invoke_main_func (init.c:629)
==26615==    by 0x40379114: scm_internal_lazy_catch (throw.c:285)
==26615==    by 0x40357092: scm_boot_guile_1 (init.c:602)
==26615==    by 0x40356DA4: scm_boot_guile (init.c:444)
==26615==    by 0x40353CF2: gh_enter (gh_init.c:72)
==26615==    by 0x8349F57: main (/usr/include/c++/3.2/iostream:62)
==26615==    by 0x42015A54: (within /lib/i686/libc-2.3.1.so)
==26615==    by 0x804BA45: XMapRaised (in 
/usr/libexec/TeXmacs-1.0.0.20/bin/texmacs.bin)
==26615==    Address 0x466332E4 is 4 bytes inside a block of size 16 free'd
==26615==    at 0x4003D0FE: __builtin_delete (vg_clientfuncs.c:194)
==26615==    by 0x4003D123: operator delete(void*) (vg_clientfuncs.c:204)
==26615==    by 0x82B3CBD: tm_writer::write(tree) (Basic/Data/tree.hh:161)
==26615==    by 0x82B3EB4: tm_writer::write(tree) (Basic/Data/tree.hh:162)
==26615==    by 0x82B4661: tree_to_texmacs(tree) (Basic/Data/tree.hh:162)
==26615==    by 0x82B4AE8: tree_to_texmacs_document(tree) 
(Basic/Types/string.hh:47)
==26615==    by 0x81741A3: save_tree(string, tree, bool) 
(Basic/Data/tree.hh:162)
==26615==    by 0x8331B8B: tm_data_rep::save(string, tm_view_rep*) 
(Basic/Data/tree.hh:162)
==26615==    by 0x8333C77: tm_data_rep::auto_save() 
(Texmacs/Data/tm_file.cc:379)
==26615==    by 0x833C4A7: 
tm_widget_rep::handle_alarm(event_ptr<alarm_event_rep>) 
(Texmacs/Layout/tm_widget.cc:340)
------------------------------------------

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