[PATCH 0/2] Fix Gwen's compile_block bug.

Here is my fix for the compiler bug, and also an assertion that will catch
similar bugs.

Paolo

Paolo Bonzini (2):
  fix GC bug in compile_block, reported by Gwen
  add assertion to add_literal to catch wrong use of the incubator

 libgst/ChangeLog | 11 +++++++++++
 libgst/comp.c    | 20 +++++++++++---------
 libgst/comp.h    |  1 +
 libgst/oop.h     |  1 +
 libgst/oop.inl   |  9 +++++++--
 5 files changed, 31 insertions(+), 11 deletions(-)

--
1.8.3.1


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

libgst:
2013-07-16  Paolo Bonzini  <[hidden email]>

        * libgst/comp.c: Do not call add_literal within an incubator context.
---
 libgst/ChangeLog |  4 ++++
 libgst/comp.c    | 16 +++++++---------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index f5a77d7..197b81e 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,7 @@
+2013-07-16  Paolo Bonzini  <[hidden email]>
+
+ * libgst/comp.c: Do not call add_literal within an incubator context.
+
 2013-06-14  Gwenael Casaccio <[hidden email]>
 
  * libgst/dict.c: Remove useless code: gst_ordered_collection structure.
diff --git a/libgst/comp.c b/libgst/comp.c
index 10330e1..1a59f2f 100644
--- a/libgst/comp.c
+++ b/libgst/comp.c
@@ -1064,7 +1064,7 @@ compile_block (tree_node blockExpr)
   bc_vector current_bytecodes, blockByteCodes;
   int argCount, tempCount;
   int stack_depth;
-  OOP blockClosureOOP, blockOOP;
+  OOP litOOP, blockOOP;
   gst_compiled_block block;
   inc_ptr incPtr;
 
@@ -1114,17 +1114,15 @@ compile_block (tree_node blockExpr)
   INCR_STACK_DEPTH ();
   block = (gst_compiled_block) OOP_TO_OBJ (blockOOP);
   if (block->header.clean == 0)
-    {
-      blockClosureOOP = make_clean_block_closure (blockOOP);
-      _gst_compile_byte (PUSH_LIT_CONSTANT, add_literal (blockClosureOOP));
-    }
+    litOOP = make_clean_block_closure (blockOOP);
   else
-    {
-      _gst_compile_byte (PUSH_LIT_CONSTANT, add_literal (blockOOP));
-      _gst_compile_byte (MAKE_DIRTY_BLOCK, 0);
-    }
+    litOOP = blockOOP;
 
   INC_RESTORE_POINTER (incPtr);
+
+  _gst_compile_byte (PUSH_LIT_CONSTANT, add_literal (litOOP));
+  if (block->header.clean != 0)
+    _gst_compile_byte (MAKE_DIRTY_BLOCK, 0);
 }
 
 
--
1.8.3.1



_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

libgst:
2013-07-16  Paolo Bonzini  <[hidden email]>

        * libgst/oop.h: Add incubator depth.
        * libgst/oop.inl: Track incubator depth and add inc_current_depth().
        * libgst/comp.c: Assert that add_literal is not called within an
        incubator context (suggested by Gwen).
---
 libgst/ChangeLog | 7 +++++++
 libgst/comp.c    | 4 ++++
 libgst/comp.h    | 1 +
 libgst/oop.h     | 1 +
 libgst/oop.inl   | 9 +++++++--
 5 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 197b81e..221d9c6 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,12 @@
 2013-07-16  Paolo Bonzini  <[hidden email]>
 
+ * libgst/oop.h: Add incubator depth.
+ * libgst/oop.inl: Track incubator depth and add inc_current_depth().
+ * libgst/comp.c: Assert that add_literal is not called within an
+ incubator context (suggested by Gwen).
+
+2013-07-16  Paolo Bonzini  <[hidden email]>
+
  * libgst/comp.c: Do not call add_literal within an incubator context.
 
 2013-06-14  Gwenael Casaccio <[hidden email]>
diff --git a/libgst/comp.c b/libgst/comp.c
index 1a59f2f..5ed20e0 100644
--- a/libgst/comp.c
+++ b/libgst/comp.c
@@ -533,6 +533,7 @@ _gst_execute_statements (OOP receiverOOP,
       _gst_compiler_state = &s;
       memset (&s, 0, sizeof (s));
       _gst_compiler_state->undeclared_temporaries = undeclared;
+      _gst_compiler_state->inc_depth = inc_current_depth();
 
       if (setjmp (_gst_compiler_state->bad_method) == 0)
         {
@@ -714,6 +715,7 @@ _gst_compile_method (tree_node method,
     _gst_compiler_state->literal_vec + LITERAL_VEC_CHUNK_SIZE;
 
   incPtr = INC_SAVE_POINTER ();
+  _gst_compiler_state->inc_depth = inc_current_depth();
 
   _gst_alloc_bytecodes ();
   _gst_push_new_scope ();
@@ -2368,6 +2370,8 @@ add_literal (OOP oop)
 
   i =_gst_compiler_state->literal_vec_curr - _gst_compiler_state->literal_vec;
   *_gst_compiler_state->literal_vec_curr++ = oop;
+
+  assert(_gst_compiler_state->inc_depth == inc_current_depth());
   INC_ADD_OOP (oop);
   return i;
 }
diff --git a/libgst/comp.h b/libgst/comp.h
index 91a1f9c..8f00735 100644
--- a/libgst/comp.h
+++ b/libgst/comp.h
@@ -231,6 +231,7 @@ typedef struct compiler_state
   scope cur_scope;
   mst_Boolean inside_block;
   mst_Boolean undeclared_temporaries;
+  int inc_depth;
   OOP *literal_vec;
   OOP *literal_vec_curr;
   OOP *literal_vec_max;
diff --git a/libgst/oop.h b/libgst/oop.h
index d41b0ae..76a5977 100644
--- a/libgst/oop.h
+++ b/libgst/oop.h
@@ -236,6 +236,7 @@ struct memory_space
 
   /* These hold onto the object incubator's state */
   OOP *inc_base, *inc_ptr, *inc_end;
+  int inc_depth;
 
   /* Objects that are at least this big (in bytes) are allocated outside
      the main heap, hoping to provide more locality of reference between
diff --git a/libgst/oop.inl b/libgst/oop.inl
index 279dd1b..d15e7a2 100644
--- a/libgst/oop.inl
+++ b/libgst/oop.inl
@@ -139,11 +139,16 @@ static inline OOP alloc_oop (PTR obj, intptr_t flags);
     *_gst_mem.inc_ptr++ = (oop))
 
 #define INC_SAVE_POINTER() \
-  (_gst_mem.inc_ptr - _gst_mem.inc_base)
+  (_gst_mem.inc_depth++, _gst_mem.inc_ptr - _gst_mem.inc_base)
 
 #define INC_RESTORE_POINTER(ptr) \
-  _gst_mem.inc_ptr = (ptr) + _gst_mem.inc_base;
+  (_gst_mem.inc_depth--, _gst_mem.inc_ptr = (ptr) + _gst_mem.inc_base)
 
+static inline intptr_t
+inc_current_depth (void)
+{
+    return _gst_mem.inc_depth;
+}
 
 
 
--
1.8.3.1


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk