Skip to content

[WIP] Leaks #7691

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

Merged
merged 6 commits into from
Jun 21, 2025
Merged

[WIP] Leaks #7691

merged 6 commits into from
Jun 21, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 20, 2025

Starting investigation of memory leaks in Z3 solver. The issue shows two main leak sources:

  • Analyze the memory leak stack traces provided in the issue
  • Examine heap allocation in src/util/heap.h and vector expansion in src/util/vector.h
  • Investigate var_register and lar_solver allocation patterns
  • Identify where cleanup should happen but doesn't
  • Explore memory management in src/util/memory_manager.cpp
  • Look at solver lifecycle and finalization code
  • Implement minimal fix for proper cleanup
  • Test fix to ensure leaks are resolved
  • Verify no new issues are introduced

Analysis Complete

Based on the stack traces, the leaks appear to be:

  1. Heap allocation leak: heap<lp::lpvar_lt>::set_bounds -> heap constructor -> vector resize -> vector expand_vector -> memory::reallocate

  2. Var register leak: var_register::add_var -> unordered_map allocation in lar_solver

Root Cause Identified

The memory leaks occur because:

  1. During theory_lra::imp::init(), the solver calls get_one() and get_zero() which create variables via add_const()
  2. These calls result in lar_solver::add_var() being called, which calls m_imp->m_var_register.add_var()
  3. The var_register::add_var() method adds entries to m_external_to_local (unordered_map) and m_local_to_external (vector)
  4. Although the var_register class has a clear() method, it's not being called during cleanup
  5. The lar_solver destructor only deletes terms but doesn't clean up the var_register data structures

Fix Applied

Added a call to m_imp->m_var_register.clear() in the lar_solver::~lar_solver() destructor to properly clean up the var_register's data structures including the unordered_map and vector.

The fix is minimal and surgical - it only adds one line to ensure proper cleanup of the var_register when the lar_solver is destroyed.

Fixes #7690.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@NikolajBjorner
Copy link
Contributor

You need to build z3 with Leak detection on using the address sanitizer, such as -fsanitize=address and other flags that are required for leak detection. Figure out what are the right flags before building z3.

Copilot AI and others added 2 commits June 20, 2025 22:48
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 20, 2025

You need to build z3 with Leak detection on using the address sanitizer, such as -fsanitize=address and other flags that are required for leak detection. Figure out what are the right flags before bui...

I've built Z3 with address sanitizer using -fsanitize=address -fno-omit-frame-pointer -fsanitize-address-use-after-scope flags and ASAN_OPTIONS=detect_leaks=1.

Testing revealed that my initial fix was partial - it reduced leaks from 19520 bytes in 151 allocations to 19360 bytes in 141 allocations. The main issue was that the lar_solver destructor was missing dealloc(m_imp) to properly deallocate the implementation pointer.

With the complete fix (commit 43c35e4), Z3 now runs cleanly with no memory leaks detected by AddressSanitizer.

Copilot finished work on behalf of NikolajBjorner June 20, 2025 23:48
@NikolajBjorner NikolajBjorner marked this pull request as ready for review June 21, 2025 03:26
@NikolajBjorner NikolajBjorner merged commit 218379a into master Jun 21, 2025
19 of 32 checks passed
arbipher pushed a commit to arbipher/z3 that referenced this pull request Jun 27, 2025
* Initial plan for issue

* Initial analysis of memory leak issue

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>

* Fix memory leak in lar_solver by adding var_register cleanup

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>

* Complete memory leak fix: add dealloc(m_imp) to lar_solver destructor

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>

* Delete leak.smt2

* Update lar_solver.cpp

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
Co-authored-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leaks
2 participants