dangling object references tracking and cleanup (#383)

* initial instructions

* rename code_stack in emit

* update release notes

* rework memory management across func calls

* add stmt object loop conditional replacement mechanism

* add func-call-loop stmt cleanup unit test

* clean up windows snprintf warnings

* add pointer printing to stmt instrs

* add stmt->exp level dangling object reference tracking and cleanup

* fixed stmt->exp dangling refs for return

* dangle ref and stack pointer unit tests

* rename unit test 220

* unit tests 226 227

VERSIONS

@@ -2,6 +2,24 @@
 ChucK VERSIONS log
 ------------------
 
+1.5.1.7 (October 2023)
+=======
+- (added) new type 'vec2' 2D vector type
+    for 2D animation (ChuGL) as well as texture mapping UV or ST 
+    coordinates; access fields with .x/.y OR .u/.v OR .s/.t
+- (added) new examples/vector/vec2.ck
+- (reworked) memory managment across function calls that return
+    Object types; now correctly tracks all instances within a 
+    statement and balances reference counts; previously there was
+    a chance that object refcount were not correctly released,
+    causing incorrect behavior including memory leaks.
+    examples (assuming foo() returns an Object of some kind):
+    `foo().bar;` or `foo(), foo()` (multi-expression stmt)
+- (fixed) incorrect reference count for certain usages of 'null'
+    where the expression was interpreted incorrectly as a
+    function call, e.g., obj == null
+
+
 1.5.1.6 (October 2023) patch release
 =======
 - (fixed) a serious issue accessing vec3/vec4 fields (.x, .y, .z. .w)

examples/vector/vecs-cast.ck

@@ -0,0 +1,13 @@
+// casting between vec2, vec3, vec4
+
+// vec2 to vec3
+@(1,2) $ vec4 => vec4 v4; <<< v4 >>>;
+// vec2 to vec4
+@(1,2) $ vec4 => v4; <<< v4 >>>;
+// vec3 to vec4
+@(1,2,3) $ vec4 => v4; <<< v4 >>>;
+
+// vec4 to vec3
+@(1,2,3,4) $ vec3 => vec3 v3; <<< v3 >>>;
+// vec4 to vec2
+@(1,2,3,4) $ vec2 => vec2 v2; <<< v2 >>>;

src/core/chuck_absyn.h

@@ -428,6 +428,8 @@ struct a_Stmt_
     ae_Stmt_Type s_type;
     // used to track control paths in non-void functions
     t_CKBOOL allControlPathsReturn; // 1.5.1.0 (ge) added
+    // number of obj refs that needs releasing after | 1.5.1.7
+    t_CKUINT numObjsToRelease;
 
     // mushed into one!
     union

src/core/chuck_emit.h

@@ -42,6 +42,7 @@
 // forward references
 struct Chuck_Instr;
 struct Chuck_Instr_Goto;
+struct Chuck_Instr_Stmt_Start;
 struct Chuck_VM_Code;
 struct Chuck_VM_Shred;
 
@@ -119,10 +120,12 @@ struct Chuck_Emitter : public Chuck_VM_Object
     // current function definition
     Chuck_Func * func;
 
-    // code stack
-    std::vector<Chuck_Code *> stack;
     // locals
     std::vector<Chuck_Local *> locals;
+    // code stack
+    std::vector<Chuck_Code *> code_stack;
+    // stmt stack
+    std::vector<Chuck_Instr_Stmt_Start *> stmt_stack;
 
     // dump
     t_CKBOOL dump;

src/core/chuck_instr.cpp

@@ -5226,7 +5226,7 @@ void Chuck_Instr_Func_Call_Member::execute( Chuck_VM * vm, Chuck_VM_Shred * shre
         if( m_func_ref && isobj(vm->env(), m_func_ref->def()->ret_type) )
         {
             // get return value as object reference
-            Chuck_VM_Object * obj = (Chuck_VM_Object *) retval.v_uint;
+            Chuck_VM_Object * obj = (Chuck_VM_Object *)retval.v_uint;
             if( obj )
             {
                 // always add reference to returned objects (should release outside)
@@ -5362,7 +5362,7 @@ void Chuck_Instr_Func_Call_Static::execute( Chuck_VM * vm, Chuck_VM_Shred * shre
         if( m_func_ref && isobj(vm->env(), m_func_ref->def()->ret_type) )
         {
             // get return value as object reference
-            Chuck_VM_Object * obj = (Chuck_VM_Object *) retval.v_uint;
+            Chuck_VM_Object * obj = (Chuck_VM_Object *)retval.v_uint;
             if( obj )
             {
                 // always add reference to returned objects (should release outside)
@@ -5488,7 +5488,7 @@ void Chuck_Instr_Func_Call_Global::execute( Chuck_VM * vm, Chuck_VM_Shred * shre
         if( m_func_ref && isobj(vm->env(), m_func_ref->def()->ret_type) )
         {
             // get return value as object reference
-            Chuck_VM_Object * obj = (Chuck_VM_Object *) retval.v_uint;
+            Chuck_VM_Object * obj = (Chuck_VM_Object *)retval.v_uint;
             if( obj )
             {
                 // always add reference to returned objects (should release outside)
@@ -5552,7 +5552,7 @@ void Chuck_Instr_Func_Call_Global::execute( Chuck_VM * vm, Chuck_VM_Shred * shre
 
 //-----------------------------------------------------------------------------
 // name: execute()
-// desc: ...
+// desc: return from a function
 //-----------------------------------------------------------------------------
 void Chuck_Instr_Func_Return::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
 {
@@ -5578,9 +5578,250 @@ void Chuck_Instr_Func_Return::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
 
 
 
+//-----------------------------------------------------------------------------
+// name: params()
+// desc: for instruction dumps
+//-----------------------------------------------------------------------------
+// for printing
+const char * Chuck_Instr_Stmt_Start::params() const
+{
+    static char buffer[CK_PRINT_BUF_LENGTH];
+    snprintf( buffer, CK_PRINT_BUF_LENGTH, "numObjReleases=%lu this=%p", (unsigned long)m_numObjReleases, this );
+    return buffer;
+}
+
+
+
+
 //-----------------------------------------------------------------------------
 // name: execute()
-// desc: ...
+// desc: executed at the start of a statement (see header for details)
+//-----------------------------------------------------------------------------
+void Chuck_Instr_Stmt_Start::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
+{
+    // if nothing to push, no op
+    if( !m_numObjReleases ) return;
+
+    // make room
+    if( !m_objectsToRelease )
+    {
+        // allocate for re-use
+        m_objectsToRelease = new t_CKUINT[m_numObjReleases];
+    }
+
+    // make room for all the object references to release
+    for( t_CKUINT i = 0; i < m_numObjReleases; i++ )
+    {
+        // zero out
+        m_objectsToRelease[i] = 0;
+    }
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: nextOffset()
+// desc: returns next offset on success; 0 if we have exceeded numObjeReleases
+//-----------------------------------------------------------------------------
+t_CKBOOL Chuck_Instr_Stmt_Start::nextOffset( t_CKUINT & offset )
+{
+    // clear offset
+    offset = 0;
+    // check
+    if( m_nextOffset >= m_numObjReleases )
+    {
+        EM_exception(
+            "(internal error) out of bounds in Stmt_Start.nextIndex(): nextOffset == %lu",
+            m_nextOffset );
+
+        // return
+        return FALSE;
+    }
+
+    // copy into return variable, then increment
+    offset = m_nextOffset++;
+    // return
+    return TRUE;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: setObject()
+// desc: set object into data region of objects to release by stmt's end
+//-----------------------------------------------------------------------------
+t_CKBOOL Chuck_Instr_Stmt_Start::setObject( Chuck_VM_Object * object, t_CKUINT offset )
+{
+    // check
+    if( offset >= m_numObjReleases )
+    {
+        EM_exception(
+            "(internal error) offset out of bounds in Stmt_Start.setObject(): offset == %lu",
+            offset );
+        // return
+        return FALSE;
+    }
+
+    // pointer arithmetic
+    t_CKUINT * pInt = m_objectsToRelease + offset;
+
+    // release if not NULL; what was previously there is no-longer accessible
+    // NOTE this could happen in the case of a loop:
+    // e.g., while( foo() ) { ... } // where foo() returns an object
+    Chuck_VM_Object * outgoing = (Chuck_VM_Object *)(*pInt);
+    CK_SAFE_RELEASE( outgoing );
+
+    // copy incoming object pointer
+    *pInt = (t_CKUINT)object;
+    // done
+    return TRUE;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: cleanupRefs()
+// desc: clean up references
+//-----------------------------------------------------------------------------
+t_CKBOOL Chuck_Instr_Stmt_Start::cleanupRefs( Chuck_VM_Shred * shred )
+{
+    // if nothing to push, no op
+    if( !m_numObjReleases ) return TRUE;
+
+    // if no stack pointer
+    if( !m_objectsToRelease )
+    {
+        // we have a problem
+        EM_exception(
+            "(internal error) NULL data region in Stmt_Start.cleanupRef() on shred[id=%lu:%s]",
+            shred->xid, shred->name.c_str() );
+        // bail out
+        return FALSE;
+    }
+
+    // cast pointer to data region as Object pointers
+    t_CKUINT * pInt = m_objectsToRelease;
+
+    // make room for all the object references to release
+    for( t_CKUINT i = 0; i < m_numObjReleases; i++ )
+    {
+        // object pointer
+        Chuck_VM_Object * object = (Chuck_VM_Object *)(*pInt);
+        // release (could be NULL)
+        CK_SAFE_RELEASE( object );
+        // advance pointer
+        pInt++;
+    }
+
+    return TRUE;
+}
+
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: params()
+// desc: for instruction dumps
+//-----------------------------------------------------------------------------
+const char * Chuck_Instr_Stmt_Remember_Object::params() const
+{
+    static char buffer[CK_PRINT_BUF_LENGTH];
+    snprintf( buffer, CK_PRINT_BUF_LENGTH, "offset=%lu start=%p", (unsigned long)m_offset, m_stmtStart );
+    return buffer;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: execute()
+// desc: remember obj ref on reg stack for stmt-related cleanup
+//-----------------------------------------------------------------------------
+void Chuck_Instr_Stmt_Remember_Object::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
+{
+    // get stack pointer
+    t_CKUINT * reg_sp = (t_CKUINT *)shred->reg->sp;
+    Chuck_VM_Object * obj = (Chuck_VM_Object *)(*(reg_sp-1));
+
+    // check
+    if( !m_stmtStart )
+    {
+        EM_exception(
+            "(internal error) missing data region information in Stmt_Remember_Object instruction..." );
+        goto error;
+    }
+
+    // clear the objects returns by function calls in the statement
+    if( !m_stmtStart->setObject( obj, m_offset ) )
+    {
+        EM_exception(
+            "(internal error) cannot setObject() in Stmt_Remember_Object instruction..." );
+        goto error;
+    }
+
+    // done
+    return;
+
+error:
+    // done
+    shred->is_running = FALSE;
+    shred->is_done = TRUE;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: params()
+// desc: for instruction dumps
+//-----------------------------------------------------------------------------
+const char * Chuck_Instr_Stmt_Cleanup::params() const
+{
+    static char buffer[CK_PRINT_BUF_LENGTH];
+    snprintf( buffer, CK_PRINT_BUF_LENGTH, "numObjRelease=%lu start=%p", (unsigned long)(m_stmtStart ? m_stmtStart->m_numObjReleases : 0), m_stmtStart );
+    return buffer;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: execute()
+// desc: clean up after a statement
+//-----------------------------------------------------------------------------
+void Chuck_Instr_Stmt_Cleanup::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
+{
+    // check
+    if( !m_stmtStart )
+    {
+        EM_exception(
+            "(internal error) missing data region in Stmt_Cleanup instruction..." );
+        goto error;
+    }
+
+    // clean up references
+    if( !m_stmtStart->cleanupRefs( shred ) )
+        goto error;
+
+    // done
+    return;
+
+error:
+    // done
+    shred->is_running = FALSE;
+    shred->is_done = TRUE;
+}
+
+
+
+
+//-----------------------------------------------------------------------------
+// name: execute()
+// desc: spork a shred from code
 //-----------------------------------------------------------------------------
 void Chuck_Instr_Spork::execute( Chuck_VM * vm, Chuck_VM_Shred * shred )
 {
@@ -7012,7 +7253,7 @@ void Chuck_Instr_Dot_Member_Data::execute( Chuck_VM * vm, Chuck_VM_Shred * shred
     {
         // 4 or 8 or 16
         // 1.3.1.0: check type to use kind instead of size
-        if( m_kind == kindof_INT ) { push_( sp, *((t_CKUINT *)data) ); } // ISSUE: 64-bit (fixed 1.3.1.0)
+        if( m_kind == kindof_INT ) { push_( sp, *((t_CKINT *)data) ); } // ISSUE: 64-bit (fixed 1.3.1.0)
         else if( m_kind == kindof_FLOAT ) { push_float( sp, *((t_CKFLOAT *)data) ); } // ISSUE: 64-bit (fixed 1.3.1.0)
         else if( m_kind == kindof_VEC2 ) { push_vec2( sp, *((t_CKVEC2 *)data) ); } // ISSUE: 64-bit (fixed 1.3.1.0) | in this context, vec2 = complex = polar
         else if( m_kind == kindof_VEC3 ) { push_vec3( sp, *((t_CKVEC3 *)data) ); } // 1.3.5.3