memray icon indicating copy to clipboard operation
memray copied to clipboard

Add supprt for new c23 deallocators

Open naamanu opened this issue 4 months ago • 3 comments

closes #725

This commit introduces 2 new functions to add support for free_sized and free_aligned_sized deallocators introduced in c23.

Tests ensure that free_sized and free_aligned_sized are skipped if unavailable otherwise we get a FREE record recorded for them.

naamanu avatar Aug 20 '25 19:08 naamanu

Codecov Report

:x: Patch coverage is 20.40816% with 39 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 90.76%. Comparing base (1425302) to head (88f0b37). :warning: Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
tests/integration/test_extensions.py 23.25% 33 Missing :warning:
src/memray/_memray/hooks.cpp 0.00% 6 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #817      +/-   ##
==========================================
- Coverage   92.19%   90.76%   -1.44%     
==========================================
  Files          95       95              
  Lines       11940    11985      +45     
  Branches      413      415       +2     
==========================================
- Hits        11008    10878     -130     
- Misses        932     1107     +175     
Flag Coverage Δ
cpp 90.76% <20.40%> (-1.44%) :arrow_down:
python_and_cython 90.76% <20.40%> (-1.44%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov-commenter avatar Aug 29 '25 18:08 codecov-commenter

Don't worry about the failing macOS test, that will be fixed by a rebase after we land #752

It looks like we're ahead of the curve here. Neither glibc nor musl libc suppport free_sized and free_aligned_sized yet (patches have been submitted to both for adding support, but not yet merged for either). Given that, I'm not sure how we can really test that this is working.

Hm. @pablogsal do you think it would be reasonable to have our tests build in C99 mode, instead of C23, and LD_PRELOAD a shared library of our own creation that provides free_sized and free_aligned_sized? Or do you think that's too different from the real case where they're provided by libc to be a trustworthy test proving that our interposition is working?

godlygeek avatar Sep 11 '25 04:09 godlygeek

Hm. @pablogsal do you think it would be reasonable to have our tests build in C99 mode, instead of C23, and LD_PRELOAD a shared library of our own creation that provides free_sized and free_aligned_sized? Or do you think that's too different from the real case where they're provided by libc to be a trustworthy test proving that our interposition is working?

I don't think this will be very useful. We already know our interposition works but what we want to check is that indeed works with GLIBC as most of the trickery can happen when we deal with the real implementation: we can be dealing with weak symbols, versioned symbols, weird redirects ...etc Having our own fake shared object would not help a lot with that

pablogsal avatar Sep 11 '25 13:09 pablogsal