Skip to content
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

Avoid illegal instructions from nocrypto #163

Merged
merged 1 commit into from
Nov 7, 2017
Merged

Avoid illegal instructions from nocrypto #163

merged 1 commit into from
Nov 7, 2017

Conversation

edwintorok
Copy link
Contributor

Nocrypto detects CPU features at build time, instead of runtime.
If the build machine is newer than the machine you install the binary on then it
would crash with an illegal instruction on startup.
See https://github.com/mirleft/ocaml-nocrypto#illegal-instructions and mirleft/ocaml-nocrypto#73

Since there is no runtime detection the only safe thing to do is to disable the
use of these instructions always.
For version 0.5.3 this is done with --disable-modernity, for 0.5.4 it'll be
NOCRYPTO_ACCELERATE=false environment variable.

Tested with opam reinstall -y nocrypto.0.5.3 -v: without this patch I see -maes in the build output, with this patch I do not. (note: you only see -maes if /proc/cpuinfo has the aes flag)

Nocrypto detects CPU features at *build time*, instead of runtime.
If the build machine is newer than the machine you install the binary on then it
would crash with an illegal instruction on startup.
See https://github.com/mirleft/ocaml-nocrypto#illegal-instructions and mirleft/ocaml-nocrypto#73

Since there is no runtime detection the only safe thing to do is to disable the
use of these instructions always.
For version 0.5.3 this is done with `--disable-modernity`, for 0.5.4 it'll be
`NOCRYPTO_ACCELERATE=false` environment variable.

Signed-off-by: Edwin Török <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants