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

c: fix all memory leaks; add sanitizer checks #3223

Merged
merged 20 commits into from
Feb 4, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Feb 3, 2024

Fix #3045. All memory leaks have been fixed!

@njzjz njzjz requested a review from wanghan-iapcm February 3, 2024 11:12
@njzjz njzjz marked this pull request as draft February 3, 2024 11:13
Signed-off-by: Jinzhe Zeng <[email protected]>
Copy link

codecov bot commented Feb 3, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (412c812) 74.78% compared to head (ed9914e) 74.90%.

Files Patch % Lines
source/api_c/include/deepmd.hpp 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3223      +/-   ##
==========================================
+ Coverage   74.78%   74.90%   +0.12%     
==========================================
  Files         362      362              
  Lines       32396    32432      +36     
  Branches     1594     1608      +14     
==========================================
+ Hits        24226    24292      +66     
- Misses       7245     7267      +22     
+ Partials      925      873      -52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@njzjz njzjz marked this pull request as ready for review February 3, 2024 11:44
@njzjz njzjz added the bug label Feb 3, 2024
njzjz and others added 5 commits February 3, 2024 16:54
@wanghan-iapcm wanghan-iapcm merged commit ab2c551 into deepmodeling:devel Feb 4, 2024
46 checks passed
@wanghan-iapcm
Copy link
Collaborator

cherry pick to v2.2 ?

@njzjz
Copy link
Member Author

njzjz commented Feb 4, 2024

cherry pick to v2.2 ?

Agree.

njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Feb 4, 2024
Fix deepmodeling#3045. All memory leaks have been fixed!

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit ab2c551)
wanghan-iapcm pushed a commit that referenced this pull request Feb 4, 2024
Fix #3045. All memory leaks have been fixed!

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit ab2c551)
njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Feb 5, 2024
A segfault sometimes appears in the tests after deepmodeling#3223. The reason is that the required shape of electric field is set to nall * 3 in the C interface, but a vector of nloc * 3 is actually given in the tests and lammps and used in the C++ interface. It's only found by sanitizer as the program doesn't write to the invalid address (only reading it won't cause segfault).

Signed-off-by: Jinzhe Zeng <[email protected]>
wanghan-iapcm pushed a commit that referenced this pull request Feb 6, 2024
[A
segfault](https://github.com/deepmodeling/deepmd-kit/actions/runs/7782245372/job/21218255452)
sometimes appears in the tests after #3223. The reason is that the
required shape of the electric field is set to nall * 3 in the C
interface, but a vector of nloc * 3 is given in the tests and lammps and
used in the C++ interface. It was not caught before, as the program
didn't write to these addresses (only reading it usually won't cause a
segfault, unless the address is invalid).
Perhaps fix #2895.

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz mentioned this pull request Apr 2, 2024
njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Apr 6, 2024
…ing#3237)

[A
segfault](https://github.com/deepmodeling/deepmd-kit/actions/runs/7782245372/job/21218255452)
sometimes appears in the tests after deepmodeling#3223. The reason is that the
required shape of the electric field is set to nall * 3 in the C
interface, but a vector of nloc * 3 is given in the tests and lammps and
used in the C++ interface. It was not caught before, as the program
didn't write to these addresses (only reading it usually won't cause a
segfault, unless the address is invalid).
Perhaps fix deepmodeling#2895.

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
(cherry picked from commit e281a8c)
njzjz added a commit that referenced this pull request Apr 6, 2024
[A
segfault](https://github.com/deepmodeling/deepmd-kit/actions/runs/7782245372/job/21218255452)
sometimes appears in the tests after #3223. The reason is that the
required shape of the electric field is set to nall * 3 in the C
interface, but a vector of nloc * 3 is given in the tests and lammps and
used in the C++ interface. It was not caught before, as the program
didn't write to these addresses (only reading it usually won't cause a
segfault, unless the address is invalid).
Perhaps fix #2895.

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
(cherry picked from commit e281a8c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants