eval/typval_encode: Fix infinite loop

Occurs when trying to dump a partial with attached self dictionary which
references that partial. “Infinite” loop should normally result in Neovim killed
by OOM killer.

Also moved the place when partials are unreferenced by clear_tv: from
…FUNC_START to …FUNC_END.

src/nvim/eval.c

@@ -19072,22 +19072,30 @@ void free_tv(typval_T *varp)
 
 #define TYPVAL_ENCODE_CONV_FUNC_START(fun, is_partial, pt) \
     do { \
-      if (is_partial) { \
+      if (!is_partial) { \
-        partial_unref(pt); \
-        tv->vval.v_partial = NULL; \
-      } else { \
         func_unref(fun); \
         if (fun != empty_string) { \
           xfree(fun); \
         } \
         tv->vval.v_string = NULL; \
+        tv->v_lock = VAR_UNLOCKED; \
       } \
-      tv->v_lock = VAR_UNLOCKED; \
     } while (0)
 
 #define TYPVAL_ENCODE_CONV_FUNC_BEFORE_ARGS(len)
 #define TYPVAL_ENCODE_CONV_FUNC_BEFORE_SELF(len)
-#define TYPVAL_ENCODE_CONV_FUNC_END()
+#define TYPVAL_ENCODE_CONV_FUNC_END() \
+    do { \
+      if (cur_mpsv->type == kMPConvPartial) { \
+        typval_T *const cur_tv = cur_mpsv->tv; \
+        partial_T *const pt = cur_mpsv->data.p.pt; \
+        partial_unref(pt); \
+        if (cur_tv != NULL) { \
+          cur_tv->vval.v_partial = NULL; \
+          cur_tv->v_lock = VAR_UNLOCKED; \
+        } \
+      } \
+    } while (0)
 
 #define TYPVAL_ENCODE_CONV_EMPTY_LIST() \
     do { \
@@ -19118,10 +19126,14 @@ void free_tv(typval_T *varp)
 #define TYPVAL_ENCODE_CONV_LIST_END() \
     do { \
       typval_T *const cur_tv = cur_mpsv->tv; \
-      assert(cur_tv->v_type == VAR_LIST); \
+      list_T *const list = cur_mpsv->data.l.list; \
-      list_unref(cur_tv->vval.v_list); \
+      list_unref(list); \
-      cur_tv->vval.v_list = NULL; \
+      if (cur_tv != NULL) { \
-      cur_tv->v_lock = VAR_UNLOCKED; \
+        assert(list == cur_tv->vval.v_list); \
+        assert(cur_tv->v_type == VAR_LIST); \
+        cur_tv->vval.v_list = NULL; \
+        cur_tv->v_lock = VAR_UNLOCKED; \
+      } \
     } while (0)
 
 #define TYPVAL_ENCODE_CONV_DICT_START(ignored) \
@@ -19143,10 +19155,14 @@ void free_tv(typval_T *varp)
 #define TYPVAL_ENCODE_CONV_DICT_END() \
     do { \
       typval_T *const cur_tv = cur_mpsv->tv; \
-      assert(cur_tv->v_type == VAR_DICT); \
+      dict_T *const dict = cur_mpsv->data.d.dict; \
       dict_unref(cur_tv->vval.v_dict); \
-      cur_tv->vval.v_dict = NULL; \
+      if (cur_tv != NULL) { \
-      cur_tv->v_lock = VAR_UNLOCKED; \
+        assert(dict == cur_tv->vval.v_dict); \
+        assert(cur_tv->v_type == VAR_DICT); \
+        cur_tv->vval.v_dict = NULL; \
+        cur_tv->v_lock = VAR_UNLOCKED; \
+      } \
     } while (0)
 
 #define TYPVAL_ENCODE_CONV_RECURSE(ignored1, ignored2)

src/nvim/eval/typval_encode.c.h

@@ -166,7 +166,8 @@
 ///
 /// After including this file it will define function
 /// `encode_vim_to_{TYPVAL_ENCODE_NAME}` with scope #TYPVAL_ENCODE_SCOPE and
-/// static function `_{TYPVAL_ENCODE_NAME}_convert_one_value`.
+/// static functions `_typval_encode_{TYPVAL_ENCODE_NAME}_convert_one_value` and
+/// `_typval_encode_{TYPVAL_ENCODE_NAME}_check_self_reference`.
 
 /// @def TYPVAL_ENCODE_FIRST_ARG_TYPE
 /// @brief Type of the first argument, which will be used to return the results
@@ -192,11 +193,50 @@
 #include "nvim/func_attr.h"
 #include "nvim/eval/typval_encode.h"
 
+static inline int _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(
+    TYPVAL_ENCODE_FIRST_ARG_TYPE TYPVAL_ENCODE_FIRST_ARG_NAME,
+    void *const val, int *const val_copyID,
+    const MPConvStack *const mpstack, const int copyID,
+    const MPConvStackValType conv_type,
+    const char *const objname)
+  REAL_FATTR_NONNULL_ARG(2, 3, 4, 7) REAL_FATTR_WARN_UNUSED_RESULT
+  REAL_FATTR_ALWAYS_INLINE;
+
+/// Function for checking whether container references itself
+///
+/// @param  TYPVAL_ENCODE_FIRST_ARG_NAME  First argument.
+/// @param[in,out]  val  Container to check.
+/// @param  val_copyID  Pointer to the container attribute that holds copyID.
+///                     After checking whether value of this attribute is
+///                     copyID (variable) it is set to copyID.
+/// @param[in]  mpstack  Stack with values to convert. Read-only, used for error
+///                      reporting.
+/// @param[in]  copyID  CopyID used by the caller.
+/// @param[in]  conv_type  Type of the conversion, @see MPConvStackValType.
+/// @param[in]  objname  Object name, used for error reporting.
+///
+/// @return NOTDONE in case of success, what to return in case of failure.
+static inline int _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(
+    TYPVAL_ENCODE_FIRST_ARG_TYPE TYPVAL_ENCODE_FIRST_ARG_NAME,
+    void *const val, int *const val_copyID,
+    const MPConvStack *const mpstack, const int copyID,
+    const MPConvStackValType conv_type,
+    const char *const objname)
+{
+  if (*val_copyID == copyID) {
+    TYPVAL_ENCODE_CONV_RECURSE(val, conv_type);
+    return OK;
+  }
+  *val_copyID = copyID;
+  return NOTDONE;
+}
+
 static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
     TYPVAL_ENCODE_FIRST_ARG_TYPE TYPVAL_ENCODE_FIRST_ARG_NAME,
-    MPConvStack *const mpstack, typval_T *const tv, const int copyID,
+    MPConvStack *const mpstack, MPConvStackVal *const cur_mpsv,
+    typval_T *const tv, const int copyID,
     const char *const objname)
-  REAL_FATTR_NONNULL_ALL REAL_FATTR_WARN_UNUSED_RESULT;
+  REAL_FATTR_NONNULL_ARG(2, 4, 6) REAL_FATTR_WARN_UNUSED_RESULT;
 
 /// Convert single value
 ///
@@ -206,9 +246,10 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
 /// @param  TYPVAL_ENCODE_FIRST_ARG_NAME  First argument, defined by the
 ///                                       includer. Only meaningful to macros
 ///                                       defined by the includer.
-/// @param[out]  mpstack  Stack with values to convert. Values which are not
+/// @param  mpstack  Stack with values to convert. Values which are not
-///                       converted completely by this function (i.e.
+///                  converted completely by this function (i.e.
-///                       non-scalars) are pushed here.
+///                  non-scalars) are pushed here.
+/// @param  cur_mpsv  Currently converted value from stack.
 /// @param  tv  Converted value.
 /// @param[in]  copyID  CopyID.
 /// @param[in]  objname  Object name, used for error reporting.
@@ -216,7 +257,8 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
 /// @return OK in case of success, FAIL in case of failure.
 static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
     TYPVAL_ENCODE_FIRST_ARG_TYPE TYPVAL_ENCODE_FIRST_ARG_NAME,
-    MPConvStack *const mpstack, typval_T *const tv, const int copyID,
+    MPConvStack *const mpstack, MPConvStackVal *const cur_mpsv,
+    typval_T *const tv, const int copyID,
     const char *const objname)
 {
   switch (tv->v_type) {
@@ -260,8 +302,8 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
         TYPVAL_ENCODE_CONV_EMPTY_LIST();
         break;
       }
-      _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(tv->vval.v_list, lv_copyID, copyID,
+      _TYPVAL_ENCODE_DO_CHECK_SELF_REFERENCE(tv->vval.v_list, lv_copyID, copyID,
-                                          kMPConvList);
+                                             kMPConvList);
       TYPVAL_ENCODE_CONV_LIST_START(tv->vval.v_list->lv_len);
       _mp_push(*mpstack, ((MPConvStackVal) {
         .type = kMPConvList,
@@ -392,8 +434,9 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
             if (val_di->di_tv.v_type != VAR_LIST) {
               goto _convert_one_value_regular_dict;
             }
-            _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(val_di->di_tv.vval.v_list,
+            _TYPVAL_ENCODE_DO_CHECK_SELF_REFERENCE(val_di->di_tv.vval.v_list,
-                                                lv_copyID, copyID, kMPConvList);
+                                                   lv_copyID, copyID,
+                                                   kMPConvList);
             TYPVAL_ENCODE_CONV_LIST_START(val_di->di_tv.vval.v_list->lv_len);
             _mp_push(*mpstack, ((MPConvStackVal) {
               .tv = tv,
@@ -423,8 +466,8 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
                 goto _convert_one_value_regular_dict;
               }
             }
-            _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(val_list, lv_copyID, copyID,
+            _TYPVAL_ENCODE_DO_CHECK_SELF_REFERENCE(val_list, lv_copyID, copyID,
-                                                kMPConvPairs);
+                                                   kMPConvPairs);
             TYPVAL_ENCODE_CONV_DICT_START(val_list->lv_len);
             _mp_push(*mpstack, ((MPConvStackVal) {
               .tv = tv,
@@ -464,8 +507,8 @@ static int _TYPVAL_ENCODE_CONVERT_ONE_VALUE(
         break;
       }
 _convert_one_value_regular_dict:
-      _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(tv->vval.v_dict, dv_copyID, copyID,
+      _TYPVAL_ENCODE_DO_CHECK_SELF_REFERENCE(tv->vval.v_dict, dv_copyID, copyID,
-                                          kMPConvDict);
+                                             kMPConvDict);
       TYPVAL_ENCODE_CONV_DICT_START(tv->vval.v_dict->dv_hashtab.ht_used);
       _mp_push(*mpstack, ((MPConvStackVal) {
         .tv = tv,
@@ -491,7 +534,7 @@ _convert_one_value_regular_dict:
 TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
     TYPVAL_ENCODE_FIRST_ARG_TYPE TYPVAL_ENCODE_FIRST_ARG_NAME,
     typval_T *const tv, const char *const objname)
-  REAL_FATTR_WARN_UNUSED_RESULT;
+  REAL_FATTR_NONNULL_ARG(2, 3) REAL_FATTR_WARN_UNUSED_RESULT;
 
 /// Convert the whole typval
 ///
@@ -510,6 +553,7 @@ TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
   MPConvStack mpstack;
   _mp_init(mpstack);
   if (_TYPVAL_ENCODE_CONVERT_ONE_VALUE(TYPVAL_ENCODE_FIRST_ARG_NAME, &mpstack,
+                                       NULL,
                                        tv, copyID, objname)
       == FAIL) {
     goto encode_vim_to__error_ret;
@@ -566,7 +610,7 @@ TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
         TYPVAL_ENCODE_CONV_SPECIAL_DICT_KEY_CHECK(
             encode_vim_to__error_ret, kv_pair->lv_first->li_tv);
         if (_TYPVAL_ENCODE_CONVERT_ONE_VALUE(TYPVAL_ENCODE_FIRST_ARG_NAME,
-                                             &mpstack,
+                                             &mpstack, cur_mpsv,
                                              &kv_pair->lv_first->li_tv,
                                              copyID,
                                              objname) == FAIL) {
@@ -587,7 +631,7 @@ TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
               TYPVAL_ENCODE_CONV_LIST_START(pt->pt_argc);
               _mp_push(mpstack, ((MPConvStackVal) {
                 .type = kMPConvPartialList,
-                .tv = tv,
+                .tv = NULL,
                 .data = {
                   .a = {
                     .arg = pt->pt_argv,
@@ -604,10 +648,21 @@ TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
             dict_T *const dict = pt == NULL ? NULL : pt->pt_dict;
             if (dict != NULL) {
               TYPVAL_ENCODE_CONV_FUNC_BEFORE_SELF(dict->dv_hashtab.ht_used);
+              const int te_csr_ret = _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(
+                  TYPVAL_ENCODE_FIRST_ARG_NAME,
+                  dict, &dict->dv_copyID, &mpstack, copyID, kMPConvDict,
+                  objname);
+              if (te_csr_ret != NOTDONE) {
+                if (te_csr_ret == FAIL) {
+                  goto encode_vim_to__error_ret;
+                } else {
+                  continue;
+                }
+              }
               TYPVAL_ENCODE_CONV_DICT_START(dict->dv_hashtab.ht_used);
               _mp_push(mpstack, ((MPConvStackVal) {
                 .type = kMPConvDict,
-                .tv = tv,
+                .tv = NULL,
                 .data = {
                   .d = {
                     .dict = dict,
@@ -644,7 +699,8 @@ TYPVAL_ENCODE_SCOPE int _TYPVAL_ENCODE_ENCODE(
     }
     assert(cur_tv != NULL);
     if (_TYPVAL_ENCODE_CONVERT_ONE_VALUE(TYPVAL_ENCODE_FIRST_ARG_NAME, &mpstack,
-                                         cur_tv, copyID, objname) == FAIL) {
+                                         cur_mpsv, cur_tv, copyID, objname)
+        == FAIL) {
       goto encode_vim_to__error_ret;
     }
   }

src/nvim/eval/typval_encode.h

@@ -249,16 +249,26 @@ static inline size_t tv_strlen(const typval_T *const tv)
 ///                      copyID (variable) it is set to copyID.
 /// @param[in]  copyID  CopyID used by the caller.
 /// @param  conv_type  Type of the conversion, @see MPConvStackValType.
-#define _TYPVAL_ENCODE_CHECK_SELF_REFERENCE(val, copyID_attr, copyID, \
+#define _TYPVAL_ENCODE_DO_CHECK_SELF_REFERENCE(val, copyID_attr, copyID, \
-                                            conv_type) \
+                                               conv_type) \
     do { \
-      if ((val)->copyID_attr == (copyID)) { \
+      const int te_csr_ret = _TYPVAL_ENCODE_CHECK_SELF_REFERENCE( \
-        TYPVAL_ENCODE_CONV_RECURSE((val), conv_type); \
+          TYPVAL_ENCODE_FIRST_ARG_NAME, \
-        return OK; \
+          (val), &(val)->copyID_attr, mpstack, copyID, conv_type, objname); \
+      if (te_csr_ret != NOTDONE) { \
+        return te_csr_ret; \
       } \
-      (val)->copyID_attr = (copyID); \
     } while (0)
 
+#define _TYPVAL_ENCODE_CHECK_SELF_REFERENCE_INNER_2(name) \
+    _typval_encode_##name##_check_self_reference
+#define _TYPVAL_ENCODE_CHECK_SELF_REFERENCE_INNER(name) \
+    _TYPVAL_ENCODE_CHECK_SELF_REFERENCE_INNER_2(name)
+
+/// Self reference checker function name
+#define _TYPVAL_ENCODE_CHECK_SELF_REFERENCE \
+    _TYPVAL_ENCODE_CHECK_SELF_REFERENCE_INNER(TYPVAL_ENCODE_NAME)
+
 #define _TYPVAL_ENCODE_ENCODE_INNER_2(name) encode_vim_to_##name
 #define _TYPVAL_ENCODE_ENCODE_INNER(name) _TYPVAL_ENCODE_ENCODE_INNER_2(name)
 

test/functional/eval/string_spec.lua

@@ -9,6 +9,7 @@ local redir_exec = helpers.redir_exec
 local funcs = helpers.funcs
 local write_file = helpers.write_file
 local NIL = helpers.NIL
+local source = helpers.source
 
 describe('string() function', function()
   before_each(clear)
@@ -137,6 +138,18 @@ describe('string() function', function()
     it('dumps references to script functions', function()
       eq('function(\'<SNR>1_Test2\')', eval('string(Test2_f)'))
     end)
+
+    it('dumps partials with self referencing a partial', function()
+      source([[
+        function TestDict() dict
+        endfunction
+        let d = {}
+        let TestDictRef = function('TestDict', d)
+        let d.tdr = TestDictRef
+      ]])
+      eq("\nE724: unable to correctly dump variable with self-referencing container\nfunction('TestDict', {'tdr': function('TestDict', {E724@1})})",
+         redir_exec('echo string(d.tdr)'))
+    end)
   end)
 
   describe('used to represent lists', function()