Skip to content

[hipDNN] Fix memory leak in Knob::UnPack during EngineDescriptor::finalize#4903

Open
BrianHarrisonAMD wants to merge 1 commit intodevelopfrom
users/bharriso/fix-knob-leak
Open

[hipDNN] Fix memory leak in Knob::UnPack during EngineDescriptor::finalize#4903
BrianHarrisonAMD wants to merge 1 commit intodevelopfrom
users/bharriso/fix-knob-leak

Conversation

@BrianHarrisonAMD
Copy link
Contributor

Summary

  • Fix memory leak of 816 bytes (18 allocations) detected by AddressSanitizer in hipdnn_backend_tests
  • Replace heap-allocated raw pointer from Knob::UnPack() with stack-allocated KnobT using UnPackTo(), eliminating the leak during knob re-serialization in EngineDescriptor::finalize()

Details

UnPack() returns a raw owning KnobT* pointer. The old code passed this directly to Knob::Pack() as a temporary, so the pointer was never freed. The fix uses UnPackTo() to populate a stack-allocated KnobT, which is automatically cleaned up at end of scope.

Closes #4901

Test plan

  • Build with -DBUILD_ADDRESS_SANITIZER=ON
  • All 3,613 unit tests pass across all test binaries
  • Zero ASAN errors (confirmed leak is eliminated)
  • Pre-commit checks pass (clang-format)

🤖 Generated with Claude Code

…lize

Use stack-allocated KnobT with UnPackTo() instead of heap-allocated
raw pointer from UnPack() to avoid leaking the intermediate native
object during knob re-serialization.

Closes #4901

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@BrianHarrisonAMD BrianHarrisonAMD requested a review from a team as a code owner February 25, 2026 23:16
@BrianHarrisonAMD BrianHarrisonAMD self-assigned this Feb 25, 2026
@BrianHarrisonAMD BrianHarrisonAMD enabled auto-merge (squash) February 25, 2026 23:19
@BrianHarrisonAMD
Copy link
Contributor Author

auto-merge enabled

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (76.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4903   +/-   ##
========================================
  Coverage    65.96%   65.96%           
========================================
  Files         1720     1720           
  Lines       267465   267466    +1     
  Branches     37091    37091           
========================================
+ Hits        176410   176416    +6     
+ Misses       75488    75484    -4     
+ Partials     15567    15566    -1     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from 0d3aca6
hipBLASLt 43.55% <ø> (ø) Carriedforward from 0d3aca6
hipCUB 82.38% <ø> (ø) Carriedforward from 0d3aca6
hipDNN 80.84% <100.00%> (+0.03%) ⬆️
hipFFT 55.93% <ø> (ø) Carriedforward from 0d3aca6
hipRAND 76.12% <ø> (ø) Carriedforward from 0d3aca6
hipSOLVER 68.81% <ø> (ø) Carriedforward from 0d3aca6
hipSPARSE 84.70% <ø> (ø) Carriedforward from 0d3aca6
rocBLAS 47.97% <ø> (ø) Carriedforward from 0d3aca6
rocFFT 52.91% <ø> (ø) Carriedforward from 0d3aca6
rocRAND 57.06% <ø> (ø) Carriedforward from 0d3aca6
rocSOLVER 76.83% <ø> (ø) Carriedforward from 0d3aca6
rocSPARSE 71.53% <ø> (ø) Carriedforward from 0d3aca6

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...ipdnn/backend/src/descriptors/EngineDescriptor.cpp 88.94% <100.00%> (+0.05%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

[hipDNN] Memory leak in Knob::UnPack during EngineDescriptor::finalize

3 participants