mirage / mirage-crypto

Cryptographic primitives for OCaml, in OCaml (also used in MirageOS)
ISC License
77 stars 43 forks source link

Update fiat #177

Closed hannesm closed 1 year ago

hannesm commented 1 year ago

/cc @samoht (or @avsm) would you mind to test whether this PR (a runtest) works fine on your macOS? It now uses the most recent fiat-crypto version. In case it doesn't work, we'll need to dig deeper.

hannesm commented 1 year ago

hmm, but since there are no code changes, I doubt it'll work on your systems.

samoht commented 1 year ago

I indeed do see the same error as before with this PR.

dmmulroy commented 1 year ago

@samoht @hannesm I'm curious what issues you were experiencing on osx? I'm running into some issues using ES256 in ocaml-jose that I've seemed to narrow down to Mirage_crypto_ec.P256.Dsa.pub_to_cstruct

https://github.com/ulrikstrid/ocaml-jose/issues/63

samoht commented 1 year ago

@dmmulroy see the discussions here: https://github.com/mit-plv/fiat-crypto/issues/1606 it seems to be due to a bug introduced in clang 14.0.3. I've reported an issue to Apple, but no news so far...

dmmulroy commented 1 year ago

@samoht Thank you for the context and link - I'll keep my eyes peeled on the issues 👀

hannesm commented 1 year ago

since the main difference is "inline everything", a performance evaluation is needed before a merge.

hannesm commented 1 year ago

since it is only inlining the change, I'll defer to a later point.