From 5af90e2ee73efd156d269b9beba6e6d572dba2b7 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Sat, 14 Jul 2018 01:05:01 -0400 Subject: [PATCH 1/5] vim-patch:8.0.0791: terminal colors depend on the system Problem: Terminal colors depend on the system. Solution: Use the highlight color lookup tables. https://github.com/vim/vim/commit/b41bf8e6b45a773456031954bca1bc4212cbffbe --- src/nvim/syntax.c | 173 +++++++++++++++++++++++----------------------- 1 file changed, 88 insertions(+), 85 deletions(-) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index ff47e443f9..6668852e0e 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -62,7 +62,7 @@ struct hl_group { int sg_cterm; ///< "cterm=" highlighting attr int sg_cterm_fg; ///< terminal fg color number + 1 int sg_cterm_bg; ///< terminal bg color number + 1 - int sg_cterm_bold; ///< bold attr was set for light color + bool sg_cterm_bold; ///< bold attr was set for light color // for RGB UIs int sg_gui; ///< "gui=" highlighting attributes ///< (combination of \ref HlAttrFlags) @@ -6385,6 +6385,89 @@ int load_colors(char_u *name) return retval; } +static char *(color_names[28]) = { + "Black", "DarkBlue", "DarkGreen", "DarkCyan", + "DarkRed", "DarkMagenta", "Brown", "DarkYellow", + "Gray", "Grey", "LightGray", "LightGrey", + "DarkGray", "DarkGrey", + "Blue", "LightBlue", "Green", "LightGreen", + "Cyan", "LightCyan", "Red", "LightRed", "Magenta", + "LightMagenta", "Yellow", "LightYellow", "White", "NONE" }; + // indices: + // 0, 1, 2, 3, + // 4, 5, 6, 7, + // 8, 9, 10, 11, + // 12, 13, + // 14, 15, 16, 17, + // 18, 19, 20, 21, 22, + // 23, 24, 25, 26, 27 +static int color_numbers_16[28] = { 0, 1, 2, 3, + 4, 5, 6, 6, + 7, 7, 7, 7, + 8, 8, + 9, 9, 10, 10, + 11, 11, 12, 12, 13, + 13, 14, 14, 15, -1 }; +// for xterm with 88 colors... +static int color_numbers_88[28] = { 0, 4, 2, 6, + 1, 5, 32, 72, + 84, 84, 7, 7, + 82, 82, + 12, 43, 10, 61, + 14, 63, 9, 74, 13, + 75, 11, 78, 15, -1 }; +// for xterm with 256 colors... +static int color_numbers_256[28] = { 0, 4, 2, 6, + 1, 5, 130, 130, + 248, 248, 7, 7, + 242, 242, + 12, 81, 10, 121, + 14, 159, 9, 224, 13, + 225, 11, 229, 15, -1 }; +// for terminals with less than 16 colors... +static int color_numbers_8[28] = { 0, 4, 2, 6, + 1, 5, 3, 3, + 7, 7, 7, 7, + 0+8, 0+8, + 4+8, 4+8, 2+8, 2+8, + 6+8, 6+8, 1+8, 1+8, 5+8, + 5+8, 3+8, 3+8, 7+8, -1 }; + +// Lookup the "cterm" value to be used for color with index "idx" in +// color_names[]. +int lookup_color(const int idx, const bool foreground) +{ + int color = color_numbers_16[idx]; + + // Use the _16 table to check if it's a valid color name. + if (color < 0) { + return -1; + } + + if (t_colors == 8) { + // t_Co is 8: use the 8 colors table + color = color_numbers_8[idx]; + if (foreground) { + // set/reset bold attribute to get light foreground + // colors (on some terminals, e.g. "linux") + if (color & 8) { + HL_TABLE()[idx].sg_cterm |= HL_BOLD; + HL_TABLE()[idx].sg_cterm_bold = true; + } else { + HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; + } + } + color &= 7; // truncate to 8 colors + } else if (t_colors == 16) { + color = color_numbers_8[idx]; + } else if (t_colors == 88) { + color = color_numbers_88[idx]; + } else if (t_colors >= 256) { + color = color_numbers_256[idx]; + } + return color; +} + /// Handle ":highlight" command /// @@ -6651,7 +6734,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) HL_TABLE()[idx].sg_set |= SG_CTERM; } HL_TABLE()[idx].sg_cterm = attr; - HL_TABLE()[idx].sg_cterm_bold = FALSE; + HL_TABLE()[idx].sg_cterm_bold = false; } } else if (*key == 'G') { if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { @@ -6673,7 +6756,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) * flag was set for a light color, reset it now */ if (key[5] == 'F' && HL_TABLE()[idx].sg_cterm_bold) { HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; - HL_TABLE()[idx].sg_cterm_bold = FALSE; + HL_TABLE()[idx].sg_cterm_bold = false; } if (ascii_isdigit(*arg)) { @@ -6695,60 +6778,6 @@ void do_highlight(const char *line, const bool forceit, const bool init) break; } } else { - static const char *color_names[] = { - "Black", "DarkBlue", "DarkGreen", "DarkCyan", - "DarkRed", "DarkMagenta", "Brown", "DarkYellow", - "Gray", "Grey", - "LightGray", "LightGrey", "DarkGray", "DarkGrey", - "Blue", "LightBlue", "Green", "LightGreen", - "Cyan", "LightCyan", "Red", "LightRed", "Magenta", - "LightMagenta", "Yellow", "LightYellow", "White", - "NONE" - }; - static const int color_numbers_16[] = { - 0, 1, 2, 3, - 4, 5, 6, 6, - 7, 7, - 7, 7, 8, 8, - 9, 9, 10, 10, - 11, 11, 12, 12, 13, - 13, 14, 14, 15, - -1 - }; - // For xterm with 88 colors: - static int color_numbers_88[] = { - 0, 4, 2, 6, - 1, 5, 32, 72, - 84, 84, - 7, 7, 82, 82, - 12, 43, 10, 61, - 14, 63, 9, 74, 13, - 75, 11, 78, 15, - -1 - }; - // For xterm with 256 colors: - static int color_numbers_256[] = { - 0, 4, 2, 6, - 1, 5, 130, 130, - 248, 248, - 7, 7, 242, 242, - 12, 81, 10, 121, - 14, 159, 9, 224, 13, - 225, 11, 229, 15, - -1 - }; - // For terminals with less than 16 colors: - static int color_numbers_8[28] = { - 0, 4, 2, 6, - 1, 5, 3, 3, - 7, 7, - 7, 7, 0+8, 0+8, - 4+8, 4+8, 2+8, 2+8, - 6+8, 6+8, 1+8, 1+8, 5+8, - 5+8, 3+8, 3+8, 7+8, - -1 - }; - // Reduce calls to STRICMP a bit, it can be slow. off = TOUPPER_ASC(*arg); for (i = ARRAY_SIZE(color_names); --i >= 0; ) { @@ -6764,33 +6793,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) break; } - // Use the _16 table to check if it's a valid color name. - color = color_numbers_16[i]; - if (color >= 0) { - if (t_colors == 8) { - // t_Co is 8: use the 8 colors table. - color = color_numbers_8[i]; - if (key[5] == 'F') { - /* set/reset bold attribute to get light foreground - * colors (on some terminals, e.g. "linux") */ - if (color & 8) { - HL_TABLE()[idx].sg_cterm |= HL_BOLD; - HL_TABLE()[idx].sg_cterm_bold = true; - } else { - HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; - } - } - color &= 7; // truncate to 8 colors - } else if (t_colors == 16 || t_colors == 88 || t_colors >= 256) { - if (t_colors == 88) { - color = color_numbers_88[i]; - } else if (t_colors >= 256) { - color = color_numbers_256[i]; - } else { - color = color_numbers_8[i]; - } - } - } + color = lookup_color(i, key[5] == 'F'); } // Add one to the argument, to avoid zero. Zero is used for // "NONE", then "color" is -1. @@ -6980,7 +6983,7 @@ static void highlight_clear(int idx) HL_TABLE()[idx].sg_attr = 0; HL_TABLE()[idx].sg_cterm = 0; - HL_TABLE()[idx].sg_cterm_bold = FALSE; + HL_TABLE()[idx].sg_cterm_bold = false; HL_TABLE()[idx].sg_cterm_fg = 0; HL_TABLE()[idx].sg_cterm_bg = 0; HL_TABLE()[idx].sg_gui = 0; From 0c0318f8a7ccbf1b286a4c52fefc168534ff7f5e Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Sat, 14 Jul 2018 01:43:48 -0400 Subject: [PATCH 2/5] vim-patch:8.0.0831: with 8 colors the bold attribute is not set properly Problem: With 8 colors the bold attribute is not set properly. Solution: Move setting HL_TABLE() out of lookup_color. (closes vim/vim#1901) https://github.com/vim/vim/commit/12d853fae1fc37c33874b5cf1e40a2dfaf04268c Use TriState on lookup_color() to avoid 'NOLINT' comments. --- src/nvim/syntax.c | 21 ++++++++++++++++----- src/nvim/syntax.h | 1 + 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index 6668852e0e..86ce259a29 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -6435,7 +6435,9 @@ static int color_numbers_8[28] = { 0, 4, 2, 6, // Lookup the "cterm" value to be used for color with index "idx" in // color_names[]. -int lookup_color(const int idx, const bool foreground) +// "boldp" will be set to TRUE or FALSE for a foreground color when using 8 +// colors, otherwise it will be unchanged. +int lookup_color(const int idx, const bool foreground, TriState *const boldp) { int color = color_numbers_16[idx]; @@ -6451,10 +6453,9 @@ int lookup_color(const int idx, const bool foreground) // set/reset bold attribute to get light foreground // colors (on some terminals, e.g. "linux") if (color & 8) { - HL_TABLE()[idx].sg_cterm |= HL_BOLD; - HL_TABLE()[idx].sg_cterm_bold = true; + *boldp = kTrue; } else { - HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; + *boldp = kFalse; } } color &= 7; // truncate to 8 colors @@ -6793,7 +6794,17 @@ void do_highlight(const char *line, const bool forceit, const bool init) break; } - color = lookup_color(i, key[5] == 'F'); + TriState bold = kNone; + color = lookup_color(i, key[5] == 'F', &bold); + + // set/reset bold attribute to get light foreground + // colors (on some terminals, e.g. "linux") + if (bold == kTrue) { + HL_TABLE()[idx].sg_cterm |= HL_BOLD; + HL_TABLE()[idx].sg_cterm_bold = true; + } else if (bold == kFalse) { + HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; + } } // Add one to the argument, to avoid zero. Zero is used for // "NONE", then "color" is -1. diff --git a/src/nvim/syntax.h b/src/nvim/syntax.h index f8282955a6..9fbad74f64 100644 --- a/src/nvim/syntax.h +++ b/src/nvim/syntax.h @@ -3,6 +3,7 @@ #include +#include "nvim/globals.h" #include "nvim/buffer_defs.h" #include "nvim/ex_cmds_defs.h" From 6b7b56dabe33a9f48c14a5d075b7796c0d2f3bb5 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Wed, 25 Jul 2018 23:36:06 -0400 Subject: [PATCH 3/5] vim-patch:8.0.1072: :highlight command causes a redraw even when nothing changed Problem: The :highlight command causes a redraw even when nothing changed. Solution: Only set "need_highlight_changed" when an attribute changed. https://github.com/vim/vim/commit/99433291b135094d9592c41f96d3ccd60073e2c1 --- src/nvim/syntax.c | 158 +++++++++++++++++++++++++--------------------- 1 file changed, 87 insertions(+), 71 deletions(-) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index 86ce259a29..dcac907c5a 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -53,7 +53,7 @@ static bool did_syntax_onoff = false; struct hl_group { char_u *sg_name; ///< highlight group name char_u *sg_name_u; ///< uppercase of sg_name - int sg_cleared; ///< "hi clear" was used + bool sg_cleared; ///< "hi clear" was used int sg_attr; ///< Screen attr @see ATTR_ENTRY int sg_link; ///< link to this highlight group ID int sg_set; ///< combination of flags in \ref SG_SET @@ -6492,10 +6492,12 @@ void do_highlight(const char *line, const bool forceit, const bool init) int attr; int id; int idx; - int dodefault = FALSE; - int doclear = FALSE; - int dolink = FALSE; - int error = FALSE; + struct hl_group *item; + struct hl_group item_before; + bool dodefault = false; + bool doclear = false; + bool dolink = false; + bool error = false; int color; bool is_normal_group = false; // "Normal" group @@ -6565,6 +6567,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) from_id = syn_check_group((const char_u *)from_start, (int)(from_end - from_start)); + item = &HL_TABLE()[from_id - 1]; if (strncmp(to_start, "NONE", 4) == 0) { to_id = 0; } else { @@ -6572,7 +6575,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) (int)(to_end - to_start)); } - if (from_id > 0 && (!init || HL_TABLE()[from_id - 1].sg_set == 0)) { + if (from_id > 0 && (!init || item->sg_set == 0)) { // Don't allow a link when there already is some highlighting // for the group, unless '!' is used if (to_id > 0 && !forceit && !init @@ -6580,19 +6583,22 @@ void do_highlight(const char *line, const bool forceit, const bool init) if (sourcing_name == NULL && !dodefault) { EMSG(_("E414: group has settings, highlight link ignored")); } - } else { - if (!init) - HL_TABLE()[from_id - 1].sg_set |= SG_LINK; - HL_TABLE()[from_id - 1].sg_link = to_id; - HL_TABLE()[from_id - 1].sg_scriptID = current_SID; - HL_TABLE()[from_id - 1].sg_cleared = false; + } else if (item->sg_link != to_id + || item->sg_scriptID != current_SID + || item->sg_cleared) { + if (!init) { + item->sg_set |= SG_LINK; + } + item->sg_link = to_id; + item->sg_scriptID = current_SID; + item->sg_cleared = false; redraw_all_later(SOME_VALID); + + // Only call highlight changed() once after multiple changes + need_highlight_changed = true; } } - // Only call highlight_changed() once, after sourcing a syntax file. - need_highlight_changed = true; - return; } @@ -6622,19 +6628,23 @@ void do_highlight(const char *line, const bool forceit, const bool init) return; } idx = id - 1; // Index is ID minus one. + item = &HL_TABLE()[idx]; // Return if "default" was used and the group already has settings if (dodefault && hl_has_settings(idx, true)) { return; } - is_normal_group = (STRCMP(HL_TABLE()[idx].sg_name_u, "NORMAL") == 0); + // Make a copy so we can check if any attribute actually changed + item_before = *item; + is_normal_group = (STRCMP(item->sg_name_u, "NORMAL") == 0); // Clear the highlighting for ":hi clear {group}" and ":hi clear". if (doclear || (forceit && init)) { highlight_clear(idx); - if (!doclear) - HL_TABLE()[idx].sg_set = 0; + if (!doclear) { + item->sg_set = 0; + } } char *key = NULL; @@ -6659,9 +6669,9 @@ void do_highlight(const char *line, const bool forceit, const bool init) linep = (const char *)skipwhite((const char_u *)linep); if (strcmp(key, "NONE") == 0) { - if (!init || HL_TABLE()[idx].sg_set == 0) { + if (!init || item->sg_set == 0) { if (!init) { - HL_TABLE()[idx].sg_set |= SG_CTERM+SG_GUI; + item->sg_set |= SG_CTERM+SG_GUI; } highlight_clear(idx); } @@ -6730,34 +6740,34 @@ void do_highlight(const char *line, const bool forceit, const bool init) break; } if (*key == 'C') { - if (!init || !(HL_TABLE()[idx].sg_set & SG_CTERM)) { + if (!init || !(item->sg_set & SG_CTERM)) { if (!init) { - HL_TABLE()[idx].sg_set |= SG_CTERM; + item->sg_set |= SG_CTERM; } - HL_TABLE()[idx].sg_cterm = attr; - HL_TABLE()[idx].sg_cterm_bold = false; + item->sg_cterm = attr; + item->sg_cterm_bold = false; } } else if (*key == 'G') { - if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { + if (!init || !(item->sg_set & SG_GUI)) { if (!init) { - HL_TABLE()[idx].sg_set |= SG_GUI; + item->sg_set |= SG_GUI; } - HL_TABLE()[idx].sg_gui = attr; + item->sg_gui = attr; } } } else if (STRCMP(key, "FONT") == 0) { // in non-GUI fonts are simply ignored } else if (STRCMP(key, "CTERMFG") == 0 || STRCMP(key, "CTERMBG") == 0) { - if (!init || !(HL_TABLE()[idx].sg_set & SG_CTERM)) { + if (!init || !(item->sg_set & SG_CTERM)) { if (!init) { - HL_TABLE()[idx].sg_set |= SG_CTERM; + item->sg_set |= SG_CTERM; } /* When setting the foreground color, and previously the "bold" * flag was set for a light color, reset it now */ - if (key[5] == 'F' && HL_TABLE()[idx].sg_cterm_bold) { - HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; - HL_TABLE()[idx].sg_cterm_bold = false; + if (key[5] == 'F' && item->sg_cterm_bold) { + item->sg_cterm &= ~HL_BOLD; + item->sg_cterm_bold = false; } if (ascii_isdigit(*arg)) { @@ -6800,21 +6810,21 @@ void do_highlight(const char *line, const bool forceit, const bool init) // set/reset bold attribute to get light foreground // colors (on some terminals, e.g. "linux") if (bold == kTrue) { - HL_TABLE()[idx].sg_cterm |= HL_BOLD; - HL_TABLE()[idx].sg_cterm_bold = true; + item->sg_cterm |= HL_BOLD; + item->sg_cterm_bold = true; } else if (bold == kFalse) { - HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; + item->sg_cterm &= ~HL_BOLD; } } // Add one to the argument, to avoid zero. Zero is used for // "NONE", then "color" is -1. if (key[5] == 'F') { - HL_TABLE()[idx].sg_cterm_fg = color + 1; + item->sg_cterm_fg = color + 1; if (is_normal_group) { cterm_normal_fg_color = color + 1; } } else { - HL_TABLE()[idx].sg_cterm_bg = color + 1; + item->sg_cterm_bg = color + 1; if (is_normal_group) { cterm_normal_bg_color = color + 1; if (!ui_rgb_attached()) { @@ -6841,58 +6851,61 @@ void do_highlight(const char *line, const bool forceit, const bool init) } } } else if (strcmp(key, "GUIFG") == 0) { - if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { - if (!init) - HL_TABLE()[idx].sg_set |= SG_GUI; + if (!init || !(item->sg_set & SG_GUI)) { + if (!init) { + item->sg_set |= SG_GUI; + } - xfree(HL_TABLE()[idx].sg_rgb_fg_name); + xfree(item->sg_rgb_fg_name); if (strcmp(arg, "NONE")) { - HL_TABLE()[idx].sg_rgb_fg_name = (char_u *)xstrdup((char *)arg); - HL_TABLE()[idx].sg_rgb_fg = name_to_color((const char_u *)arg); + item->sg_rgb_fg_name = (char_u *)xstrdup((char *)arg); + item->sg_rgb_fg = name_to_color((const char_u *)arg); } else { - HL_TABLE()[idx].sg_rgb_fg_name = NULL; - HL_TABLE()[idx].sg_rgb_fg = -1; + item->sg_rgb_fg_name = NULL; + item->sg_rgb_fg = -1; } } if (is_normal_group) { - normal_fg = HL_TABLE()[idx].sg_rgb_fg; + normal_fg = item->sg_rgb_fg; } } else if (STRCMP(key, "GUIBG") == 0) { - if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { - if (!init) - HL_TABLE()[idx].sg_set |= SG_GUI; + if (!init || !(item->sg_set & SG_GUI)) { + if (!init) { + item->sg_set |= SG_GUI; + } - xfree(HL_TABLE()[idx].sg_rgb_bg_name); + xfree(item->sg_rgb_bg_name); if (STRCMP(arg, "NONE") != 0) { - HL_TABLE()[idx].sg_rgb_bg_name = (char_u *)xstrdup((char *)arg); - HL_TABLE()[idx].sg_rgb_bg = name_to_color((const char_u *)arg); + item->sg_rgb_bg_name = (char_u *)xstrdup((char *)arg); + item->sg_rgb_bg = name_to_color((const char_u *)arg); } else { - HL_TABLE()[idx].sg_rgb_bg_name = NULL; - HL_TABLE()[idx].sg_rgb_bg = -1; + item->sg_rgb_bg_name = NULL; + item->sg_rgb_bg = -1; } } if (is_normal_group) { - normal_bg = HL_TABLE()[idx].sg_rgb_bg; + normal_bg = item->sg_rgb_bg; } } else if (strcmp(key, "GUISP") == 0) { - if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { - if (!init) - HL_TABLE()[idx].sg_set |= SG_GUI; + if (!init || !(item->sg_set & SG_GUI)) { + if (!init) { + item->sg_set |= SG_GUI; + } - xfree(HL_TABLE()[idx].sg_rgb_sp_name); + xfree(item->sg_rgb_sp_name); if (strcmp(arg, "NONE") != 0) { - HL_TABLE()[idx].sg_rgb_sp_name = (char_u *)xstrdup((char *)arg); - HL_TABLE()[idx].sg_rgb_sp = name_to_color((const char_u *)arg); + item->sg_rgb_sp_name = (char_u *)xstrdup((char *)arg); + item->sg_rgb_sp = name_to_color((const char_u *)arg); } else { - HL_TABLE()[idx].sg_rgb_sp_name = NULL; - HL_TABLE()[idx].sg_rgb_sp = -1; + item->sg_rgb_sp_name = NULL; + item->sg_rgb_sp = -1; } } if (is_normal_group) { - normal_sp = HL_TABLE()[idx].sg_rgb_sp; + normal_sp = item->sg_rgb_sp; } } else if (strcmp(key, "START") == 0 || strcmp(key, "STOP") == 0) { // Ignored for now @@ -6901,11 +6914,11 @@ void do_highlight(const char *line, const bool forceit, const bool init) error = true; break; } - HL_TABLE()[idx].sg_cleared = false; + item->sg_cleared = false; // When highlighting has been given for a group, don't link it. - if (!init || !(HL_TABLE()[idx].sg_set & SG_LINK)) { - HL_TABLE()[idx].sg_link = 0; + if (!init || !(item->sg_set & SG_LINK)) { + item->sg_link = 0; } // Continue with next argument. @@ -6934,14 +6947,17 @@ void do_highlight(const char *line, const bool forceit, const bool init) } else { set_hl_attr(idx); } - HL_TABLE()[idx].sg_scriptID = current_SID; - redraw_all_later(NOT_VALID); + item->sg_scriptID = current_SID; } xfree(key); xfree(arg); - // Only call highlight_changed() once, after sourcing a syntax file - need_highlight_changed = true; + // Only call highlight_changed() once, after a sequence of highlight + // commands, and only if an attribute actually changed + if (memcmp(item, &item_before, sizeof(item_before)) != 0) { + redraw_all_later(NOT_VALID); + need_highlight_changed = true; + } } #if defined(EXITFREE) From f0ca2283b08aa1f43eb14d6d4bae18272c2111f1 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Wed, 25 Jul 2018 23:36:32 -0400 Subject: [PATCH 4/5] vim-patch:8.0.1078: using freed memory with ":hi Normal" Problem: Using freed memory with ":hi Normal". Solution: Get "item" again after updating the table. https://github.com/vim/vim/commit/b4ea1914b8ca7c368253bd96e6b3cb9e3392da1c --- src/nvim/syntax.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index dcac907c5a..39a2abe03d 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -6500,6 +6500,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) bool error = false; int color; bool is_normal_group = false; // "Normal" group + bool did_highlight_changed = false; // If no argument, list current highlighting. if (ends_excmd((uint8_t)(*line))) { @@ -6944,6 +6945,9 @@ void do_highlight(const char *line, const bool forceit, const bool init) // redraw below will still handle usages of guibg=fg etc. ui_default_colors_set(); } + item = &HL_TABLE()[idx]; + did_highlight_changed = true; + redraw_all_later(NOT_VALID); } else { set_hl_attr(idx); } @@ -6954,7 +6958,8 @@ void do_highlight(const char *line, const bool forceit, const bool init) // Only call highlight_changed() once, after a sequence of highlight // commands, and only if an attribute actually changed - if (memcmp(item, &item_before, sizeof(item_before)) != 0) { + if (memcmp(item, &item_before, sizeof(item_before)) != 0 + && !did_highlight_changed) { redraw_all_later(NOT_VALID); need_highlight_changed = true; } From 3e6d3bf3bdd72d1f93e4683551fc7ca4abca94e1 Mon Sep 17 00:00:00 2001 From: Jan Edmund Lazo Date: Wed, 25 Jul 2018 23:56:08 -0400 Subject: [PATCH 5/5] vim-patch:8.0.1088: occasional memory use after free Problem: Occasional memory use after free. Solution: Use the highlight table directly, don't keep a pointer. https://github.com/vim/vim/commit/414168d97fad45387a3d7dd16449d15b27079ad8 --- src/nvim/syntax.c | 122 ++++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 63 deletions(-) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index 39a2abe03d..b475ca16c8 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -6492,7 +6492,6 @@ void do_highlight(const char *line, const bool forceit, const bool init) int attr; int id; int idx; - struct hl_group *item; struct hl_group item_before; bool dodefault = false; bool doclear = false; @@ -6568,7 +6567,6 @@ void do_highlight(const char *line, const bool forceit, const bool init) from_id = syn_check_group((const char_u *)from_start, (int)(from_end - from_start)); - item = &HL_TABLE()[from_id - 1]; if (strncmp(to_start, "NONE", 4) == 0) { to_id = 0; } else { @@ -6576,7 +6574,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) (int)(to_end - to_start)); } - if (from_id > 0 && (!init || item->sg_set == 0)) { + if (from_id > 0 && (!init || HL_TABLE()[from_id - 1].sg_set == 0)) { // Don't allow a link when there already is some highlighting // for the group, unless '!' is used if (to_id > 0 && !forceit && !init @@ -6584,15 +6582,15 @@ void do_highlight(const char *line, const bool forceit, const bool init) if (sourcing_name == NULL && !dodefault) { EMSG(_("E414: group has settings, highlight link ignored")); } - } else if (item->sg_link != to_id - || item->sg_scriptID != current_SID - || item->sg_cleared) { + } else if (HL_TABLE()[from_id - 1].sg_link != to_id + || HL_TABLE()[from_id - 1].sg_scriptID != current_SID + || HL_TABLE()[from_id - 1].sg_cleared) { if (!init) { - item->sg_set |= SG_LINK; + HL_TABLE()[from_id - 1].sg_set |= SG_LINK; } - item->sg_link = to_id; - item->sg_scriptID = current_SID; - item->sg_cleared = false; + HL_TABLE()[from_id - 1].sg_link = to_id; + HL_TABLE()[from_id - 1].sg_scriptID = current_SID; + HL_TABLE()[from_id - 1].sg_cleared = false; redraw_all_later(SOME_VALID); // Only call highlight changed() once after multiple changes @@ -6629,7 +6627,6 @@ void do_highlight(const char *line, const bool forceit, const bool init) return; } idx = id - 1; // Index is ID minus one. - item = &HL_TABLE()[idx]; // Return if "default" was used and the group already has settings if (dodefault && hl_has_settings(idx, true)) { @@ -6637,14 +6634,14 @@ void do_highlight(const char *line, const bool forceit, const bool init) } // Make a copy so we can check if any attribute actually changed - item_before = *item; - is_normal_group = (STRCMP(item->sg_name_u, "NORMAL") == 0); + item_before = HL_TABLE()[idx]; + is_normal_group = (STRCMP(HL_TABLE()[idx].sg_name_u, "NORMAL") == 0); // Clear the highlighting for ":hi clear {group}" and ":hi clear". if (doclear || (forceit && init)) { highlight_clear(idx); if (!doclear) { - item->sg_set = 0; + HL_TABLE()[idx].sg_set = 0; } } @@ -6670,9 +6667,9 @@ void do_highlight(const char *line, const bool forceit, const bool init) linep = (const char *)skipwhite((const char_u *)linep); if (strcmp(key, "NONE") == 0) { - if (!init || item->sg_set == 0) { + if (!init || HL_TABLE()[idx].sg_set == 0) { if (!init) { - item->sg_set |= SG_CTERM+SG_GUI; + HL_TABLE()[idx].sg_set |= SG_CTERM+SG_GUI; } highlight_clear(idx); } @@ -6741,34 +6738,34 @@ void do_highlight(const char *line, const bool forceit, const bool init) break; } if (*key == 'C') { - if (!init || !(item->sg_set & SG_CTERM)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_CTERM)) { if (!init) { - item->sg_set |= SG_CTERM; + HL_TABLE()[idx].sg_set |= SG_CTERM; } - item->sg_cterm = attr; - item->sg_cterm_bold = false; + HL_TABLE()[idx].sg_cterm = attr; + HL_TABLE()[idx].sg_cterm_bold = false; } } else if (*key == 'G') { - if (!init || !(item->sg_set & SG_GUI)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { if (!init) { - item->sg_set |= SG_GUI; + HL_TABLE()[idx].sg_set |= SG_GUI; } - item->sg_gui = attr; + HL_TABLE()[idx].sg_gui = attr; } } } else if (STRCMP(key, "FONT") == 0) { // in non-GUI fonts are simply ignored } else if (STRCMP(key, "CTERMFG") == 0 || STRCMP(key, "CTERMBG") == 0) { - if (!init || !(item->sg_set & SG_CTERM)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_CTERM)) { if (!init) { - item->sg_set |= SG_CTERM; + HL_TABLE()[idx].sg_set |= SG_CTERM; } /* When setting the foreground color, and previously the "bold" * flag was set for a light color, reset it now */ - if (key[5] == 'F' && item->sg_cterm_bold) { - item->sg_cterm &= ~HL_BOLD; - item->sg_cterm_bold = false; + if (key[5] == 'F' && HL_TABLE()[idx].sg_cterm_bold) { + HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; + HL_TABLE()[idx].sg_cterm_bold = false; } if (ascii_isdigit(*arg)) { @@ -6811,21 +6808,21 @@ void do_highlight(const char *line, const bool forceit, const bool init) // set/reset bold attribute to get light foreground // colors (on some terminals, e.g. "linux") if (bold == kTrue) { - item->sg_cterm |= HL_BOLD; - item->sg_cterm_bold = true; + HL_TABLE()[idx].sg_cterm |= HL_BOLD; + HL_TABLE()[idx].sg_cterm_bold = true; } else if (bold == kFalse) { - item->sg_cterm &= ~HL_BOLD; + HL_TABLE()[idx].sg_cterm &= ~HL_BOLD; } } // Add one to the argument, to avoid zero. Zero is used for // "NONE", then "color" is -1. if (key[5] == 'F') { - item->sg_cterm_fg = color + 1; + HL_TABLE()[idx].sg_cterm_fg = color + 1; if (is_normal_group) { cterm_normal_fg_color = color + 1; } } else { - item->sg_cterm_bg = color + 1; + HL_TABLE()[idx].sg_cterm_bg = color + 1; if (is_normal_group) { cterm_normal_bg_color = color + 1; if (!ui_rgb_attached()) { @@ -6852,61 +6849,61 @@ void do_highlight(const char *line, const bool forceit, const bool init) } } } else if (strcmp(key, "GUIFG") == 0) { - if (!init || !(item->sg_set & SG_GUI)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { if (!init) { - item->sg_set |= SG_GUI; + HL_TABLE()[idx].sg_set |= SG_GUI; } - xfree(item->sg_rgb_fg_name); + xfree(HL_TABLE()[idx].sg_rgb_fg_name); if (strcmp(arg, "NONE")) { - item->sg_rgb_fg_name = (char_u *)xstrdup((char *)arg); - item->sg_rgb_fg = name_to_color((const char_u *)arg); + HL_TABLE()[idx].sg_rgb_fg_name = (char_u *)xstrdup((char *)arg); + HL_TABLE()[idx].sg_rgb_fg = name_to_color((const char_u *)arg); } else { - item->sg_rgb_fg_name = NULL; - item->sg_rgb_fg = -1; + HL_TABLE()[idx].sg_rgb_fg_name = NULL; + HL_TABLE()[idx].sg_rgb_fg = -1; } } if (is_normal_group) { - normal_fg = item->sg_rgb_fg; + normal_fg = HL_TABLE()[idx].sg_rgb_fg; } } else if (STRCMP(key, "GUIBG") == 0) { - if (!init || !(item->sg_set & SG_GUI)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { if (!init) { - item->sg_set |= SG_GUI; + HL_TABLE()[idx].sg_set |= SG_GUI; } - xfree(item->sg_rgb_bg_name); + xfree(HL_TABLE()[idx].sg_rgb_bg_name); if (STRCMP(arg, "NONE") != 0) { - item->sg_rgb_bg_name = (char_u *)xstrdup((char *)arg); - item->sg_rgb_bg = name_to_color((const char_u *)arg); + HL_TABLE()[idx].sg_rgb_bg_name = (char_u *)xstrdup((char *)arg); + HL_TABLE()[idx].sg_rgb_bg = name_to_color((const char_u *)arg); } else { - item->sg_rgb_bg_name = NULL; - item->sg_rgb_bg = -1; + HL_TABLE()[idx].sg_rgb_bg_name = NULL; + HL_TABLE()[idx].sg_rgb_bg = -1; } } if (is_normal_group) { - normal_bg = item->sg_rgb_bg; + normal_bg = HL_TABLE()[idx].sg_rgb_bg; } } else if (strcmp(key, "GUISP") == 0) { - if (!init || !(item->sg_set & SG_GUI)) { + if (!init || !(HL_TABLE()[idx].sg_set & SG_GUI)) { if (!init) { - item->sg_set |= SG_GUI; + HL_TABLE()[idx].sg_set |= SG_GUI; } - xfree(item->sg_rgb_sp_name); + xfree(HL_TABLE()[idx].sg_rgb_sp_name); if (strcmp(arg, "NONE") != 0) { - item->sg_rgb_sp_name = (char_u *)xstrdup((char *)arg); - item->sg_rgb_sp = name_to_color((const char_u *)arg); + HL_TABLE()[idx].sg_rgb_sp_name = (char_u *)xstrdup((char *)arg); + HL_TABLE()[idx].sg_rgb_sp = name_to_color((const char_u *)arg); } else { - item->sg_rgb_sp_name = NULL; - item->sg_rgb_sp = -1; + HL_TABLE()[idx].sg_rgb_sp_name = NULL; + HL_TABLE()[idx].sg_rgb_sp = -1; } } if (is_normal_group) { - normal_sp = item->sg_rgb_sp; + normal_sp = HL_TABLE()[idx].sg_rgb_sp; } } else if (strcmp(key, "START") == 0 || strcmp(key, "STOP") == 0) { // Ignored for now @@ -6915,11 +6912,11 @@ void do_highlight(const char *line, const bool forceit, const bool init) error = true; break; } - item->sg_cleared = false; + HL_TABLE()[idx].sg_cleared = false; // When highlighting has been given for a group, don't link it. - if (!init || !(item->sg_set & SG_LINK)) { - item->sg_link = 0; + if (!init || !(HL_TABLE()[idx].sg_set & SG_LINK)) { + HL_TABLE()[idx].sg_link = 0; } // Continue with next argument. @@ -6945,20 +6942,19 @@ void do_highlight(const char *line, const bool forceit, const bool init) // redraw below will still handle usages of guibg=fg etc. ui_default_colors_set(); } - item = &HL_TABLE()[idx]; did_highlight_changed = true; redraw_all_later(NOT_VALID); } else { set_hl_attr(idx); } - item->sg_scriptID = current_SID; + HL_TABLE()[idx].sg_scriptID = current_SID; } xfree(key); xfree(arg); // Only call highlight_changed() once, after a sequence of highlight // commands, and only if an attribute actually changed - if (memcmp(item, &item_before, sizeof(item_before)) != 0 + if (memcmp(&HL_TABLE()[idx], &item_before, sizeof(item_before)) != 0 && !did_highlight_changed) { redraw_all_later(NOT_VALID); need_highlight_changed = true;