Skip to content

Fix MSan use-of-uninitialized-value in SimSIMD SVE functions#101239

Open
alexey-milovidov wants to merge 3 commits intomasterfrom
fix-simsimd-sve-msan
Open

Fix MSan use-of-uninitialized-value in SimSIMD SVE functions#101239
alexey-milovidov wants to merge 3 commits intomasterfrom
fix-simsimd-sve-msan

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Mar 30, 2026 •
edited
Loading

Update SimSIMD submodule to include fix for svmla_*_x / svmls_*_x in SVE accumulator operations. The _x (don't-care) variant left inactive lanes undefined, but svaddv(svptrue, ...) summed all lanes including those undefined ones. Changed to _m (merge) which preserves the accumulator value for inactive lanes.

Fixes #101232

Contrib PR: ClickHouse/SimSIMD#18
Upstream PR: ashvardanian/NumKong#331

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100177&sha=a7031a78dd031fc0e90fbeebc0c95df386c5c1a7&name_0=PR&name_1=Stress%20test%20%28arm_msan%29

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

...

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@alexey-milovidov @claude
Fix MSan use-of-uninitialized-value in SimSIMD SVE functions
ef8c9a8 
Update SimSIMD submodule to include fix for `svmla_*_x` / `svmls_*_x`
in SVE accumulator operations. The `_x` (don't-care) variant left
inactive lanes undefined, but `svaddv(svptrue, ...)` summed all lanes
including those undefined ones. Changed to `_m` (merge) which preserves
the accumulator value for inactive lanes.

Fixes #101232

https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100177&sha=a7031a78dd031fc0e90fbeebc0c95df386c5c1a7&name_0=PR&name_1=Stress%20test%20%28arm_msan%29

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 30, 2026 •
edited
Loading

Workflow [PR], commit [a43cadd]

Summary: ❌

job_name test_name status info comment
Integration tests (amd_llvm_coverage, 2/5) failure
test_overcommit_tracker/test.py::test_user_overcommit FAIL cidb
Stress test (arm_msan) failure
MemorySanitizer: use-of-uninitialized-value (STID: 1003-358c) FAIL cidb, issue
Stateless tests (amd_tsan, s3 storage, parallel, 1/2) error
Unit tests (asan_ubsan) error

AI Review

Summary

This PR updates only the contrib/SimSIMD submodule pointer to include upstream fix 5a0677af82f7 for SVE accumulator operations, replacing _x intrinsics with _m so inactive lanes preserve prior accumulator values before full-lane reductions. Based on the submodule delta (single commit, focused to include/simsimd/dot.h and include/simsimd/spatial.h), this is a targeted correctness fix for uninitialized-lane behavior under MSan, and I did not find additional issues in the ClickHouse-side change.

ClickHouse Rules
Item Status Notes
Deletion logging ➖
Serialization versioning ➖
Core-area scrutiny ✅
No test removal ✅
Experimental gate ➖
No magic constants ➖
Backward compatibility ✅
SettingsChangesHistory.cpp ➖
PR metadata quality ✅
Safe rollout ✅
Compilation time ✅
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh bot added pr-ci submodule changed At least one submodule changed in this PR. labels Mar 30, 2026
@nikitamikhaylov
Copy link
Copy Markdown
Member

nikitamikhaylov commented Mar 30, 2026

@groeneai It was right to update the submodule rather than disabling the MSAN in the SimSIMD like you did here #100862

@groeneai
Copy link
Copy Markdown
Contributor

groeneai commented Mar 30, 2026

Thanks @nikitamikhaylov — agreed, the submodule update fixing the _x → _m SVE accumulator variants is the proper approach since it addresses the root cause in SimSIMD directly. Our #100862 was a workaround (disable SVE under MSAN) that we already closed in favor of this.

This was referenced Apr 1, 2026
Merged
Merged
@nikitamikhaylov
Copy link
Copy Markdown
Member

nikitamikhaylov commented Apr 1, 2026

@groeneai The issue still reproduces. Do you know what's wrong here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

pr-ci submodule changed At least one submodule changed in this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MemorySanitizer: use-of-uninitialized-value (STID: 1003-358c)

3 participants

@alexey-milovidov @nikitamikhaylov @groeneai