vim-patch:partial:9.0.0013: reproducing memory access errors can be difficult

Problem:    Reproducing memory access errors can be difficult.
Solution:   When testing, copy each line to allocated memory, so that valgrind
            can detect accessing memory before and/or after it.  Fix uncovered
            problems.

fa4873ccfc

Since test_override() is N/A, enable ml_get_alloc_lines when ASAN is
enabled instead, so it also applies to functional tests.

Use xstrdup() to copy the line as ml_line_len looks hard to port.

Squash the test changes from patch 9.0.0016.

Co-authored-by: Bram Moolenaar <Bram@vim.org>

src/nvim/change.c

@@ -1162,12 +1162,16 @@ int open_line(int dir, int flags, int second_line_indent, bool *did_do_comment)
               if (p[0] == '/' && p[-1] == '*') {
                 // End of C comment, indent should line up
                 // with the line containing the start of
-                // the comment
+                // the comment.
                 curwin->w_cursor.col = (colnr_T)(p - ptr);
                 if ((pos = findmatch(NULL, NUL)) != NULL) {
                   curwin->w_cursor.lnum = pos->lnum;
                   newindent = get_indent();
+                  break;
                 }
+                // this may make "ptr" invalid, get it again
+                ptr = ml_get(curwin->w_cursor.lnum);
+                p = ptr + curwin->w_cursor.col;
               }
             }
           }

src/nvim/edit.c

@@ -4653,9 +4653,9 @@ int ins_copychar(linenr_T lnum)
   }
 
   // try to advance to the cursor column
+  validate_virtcol();
   line = ml_get(lnum);
   prev_ptr = line;
-  validate_virtcol();
 
   chartabsize_T cts;
   init_chartabsize_arg(&cts, curwin, lnum, 0, line, line);

src/nvim/indent_c.c

@@ -2529,8 +2529,6 @@ int get_c_indent(void)
                 break;
               }
 
-              l = get_cursor_line_ptr();
-
               // If we're in a comment or raw string now, skip to
               // the start of it.
               trypos = ind_find_start_CORS(NULL);
@@ -2540,9 +2538,9 @@ int get_c_indent(void)
                 continue;
               }
 
-              //
+              l = get_cursor_line_ptr();
+
               // Skip preprocessor directives and blank lines.
-              //
               if (cin_ispreproc_cont(&l, &curwin->w_cursor.lnum, &amount)) {
                 continue;
               }
@@ -2640,8 +2638,6 @@ int get_c_indent(void)
                   break;
                 }
 
-                l = get_cursor_line_ptr();
-
                 // If we're in a comment or raw string now, skip
                 // to the start of it.
                 trypos = ind_find_start_CORS(NULL);
@@ -2651,6 +2647,8 @@ int get_c_indent(void)
                   continue;
                 }
 
+                l = get_cursor_line_ptr();
+
                 // Skip preprocessor directives and blank lines.
                 if (cin_ispreproc_cont(&l, &curwin->w_cursor.lnum, &amount)) {
                   continue;
@@ -2916,12 +2914,16 @@ int get_c_indent(void)
               trypos = NULL;
             }
 
+            l = get_cursor_line_ptr();
+
             // If we are looking for ',', we also look for matching
             // braces.
-            if (trypos == NULL && terminated == ','
+            if (trypos == NULL && terminated == ',') {
-                && find_last_paren(l, '{', '}')) {
+              if (find_last_paren(l, '{', '}')) {
                 trypos = find_start_brace();
               }
+              l = get_cursor_line_ptr();
+            }
 
             if (trypos != NULL) {
               // Check if we are on a case label now.  This is
@@ -2951,6 +2953,7 @@ int get_c_indent(void)
                 curwin->w_cursor.lnum--;
                 curwin->w_cursor.col = 0;
               }
+              l = get_cursor_line_ptr();
             }
 
             // Get indent and pointer to text for current line,

src/nvim/memline.c

@@ -244,6 +244,10 @@ typedef enum {
 # include "memline.c.generated.h"
 #endif
 
+#if __has_feature(address_sanitizer)
+# define ML_GET_ALLOC_LINES
+#endif
+
 /// Open a new memline for "buf".
 ///
 /// @return  FAIL for failure, OK otherwise.
@@ -535,7 +539,8 @@ void ml_close(buf_T *buf, int del_file)
     return;
   }
   mf_close(buf->b_ml.ml_mfp, del_file);       // close the .swp file
