Merge pull request #36773 from zeertzjq/funccal-refcount

fix(eval): fix crash with :breakadd expr when calling user func

src/nvim/debugger.c

@@ -828,17 +828,21 @@ static linenr_T debuggy_find(bool file, char *fname, linenr_T after, garray_T *g
       typval_T *const tv = eval_expr_no_emsg(bp);
       if (tv != NULL) {
         if (bp->dbg_val == NULL) {
+          xfree(debug_oldval);
           debug_oldval = typval_tostring(NULL, true);
           bp->dbg_val = tv;
+          xfree(debug_newval);
           debug_newval = typval_tostring(bp->dbg_val, true);
           line = true;
         } else {
           if (typval_compare(tv, bp->dbg_val, EXPR_IS, false) == OK
               && tv->vval.v_number == false) {
             line = true;
+            xfree(debug_oldval);
             debug_oldval = typval_tostring(bp->dbg_val, true);
             // Need to evaluate again, typval_compare() overwrites "tv".
             typval_T *const v = eval_expr_no_emsg(bp);
+            xfree(debug_newval);
             debug_newval = typval_tostring(v, true);
             tv_free(bp->dbg_val);
             bp->dbg_val = v;
@@ -846,7 +850,9 @@ static linenr_T debuggy_find(bool file, char *fname, linenr_T after, garray_T *g
           tv_free(tv);
         }
       } else if (bp->dbg_val != NULL) {
+        xfree(debug_oldval);
         debug_oldval = typval_tostring(bp->dbg_val, true);
+        xfree(debug_newval);
         debug_newval = typval_tostring(NULL, true);
         tv_free(bp->dbg_val);
         bp->dbg_val = NULL;

src/nvim/eval/userfunc.c

@@ -3906,41 +3906,57 @@ funccall_T *get_funccal(void)
   return funccal;
 }
 
+/// @return  dict used for local variables in the current funccal or
+///          NULL if there is no current funccal.
+dict_T *get_funccal_local_dict(void)
+{
+  if (current_funccal == NULL || current_funccal->fc_l_vars.dv_refcount == 0) {
+    return NULL;
+  }
+  return &get_funccal()->fc_l_vars;
+}
+
 /// @return  hashtable used for local variables in the current funccal or
 ///          NULL if there is no current funccal.
 hashtab_T *get_funccal_local_ht(void)
 {
-  if (current_funccal == NULL) {
-    return NULL;
-  }
-  return &get_funccal()->fc_l_vars.dv_hashtab;
+  dict_T *d = get_funccal_local_dict();
+  return d != NULL ? &d->dv_hashtab : NULL;
 }
 
 /// @return   the l: scope variable or
 ///           NULL if there is no current funccal.
 dictitem_T *get_funccal_local_var(void)
 {
-  if (current_funccal == NULL) {
+  if (current_funccal == NULL || current_funccal->fc_l_vars.dv_refcount == 0) {
     return NULL;
   }
   return (dictitem_T *)&get_funccal()->fc_l_vars_var;
 }
 
+/// @return  the dict used for argument in the current funccal or
+///          NULL if there is no current funccal.
+dict_T *get_funccal_args_dict(void)
+{
+  if (current_funccal == NULL || current_funccal->fc_l_vars.dv_refcount == 0) {
+    return NULL;
+  }
+  return &get_funccal()->fc_l_avars;
+}
+
 /// @return  the hashtable used for argument in the current funccal or
 ///          NULL if there is no current funccal.
 hashtab_T *get_funccal_args_ht(void)
 {
-  if (current_funccal == NULL) {
-    return NULL;
-  }
-  return &get_funccal()->fc_l_avars.dv_hashtab;
+  dict_T *d = get_funccal_args_dict();
+  return d != NULL ? &d->dv_hashtab : NULL;
 }
 
 /// @return  the a: scope variable or
 ///          NULL if there is no current funccal.
 dictitem_T *get_funccal_args_var(void)
 {
-  if (current_funccal == NULL) {
+  if (current_funccal == NULL || current_funccal->fc_l_vars.dv_refcount == 0) {
     return NULL;
   }
   return (dictitem_T *)&get_funccal()->fc_l_avars_var;
@@ -3949,7 +3965,7 @@ dictitem_T *get_funccal_args_var(void)
 /// List function variables, if there is a function.
 void list_func_vars(int *first)
 {
-  if (current_funccal != NULL) {
+  if (current_funccal != NULL && current_funccal->fc_l_vars.dv_refcount > 0) {
     list_hashtable_vars(&current_funccal->fc_l_vars.dv_hashtab, "l:", false,
                         first);
   }

src/nvim/eval/vars.c

@@ -2483,7 +2483,6 @@ dictitem_T *find_var_in_ht(hashtab_T *const ht, int htname, const char *const va
 static hashtab_T *find_var_ht_dict(const char *name, const size_t name_len, const char **varname,
                                    dict_T **d)
 {
-  funccall_T *funccal = get_funccal();
   *d = NULL;
 
   if (name_len == 0) {
@@ -2503,11 +2502,12 @@ static hashtab_T *find_var_ht_dict(const char *name, const size_t name_len, cons
       return &compat_hashtab;
     }
 
-    if (funccal == NULL) {  // global variable
-      *d = get_globvar_dict();
-    } else {  // l: variable
-      *d = &funccal->fc_l_vars;
+    *d = get_funccal_local_dict();
+    if (*d != NULL) {  // local variable
+      goto end;
     }
+
+    *d = get_globvar_dict();  // global variable
     goto end;
   }
 
@@ -2529,10 +2529,10 @@ static hashtab_T *find_var_ht_dict(const char *name, const size_t name_len, cons
     *d = curtab->tp_vars;
   } else if (*name == 'v') {  // v: variable
     *d = get_vimvar_dict();
-  } else if (*name == 'a' && funccal != NULL) {  // function argument
-    *d = &funccal->fc_l_avars;
-  } else if (*name == 'l' && funccal != NULL) {  // local variable
-    *d = &funccal->fc_l_vars;
+  } else if (*name == 'a') {  // a: function argument
+    *d = get_funccal_args_dict();
+  } else if (*name == 'l') {  // l: local variable
+    *d = get_funccal_local_dict();
   } else if (*name == 's'  // script variable
              && (current_sctx.sc_sid > 0 || current_sctx.sc_sid == SID_STR
                  || current_sctx.sc_sid == SID_LUA)

test/functional/legacy/debugger_spec.lua

@@ -13,64 +13,112 @@ describe('debugger', function()
   local screen
 
   before_each(function()
-    screen = Screen.new(999, 10)
+    screen = Screen.new(999, 7)
   end)
 
   -- oldtest: Test_Debugger_breakadd_expr()
+  -- This doesn't seem to work as documented. The breakpoint is not
+  -- triggered until the next function call.
   it(':breakadd expr', function()
-    write_file('XdebugBreakExpr.vim', 'let g:Xtest_var += 1')
+    write_file(
+      'XbreakExpr.vim',
+      [[
+      func Foo()
+        eval 1
+        eval 2
+      endfunc
+
+      let g:Xtest_var += 1
+      call Foo()
+      let g:Xtest_var += 1
+      call Foo()]]
+    )
     finally(function()
-      os.remove('XdebugBreakExpr.vim')
+      os.remove('XbreakExpr.vim')
     end)
 
-    command('edit XdebugBreakExpr.vim')
+    command('edit XbreakExpr.vim')
     command(':let g:Xtest_var = 10')
     command(':breakadd expr g:Xtest_var')
-    feed(':source %<CR>')
-    screen:expect {
-      grid = [[
-      ^let g:Xtest_var += 1{MATCH: *}|
-      {1:~{MATCH: *}}|*8
-      :source %{MATCH: *}|
-    ]],
-    }
-    feed(':source %<CR>')
-    screen:expect {
-      grid = [[
+    local initial_screen = [[
+      ^func Foo(){MATCH: *}|
+        eval 1{MATCH: *}|
+        eval 2{MATCH: *}|
+      endfunc{MATCH: *}|
+      {MATCH: *}|
       let g:Xtest_var += 1{MATCH: *}|
-      {1:~{MATCH: *}}|
-      {3:{MATCH: *}}|
-      Breakpoint in "{MATCH:.*}XdebugBreakExpr.vim" line 1{MATCH: *}|
+      {MATCH: *}|
+    ]]
+    screen:expect(initial_screen)
+
+    feed(':source %<CR>')
+    screen:expect([[
+      Breakpoint in "Foo" line 1{MATCH: *}|
       Entering Debug mode.  Type "cont" to continue.{MATCH: *}|
       Oldval = "10"{MATCH: *}|
       Newval = "11"{MATCH: *}|
-      {MATCH:.*}XdebugBreakExpr.vim{MATCH: *}|
-      line 1: let g:Xtest_var += 1{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[7]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
       >^{MATCH: *}|
-    ]],
-    }
+    ]])
     feed('cont<CR>')
-    screen:expect {
-      grid = [[
-      ^let g:Xtest_var += 1{MATCH: *}|
-      {1:~{MATCH: *}}|*8
-      {MATCH: *}|
-    ]],
-    }
-    feed(':source %<CR>')
-    screen:expect {
-      grid = [[
-      let g:Xtest_var += 1{MATCH: *}|
-      {1:~{MATCH: *}}|
-      {3:{MATCH: *}}|
-      Breakpoint in "{MATCH:.*}XdebugBreakExpr.vim" line 1{MATCH: *}|
-      Entering Debug mode.  Type "cont" to continue.{MATCH: *}|
+    screen:expect([[
+      >cont{MATCH: *}|
+      Breakpoint in "Foo" line 1{MATCH: *}|
       Oldval = "11"{MATCH: *}|
       Newval = "12"{MATCH: *}|
-      {MATCH:.*}XdebugBreakExpr.vim{MATCH: *}|
-      line 1: let g:Xtest_var += 1{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[9]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
       >^{MATCH: *}|
-    ]],
-    }
+    ]])
+    feed('cont<CR>')
+    screen:expect(initial_screen)
+
+    -- Check the behavior without the g: prefix.
+    -- The Oldval and Newval don't look right here.
+    command(':breakdel *')
+    command(':breakadd expr Xtest_var')
+    feed(':source %<CR>')
+    screen:expect([[
+      Breakpoint in "Foo" line 1{MATCH: *}|
+      Entering Debug mode.  Type "cont" to continue.{MATCH: *}|
+      Oldval = "13"{MATCH: *}|
+      Newval = "(does not exist)"{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[7]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
+      >^{MATCH: *}|
+    ]])
+    feed('cont<CR>')
+    screen:expect([[
+      {MATCH:.*}XbreakExpr.vim[7]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
+      >cont{MATCH: *}|
+      Breakpoint in "Foo" line 2{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[7]..function Foo{MATCH: *}|
+      line 2: eval 2{MATCH: *}|
+      >^{MATCH: *}|
+    ]])
+    feed('cont<CR>')
+    screen:expect([[
+      >cont{MATCH: *}|
+      Breakpoint in "Foo" line 1{MATCH: *}|
+      Oldval = "14"{MATCH: *}|
+      Newval = "(does not exist)"{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[9]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
+      >^{MATCH: *}|
+    ]])
+    feed('cont<CR>')
+    screen:expect([[
+      {MATCH:.*}XbreakExpr.vim[9]..function Foo{MATCH: *}|
+      line 1: eval 1{MATCH: *}|
+      >cont{MATCH: *}|
+      Breakpoint in "Foo" line 2{MATCH: *}|
+      {MATCH:.*}XbreakExpr.vim[9]..function Foo{MATCH: *}|
+      line 2: eval 2{MATCH: *}|
+      >^{MATCH: *}|
+    ]])
+    feed('cont<CR>')
+    screen:expect(initial_screen)
   end)
 end)