Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There is a memory leak defect at line 1303 in the file /vim/src/dict.c. #14477

Closed
LuMingYinDetect opened this issue Apr 10, 2024 · 4 comments
Closed
Labels

Comments

@LuMingYinDetect
Copy link

Steps to reproduce

1.A pointer named d1 is defined at line 1272 in the file /vim/src/dict.c. This pointer is initialized to argvars[0].vval.v_dict at line 1276. Subsequently, this pointer is passed as an argument to the function dict_copy for copying at line 1291, and the copied memory space is assigned to the pointer d1,When the if statement at line 1302 returns true, the program returns at line 1303. as shown in the diagram below:
https://github.com/LuMingYinDetect/vim_defects/blob/main/vim_24.png

2.Previously, I reported this bug in #14238. Recently, I revisited the dict_copy function. In this function, the memory area being copied is named orig. At line 297 of dict_copy function, a pointer named copy is defined. This pointer is allocated a dynamic memory area using the dict_alloc function at line 305. Then, at line 312, orig->dv_copydict = copy is executed, assigning the dynamic memory area pointed to by copy to orig->dv_copydict. Finally, at line 355, the function returns. Both the return value in dict_copy function and orig->dv_copydict point to the dynamic memory area allocated by the dict_alloc function.
However, after dict_copy function execution, when the program returns from line 1303, only orig->dv_copydict points to the dynamic memory area allocated by the dict_alloc function. But this doesn't seem to be the intended behavior of this code. Testing in the issue also confirms this (continuous increase in Vim's memory). as shown in the diagram below:
https://github.com/LuMingYinDetect/vim_defects/blob/main/vim_25.png

Expected behaviour

If this defect is indeed true, it is advisable to fix it.

Version of Vim

9.1.0296

Environment

Ubuntu 20.04

Logs and stack traces

No response

@chrisbra
Copy link
Member

This one looks like a false positive. d1 comes from the argvars pointer, which is the pointer to the zero argument in this case.

Those pointers will then later be freed in call_func_tv

@LuMingYinDetect
Copy link
Author

This one looks like a false positive. d1 comes from the argvars pointer, which is the pointer to the zero argument in this case.

Those pointers will then later be freed in call_func_tv

Thank you for your patient explanation! I checked the file /vim/src/dict.c and indeed, on line 1276 of the program, the statement d1=argvars[0].vval.v_dict; is executed, which assigns the value of argvars[0].vval.v_dict to the d1 pointer. However, on line 1291 of the program, the statement d1=dict_copy(d1,False,True,getcopyID()); is executed, which calls the function dict_copy. Inside dict_copy, the function dict_alloc is used to allocate a new block of dynamic memory and returns it. Therefore, after dict_copy is executed, d1 seems to point to a new memory area. But I noticed that the program also assigns the newly allocated dynamic memory area to the original d1 (i.e., argvars[0].vval.v_dict)'s dv_copydict. When the program follows the defect path given above, the newly allocated dynamic memory area is not used, but only pointed to by argvars[0].vval.v_dict->dv_copydict. I'm not sure if this was intentional or an unintentional mistake. According to what you said, this newly allocated dynamic memory area should be released in the call_func_tv function, but the test provided in the issue #14238 suggests that the memory may not be released.

@chrisbra
Copy link
Member

ah, so this is a duplicate of #14238 ? Please don't report duplicate issues.

chrisbra added a commit to chrisbra/vim that referenced this issue Apr 15, 2024
fixes: vim#14477
fixes: vim#14238
Signed-off-by: Christian Brabandt <cb@256bit.org>
zeertzjq added a commit to zeertzjq/neovim that referenced this issue Apr 16, 2024
Problem:  a few memory leaks are found
          (LuMingYinDetect )
Solution: properly free the memory

Fixes the following problems:
- Memory leak in f_maplist()
  fixes: vim/vim#14486

- Memory leak in option.c
  fixes: vim/vim#14485

- Memory leak in f_resolve()
  fixes: vim/vim#14484

- Memory leak in f_autocmd_get()
  related: vim/vim#14474

- Memory leak in dict_extend_func()
  fixes: vim/vim#14477
  fixes: vim/vim#14238

closes: vim/vim#14517

vim/vim@29269a7

Co-authored-by: Christian Brabandt <cb@256bit.org>
zeertzjq added a commit to neovim/neovim that referenced this issue Apr 16, 2024
Problem:  a few memory leaks are found
          (LuMingYinDetect )
Solution: properly free the memory

Fixes the following problems:
- Memory leak in f_maplist()
  fixes: vim/vim#14486

- Memory leak in option.c
  fixes: vim/vim#14485

- Memory leak in f_resolve()
  fixes: vim/vim#14484

- Memory leak in f_autocmd_get()
  related: vim/vim#14474

- Memory leak in dict_extend_func()
  fixes: vim/vim#14477
  fixes: vim/vim#14238

closes: vim/vim#14517

vim/vim@29269a7

Co-authored-by: Christian Brabandt <cb@256bit.org>
famiu pushed a commit to famiu/neovim that referenced this issue Apr 18, 2024
Problem:  a few memory leaks are found
          (LuMingYinDetect )
Solution: properly free the memory

Fixes the following problems:
- Memory leak in f_maplist()
  fixes: vim/vim#14486

- Memory leak in option.c
  fixes: vim/vim#14485

- Memory leak in f_resolve()
  fixes: vim/vim#14484

- Memory leak in f_autocmd_get()
  related: vim/vim#14474

- Memory leak in dict_extend_func()
  fixes: vim/vim#14477
  fixes: vim/vim#14238

closes: vim/vim#14517

vim/vim@29269a7

Co-authored-by: Christian Brabandt <cb@256bit.org>
@LuMingYinDetect
Copy link
Author

ah, so this is a duplicate of #14238 ? Please don't report duplicate issues.

I'm sorry for any inconvenience caused! I just had a new discovery regarding this defect, so I opened a new question at that time. I should have explained it in the original question. I'll pay attention to that next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants