Skip to content

new[CI]: add linux sanitizer tests#2860

Merged
awni merged 4 commits into
ml-explore:mainfrom
incertum:new/linux-sanitizer-tests-ci
Dec 18, 2025
Merged

new[CI]: add linux sanitizer tests#2860
awni merged 4 commits into
ml-explore:mainfrom
incertum:new/linux-sanitizer-tests-ci

Conversation

@incertum

@incertum incertum commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

Proposed changes

new[CI]: add linux sanitizer tests

CC @madrob

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

Comment thread .github/scripts/build-sanitizer-tests.sh
@madrob

madrob commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

Did this find any bugs?

@madrob madrob requested a review from awni December 3, 2025 16:27
Comment thread CMakeLists.txt
@incertum

incertum commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

Did this find any bugs?

Now with the fix on top checkout https://github.com/ml-explore/mlx/actions/runs/19943012002/job/57185456349?pr=2860 ... it seems that we could improve a few things, however note that to be more serious we would need to add fuzzing! Also note that for now I turned of the leak detector etc (see settings).

@incertum

incertum commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

@awni: @madrob and I chatted and are proposing to defer enabling ASAN and TSAN CI checks until follow up PR(s) where issues will get addressed.
We left UBSAN as it's current configured to not halt on error as UBSAN is generally more tricky to address, but that way we can view the logs if we wanted to.

What do you think?

@awni

awni commented Dec 5, 2025

Copy link
Copy Markdown
Member

Fine with me. Do you plan do to send some of those follow ups?

@incertum

incertum commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

Fine with me. Do you plan do to send some of those follow ups?

Yes I'll try to look into it :)

Signed-off-by: Melissa Kilby <mkilby@apple.com>
…s PUBLIC

Signed-off-by: Melissa Kilby <mkilby@apple.com>
Signed-off-by: Melissa Kilby <mkilby@apple.com>
@incertum

Copy link
Copy Markdown
Contributor Author

@angeloskath and @awni I have just rebased this PR and the ASAN tests (CPU backend) look good now. The todo comment is also up-to-date plus I created #2915.

What would be needed from your perspective to get a v1 of these new checks merged in?

Signed-off-by: Melissa Kilby <mkilby@apple.com>

@awni awni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@awni awni merged commit ff26b00 into ml-explore:main Dec 18, 2025
14 checks passed
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.

3 participants