patch 8.1.0613: when executing an insecure function the secure flag is stuck

Problem:    When executing an insecure function the secure flag is stuck.
            (Gabriel Barta)
Solution:   Restore "secure" instead of decrementing it. (closes #3705)

Author: brammool

src/buffer.c

@@ -5519,6 +5519,7 @@ chk_modeline(
 
 	    if (*s != NUL)		/* skip over an empty "::" */
 	    {
+		int secure_save = secure;
 #ifdef FEAT_EVAL
 		save_current_sctx = current_sctx;
 		current_sctx.sc_sid = SID_MODELINE;
@@ -5530,7 +5531,7 @@ chk_modeline(
 
 		retval = do_set(s, OPT_MODELINE | OPT_LOCAL | flags);
 
-		--secure;
+		secure = secure_save;
 #ifdef FEAT_EVAL
 		current_sctx = save_current_sctx;
 #endif

src/option.c

@@ -5214,7 +5214,7 @@ do_set(
 
 			{
 			    long_u *p = insecure_flag(opt_idx, opt_flags);
-			    int	    did_inc_secure = FALSE;
+			    int	    secure_saved = secure;
 
 			    // When an option is set in the sandbox, from a
 			    // modeline or in secure mode, then deal with side
@@ -5227,21 +5227,18 @@ do_set(
 #endif
 				    || (opt_flags & OPT_MODELINE)
 				    || (!value_is_replaced && (*p & P_INSECURE)))
-			    {
-				did_inc_secure = TRUE;
 				++secure;
-			    }
 
-			    // Handle side effects, and set the global value for
-			    // ":set" on local options. Note: when setting 'syntax'
-			    // or 'filetype' autocommands may be triggered that can
-			    // cause havoc.
-			    errmsg = did_set_string_option(opt_idx, (char_u **)varp,
+			    // Handle side effects, and set the global value
+			    // for ":set" on local options. Note: when setting
+			    // 'syntax' or 'filetype' autocommands may be
+			    // triggered that can cause havoc.
+			    errmsg = did_set_string_option(
+				    opt_idx, (char_u **)varp,
 				    new_value_alloced, oldval, errbuf,
 				    opt_flags, &value_checked);
 
-			    if (did_inc_secure)
-				--secure;
+			    secure = secure_saved;
 			}
 
 #if defined(FEAT_EVAL)

src/testdir/test_autocmd.vim

@@ -650,6 +650,28 @@ func Test_OptionSet_diffmode_close()
   "delfunc! AutoCommandOptionSet
 endfunc
 
+func Test_OptionSet_modeline()
+  call test_override('starting', 1)
+  au! OptionSet
+  augroup set_tabstop
+    au OptionSet tabstop call timer_start(1, {-> execute("echo 'Handler called'", "")})
+  augroup END
+  call writefile(['vim: set ts=7 sw=5 :', 'something'], 'XoptionsetModeline')
+  set modeline
+  let v:errmsg = ''
+  call assert_fails('split XoptionsetModeline', 'E12:')
+  call assert_equal(7, &ts)
+  call assert_equal('', v:errmsg)
+
+  augroup set_tabstop
+    au!
+  augroup END
+  bwipe!
+  set ts&
+  call delete('XoptionsetModeline')
+  call test_override('starting', 0)
+endfunc
+
 " Test for Bufleave autocommand that deletes the buffer we are about to edit.
 func Test_BufleaveWithDelete()
   new | edit Xfile1

src/version.c

@@ -799,6 +799,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    613,
 /**/
     612,
 /**/