-  if (buf->b_ml.ml_line_lnum != 0 && (buf->b_ml.ml_flags & ML_LINE_DIRTY)) {
+  if (buf->b_ml.ml_line_lnum != 0
+      && (buf->b_ml.ml_flags & (ML_LINE_DIRTY | ML_ALLOCATED))) {
     xfree(buf->b_ml.ml_line_ptr);
   }
   xfree(buf->b_ml.ml_stack);
@@ -1780,7 +1785,6 @@ char *ml_get_buf(buf_T *buf, linenr_T lnum, bool will_change)
       recursive--;
     }
     ml_flush_line(buf);
-    buf->b_ml.ml_flags &= ~ML_LINE_DIRTY;
 errorret:
     STRCPY(questions, "???");
     buf->b_ml.ml_line_lnum = lnum;
@@ -1824,18 +1828,36 @@ errorret:
     char *ptr = (char *)dp + (dp->db_index[lnum - buf->b_ml.ml_locked_low] & DB_INDEX_MASK);
     buf->b_ml.ml_line_ptr = ptr;
     buf->b_ml.ml_line_lnum = lnum;
-    buf->b_ml.ml_flags &= ~ML_LINE_DIRTY;
+    buf->b_ml.ml_flags &= ~(ML_LINE_DIRTY | ML_ALLOCATED);
   }
   if (will_change) {
     buf->b_ml.ml_flags |= (ML_LOCKED_DIRTY | ML_LOCKED_POS);
+#ifdef ML_GET_ALLOC_LINES
+    if (buf->b_ml.ml_flags & ML_ALLOCATED) {
+      // can't make the change in the data block
+      buf->b_ml.ml_flags |= ML_LINE_DIRTY;
+    }
+#endif
     ml_add_deleted_len_buf(buf, buf->b_ml.ml_line_ptr, -1);
   }
 
+#ifdef ML_GET_ALLOC_LINES
+  if ((buf->b_ml.ml_flags & (ML_LINE_DIRTY | ML_ALLOCATED)) == 0) {
+    // make sure the text is in allocated memory
+    buf->b_ml.ml_line_ptr = xstrdup(buf->b_ml.ml_line_ptr);
+    buf->b_ml.ml_flags |= ML_ALLOCATED;
+    if (will_change) {
+      // can't make the change in the data block
+      buf->b_ml.ml_flags |= ML_LINE_DIRTY;
+    }
+  }
+#endif
   return buf->b_ml.ml_line_ptr;
 }
 
 /// Check if a line that was just obtained by a call to ml_get
 /// is in allocated memory.
+/// This ignores ML_ALLOCATED to get the same behavior as without ML_GET_ALLOC_LINES.
 int ml_line_alloced(void)
 {
   return curbuf->b_ml.ml_flags & ML_LINE_DIRTY;
@@ -2352,24 +2374,23 @@ int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy)
     return FAIL;
   }
 
-  bool readlen = true;
-
   if (copy) {
     line = xstrdup(line);
   }
-  if (buf->b_ml.ml_line_lnum != lnum) {  // other line buffered
-    ml_flush_line(buf);  // flush it
-  } else if (buf->b_ml.ml_flags & ML_LINE_DIRTY) {  // same line allocated
-    ml_add_deleted_len_buf(buf, buf->b_ml.ml_line_ptr, -1);
-    readlen = false;  // already added the length
 
-    xfree(buf->b_ml.ml_line_ptr);  // free it
+  if (buf->b_ml.ml_line_lnum != lnum) {
+    // another line is buffered, flush it
+    ml_flush_line(buf);
   }
 
-  if (readlen && kv_size(buf->update_callbacks)) {
+  if (kv_size(buf->update_callbacks)) {
     ml_add_deleted_len_buf(buf, ml_get_buf(buf, lnum, false), -1);
   }
 
+  if (buf->b_ml.ml_flags & (ML_LINE_DIRTY | ML_ALLOCATED)) {
+    xfree(buf->b_ml.ml_line_ptr);  // free allocated line
+  }
+
   buf->b_ml.ml_line_ptr = line;
   buf->b_ml.ml_line_lnum = lnum;
   buf->b_ml.ml_flags = (buf->b_ml.ml_flags | ML_LINE_DIRTY) & ~ML_EMPTY;
@@ -2692,8 +2713,11 @@ static void ml_flush_line(buf_T *buf)
     xfree(new_line);
 
     entered = false;
+  } else if (buf->b_ml.ml_flags & ML_ALLOCATED) {
+    xfree(buf->b_ml.ml_line_ptr);
   }
 
+  buf->b_ml.ml_flags &= ~(ML_LINE_DIRTY | ML_ALLOCATED);
   buf->b_ml.ml_line_lnum = 0;
   buf->b_ml.ml_line_offset = 0;
 }

src/nvim/memline_defs.h

@@ -49,10 +49,11 @@ typedef struct memline {
   int ml_stack_top;             // current top of ml_stack
   int ml_stack_size;            // total number of entries in ml_stack
 
-#define ML_EMPTY        1       // empty buffer
+#define ML_EMPTY        0x01    // empty buffer
-#define ML_LINE_DIRTY   2       // cached line was changed and allocated
+#define ML_LINE_DIRTY   0x02    // cached line was changed and allocated
-#define ML_LOCKED_DIRTY 4       // ml_locked was changed
+#define ML_LOCKED_DIRTY 0x04    // ml_locked was changed
-#define ML_LOCKED_POS   8       // ml_locked needs positive block number
+#define ML_LOCKED_POS   0x08    // ml_locked needs positive block number
+#define ML_ALLOCATED    0x10    // ml_line_ptr is an allocated copy
   int ml_flags;
 
   linenr_T ml_line_lnum;        // line number of cached line, 0 if not valid

src/nvim/testdir/test_breakindent.vim

@@ -10,7 +10,9 @@ CheckOption breakindent
 source view_util.vim
 source screendump.vim
 
-let s:input ="\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP"
+func SetUp()
+  let s:input ="\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP"
+endfunc
 
 func s:screen_lines(lnum, width) abort
   return ScreenLines([a:lnum, a:lnum + 2], a:width)
@@ -837,12 +839,12 @@ func Test_breakindent20_list()
   call s:compare_lines(expect, lines)
   " check formatlistpat indent with different list levels
   let &l:flp = '^\s*\*\+\s\+'
-  redraw!
   %delete _
   call setline(1, ['* Congress shall make no law',
         \ '*** Congress shall make no law',
         \ '**** Congress shall make no law'])
   norm! 1gg
+  redraw!
   let expect = [
 	\ "* Congress shall    ",
 	\ "  make no law       ",
@@ -956,9 +958,9 @@ func Test_no_spurious_match()
   let @/ = '\%>3v[y]'
   redraw!
   call searchcount().total->assert_equal(1)
+
   " cleanup
   set hls&vim
-  let s:input = "\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP"
   bwipeout!
 endfunc
 
@@ -1023,8 +1025,6 @@ func Test_no_extra_indent()
 endfunc
 
 func Test_breakindent_column()
-  " restore original
-  let s:input ="\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP"
   call s:test_windows('setl breakindent breakindentopt=column:10')
   redraw!
   " 1) default: does not indent, too wide :(

src/nvim/testdir/test_edit.vim

@@ -1890,6 +1890,9 @@ func Test_edit_insertmode_ex_edit()
   call writefile(lines, 'Xtest_edit_insertmode_ex_edit')
 
   let buf = RunVimInTerminal('-S Xtest_edit_insertmode_ex_edit', #{rows: 6})
+  " Somehow this can be very slow with valgrind. A separate TermWait() works
+  " better than a longer time with WaitForAssert() (why?)
+  call TermWait(buf, 1000)
   call WaitForAssert({-> assert_match('^-- INSERT --\s*$', term_getline(buf, 6))})
   call term_sendkeys(buf, "\<C-B>\<C-L>")
   call WaitForAssert({-> assert_notmatch('^-- INSERT --\s*$', term_getline(buf, 6))})