HOL-Light: improve tooling and CI#1634
Conversation
CBMC Results (ML-KEM-768)Full Results (191 proofs)
|
CBMC Results (ML-KEM-512)Full Results (191 proofs)
|
CBMC Results (ML-KEM-1024)Full Results (191 proofs)
|
724e0b1 to
9a3e91a
Compare
|
Modified the two remaining occurences of _MEMSAFE to _SAFE in order to fit the expected format for aarch proofs. |
25cad96 to
3e258d8
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @L-series - I left a couple of comments.
This PR actually takes another approach that I had in mind: I thought that after running each HOL-Light proof we check for the expected theorems to be present.
The approach implemented here is more like a linter. But I support the addition!
The -a flag is highly appreciated - thank you!
| [[ $ARCH == "x86_64" && $routine == "mlkem_rej_uniform" ]] && suffixes=("CORRECT") | ||
|
|
||
| for sfx in "${suffixes[@]}"; do | ||
| if ! grep -q "_${sfx}" "$PROOFS_DIR/${routine}.ml"; then |
There was a problem hiding this comment.
can you actually grep for {suffix} = prove or {suffix} = time prove?
There was a problem hiding this comment.
Done, however please note that I modified some of the proofs as they included double whitespaces before the = prove. I opted for this as it seems cleaner than having a more complicated regexp. As an aside, perhaps there is some tool to apply standard formatting onto the *.ml files?
Thank you for the review! Indeed, the idea I had in mind was that one should avoid using compute on proofs which do not contain the required theorems. However, I see now that there is still value running the HOL CI even if the routine is only partially proven. |
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @L-series for the changes! I'm happy with the overall approach now.
Two comments still need to be addressed. Thank you!
| if [[ $arch == "x86_64" && ${routine} == "mlkem_rej_uniform" ]]; then | ||
| suffixes=("CORRECT") | ||
| elif [[ $arch == "x86_64" ]]; then | ||
| suffixes+=("SUBROUTINE_CORRECT" "NOIBT_SUBROUTINE_CORRECT" "NOIBT_SUBROUTINE_SAFE") |
There was a problem hiding this comment.
I tried removing the SUBROUTINE_SAFE proof from the mlkem_poly_compress_d4.ml proof and the script did not catch it. Is that because NOIBT_SUBROUTINE_SAFE matches the SUBROUTINE_SAFE suffix? That should be fixed.
We can actually check the full theorem name, not just the suffix. It should be possible to derive the theorem names from the file names - we should enforce that and adjust theorem names where needed.
| if [[ $arch == "x86_64" && ${routine} == "mlkem_rej_uniform" ]]; then | ||
| suffixes=("CORRECT") |
There was a problem hiding this comment.
Why is there this exceptions? There are memory safety proofs for this routine?
| (* ========================================================================= *) | ||
|
|
||
| let MLKEM_REJ_UNIFORM_MEMSAFE = prove | ||
| let MLKEM_REJ_UNIFORM_SAFE = prove |
There was a problem hiding this comment.
This was intentionally named this way and I would prefer to keep it like that because we do not have any constant time proofs at this time.
Can you implement it as: if mlkem_rej_uniform then MEMSAFE else SAFE? That would be better, I believe.
|
@L-series, gentle ping. Could you please adjust this PR so we can get this merged? |
|
@L-series, are you still working on this? |
|
@L-series, do you expect to finish this PR soon? |
This commit introduces a new flag --arch to the hol_light command of the tests script that allows user specification of which architecture to run/list the proofs for. If not passed, the behavior is unchanged. Signed-off-by: Andreas Hatziiliou <andreas.hatziiliou@savoirfairelinux.com>
Apologies for the long delay, I was away traveling this last month. I just pushed some changes and I've reworked However, I just wanted to make sure that this is okay with you as it would require renaming around 15 proofs. |
Thanks for picking this upa gain @L-series! Welcome back! |
We add a check to the linting script called check-theorems that ensures that all HOL-Light proofs provide the expected set of theorems depending on the architecture. Signed-off-by: Andreas Hatziiliou <andreas.hatziiliou@savoirfairelinux.com>
This commit aims to resolve the two following issues:
testsscript to allow cross-platform proof checking #1585Note that the
check-theoremsscript will currently fail the HOL-Light CI because for AArch, the naming convention for the memory safety proof (usually _SAFE) is not respected inmlkem_rej_uniform(it uses _MEMSAFE). @hanno-becker please advise if I should modify this.