91 lines
3.2 KiB
Diff
91 lines
3.2 KiB
Diff
|
From df63da98d8dc284b1c76cfe1b17fa0acbd6094d8 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Christian Brabandt <cb@256bit.org>
|
||
|
|
Date: Thu, 23 Nov 2023 20:14:28 +0100
|
||
|
|
Subject: [PATCH] patch 9.0.2123: Problem with initializing the length of
|
||
|
|
range() lists
|
||
|
|
|
||
|
|
Problem: Problem with initializing the length of range() lists
|
||
|
|
Solution: Set length explicitly when it shouldn't contain any items
|
||
|
|
|
||
|
|
range() may cause a wrong calculation of list length, which may later
|
||
|
|
then cause a segfault in list_find(). This is usually not a problem,
|
||
|
|
because range_list_materialize() calculates the length, when it
|
||
|
|
materializes the list.
|
||
|
|
|
||
|
|
In addition, in list_find() when the length of the range was wrongly
|
||
|
|
initialized, it may seem to be valid, so the check for list index
|
||
|
|
out-of-bounds will not be true, because it is called before the list is
|
||
|
|
actually materialized. And so we may eventually try to access a null
|
||
|
|
pointer, causing a segfault.
|
||
|
|
|
||
|
|
So this patch does 3 things:
|
||
|
|
|
||
|
|
- In f_range(), when we know that the list should be empty, explicitly
|
||
|
|
set the list->lv_len value to zero. This should happen, when
|
||
|
|
start is larger than end (in case the stride is positive) or
|
||
|
|
end is larger than start when the stride is negative.
|
||
|
|
This should fix the underlying issue properly. However,
|
||
|
|
|
||
|
|
- as a safety measure, let's check that the requested index is not
|
||
|
|
out of range one more time, after the list has been materialized
|
||
|
|
and return NULL in case it suddenly is.
|
||
|
|
|
||
|
|
- add a few more tests to verify the behaviour.
|
||
|
|
|
||
|
|
fixes: #13557
|
||
|
|
closes: #13563
|
||
|
|
|
||
|
|
Co-authored-by: Tim Pope <tpope@github.com>
|
||
|
|
Signed-off-by: Christian Brabandt <cb@256bit.org>
|
||
|
|
---
|
||
|
|
src/evalfunc.c | 5 ++++-
|
||
|
|
src/list.c | 4 ++++
|
||
|
|
src/testdir/test_functions.vim | 3 +++
|
||
|
|
3 files changed, 11 insertions(+), 1 deletion(-)
|
||
|
|
|
||
|
|
diff --git a/src/evalfunc.c b/src/evalfunc.c
|
||
|
|
index 7f7914eca7d91..fa27d0d274a64 100644
|
||
|
|
--- a/src/evalfunc.c
|
||
|
|
+++ b/src/evalfunc.c
|
||
|
|
@@ -8646,7 +8646,10 @@ f_range(typval_T *argvars, typval_T *rettv)
|
||
|
|
list->lv_u.nonmat.lv_start = start;
|
||
|
|
list->lv_u.nonmat.lv_end = end;
|
||
|
|
list->lv_u.nonmat.lv_stride = stride;
|
||
|
|
- list->lv_len = (end - start) / stride + 1;
|
||
|
|
+ if (stride > 0 ? end < start : end > start)
|
||
|
|
+ list->lv_len = 0;
|
||
|
|
+ else
|
||
|
|
+ list->lv_len = (end - start) / stride + 1;
|
||
|
|
}
|
||
|
|
|
||
|
|
/*
|
||
|
|
diff --git a/src/list.c b/src/list.c
|
||
|
|
index d1494c67d56e9..ce1ccaa1c045c 100644
|
||
|
|
--- a/src/list.c
|
||
|
|
+++ b/src/list.c
|
||
|
|
@@ -415,6 +415,10 @@ list_find(list_T *l, long n)
|
||
|
|
|
||
|
|
CHECK_LIST_MATERIALIZE(l);
|
||
|
|
|
||
|
|
+ // range_list_materialize may reset l->lv_len
|
||
|
|
+ if (n >= l->lv_len)
|
||
|
|
+ return NULL;
|
||
|
|
+
|
||
|
|
// When there is a cached index may start search from there.
|
||
|
|
if (l->lv_u.mat.lv_idx_item != NULL)
|
||
|
|
{
|
||
|
|
diff --git a/src/testdir/test_functions.vim b/src/testdir/test_functions.vim
|
||
|
|
index 49b688c25f347..0801d2b695b76 100644
|
||
|
|
--- a/src/testdir/test_functions.vim
|
||
|
|
+++ b/src/testdir/test_functions.vim
|
||
|
|
@@ -3052,6 +3052,9 @@ func Test_range()
|
||
|
|
" get()
|
||
|
|
call assert_equal(4, get(range(1, 10), 3))
|
||
|
|
call assert_equal(-1, get(range(1, 10), 42, -1))
|
||
|
|
+ call assert_equal(0, get(range(1, 0, 2), 0))
|
||
|
|
+ call assert_equal(0, get(range(0, -1, 2), 0))
|
||
|
|
+ call assert_equal(0, get(range(-2, -1, -2), 0))
|
||
|
|
|
||
|
|
" index()
|
||
|
|
call assert_equal(1, index(range(1, 5), 2))
|