Skip to content

Commit 3bb9763

Browse files
committed
Use SipHasher rather than SHA-512 for ISLE manifest.
Fixes #3609. It turns out that `sha2` is a nontrivial dependency for Cranelift in many contexts, partly because it pulls in a number of other crates as well. One option is to remove the hash check under certain circumstances, as implemented in #3616. However, this is undesirable for other reasons: having different dependency options in Wasmtime in particular for crates.io vs. local builds is not really possible, and so either we still have the higher build cost in Wasmtime, or we turn off the checks by default, which goes against the original intent of ensuring developer safety (no mysterious stale-source bugs). This PR uses `SipHash` instead, which is built into the standard library. `SipHash` is deprecated, but it's fixed and deterministic (across runs and across Rust versions), which is what we need, unlike the suggested replacement `std::collections::hash_map::DefaultHasher`. The result is only 64 bits, and is not cryptographically secure, but we never needed that; we just need a simple check to indicate when we forget a `rebuild-isle`.
1 parent e94ebc2 commit 3bb9763

File tree

5 files changed

+34
-15
lines changed

5 files changed

+34
-15
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cranelift/codegen/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ criterion = "0.3"
3737
cranelift-codegen-meta = { path = "meta", version = "0.79.0" }
3838
cranelift-isle = { path = "../isle/isle", version = "=0.79.0", optional = true }
3939
miette = { version = "3", features = ["fancy"], optional = true }
40-
sha2 = "0.9.8"
4140

4241
[features]
4342
default = ["std", "unwind"]

cranelift/codegen/build.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,31 @@
1414
// The build script expects to be run from the directory where this build.rs file lives. The
1515
// current directory is used to find the sources.
1616

17+
// We use the deprecated SipHasher from std::hash in order to verify
18+
// that ISLE sources haven't changed since the generated source was
19+
// last regenerated.
20+
//
21+
// We use this instead of a stronger and more usual content hash, like
22+
// SHA-{160,256,512}, because it's built into the standard library and
23+
// we don't want to pull in a separate crate. We try to keep Cranelift
24+
// crate dependencies as intentionally small as possible. In fact, we
25+
// used to use the `sha2` crate for SHA-512 and this turns out to be
26+
// undesirable for downstream consumers (see #3609).
27+
//
28+
// Why not the recommended replacement
29+
// `std::collections::hash_map::DefaultHasher`? Because we need the
30+
// hash to be deterministic, both between runs (i.e., not seeded with
31+
// random state) and across Rust versions.
32+
//
33+
// If `SipHasher` is ever actually removed from `std`, we'll need to
34+
// find a new option, either a very small crate or something else
35+
// that's built-in.
36+
#![allow(deprecated)]
37+
1738
use cranelift_codegen_meta as meta;
1839

19-
use sha2::{Digest, Sha512};
2040
use std::env;
41+
use std::hash::{Hasher, SipHasher};
2142
use std::io::Read;
2243
use std::process;
2344
use std::time::Instant;
@@ -176,11 +197,11 @@ impl IsleCompilation {
176197
// to `\r\n`; canonicalize the source that we hash to
177198
// Unix-style (`\n`) so hashes will match.
178199
let content = content.replace("\r\n", "\n");
179-
// One line in the manifest: <filename> <sha-512 hash>.
180-
let mut hasher = Sha512::default();
181-
hasher.update(content.as_bytes());
200+
// One line in the manifest: <filename> <siphash>.
201+
let mut hasher = SipHasher::new_with_keys(0, 0); // fixed keys for determinism
202+
hasher.write(content.as_bytes());
182203
let filename = format!("{}", filename.display()).replace("\\", "/");
183-
writeln!(&mut manifest, "{} {:x}", filename, hasher.finalize())?;
204+
writeln!(&mut manifest, "{} {:x}", filename, hasher.finish())?;
184205
}
185206

186207
Ok(manifest)
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
src/clif.isle be1359b4b6b153f378517c1dd95cd80f4a6bed0c7b86eaba11c088fd71b7bfe80a3c868ace245b2da0bfbbd6ded262ea9576c8e0eeacbf89d03c34a17a709602
2-
src/prelude.isle 15c8dd937171bd0f619179e219422d43af0eb0ef9a6e88f23b2aa55776712e27342309dd3a4441876b2dfec7f16ce7fe13b3a926ace89b25cfc9577e7b1d1578
3-
src/isa/aarch64/inst.isle a6921329a85a252b8059657056ee0c4477304dff461bd58c39133a2870666b23a34c911be55e25e7887074cb54b640ff09998730af09b3c1ba792f434309af24
4-
src/isa/aarch64/lower.isle 3cc84f8e3907818da7e0cbb4afe7f269da7090f3d504c74ecf02ef57463b281f4a320e52656d9397720ec8e4af98c1ecea05c4ffbe3f0c4126af4ed95d2734cc
1+
src/clif.isle f176ef3bba99365
2+
src/prelude.isle babc931e5dc5b4cf
3+
src/isa/aarch64/inst.isle abbd648f4a0b479a
4+
src/isa/aarch64/lower.isle c46d0692df0a9c0d
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
src/clif.isle be1359b4b6b153f378517c1dd95cd80f4a6bed0c7b86eaba11c088fd71b7bfe80a3c868ace245b2da0bfbbd6ded262ea9576c8e0eeacbf89d03c34a17a709602
2-
src/prelude.isle 15c8dd937171bd0f619179e219422d43af0eb0ef9a6e88f23b2aa55776712e27342309dd3a4441876b2dfec7f16ce7fe13b3a926ace89b25cfc9577e7b1d1578
3-
src/isa/x64/inst.isle 1a44ccc0c2cad90447762848461fcae714216ef058d42bdba89330a6008061526e92bbf1c17055c465b20fc75d98d1faa34feda8b22fa7ae0504a0f808798b41
4-
src/isa/x64/lower.isle c7943201b32e9eb9726466e8cc417f7e84c4c4052de31e05ab6e0ad7502a587cf1d7d9835703c4ff5a506390f7a0668741e7f3feaa1edda6396571a425949fc9
1+
src/clif.isle f176ef3bba99365
2+
src/prelude.isle babc931e5dc5b4cf
3+
src/isa/x64/inst.isle fb5d3ac8e68c46d2
4+
src/isa/x64/lower.isle d39e01add89178d5

0 commit comments

Comments
 (0)