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

Fix bugs detected by the address sanitizer #2212

Merged
merged 23 commits into from
Feb 17, 2024

Conversation

maxaehle
Copy link
Contributor

@maxaehle maxaehle commented Feb 14, 2024

Steps to reproduce the underlying issue

Compile SU2 with Clang and -fsanitize=address (and possibly -fno-omit-frame-pointer), and run the testcases as usual. AddressSanitizer warnings appear in the output (if any).

So far, only the tests in serial_regression.py have been analyzed.

Proposed Changes

Depending on the context of the memory leak,

  • add delete / delete[] statement
  • turn new-allocated arrays into std::vectors, or
  • for one bug caused by a delete to a base class pointer, make its destructor virtual.

Related Work

#2211 and #2213 have been detected in the same way, but I'm not sure how to properly fix them.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • [not necessary] I have added a test case that demonstrates my contribution, if necessary.
  • [not necessary] I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

Max Aehle added 6 commits February 14, 2024 14:16
In CPhysicalGeometry.cpp, instances of derived classes are
delete'd via CMeshReaderFVM pointers. Without a virtual
destructor, destructors of members added by these derived classes
are not called. With respect to e.g. the std::vector members of
CCGNSMeshReaderFVM, this leads to memory leaks.
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catches

Common/include/option_structure.inl Outdated Show resolved Hide resolved
jblueh and others added 5 commits February 15, 2024 10:45
This helps to avoid memory leaks and double frees from the
respective COptionActDisk instances, and allows to check that
identical marker lists were supplied.
@jblueh
Copy link
Contributor

jblueh commented Feb 17, 2024

The contributions I made are from investigating serial_regression_AD.py in the same manner. parallel_regression.py and parallel_regression_AD.py are checked to a certain extent, too.

@jblueh jblueh marked this pull request as ready for review February 17, 2024 01:27
@jblueh jblueh merged commit 0f211b5 into develop Feb 17, 2024
30 of 31 checks passed
@jblueh jblueh deleted the fix_addresssanitizer_findings branch February 17, 2024 01:28
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.

3 participants