Crash with signcolumn=number and numberwidth=1

[godlygeek]

Problem

I caught a:

*** Error in `nvim': free(): invalid next size (normal): 0x000000000260b430 ***

and abort in neovim 0.10.0

I was able to use Valgrind to track down the location:

==29288== Invalid write of size 4
==29288==    at 0x4DE07A: draw_sign (drawline.c:468)
==29288==    by 0x4DE469: draw_lnum_col (drawline.c:553)
==29288==    by 0x4E1A5B: win_line (drawline.c:1547)
==29288==    by 0x4EBC1C: win_update (drawscreen.c:2286)
==29288==    by 0x4E73E5: update_screen (drawscreen.c:647)
==29288==    by 0x65995B: normal_redraw (normal.c:1353)
==29288==    by 0x659BE7: normal_check (normal.c:1455)
==29288==    by 0x733879: state_enter (state.c:40)
==29288==    by 0x657839: normal_enter (normal.c:518)
==29288==    by 0x60027B: main (main.c:664)
==29288==  Address 0x5e5c93c is 4 bytes before a block of size 1,096 alloc'd
==29288==    at 0x402D7BB: malloc (vg_replace_malloc.c:393)
==29288==    by 0x638A59: try_malloc (memory.c:98)
==29288==    by 0x638AD0: xmalloc (memory.c:132)
==29288==    by 0x5A8192: grid_alloc (grid.c:860)
==29288==    by 0x4E6686: default_grid_alloc (drawscreen.c:202)
==29288==    by 0x4E6A51: screen_resize (drawscreen.c:308)
==29288==    by 0x773A45: ui_refresh (ui.c:241)
==29288==    by 0x773FC6: ui_attach_impl (ui.c:401)
==29288==    by 0x486AA8: nvim_ui_attach (ui.c:194)
==29288==    by 0x474409: handle_nvim_ui_attach (dispatch_wrappers.generated.h:6155)
==29288==    by 0x6530A9: request_event (channel.c:407)
==29288==    by 0x55564E: multiqueue_process_events (multiqueue.c:149)

The problem is that, in draw_sign, the computed sign_pos can wind up being negative if numberwidth=1 and signcolumn=number.

I think this only happens with an extmark, not a normal sign.

Steps to reproduce

Add this patch to provoke a crash if sign_pos ever winds up negative:

diff --git a/src/nvim/drawline.c b/src/nvim/drawline.c
index 283f7d9d6..39004d780 100644
--- a/src/nvim/drawline.c
+++ b/src/nvim/drawline.c
@@ -465,6 +465,7 @@ static void draw_sign(bool nrcol, win_T *wp, winlinevars_T *wlv, int sign_idx, i
     int fill = nrcol ? number_width(wp) + 1 : SIGN_WIDTH;
     draw_col_fill(wlv, schar_from_ascii(' '), fill, attr);
     int sign_pos = wlv->off - SIGN_WIDTH - (int)nrcol;
+    assert(sign_pos >= 0);
     linebuf_char[sign_pos] = sattr.text[0];
     linebuf_char[sign_pos + 1] = sattr.text[1];
   } else {

Then make a debug build (CMAKE_BUILD_TYPE=Debug) and run with:

vim --clean -c 'set number numberwidth=1 signcolumn=number' -c 'au VimEnter * call nvim_buf_set_extmark(1, nvim_create_namespace(""), 0, 0, {"sign_text": "X"})'

Note that it doesn't reproduce without the VimEnter autocommand. If you just do -c 'call nvim_buf_set_extmark(...)' the number column winds up being grown to 2 spaces wide, instead. I think what's happening is that when the sign is placed asynchronously, the number column isn't being grown, and neovim is instead trying to draw a 2 character wide sign in a 1 character wide column, and winds up overwriting the byte before the start of the buffer.

Expected behavior

No crash 😄

Neovim version (nvim -v)

0.10.0 commit 27fb629

Vim (not Nvim) behaves the same?

n/a (no extmarks)

Operating system/version

RHEL 7

Terminal name/version

wsltty

$TERM environment variable

screen-256color

Installation

build from repo

[luukvbaal]

number_width()'s cached value is incorrect in this repro. Here's one way to fix the crash by making sure the numberwidth cache is invalidated(there might be a better way):

@@ -189,6 +190,13 @@ void buf_put_decor_sh(buf_T *buf, DecorSignHighlight *sh, int row1, int row2)
 {
   if (sh->flags & kSHIsSign) {
     sh->sign_add_id = sign_add_id++;
+    if (buf_meta_total(buf, kMTMetaSignText) == 1) {
+      FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
+        if (wp->w_buffer == buf) {
+          wp->w_nrwidth_line_count = 0;
+        }
+      }
+    }
     if (sh->text[0]) {
       buf_signcols_count_range(buf, row1, row2, 1, kFalse);
     }

Or just directly set the numberwidth:

if (wp->w_buffer == buf && wp->w_minscwidth == SCL_NUM && wp->w_nrwidth_width < 2) {
  wp->w_nrwidth_width = 2;
}