-
Notifications
You must be signed in to change notification settings - Fork 25
HOL-Light: Add HOL-Light proof framework and x86 AVX2 NTT proof #640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0f77c6c to
c321a88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jakemas - this is awesome progress!
I tried to run the proofs on a c8g.4x-large in the nix develop .#hol_light shell, but
it does not work as x86_64-linux-gnu-objdump and x86_64-linux-gnu-as are not installed.
We can probably fix cross compilation in a follow-up PR -- running it on x86 now.
ubuntu@ip-172-31-34-86:~/mldsa-native tests hol_light -p mldsa_ntt
INFO > HOL_LIGHT (1/1) None (native): Starting HOL-Light proof for mldsa_ntt
ERROR > HOL_LIGHT (1/1) None (native): FAILED (after 0s)
ERROR > HOL_LIGHT (1/1) None (native): sh: line 1: x86_64-linux-gnu-as: command not found
sh: line 1: x86_64-linux-gnu-objdump: command not found
sh: line 1: x86_64-linux-gnu-as: command not found
make: *** [Makefile:70: mldsa/mldsa_ntt.o] Error 127
1 tests FAILED
* HOL-Light proof for mldsa_ntt
ubuntu@ip-172-31-34-86:~/mldsa-native$ make -C proofs/hol_light/x86_64
make: Entering directory '/home/ubuntu/mldsa-native/proofs/hol_light/x86_64'
Preparing mldsa/mldsa_ntt.o ...
sh: line 1: x86_64-linux-gnu-as: command not found
AS:
sh: line 1: x86_64-linux-gnu-objdump: command not found
OBJDUMP:
[ -d mldsa ] || mkdir -p mldsa
cat mldsa/mldsa_ntt.S | gcc -E -xassembler-with-cpp - | tr ';' '\n' | x86_64-linux-gnu-as -o mldsa/mldsa_ntt.o -
sh: line 1: x86_64-linux-gnu-as: command not found
make: *** [Makefile:70: mldsa/mldsa_ntt.o] Error 127
make: Leaving directory '/home/ubuntu/mldsa-native/proofs/hol_light/x86_64'
This aligns the lint script with mlkem-native: - Fixing a typo that was discovered in #640 - Uncommenting the shell linting I have no memories why the latter was commented out. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
This aligns the lint script with mlkem-native: - Fixing a typo that was discovered in #640 - Uncommenting the shell linting I have no memories why the latter was commented out. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
87f4d53 to
24a1562
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jakemas. I took the liberty to also add it to the root README.
I verified that the proof runs okay.
I tested on a m4.4xlarge machine and the proofs completed after 20393s.
I tried to understand the main theorem that is being proven - the bounds look good and are indeed much tighter than what we require.
It currently only works on x86, but we can address that in a follow up.
Here is a list of follow-up issues that we should tackle:
@hanno-becker, WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @jakemas @jargh for the work! 🎉 It's great to see the first correctness proof for x86_64, let alone such a complicated one.
A few things:
- The documentation still needs adjusting after copy-over from mlkem-native. Most importantly, we cannot claim to have verified all x86_64 assembly, which is currently stated at the top of
proofs/hol_light/x86_64/README.md. - I would like to stick a homogeneous assembly syntax, for now AT&T everywhere. We can uniformly change this at a later point, but I don't want different syntaxes across mlkem-native and mldsa-native, or even within mldsa-native.
- We should have CI and autogen in place before we merge.
@jakemas Can you have a stab at those? 1 and 2 should be straightforward (note that simpasm should give you AT&T by default); if you have issues with 3, let us know. I may get to it as well, but the alpha release has priority.
If we can avoid the issue by swapping |
Ok fixed (1) and (2) I will test on my x86 machine then clean up the commits. (I'd rather add the non-simpasm ASMB as it appears upstream (https://github.com/pq-crystals/dilithium/blob/master/avx2/ntt.S) with macros etc. to demonstrate clearly the applicability to the reference -- fortunately, John requires in s2n-bignum that the AT&T versions are generated from the Intel, so we have both).
By this do you mean these merge first: because the bytecode check happens as part of CI... part of this PR or another? |
414790e to
3914869
Compare
Testing - Automatic bytecode generation/insertionOk I hooked up the was correctly found by By then running To generate the bytecode: |
HOL-Light Proofs in CII've added As in mlkem-native a three part test:
The interactive shell test in mlkem-native https://github.com/pq-code-package/mlkem-native/blob/main/.github/workflows/hol_light.yml#L66-L68 performs |
44e99b1 to
2b14a7b
Compare
c3596c3 to
fdd4867
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jakemas. The CI looks good to me.
Three minor comments on autogen above.
Can you also autogenerate mldsa_ntt.S please (see gen_hol_light_asm in mlkem-native)?
42d9098 to
01dda75
Compare
Okay, also added autogen. |
01dda75 to
c997ca7
Compare
|
On the problem of clang-based and gcc-based assemblers behaving differently, (the former selectively swapping arguments of commutative functions like VPADD to get a more compact encoding), there could be an argument for forcing the optimization by switching the order in the code, in the hope that it becomes more platform-independent. A possible issue with doing that simply via the clean code is that the triggering of the optimization depends on the register values, so it might be tricky to enforce at the macro level. It would be interesting to know exactly when the swap gets triggered. Maybe it's when one argument is >= ymm8 and the other is <= ymm7 (which I think is the only situation that makes the encoding shorter) or maybe it's simply when one register code is greater than another. |
|
That is, I'd instinctively vote for option (2) abiove if it isn't too troublesome
|
15463c4 to
75ae090
Compare
03b23ac to
dc96842
Compare
dc96842 to
e1ba952
Compare
Add HOL-Light framework with NTT proof. This includes: - list_proofs.sh, build-proof.sh, dump_bytecode.ml utilities - mldsa-specs.ml and mldsa-utils.ml to support HOL-Light proofs - HOL-Light proof for ML-DSA x86 AVX2 NTT in mldsa_ntt.ml - Addition of hol_light to scripts/tests to wrap and run proofs - READMEs for usage descriptions - Autogenerate embedded bytecode using dump_bytecode.ml - Add CI jobs to test and run hol-light proofs - Add autogen for asmb files Signed-off-by: Jake Massimo <jakemas@amazon.com>
e1ba952 to
67f7d5c
Compare
This PR includes:
list_proofs.sh,build-proof.sh,dump_bytecode.mlutilitiesmldsa-specs.mlandmldsa-utils.mlto support HOL-Light proofsmldsa_ntt.mlhol_lighttoscripts/teststo wrap and run proofsdump_bytecode.mlby the argument--update-hol-light-bytecodeand--dry-runto check/updategen_hol_light_asm_fileandgen_hol_light_asmSee the added proofs/hol_light/x86_64/README.md for details.
Running
make -C proofs/hol_light/x86_64produces anmldsa_ntt.correctfile that ends in the following:(~5hour on my test EC2 m5.2xlarge, Successful in 249m in CI).
Discussion
There is an issue where the x86 bytecode generated on clang ARM Mac is different to that when compiled on a gcc Linux. The clang compiler performs some optimization of instructions by switching the source operands of
VPADDDetc. to get a shorter encoding. This means thedefine_assert_from_elfcheck will fail when compiled on clang ARM Mac, as the proof is set with the machine code as compiled on gcc Linux (I do have proofs for both). We will have the same issue in mlkem-native when the x86 proofs arrive. So looking for opinions on (1) being okay with the x86 proofs not working on clang ARM Mac environments (2) fix the selective swapping of source operands toVPADDDto "optimise" the instructions down to the shorter size on gcc Linux, so both compile consistently (3) adding both proof types... likely all but (1) is out of scope of this PR -- just keeping notes.