Use heap allocation + valgrind in backend unit test#1089
Merged
mkannwischer merged 3 commits intomainfrom May 4, 2026
Merged
Conversation
c694808 to
781325d
Compare
Contributor
CBMC Results (ML-DSA-44, REDUCE-RAM)Full Results (200 proofs)
|
Contributor
CBMC Results (ML-DSA-44)Full Results (200 proofs)
|
Contributor
CBMC Results (ML-DSA-65, REDUCE-RAM)Full Results (200 proofs)
|
Contributor
CBMC Results (ML-DSA-87, REDUCE-RAM)Full Results (200 proofs)
|
Contributor
CBMC Results (ML-DSA-87)Full Results (200 proofs)
|
Contributor
CBMC Results (ML-DSA-65)Full Results (200 proofs)
|
mkannwischer
requested changes
May 3, 2026
Contributor
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @hanno-becker. The port looks good to me.
Can we also cover the rejction sampling, please?
The commit message could use updating:
* Build the unit test objects with custom_heap_alloc_config.h by adding
the appropriate -DMLD_CONFIG_FILE, -std=c11, and -D_GNU_SOURCE flags
in components.mk, factored through a new UNIT_CFLAGS variable.
Contributor
Author
|
@mkannwischer This is a pre-existing test gap? It should be addressed, but seems orthogonal to this PR. |
Contributor
Author
|
Opened #1090 and addressed in the follow-up commit. Also fixed the commit message. |
mkannwischer
approved these changes
May 4, 2026
Contributor
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @hanno-becker for adding that. LGTM. Happy if CI is.
Port of mlkem-native#1633 to mldsa-native. * Replace aligned_alloc + MLD_ALIGN_UP with posix_memalign in custom_heap_alloc_config.h. Unlike aligned_alloc, posix_memalign does not require the size to be a multiple of the alignment, removing the need for MLD_ALIGN_UP rounding. This ensures that allocations are exact-sized, allowing memory-safety tests like valgrind and ASan to detect overflows at precise buffer boundaries. On Windows, _aligned_malloc is used instead. Also adds the missing configs.yml entry so the file is tracked by autogen. * Replaced stack-allocated buffers in test_unit.c with allocations based on customizable MLD_ALLOC/FREE. The existing unit test run in CI remains stack-based for portability. * Add separate unit_valgrind job to ci.yml that, on x86_64 and aarch64 runners, runs the unit tests using valgrind + heap-based MLD_ALLOC/FREE. This catches buffer overflows in hand-written assembly that ASan cannot detect, since ASan only instruments compiler-generated code. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Add native-vs-C consistency tests for previously untested backends: - mld_rej_uniform_native: compare against mld_rej_uniform_c - mld_rej_uniform_eta2_native: compare against mld_rej_eta_c - mld_rej_uniform_eta4_native: compare against mld_rej_eta_c In line with mlkem-native, these tests call the native backends directly. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Fixes #1092 Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
Contributor
Author
|
@mkannwischer Multiplexed the unit test for |
mkannwischer
approved these changes
May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port of mlkem-native#1633 to mldsa-native.
Replace aligned_alloc + MLD_ALIGN_UP with posix_memalign in custom_heap_alloc_config.h. Unlike aligned_alloc, posix_memalign does not require the size to be a multiple of the alignment, removing the need for MLD_ALIGN_UP rounding. This ensures that allocations are exact-sized, allowing memory-safety tests like valgrind and ASan to detect overflows at precise buffer boundaries. On Windows, _aligned_malloc is used instead. Also adds the missing configs.yml entry so the file is tracked by autogen.
Replace all stack-allocated buffers in test_unit.c with heap allocations via MLD_ALLOC/MLD_FREE, using the custom_heap_alloc_config. This enables valgrind to detect buffer overflows in assembly backends, which operate on these buffers.
Build the unit test objects with custom_heap_alloc_config.h by adding the appropriate -DMLD_CONFIG_FILE, -std=c11, and -D_GNU_SOURCE flags in components.mk, factored through a new UNIT_CFLAGS variable.
Add unit_valgrind job to ci.yml that runs the unit tests under valgrind on x86_64 and aarch64 runners. This catches buffer overflows in hand-written assembly that ASan cannot detect, since ASan only instruments compiler-generated code.