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 memory leak in ansi-c frontend Fixes: #569 #831

Merged
merged 1 commit into from
Apr 17, 2017

Conversation

reuk
Copy link
Contributor

@reuk reuk commented Apr 14, 2017

If you swap an irep with an irep that it contains, it creates a circular reference which lives until the program quits. Unfortunately, we were doing that in the ansi-c front end, causing the leaks described in #569.

If you swap an irep with an irep that it contains, I think it
creates a circular reference which lives until the program quits.
@tautschnig
Copy link
Collaborator

I do agree with the assessment, but I'm wondering 1) how you've found it (there might be more hiding), 2) whether address sanitizer runs confirm this, and 3) how we can avoid this in future.

@reuk
Copy link
Contributor Author

reuk commented Apr 15, 2017

  1. Valgrind, ASan, reading the code. Given that the only memory leaked came from inside ireps, it was likely to be a reference counting problem. Then, you just have to read the code following the leaked allocations, and look for anything which might affect the refcounts in unexpected ways.
  2. Yes, but don't trust me. I built with make 'CXX=ccache clang++' 'CXXFLAGS=-g -O0 -fsanitize=address -fno-omit-frame-pointer' 'LINKFLAGS=-fsanitize=address' -j6, and then ran the majority of the failed tests from indirect leak in irept::detach #569 (not all, because they're pretty slow in a VM). I'd recommend that we add address sanitizer to the Linux/clang CI so that we can make sure this fixes the issue, and so that further issues are not introduced.
  3. Unit tests which are run under ASan would be the process way to avoid this.

@tautschnig
Copy link
Collaborator

Thanks a lot for all the details!

@mgudemann
Copy link
Contributor

@reuk thanks for fixing this. Once this PR is accepted, please close #569, too.

@tautschnig
Copy link
Collaborator

@mgudemann good catch; @reuk you might want to add "Fixes: #569" to the commit message to automate this step.

@reuk reuk changed the title Fix memory leak in ansi-c frontend Fix memory leak in ansi-c frontend Fixes: #569 Apr 15, 2017
@kroening kroening merged commit 4cfcb87 into diffblue:master Apr 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants