Add debug assertions for reduce.c and polyvec.c #416

willieyz · 2025-08-07T11:48:27Z

Resolves Add debug assertions for reduce.c and polyvec.c #216
All implementations of functions in poly.c, polyvec.c have bounds assertions
for inputs and outputs added that match the CBMC specifications.
As for mlkem-native, if CBMC is enabled, the bounds assertions
are interpreted as CBMC proof obligations.

mkannwischer

Thanks @willieyz. This is a nice addition!
There are still quite a few things that could be improved.

mkannwischer · 2025-08-13T05:39:13Z

mldsa/polyvec.c

@@ -384,11 +421,15 @@ void mld_polyveck_sub(mld_polyveck *u, const mld_polyveck *v)
  {
    mld_poly_sub(&u->vec[i], &v->vec[i]);
  }
+
+  mld_assert_bound_2d(u->vec, MLDSA_K, MLDSA_N, (int64_t)INT32_MIN,


This cast should not be needed.

mkannwischer · 2025-08-13T05:39:39Z

mldsa/polyvec.c

 }

 void mld_polyvecl_reduce(mld_polyvecl *v)
 {
  unsigned int i;
+  mld_assert_bound_2d(v->vec, MLDSA_L, MLDSA_N, (int64_t)INT32_MIN,


This cast should not be needed.

mldsa/poly.c

mkannwischer · 2025-08-13T05:47:07Z

mldsa/poly.c

@@ -379,6 +393,8 @@ void mld_poly_uniform(mld_poly *a, const uint8_t seed[MLDSA_SEEDBYTES + 2])

  /* FIPS 204. Section 3.6.3 Destruction of intermediate values. */
  mld_zeroize(buf, sizeof(buf));
+
+  mld_assert_bound(a->coeffs, MLDSA_N, 0, MLDSA_Q);


Can we have all assertions before the zeroize call please? Just to be consistent with mlkem-native that zeroize is the last step in the function.

Fixed, Thank you for your reviewing!

mkannwischer · 2025-08-13T05:51:17Z

mldsa/poly.c

@@ -84,6 +86,8 @@ void mld_poly_sub(mld_poly *r, const mld_poly *b)
  {
    r->coeffs[i] = r->coeffs[i] - b->coeffs[i];
  }
+
+  mld_assert_bound(r->coeffs, MLDSA_N, (int64_t)INT32_MIN, REDUCE32_DOMAIN_MAX);


This cast should not be needed

mkannwischer · 2025-08-13T05:57:07Z

mldsa/polyvec.c

 }

 void mld_polyvecl_pointwise_acc_montgomery(mld_poly *w, const mld_polyvecl *u,
                                           const mld_polyvecl *v)
 {
  unsigned int i, j;
+  mld_assert_bound_2d(u->vec, MLDSA_L, MLDSA_N, 0, MLDSA_Q);


isn't this missing a pre-condition on v?

Yes, sorry about the miss..
I had fixed it now, thank you for your reviewing!

mkannwischer · 2025-08-13T06:00:11Z

mldsa/polyvec.c

@@ -372,6 +407,8 @@ void mld_polyveck_add(mld_polyveck *u, const mld_polyveck *v)
 void mld_polyveck_sub(mld_polyveck *u, const mld_polyveck *v)


mld_polyveck_add can have a post-condition in the contract - just like mld_polyvecl_add. That should be added and also be asserted.

Added, thank you for your reviewing!

mkannwischer · 2025-08-13T06:01:21Z

mldsa/polyvec.c

@@ -431,12 +477,15 @@ void mld_polyveck_invntt_tomont(mld_polyveck *v)
  {
    mld_poly_invntt_tomont(&v->vec[i]);
  }
+
+  mld_assert_abs_bound_2d(v->vec, MLDSA_K, MLDSA_N, MLD_NTT_BOUND);


This should be INTT_BOUND.

mkannwischer · 2025-08-13T06:09:32Z

mldsa/poly.c

Can the assertion for mld_poly_reduce now be added?

Fixed,
I had added the assert for pre-condition and post-condition,
Thank you for your reviewing!

mkannwischer · 2025-08-13T06:12:14Z

mldsa/poly.c

@@ -141,6 +146,9 @@ void mld_poly_pointwise_montgomery(mld_poly *c, const mld_poly *a,
 {


mld_poly_invntt_tomont should have an MLD_INTT_BOUND output bound.

Added, thank you for the reviewing!

mkannwischer

Thanks @willieyz. I think we settled on changing the bounds back to q*3/4, right? Could you please change that?

- This commit is ported from mlkem PR #214 - All implementations of functions in poly.c have bounds assertions for inputs and outputs added that match the CBMC specifications. Signed-off-by: willieyz <willie.zhao@chelpis.com>

Signed-off-by: willieyz <willie.zhao@chelpis.com>

- This commit is ported from mlkem PR #214 - All implementations of functions in polyvec.c have bounds assertions for inputs and outputs added that match the CBMC specifications. Signed-off-by: willieyz <willie.zhao@chelpis.com>

…test error Signed-off-by: willieyz <willie.zhao@chelpis.com>

willieyz force-pushed the cbmc-bound-assertions branch 8 times, most recently from 407a414 to a4cfae5 Compare August 8, 2025 04:02

willieyz changed the title ~~Debug: Add bounds assertions macros (port mlkem-native #214)~~ Debug: Add bounds assertions macros (port PR#214) Aug 8, 2025

willieyz changed the title ~~Debug: Add bounds assertions macros (port PR#214)~~ Add debug assertions for reduce.c and polyvec.c #216 Aug 8, 2025

willieyz force-pushed the cbmc-bound-assertions branch 5 times, most recently from b99b71f to 1a95672 Compare August 11, 2025 08:53

willieyz marked this pull request as ready for review August 11, 2025 09:19

willieyz requested a review from a team as a code owner August 11, 2025 09:19

mkannwischer requested changes Aug 13, 2025

View reviewed changes

willieyz marked this pull request as draft August 13, 2025 08:57

willieyz force-pushed the cbmc-bound-assertions branch 3 times, most recently from 7e05da5 to d36b82b Compare August 15, 2025 02:03

willieyz force-pushed the cbmc-bound-assertions branch 8 times, most recently from 4fca5db to b13c665 Compare August 25, 2025 08:14

willieyz force-pushed the cbmc-bound-assertions branch 13 times, most recently from 36bd946 to 1cbb91f Compare August 28, 2025 03:55

willieyz changed the title ~~Add debug assertions for reduce.c and polyvec.c #216~~ Add debug assertions for reduce.c and polyvec.c Aug 29, 2025

willieyz force-pushed the cbmc-bound-assertions branch 4 times, most recently from a46549a to fae1af6 Compare September 3, 2025 07:00

willieyz requested a review from mkannwischer September 3, 2025 08:05

willieyz marked this pull request as ready for review September 3, 2025 08:05

mkannwischer requested changes Sep 16, 2025

View reviewed changes

willieyz added 3 commits September 17, 2025 10:26

Debug: Add bounds assertions in poly.c

7c32239

- This commit is ported from mlkem PR #214 - All implementations of functions in poly.c have bounds assertions for inputs and outputs added that match the CBMC specifications. Signed-off-by: willieyz <willie.zhao@chelpis.com>

fix: fix the overflow problem for the exclusive lower bound

ed98635

Signed-off-by: willieyz <willie.zhao@chelpis.com>

Debug: Add bounds assertions in polyvec.c

90a6698

- This commit is ported from mlkem PR #214 - All implementations of functions in polyvec.c have bounds assertions for inputs and outputs added that match the CBMC specifications. Signed-off-by: willieyz <willie.zhao@chelpis.com>

willieyz force-pushed the cbmc-bound-assertions branch from fae1af6 to fbbfb99 Compare September 17, 2025 02:27

Fix: change the MLD_INTT_BOUND to (MLDSA_Q * 3 / 4), fix the debug …

3eba7b2

…test error Signed-off-by: willieyz <willie.zhao@chelpis.com>

willieyz force-pushed the cbmc-bound-assertions branch from fbbfb99 to 3eba7b2 Compare September 17, 2025 02:31

		@@ -372,6 +407,8 @@ void mld_polyveck_add(mld_polyveck u, const mld_polyveck v)
		void mld_polyveck_sub(mld_polyveck u, const mld_polyveck v)

		@@ -141,6 +146,9 @@ void mld_poly_pointwise_montgomery(mld_poly c, const mld_poly a,
		{

Add debug assertions for reduce.c and polyvec.c #416

Are you sure you want to change the base?

Add debug assertions for reduce.c and polyvec.c #416

Conversation

willieyz commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkannwischer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willieyz Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkannwischer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

willieyz commented Aug 7, 2025 •

edited

Loading

willieyz Aug 13, 2025 •

edited

Loading