| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem:
gcc and Coverity report potential `size + len `overflow kv_concat_len().
CID 656305: Null pointer dereferences (FORWARD_NULL)
/src/nvim/input_cmdatom.c: 474 in atom_push_raw()
468 atom_captures++;
469 }
470 if (collect) {
471 composite.queued = true;
472 }
473 kv_push(vatom.atoms, *atom);
>>> CID 656305: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "cur_frame".
474 vatom.frame = cur_frame->id;
475 return;
476 }
477 atom_captures++;
478 if (atom->type == kAVisual && kv_size(atom->atoms) > 0) {
479 // The completing operator is the only subatom that could have edited.
CID 656304: Memory - illegal accesses (OVERRUN)
/src/nvim/input.c: 718 in sb_add_spec()
712 /// bytes. Unlike sb_add_lit(), no CTRL-V: for text read raw (<Cmd>), not typed into a cmdline.
713 void sb_add_spec(StringBuilder *buf, const char *s)
714 {
715 while (*s != NUL) {
716 if ((uint8_t)(*s) == K_SPECIAL && s[1] != NUL && s[2] != NUL) {
717 // Insert special key literally.
>>> CID 656304: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "buf->items" at offset corresponding to index variable "buf->size" through dereference in call to "memcpy".
718 kv_concat_len(*buf, s, 3);
719 s += 3;
720 } else {
721 sb_add_char(buf, mb_cptr2char_adv(&s));
722 }
723 }
Solution:
Both are false positives; assert or check to avoid the warnings.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem:
The kvec append and reserve macros do not parenthesize length arguments.
Compound expressions can change the allocation and copy sizes, causing
buffer overflows for elements larger than one byte. The append macros
also expand to bare if statements, making them unsafe to use in unbraced
if/else statements.
Solution:
Parenthesize length and data arguments and wrap both append macros in
do/while statements.
AI-assisted
|
| |
|
|
| |
Some names/comments are misleading.
Also add some asserts.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Coverity can't follow kv_ensure_space()'s `kv_roundup32()` bit math, so
every kv_concat_len() looks like an overrun; and it doesn't know
`kv_push()` allocates when `size == capacity`.
_____________________________________________________________________________________________
CID 653191: Memory - illegal accesses (OVERRUN)
/src/nvim/input.c: 3536 in paste_store()
3530
3531 if (s > start) {
3532 if (need_redo) {
3533 kv_concat_len(redobuff.cur.keys, start, (size_t)(s - start));
3534 }
3535 if (need_record) {
>>> CID 653191: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "recordbuff.items" at offset corresponding to index variable "recordbuff.size" through dereference in call to "memcpy".
3536 kv_concat_len(recordbuff, start, (size_t)(s - start));
3537 }
3538 }
3539
3540 if (s < str_end) {
3541 int c = (uint8_t)(*s++);
_____________________________________________________________________________________________
CID 653190: Memory - illegal accesses (OVERRUN)
/src/nvim/input.c: 730 in redo_append_spec()
724 return;
725 }
726
727 while (*s != NUL) {
728 if ((uint8_t)(*s) == K_SPECIAL && s[1] != NUL && s[2] != NUL) {
729 // Insert special key literally.
>>> CID 653190: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "redobuff.cur.keys.items" at offset corresponding to index variable "redobuff.cur.keys.size" through dereference in call to "memcpy".
730 kv_concat_len(redobuff.cur.keys, s, 3);
731 s += 3;
732 } else {
733 sb_add_char(&redobuff.cur.keys, mb_cptr2char_adv(&s));
734 }
735 }
CID 653189: (OVERRUN)
/src/nvim/input.c: 3533 in paste_store()
/src/nvim/input.c: 3536 in paste_store()
_____________________________________________________________________________________________
CID 653189: (OVERRUN)
/src/nvim/input.c: 3533 in paste_store()
3527 && *s != NL && !(crlf && *s == CAR)) {
3528 s++;
3529 }
3530
3531 if (s > start) {
3532 if (need_redo) {
>>> CID 653189: (OVERRUN)
>>> Overrunning dynamic array "redobuff.cur.keys.items" at offset corresponding to index variable "redobuff.cur.keys.size" through dereference in call to "memcpy".
3533 kv_concat_len(redobuff.cur.keys, start, (size_t)(s - start));
3534 }
3535 if (need_record) {
3536 kv_concat_len(recordbuff, start, (size_t)(s - start));
3537 }
3538 }
/src/nvim/input.c: 3536 in paste_store()
3530
3531 if (s > start) {
3532 if (need_redo) {
3533 kv_concat_len(redobuff.cur.keys, start, (size_t)(s - start));
3534 }
3535 if (need_record) {
>>> CID 653189: (OVERRUN)
>>> Overrunning dynamic array "recordbuff.items" at offset corresponding to index variable "recordbuff.size" through dereference in call to "memcpy".
3536 kv_concat_len(recordbuff, start, (size_t)(s - start));
3537 }
3538 }
3539
3540 if (s < str_end) {
3541 int c = (uint8_t)(*s++);
_____________________________________________________________________________________________
CID 653188: Memory - illegal accesses (OVERRUN)
/src/nvim/input_cmdatom.c: 216 in atoms_concat_keys()
210
211 /// Concatenates the keys of multiple atoms into one (allocated) string.
212 static String atoms_concat_keys(CmdAtomVec atoms)
213 {
214 StringBuilder keys = KV_INITIAL_VALUE;
215 for (size_t i = 0; i < kv_size(atoms); i++) {
>>> CID 653188: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "keys.items" at offset corresponding to index variable "keys.size" through dereference in call to "memcpy".
216 kv_concat(keys, kv_A(atoms, i).keys);
217 }
218 size_t len = kv_size(keys);
219 kv_push(keys, NUL);
220 return (String){ .data = keys.items, .size = len };
221 }
CID 653187: Null pointer dereferences (FORWARD_NULL)
_____________________________________________________________________________________________
CID 653186: Null pointer dereferences (FORWARD_NULL)
/src/nvim/input_cmdatom.c: 173 in atom_compose_keys()
167 StringBuilder sb = KV_INITIAL_VALUE;
168 redo_prefix(&spec, &sb, false);
169 redo_chars(&spec, &sb, false);
170 if (sb.size == 0) {
171 return NULL;
172 }
>>> CID 653186: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "((sb.size == sb.capacity) ? (sb.capacity = (sb.capacity ? sb.capacity << 1 : 8UL)) , (sb.items = xrealloc(sb.items, 1UL * sb.capacity)) , 0 : 0) , (sb.items + sb.size++)".
173 kv_push(sb, NUL);
174 return sb.items;
175 }
176
177 /// The pending change as a CmdAtom: the composed keysequence plus the structured fields.
178 /// Caller owns `keys`.
_____________________________________________________________________________________________
CID 653185: Null pointer dereferences (FORWARD_NULL)
/src/nvim/input.c: 296 in redo_compose()
290 StringBuilder buf = KV_INITIAL_VALUE;
291 redo_prefix(&r->spec, &buf, false);
292 kv_splice(buf, r->keys);
293 if (buf.size == 0) {
294 return (String)STRING_INIT;
295 }
>>> CID 653185: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "((buf.size == buf.capacity) ? (buf.capacity = (buf.capacity ? buf.capacity << 1 : 8UL)) , (buf.items = xrealloc(buf.items, 1UL * buf.capacity)) , 0 : 0) , (buf.items + buf.size++)".
296 kv_push(buf, NUL);
297 return cbuf_as_string(buf.items, buf.size - 1);
298 }
299
301 String redo_keys(void)
_____________________________________________________________________________________________
CID 653184: Memory - illegal accesses (OVERRUN)
/src/nvim/input.c: 3533 in paste_store()
3527 && *s != NL && !(crlf && *s == CAR)) {
3528 s++;
3529 }
3530
3531 if (s > start) {
3532 if (need_redo) {
>>> CID 653184: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "redobuff.cur.keys.items" at offset corresponding to index variable "redobuff.cur.keys.size" through dereference in call to "memcpy".
3533 kv_concat_len(redobuff.cur.keys, start, (size_t)(s - start));
3534 }
3535 if (need_record) {
3536 kv_concat_len(recordbuff, start, (size_t)(s - start));
3537 }
3538 }
_____________________________________________________________________________________________
CID 653183: Memory - illegal accesses (OVERRUN)
/src/nvim/input.c: 730 in redo_append_spec()
724 return;
725 }
726
727 while (*s != NUL) {
728 if ((uint8_t)(*s) == K_SPECIAL && s[1] != NUL && s[2] != NUL) {
729 // Insert special key literally.
>>> CID 653183: Memory - illegal accesses (OVERRUN)
>>> Overrunning dynamic array "redobuff.cur.keys.items" at offset corresponding to index variable "redobuff.cur.keys.size" through dereference in call to "memcpy".
730 kv_concat_len(redobuff.cur.keys, s, 3);
731 s += 3;
732 } else {
733 sb_add_char(&redobuff.cur.keys, mb_cptr2char_adv(&s));
734 }
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
"wininfo" is going to be my next victim. The main problem with wininfo
is that it is "all or nothing", i e either all state about a buffer in a
window is considered valid or none of it is. This needs to be fixed to
address some long running grievances.
For now this is just a warmup: refactor it from a linked list to a
vector.
|
| |
|
|
|
|
|
|
|
|
|
| |
loop->children might have been a linked list because used to be
modified in place while looped over. However the loops that exists
rather schedules events to be processed later, outside of the loop,
so this can not happen anymore.
When a linked list is otherwise useful it is better to use
lib/queue_defs.h which defines an _intrusive_ linked list (i e it
doesn't need to do allocations for list items like klist ).
|
| |
|
| |
Keys before mapping (i.e. typed keys) are passed as the second argument.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
(#27022)
Coverity warns about a possible null pointer dereference in the `memcpy`
call in `kv_concat_len`. The `memcpy` follows `kv_ensure_space` which
(re)allocates the `items` pointer if the vector's capacity is not large
enough to contain all of the items being appended. The only way `items`
would be NULL at this point is if `capacity` were mistakenly set to some
large number without `items` ever having being set in the first place.
This should not happen when using the kvec API so if this condition is
ever false it is a bug, which the `assert` will catch.
|
| | |
|
| |
|
|
|
|
| |
FUNC_ATTR_* should only be used in .c files with generated headers.
Defining FUNC_ATTR_* as empty in headers causes misuses of them to be
silently ignored. Instead don't define them by default, and only define
them as empty after a .c file has included its generated header.
|
| |
|
|
|
|
|
| |
We already have an extensive suite of static analysis tools we use,
which causes a fair bit of redundancy as we get duplicate warnings. PVS
is also prone to give false warnings which creates a lot of work to
identify and disable.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The removes the previous restriction that nvim_buf_set_extmark()
could not be used to highlight arbitrary multi-line regions
The problem can be summarized as follows: let's assume an extmark with a
hl_group is placed covering the region (5,0) to (50,0) Now, consider
what happens if nvim needs to redraw a window covering the lines 20-30.
It needs to be able to ask the marktree what extmarks cover this region,
even if they don't begin or end here.
Therefore the marktree needs to be augmented with the information covers
a point, not just what marks begin or end there. To do this, we augment
each node with a field "intersect" which is a set the ids of the
marks which overlap this node, but only if it is not part of the set of
any parent. This ensures the number of nodes that need to be explicitly
marked grows only logarithmically with the total number of explicitly
nodes (and thus the number of of overlapping marks).
Thus we can quickly iterate all marks which overlaps any query position
by looking up what leaf node contains that position. Then we only need
to consider all "start" marks within that leaf node, and the "intersect"
set of that node and all its parents.
Now, and the major source of complexity is that the tree restructuring
operations (to ensure that each node has T-1 <= size <= 2*T-1) also need
to update these sets. If a full inner node is split in two, one of the
new parents might start to completely overlap some ranges and its ids
will need to be moved from its children's sets to its own set.
Similarly, if two undersized nodes gets joined into one, it might no
longer completely overlap some ranges, and now the children which do
needs to have the have the ids in its set instead. And then there are
the pivots! Yes the pivot operations when a child gets moved from one
parent to another.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This involves two redesigns of the map.c implementations:
1. Change of macro style and code organization
The old khash.h and map.c implementation used huge #define blocks with a
lot of backslash line continuations.
This instead uses the "implementation file" .c.h pattern. Such a file is
meant to be included multiple times, with different macros set prior to
inclusion as parameters. we already use this pattern e.g. for
eval/typval_encode.c.h to implement different typval encoders reusing a
similar structure.
We can structure this code into two parts. one that only depends on key
type and is enough to implement sets, and one which depends on both key
and value to implement maps (as a wrapper around sets, with an added
value[] array)
2. Separate the main hash buckets from the key / value arrays
Change the hack buckets to only contain an index into separate key /
value arrays
This is a common pattern in modern, state of the art hashmap
implementations. Even though this leads to one more allocated array, it
is this often is a net reduction of memory consumption. Consider
key+value consuming at least 12 bytes per pair. On average, we will have
twice as many buckets per item.
Thus old implementation:
2*12 = 24 bytes per item
New implementation
1*12 + 2*4 = 20 bytes per item
And the difference gets bigger with larger items.
One might think we have pulled a fast one here, as wouldn't the average size of
the new key/value arrays be 1.5 slots per items due to amortized grows?
But remember, these arrays are fully dense, and thus the accessed memory,
measured in _cache lines_, the unit which actually matters, will be the
fully used memory but just rounded up to the nearest cache line
boundary.
This has some other interesting properties, such as an insert-only
set/map will be fully ordered by insert only. Preserving this ordering
in face of deletions is more tricky tho. As we currently don't use
ordered maps, the "delete" operation maintains compactness of the item
arrays in the simplest way by breaking the ordering. It would be
possible to implement an order-preserving delete although at some cost,
like allowing the items array to become non-dense until the next rehash.
Finally, in face of these two major changes, all code used in khash.h
has been integrated into map.c and friends. Given the heavy edits it
makes no sense to "layer" the code into a vendored and a wrapper part.
Rather, the layered cake follows the specialization depth: code shared
for all maps, code specialized to a key type (and its equivalence
relation), and finally code specialized to value+key type.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reduces the total number of khash_t instantiations from 22 to 8.
Make the khash internal functions take the size of values as a runtime
parameter. This is abstracted with typesafe Map containers which
are still specialized for both key, value type.
Introduce `Set(key)` type for when there is no value.
Refactor shada.c to use Map/Set instead of khash directly.
This requires `map_ref` operation to be more flexible.
Return pointers to both key and value, plus an indicator for new_item.
As a bonus, `map_key` is now redundant.
Instead of Map(cstr_t, FileMarks), use a pointer map as the FileMarks struct is
humongous.
Make `event_strings` actually work like an intern pool instead of wtf it
was doing before.
|
| |
|
|
|
|
| |
libnvim couldn't be easily used in C++ due to the use of reserved keywords.
Additionally, add explicit casts to *alloc function calls used in inline
functions, as C++ doesn't allow implicit casts from void pointers.
|
| |
|
|
| |
Work on https://github.com/neovim/neovim/issues/459
|
|
|
It's confusing to mix vendored dependencies with neovim source code. A
clean separation is simpler to keep track of and simpler to document.
|
