From 04e42f2ae409294d551c9b589aee4cbfd2616d68 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Fri, 14 Nov 2014 20:44:20 +0100 Subject: Fix warnings: tag.c: test_for_static()/get_tags(): Various (2): FP. Problems : Assigned value is garbage or undefined @ 2191. Uninitialized argument value @ 2796. Diagnostic : False positives. Rationale : Both problems share the same cause. Error happens in get_tags(), if parse_match() fails because of parse_tag_line() failing before. Then, `tp` is not correctly initialized and subsequent code accesses garbage values. This is not really possible, as parse_tag_line() should not fail after find_tags() has been successful. That is because find_tags() already does tag line parsing, using parse_tag_line() itself for it (or a quicker alternative that should produce same result). That's why return value of parse_match() is ignored, and subsequent code assumes it is successful. Resolution : Assert parse_match() always successful. --- src/nvim/tag.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nvim/tag.c b/src/nvim/tag.c index e267280bbd..816e9902fe 100644 --- a/src/nvim/tag.c +++ b/src/nvim/tag.c @@ -10,6 +10,7 @@ * Code to handle tags and the tag stack */ +#include #include #include #include @@ -2779,7 +2780,8 @@ int get_tags(list_T *list, char_u *pat) TAG_REGEXP | TAG_NOIC, (int)MAXCOL, NULL); if (ret == OK && num_matches > 0) { for (i = 0; i < num_matches; ++i) { - parse_match(matches[i], &tp); + int parse_result = parse_match(matches[i], &tp); + assert(parse_result == OK); is_static = test_for_static(&tp); /* Skip pseudo-tag lines. */ -- cgit From 3f420ce0d829a8cdff9d55f6178989ca8e0406c9 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Fri, 14 Nov 2014 21:26:24 +0100 Subject: Fix warnings: tag.c: jumpto_tag(): Np dereference: MI. Problem : Dereference of null pointer @ 2399. Diagnostic : Multithreading issue. Rationale : Error can only occur if global `g_do_tagpreview` changes while the function is executing. Resolution : Use local copy of global var. --- src/nvim/tag.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/nvim/tag.c b/src/nvim/tag.c index 816e9902fe..724261f08e 100644 --- a/src/nvim/tag.c +++ b/src/nvim/tag.c @@ -2308,6 +2308,7 @@ jumpto_tag ( win_T *curwin_save = NULL; char_u *full_fname = NULL; int old_KeyTyped = KeyTyped; /* getting the file may reset it */ + const int l_g_do_tagpreview = g_do_tagpreview; pbuf = xmalloc(LSIZE); @@ -2364,7 +2365,7 @@ jumpto_tag ( ++RedrawingDisabled; - if (g_do_tagpreview != 0) { + if (l_g_do_tagpreview != 0) { postponed_split = 0; /* don't split again below */ curwin_save = curwin; /* Save current window */ @@ -2396,7 +2397,7 @@ jumpto_tag ( if (keep_help) { /* A :ta from a help file will keep the b_help flag set. For ":ptag" * we need to use the flag from the window where we came from. */ - if (g_do_tagpreview != 0) + if (l_g_do_tagpreview != 0) keep_help_flag = curwin_save->w_buffer->b_help; else keep_help_flag = curbuf->b_help; @@ -2542,7 +2543,7 @@ jumpto_tag ( foldOpenCursor(); } - if (g_do_tagpreview != 0 + if (l_g_do_tagpreview != 0 && curwin != curwin_save && win_valid(curwin_save)) { /* Return cursor to where we were */ validate_cursor(); -- cgit From b4ae878407223965cdee6c88e7a55caf08b2a130 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Fri, 14 Nov 2014 22:12:41 +0100 Subject: Fix warnings: tag.c: get_tags(): Uninitialized arg: RI. Problem : Uninitialized argument value @ 2798. Diagnostic : Real issue. Rationale : Tags doesn't have to have a kind. When they have one, both `tp.tagkind` and `tp.tagkind_end` are nonnull. But when they don't, `tp.tagkind` will we null (but defined), while `tp.tagkind_end` will be undefined. Therefore, reported invocation is indeed using a garbage value for a tag with no kind. Problem doesn't have consequences because `add_tag_field()` doesn't use `end` param if `start` param is null. Resolution : Don't use `tp.tagkind_end` if `tp.tagkind` is null. --- src/nvim/tag.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nvim/tag.c b/src/nvim/tag.c index 724261f08e..6ac6dc5cd2 100644 --- a/src/nvim/tag.c +++ b/src/nvim/tag.c @@ -2799,7 +2799,7 @@ int get_tags(list_T *list, char_u *pat) || add_tag_field(dict, "cmd", tp.command, tp.command_end) == FAIL || add_tag_field(dict, "kind", tp.tagkind, - tp.tagkind_end) == FAIL + tp.tagkind ? tp.tagkind_end : NULL) == FAIL || dict_add_nr_str(dict, "static", is_static, NULL) == FAIL) ret = FAIL; -- cgit From c802869e99f8a56ba2eceea2f62d29c0cc673210 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sat, 15 Nov 2014 23:09:58 +0100 Subject: Fix warnings: window.c: win_rotate(): Np dereference: FP. Problem : Dereference of null pointer @ 1268. Diagnostic : False positive. Rationale : Suggested error path implies current window's frame to be the only child of its parent, which is ruled out by `if (firstwin == lastwin) {` check at the beginning. Resolution : Assert another child remains after removing current frame. Strictly, assert is only needed in false branch of conditional, but we add it the same in the true branch to reduce reader surprise. Several forms of a single assert after `if (firstwin == lastwin) {` were tried, but analyzer cannot follow implications that way. --- src/nvim/window.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index 9345d740d1..0ca3b1ca34 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -1230,7 +1230,6 @@ static void win_rotate(int upwards, int count) return; } - /* Check if all frames in this row/col have one window. */ for (frp = curwin->w_frame->fr_parent->fr_child; frp != NULL; frp = frp->fr_next) @@ -1246,6 +1245,7 @@ static void win_rotate(int upwards, int count) wp1 = frp->fr_win; win_remove(wp1, NULL); frame_remove(frp); + assert(frp->fr_parent->fr_child); /* find last frame and append removed window/frame after it */ for (; frp->fr_next != NULL; frp = frp->fr_next) @@ -1263,6 +1263,7 @@ static void win_rotate(int upwards, int count) wp2 = wp1->w_prev; /* will become last window */ win_remove(wp1, NULL); frame_remove(frp); + assert(frp->fr_parent->fr_child); /* append the removed window/frame before the first in the list */ win_append(frp->fr_parent->fr_child->fr_win->w_prev, wp1); -- cgit From 5955f44adea310b735aae307407a807d7acd74c0 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sun, 16 Nov 2014 12:26:20 +0100 Subject: Fix warnings: window.c: winframe_remove(): Np dereference: FP. Problem : Dereference of null pointer @ 2196. Diagnostic : False positive. Rationale : Suggested error path implies `frp->child == NULL` while being under condition `frp2->fr_layout == frp->fr_layout`, which is impossible: - If frp2 is frp's parent, then frp2's layout is FR_COL or FR_ROW; - if frp->child is NULL, the frp's layout is FR_LEAF. - Therefore, they can't be equal. Resolution : Assert frp->child not null. --- src/nvim/window.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nvim/window.c b/src/nvim/window.c index 0ca3b1ca34..96f074c9a5 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2194,6 +2194,7 @@ winframe_remove ( * the frames into this list. */ if (frp2->fr_child == frp) frp2->fr_child = frp->fr_child; + assert(frp->fr_child); frp->fr_child->fr_prev = frp->fr_prev; if (frp->fr_prev != NULL) frp->fr_prev->fr_next = frp->fr_child; -- cgit From f544d976faf23649a5015eb25f3296f715a7a1bc Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sun, 16 Nov 2014 13:34:57 +0100 Subject: Fix warnings: window.c: win_drag_vsep_line(): Np dereference: FP. Problem : Dereference of null pointer @ 4512. Diagnostic : False positive. Rationale : Suggested error path implies `fr == NULL` after 4504. That's not possible, because: - curfr and curfr->next must be both nonnull, as we are dragging the divider between the two. - after conditional, fr is one of those two (the one that grows). Resolution : Assert fr. --- src/nvim/window.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nvim/window.c b/src/nvim/window.c index 96f074c9a5..63c46f243e 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -4504,6 +4504,7 @@ void win_drag_vsep_line(win_T *dragwin, int offset) room += fr->fr_width - frame_minwidth(fr, NULL); fr = curfr; /* put fr at window that grows */ } + assert(fr); if (room < offset) /* Not enough room */ offset = room; /* Move as far as we can */ -- cgit From f47d52ea4f260ad07e2eed8c7c2ae39a484dc282 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sun, 16 Nov 2014 14:01:28 +0100 Subject: Fix warnings: window.c: tabline_height(): Np dereference: FP. Problem : Dereference of null pointer @ 4978. Diagnostic : False positive. Rationale : tabline_height() shouldn't be called when a tab doesn't exist yet (this is, before initialization). Resolution : Assert function precondition. --- src/nvim/window.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nvim/window.c b/src/nvim/window.c index 63c46f243e..315d5f07de 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -4976,6 +4976,7 @@ static void last_status_rec(frame_T *fr, int statusline) */ int tabline_height(void) { + assert(first_tabpage); switch (p_stal) { case 0: return 0; case 1: return (first_tabpage->tp_next == NULL) ? 0 : 1; -- cgit From d3f413ba6a4e9fc36712f787bc120eb028f39cae Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sun, 16 Nov 2014 19:35:46 +0100 Subject: Fix warnings: eval.c: set_var_lval(): Np dereference: FP. Problem : Dereference of null pointer @ 2273. Diagnostic : False positive. Rationale : Suggested error would happen when assigning an rvalue with more items than the lvalue. Then we would enter conditional at: ``` if (lp->ll_li->li_next == NULL) { /* Need to add an empty item. */ list_append_number(lp->ll_list, 0); } lp->ll_li = lp->ll_li->li_next; ``` Analyzer thinks the value assigned to lp->ll_li is still NULL and is hit on the next iteration. Resolution : Assert lp->ll_li->li_next is not null anymore after list_append_number(). --- src/nvim/eval.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 498795dc38..d7f820ae78 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -2272,6 +2272,7 @@ static void set_var_lval(lval_T *lp, char_u *endp, typval_T *rettv, int copy, ch if (lp->ll_li->li_next == NULL) { /* Need to add an empty item. */ list_append_number(lp->ll_list, 0); + assert(lp->ll_li->li_next); } lp->ll_li = lp->ll_li->li_next; ++lp->ll_n1; -- cgit From be90cdf4f9891ebd6cdf6b2ec6c34f3bcf465643 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sun, 16 Nov 2014 20:26:47 +0100 Subject: Fix warnings: eval.c: dictitem_alloc(): Out-of-bounds access: FP. Problem : Out-of-bound array access @ 5737. Diagnostic : False positive. Rationale : Situation is intentional. `dictitem_T` is a prefix all dict items whill share, but actual size of each item will be different depending on its key length. `di_key` array field is declared of size 1 just to have a field name, but real size will vary for each item. Resolution : Make analyzer ignore it. This could be refactored to use C99-allowed variable length arrays, but eval.c is bound to dissappear, so no effort is done in that sense. --- src/nvim/eval.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index d7f820ae78..9acde49105 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -5734,7 +5734,9 @@ dict_free ( dictitem_T *dictitem_alloc(char_u *key) FUNC_ATTR_NONNULL_RET { dictitem_T *di = xmalloc(sizeof(dictitem_T) + STRLEN(key)); +#ifndef __clang_analyzer__ STRCPY(di->di_key, key); +#endif di->di_flags = 0; return di; } -- cgit From ece19651c60c34e2fa5bed623dff82b9af4d8e11 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 09:31:54 +0100 Subject: Fix warnings: eval.c: item_compare(): Garbage value: MI. Problem : Result of operation is garbage or undefined @ 13565. Diagnostic : Multithreading issue. Rationale : Problem occurs only if global (static) variable `item_compare_keep_zero` changes after being used by `do_sort_uniq` but before being used by `item_compare` or `item_compare2`. Resolution : This is not an intra-function problem, as other MI's before, but rather an inter-function one. Thus, it can't be solved by using local copy of global. Therefore, we are forced to do a bit refactoring. We can't simply add a bool param to item_compare/item_compare2, as they couldn't be passed to qsort() that way. So, item_compare/item_compare2 are added a bool param and curried versions of them are added and used in their place. --- src/nvim/eval.c | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 9acde49105..44ea2b2332 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -13545,13 +13545,12 @@ static bool item_compare_numeric; static char_u *item_compare_func; static dict_T *item_compare_selfdict; static int item_compare_func_err; -static bool item_compare_keep_zero; #define ITEM_COMPARE_FAIL 999 /* * Compare functions for f_sort() and f_uniq() below. */ -static int item_compare(const void *s1, const void *s2) +static int item_compare(const void *s1, const void *s2, bool keep_zero) { sortItem_T *si1, *si2; char_u *p1, *p2; @@ -13604,7 +13603,7 @@ static int item_compare(const void *s1, const void *s2) // When the result would be zero, compare the item indexes. Makes the // sort stable. - if (res == 0 && !item_compare_keep_zero) { + if (res == 0 && !keep_zero) { res = si1->idx > si2->idx ? 1 : -1; } @@ -13613,7 +13612,17 @@ static int item_compare(const void *s1, const void *s2) return res; } -static int item_compare2(const void *s1, const void *s2) +static int item_compare_keeping_zero(const void *s1, const void *s2) +{ + return item_compare(s1, s2, true); +} + +static int item_compare_not_keeping_zero(const void *s1, const void *s2) +{ + return item_compare(s1, s2, false); +} + +static int item_compare2(const void *s1, const void *s2, bool keep_zero) { sortItem_T *si1, *si2; int res; @@ -13650,13 +13659,23 @@ static int item_compare2(const void *s1, const void *s2) // When the result would be zero, compare the pointers themselves. Makes // the sort stable. - if (res == 0 && !item_compare_keep_zero) { + if (res == 0 && !keep_zero) { res = si1->idx > si2->idx ? 1 : -1; } return res; } +static int item_compare2_keeping_zero(const void *s1, const void *s2) +{ + return item_compare2(s1, s2, true); +} + +static int item_compare2_not_keeping_zero(const void *s1, const void *s2) +{ + return item_compare2(s1, s2, false); +} + /* * "sort({list})" function */ @@ -13737,15 +13756,16 @@ static void do_sort_uniq(typval_T *argvars, typval_T *rettv, bool sort) } item_compare_func_err = FALSE; - item_compare_keep_zero = false; // Test the compare function. if (item_compare_func != NULL - && item_compare2(&ptrs[0], &ptrs[1]) == ITEM_COMPARE_FAIL) { + && item_compare2_not_keeping_zero(&ptrs[0], &ptrs[1]) + == ITEM_COMPARE_FAIL) { EMSG(_("E702: Sort compare function failed")); } else { // Sort the array with item pointers. qsort(ptrs, (size_t)len, sizeof (sortItem_T), - item_compare_func == NULL ? item_compare : item_compare2); + item_compare_func == NULL ? item_compare_not_keeping_zero : + item_compare2_not_keeping_zero); if (!item_compare_func_err) { // Clear the list and append the items in the sorted order. @@ -13764,8 +13784,8 @@ static void do_sort_uniq(typval_T *argvars, typval_T *rettv, bool sort) // f_uniq(): ptrs will be a stack of items to remove. item_compare_func_err = FALSE; - item_compare_keep_zero = true; - item_compare_func_ptr = item_compare_func ? item_compare2 : item_compare; + item_compare_func_ptr = item_compare_func ? item_compare2_keeping_zero : + item_compare_keeping_zero; for (li = l->lv_first; li != NULL && li->li_next != NULL; li = li->li_next) { if (item_compare_func_ptr(&li, &li->li_next) == 0) { -- cgit From eb15d8777be0fe9a044b153a5f0991e1eb90faa9 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 10:05:08 +0100 Subject: Fix warnings: eval.c: clear_tv(): Bad free: RI. Problem : Bad free @ 16076. Diagnostic : Real issue. Rationale : A non-allocated string is set at 4127, which later on can be tried to be freed if aborting. Resolution : Detect particular case (func with empty name) and don't free in that case. Another solution (use allocated string) was tried before, but it produced a leak difficult to solve. Finally applied solution works, but it produces a new false positive warning (Np dereference at 13763), deactivated by `assert(ptrs[i].item->li_next)`. --- src/nvim/eval.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 44ea2b2332..b8d1799e9a 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -178,6 +178,8 @@ static char *e_nofunc = N_("E130: Unknown function: %s"); static char *e_illvar = N_("E461: Illegal variable name: %s"); static char *e_float_as_string = N_("E806: using Float as a String"); +static char_u * const empty_string = (char_u *)""; + static dictitem_T globvars_var; /* variable used for g: */ #define globvarht globvardict.dv_hashtab @@ -4124,7 +4126,7 @@ eval7 ( * get_func_tv, but it's needed in handle_subscript() to parse * what follows. So set it here. */ if (rettv->v_type == VAR_UNKNOWN && !evaluate && **arg == '(') { - rettv->vval.v_string = (char_u *)""; + rettv->vval.v_string = empty_string; rettv->v_type = VAR_FUNC; } @@ -13799,6 +13801,7 @@ static void do_sort_uniq(typval_T *argvars, typval_T *rettv, bool sort) if (!item_compare_func_err) { while (--i >= 0) { + assert(ptrs[i].item->li_next); li = ptrs[i].item->li_next; ptrs[i].item->li_next = li->li_next; if (li->li_next != NULL) { @@ -16134,7 +16137,11 @@ void clear_tv(typval_T *varp) switch (varp->v_type) { case VAR_FUNC: func_unref(varp->vval.v_string); - /*FALLTHROUGH*/ + if (varp->vval.v_string != empty_string) { + free(varp->vval.v_string); + } + varp->vval.v_string = NULL; + break; case VAR_STRING: free(varp->vval.v_string); varp->vval.v_string = NULL; -- cgit From 34a4a01e4efb690743de0fe60d98a3ebed1bd681 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 10:24:07 +0100 Subject: Fix warnings: eval.c: get_user_func_name(): Np dereference: FP. Problem : Dereference of null pointer @ 18216. Diagnostic : False positive. Rationale : `hi` and `done` are static. Intended usage is for the first call to have idx == 0, so that they are initialized. Resolution : Assert hi after (optional) initialization. --- src/nvim/eval.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index b8d1799e9a..af7a2d0118 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -18280,6 +18280,7 @@ char_u *get_user_func_name(expand_T *xp, int idx) done = 0; hi = func_hashtab.ht_array; } + assert(hi); if (done < func_hashtab.ht_used) { if (done++ > 0) ++hi; -- cgit From 3bf32fe43f86a2c966c77b130f7ea7c71b6ac099 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 10:39:40 +0100 Subject: Fix warnings: eval.c: call_user_func(): Out of bounds: FP. Problem : Out-of-bound array access @ 18429. Diagnostic : False positive. Rationale : Situation is intentional. `dictitem_T` is a prefix all dict items whill share, but actual size of each item will be different depending on its key length. `di_key` array field is declared of size 1 just to have a field name, but real size will vary for each item. Resolution : Make analyzer ignore it. This could be refactored to use C99-allowed variable length arrays, but eval.c is bound to dissappear, so no effort is done in that sense. --- src/nvim/eval.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index af7a2d0118..d2ab8aa6ee 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -18496,8 +18496,10 @@ call_user_func ( /* Set l:self to "selfdict". Use "name" to avoid a warning from * some compiler that checks the destination size. */ v = &fc->fixvar[fixvar_idx++].var; +#ifndef __clang_analyzer__ name = v->di_key; STRCPY(name, "self"); +#endif v->di_flags = DI_FLAGS_RO + DI_FLAGS_FIX; hash_add(&fc->l_vars.dv_hashtab, DI2HIKEY(v)); v->di_tv.v_type = VAR_DICT; @@ -18517,8 +18519,10 @@ call_user_func ( /* Use "name" to avoid a warning from some compiler that checks the * destination size. */ v = &fc->fixvar[fixvar_idx++].var; +#ifndef __clang_analyzer__ name = v->di_key; STRCPY(name, "000"); +#endif v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX; hash_add(&fc->l_avars.dv_hashtab, DI2HIKEY(v)); v->di_tv.v_type = VAR_LIST; -- cgit From 97be8d45f4e52aa33d37a831c26e24235ec55220 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 10:41:05 +0100 Subject: Fix warnings: eval.c: add_nr_var(): Out of bounds: FP. Problem : Out-of-bound array access @ 18737. Diagnostic : False positive. Rationale : Situation is intentional. `dictitem_T` is a prefix all dict items whill share, but actual size of each item will be different depending on its key length. `di_key` array field is declared of size 1 just to have a field name, but real size will vary for each item. Resolution : Make analyzer ignore it. This could be refactored to use C99-allowed variable length arrays, but eval.c is bound to dissappear, so no effort is done in that sense. --- src/nvim/eval.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index d2ab8aa6ee..b4eb6053a7 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -18809,7 +18809,9 @@ free_funccal ( */ static void add_nr_var(dict_T *dp, dictitem_T *v, char *name, varnumber_T nr) { +#ifndef __clang_analyzer__ STRCPY(v->di_key, name); +#endif v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX; hash_add(&dp->dv_hashtab, DI2HIKEY(v)); v->di_tv.v_type = VAR_NUMBER; -- cgit From 5c00f8c13244727845cd2de194f4e09c04ed4d8a Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 11:36:48 +0100 Subject: Fix warnings: eval.c: do_return(): Np dereference: FP. Problem : Dereference of null pointer @ 18841. Diagnostic : False positive. Rationale : Suggested error path takes `reanimate` branch at 18827, assigning `rettv = current_funccal->rettv`. Then, inmediately after, it supposes rettv is null, which cannot happen, since current_funccal->rettv should always be non null. Resolution : Assert current_funccal->rettv non null. --- src/nvim/eval.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index b4eb6053a7..393fab1404 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -18901,8 +18901,10 @@ int do_return(exarg_T *eap, int reanimate, int is_cmd, void *rettv) else { /* When undoing a return in order to make it pending, get the stored * return rettv. */ - if (reanimate) + if (reanimate) { + assert(current_funccal->rettv); rettv = current_funccal->rettv; + } if (rettv != NULL) { /* Store the value of the pending return. */ -- cgit From ee134fc69866d059fa551d83fed5ae080c0d6f6b Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 17 Nov 2014 12:01:06 +0100 Subject: Fix warnings: screen.c: screenalloc(): Np arg (2): MI. Problems : Null pointer argument in call to memory copy function @ 6465. Null pointer argument in call to memory copy function @ 6475. Diagnostic : Multithreading issues. Rationale : Problem occurs if globals `enc_utf8` and `enc_dbcs` are modified while function is executing. Resolution : Use local copy of globals. --- src/nvim/screen.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/nvim/screen.c b/src/nvim/screen.c index 58faac1ae7..3cbdbdd81f 100644 --- a/src/nvim/screen.c +++ b/src/nvim/screen.c @@ -6301,6 +6301,8 @@ void screenalloc(bool doclear) static int entered = FALSE; /* avoid recursiveness */ static int done_outofmem_msg = FALSE; /* did outofmem message */ int retry_count = 0; + const bool l_enc_utf8 = enc_utf8; + const int l_enc_dbcs = enc_dbcs; retry: /* @@ -6311,8 +6313,8 @@ retry: if ((ScreenLines != NULL && Rows == screen_Rows && Columns == screen_Columns - && enc_utf8 == (ScreenLinesUC != NULL) - && (enc_dbcs == DBCS_JPNU) == (ScreenLines2 != NULL) + && l_enc_utf8 == (ScreenLinesUC != NULL) + && (l_enc_dbcs == DBCS_JPNU) == (ScreenLines2 != NULL) && p_mco == Screen_mco ) || Rows == 0 @@ -6358,13 +6360,13 @@ retry: new_ScreenLines = xmalloc((size_t)((Rows + 1) * Columns * sizeof(schar_T))); memset(new_ScreenLinesC, 0, sizeof(u8char_T *) * MAX_MCO); - if (enc_utf8) { + if (l_enc_utf8) { new_ScreenLinesUC = xmalloc( (size_t)((Rows + 1) * Columns * sizeof(u8char_T))); for (i = 0; i < p_mco; ++i) new_ScreenLinesC[i] = xcalloc((Rows + 1) * Columns, sizeof(u8char_T)); } - if (enc_dbcs == DBCS_JPNU) + if (l_enc_dbcs == DBCS_JPNU) new_ScreenLines2 = xmalloc( (size_t)((Rows + 1) * Columns * sizeof(schar_T))); new_ScreenAttrs = xmalloc((size_t)((Rows + 1) * Columns * sizeof(sattr_T))); @@ -6383,8 +6385,8 @@ retry: if (new_ScreenLinesC[i] == NULL) break; if (new_ScreenLines == NULL - || (enc_utf8 && (new_ScreenLinesUC == NULL || i != p_mco)) - || (enc_dbcs == DBCS_JPNU && new_ScreenLines2 == NULL) + || (l_enc_utf8 && (new_ScreenLinesUC == NULL || i != p_mco)) + || (l_enc_dbcs == DBCS_JPNU && new_ScreenLines2 == NULL) || new_ScreenAttrs == NULL || new_LineOffset == NULL || new_LineWraps == NULL @@ -6432,7 +6434,7 @@ retry: if (!doclear) { (void)memset(new_ScreenLines + new_row * Columns, ' ', (size_t)Columns * sizeof(schar_T)); - if (enc_utf8) { + if (l_enc_utf8) { (void)memset(new_ScreenLinesUC + new_row * Columns, 0, (size_t)Columns * sizeof(u8char_T)); for (i = 0; i < p_mco; ++i) @@ -6440,7 +6442,7 @@ retry: + new_row * Columns, 0, (size_t)Columns * sizeof(u8char_T)); } - if (enc_dbcs == DBCS_JPNU) + if (l_enc_dbcs == DBCS_JPNU) (void)memset(new_ScreenLines2 + new_row * Columns, 0, (size_t)Columns * sizeof(schar_T)); (void)memset(new_ScreenAttrs + new_row * Columns, @@ -6453,12 +6455,12 @@ retry: len = Columns; /* When switching to utf-8 don't copy characters, they * may be invalid now. Also when p_mco changes. */ - if (!(enc_utf8 && ScreenLinesUC == NULL) + if (!(l_enc_utf8 && ScreenLinesUC == NULL) && p_mco == Screen_mco) memmove(new_ScreenLines + new_LineOffset[new_row], ScreenLines + LineOffset[old_row], (size_t)len * sizeof(schar_T)); - if (enc_utf8 && ScreenLinesUC != NULL + if (l_enc_utf8 && ScreenLinesUC != NULL && p_mco == Screen_mco) { memmove(new_ScreenLinesUC + new_LineOffset[new_row], ScreenLinesUC + LineOffset[old_row], @@ -6469,7 +6471,7 @@ retry: ScreenLinesC[i] + LineOffset[old_row], (size_t)len * sizeof(u8char_T)); } - if (enc_dbcs == DBCS_JPNU && ScreenLines2 != NULL) + if (l_enc_dbcs == DBCS_JPNU && ScreenLines2 != NULL) memmove(new_ScreenLines2 + new_LineOffset[new_row], ScreenLines2 + LineOffset[old_row], (size_t)len * sizeof(schar_T)); -- cgit From 7aa7ce253d462dc6b98dcf781d95be56c792f9d2 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Tue, 18 Nov 2014 22:20:39 +0100 Subject: Fix warnings: eval.c: f_rpcrequest(): Garbage value: MI. Problem : Assigned value is garbage or undefined @ 12578. Diagnostic : Multithreading issue. Rationale : Error can only occur if global `provider_call_nesting` is changed while function is executing. Resolution : Use local copy of global. --- src/nvim/eval.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 393fab1404..1a80198515 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -12523,6 +12523,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv) { rettv->v_type = VAR_NUMBER; rettv->vval.v_number = 0; + const int l_provider_call_nesting = provider_call_nesting; if (check_restricted() || check_secure()) { return; @@ -12550,7 +12551,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv) int save_autocmd_fname_full, save_autocmd_bufnr; void *save_funccalp; - if (provider_call_nesting) { + if (l_provider_call_nesting) { // If this is called from a provider function, restore the scope // information of the caller. save_current_SID = current_SID; @@ -12579,7 +12580,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv) args, &err); - if (provider_call_nesting) { + if (l_provider_call_nesting) { current_SID = save_current_SID; sourcing_name = save_sourcing_name; sourcing_lnum = save_sourcing_lnum; -- cgit