Fix Miri testing of ChaCha

[dhardy]

Turns out the reason the Miri test on #789 passed is because it didn't run (@RalfJung: told you skipping the test when Miri is unavailable would also have problems, but never mind).

So we can either hack around this by not testing rand_chacha and the affected parts of Rand with Miri or we can hack around this by using a cfg to adjust the ChaCha code. @kazcw any idea how easy this is?

Related: #792 is failing both Emscripten and Miri testing due to the new ChaCha code.

[vks]

I think we should make c2-chacha compatible with Miri and Emscripten.

[vks]

Did you try removing the default simd feature from c2-chacha for Miri and Emscripten? That should probably work. Edit: It seems like this is already done for Miri, but possibly not for Emscripten.

[RalfJung]

Does c2-chacha have a non-SIMD fallback, or does rand use some other implementation for non-SIMD platforms? The usual approach is to also use that fallback for Miri. (This is, e.g., what hashbrown does.)

[vks]

@RalfJung Yes, and we are running Miri without the simd feature already:

• https://github.com/dhardy/rand/blob/e182cfce6a9b71148d59b088dfab47d0d4a58821/utils/ci/miri.sh#L13
• https://github.com/dhardy/rand/blob/e182cfce6a9b71148d59b088dfab47d0d4a58821/rand_chacha/Cargo.toml#L31
• https://github.com/cryptocorrosion/cryptocorrosion/blob/e77f1623c012d2bae022d69c21668391019bc3b2/stream-ciphers/chacha/Cargo.toml#L27

It seems to be related to the runtime dispatch of the architecture: #792 (comment)

[RalfJung]

Ah, it's doing a run-time check? Yeah, that's different from what I saw elsewhere.

Also, these feature games won't help projects that depend on rand -- they would have to all propagate this. I don't think features are a good solution for target-specific changes; cfg works better for that.

(This also applies to emcscripten: if you make this a feature, every library that indirectly uses rand has to add this feature and propagate it outwards if it wants to be emscripten-compatible.)

[RalfJung]

(@RalfJung: told you skipping the test when Miri is unavailable would also have problems, but never mind).

Yeah, I never said it didn't have problems. :/ If you prefer to have development here stop when Miri is not available as a component, don't let me stop you! But right now we've had Miri broken for >1 week, so that also does not seem great.

[dhardy]

But right now we've had Miri broken for >1 week, so that also does not seem great.

Of course not. Any idea when it will be a reliable component of rustc?

Yes, it sounds like a cfg in the code is needed, probably somewhere in c2-chacha which is @kazcw's code.

[RalfJung]

Of course not. Any idea when it will be a reliable component of rustc?

Clippy and RLS also break regularly for nightly, and I don't see that changing. But hopefully at some point we will be able to ask rustup "please install the latest nightly that has Miri".

Yes, it sounds like a cfg in the code is needed, probably somewhere in c2-chacha which is @kazcw's code.

Basically, in cases like this, feature = "simd" needs to be extended to all(feature = "simd", not(miri)) (or some version of that) -- and a similar clause for emcscripten.

[kazcw]

Fixing for miri will be easy, I can do it this weekend (if I don't get time sooner). It's not obvious to me why the wasm32 build wouldn't be compiling purely portable code but I can look into that this weekend too.

[kazcw]

I fixed build for miri in ppv-lite86: 0.2.3 -> 0.2.4.

The emscripten failure is caused by emscripten-core/emscripten-fastcomp#169. Avoiding usage of 128-bit types in ppv-lite86 would be onerous; it's a 128-bit-types math library. It works on every complete Rust implementation.

[dhardy]

Thanks @kazcw. I see what you mean; supporting Emscripten with the new ChaCha code would not be easy.

I wish we had some data on the usage of Rand on Emscripten. @huangjj27, @CryZe, @pitdicker, @jsheard and @svenstaro have all contributed to Emscripten support; is this still an important target now that we have WASM+WASI? If we must support Emscripten then using a different PRNG (or different ChaCha implementation) may be the best option.

[svenstaro]

I'm definitely still using Rand and emscripten and I'm pretty happy with that. My project can't yet be implemented in non-emscripten backed wasm I think so I'd be pretty distraught if that went away.

[vks]

@kazcw Could you please publish ppv-lite86 0.2.4 on crates.io?

@dhardy I think the easiest fix would be to use rand_hc on Emscripten. Alternatively, we would have to add a feature to rand_chacha to use the old implementation, which would be an unfortunate duplication of code.

[huangjj27]

which would be an unfortunate duplication of code.

never mind the deplication of codes because there is no overhead for the users while compiling.

personally, I use wasm32-unknown-unknown/wasm32-wasi target in most of my tiny projects, but there are some projects that still support Emscripten (like yew). How about follow the supportment untill most of down stream has moved to wasm32-unknown-unknown/wasm32-wasi?

[dhardy]

@dhardy I think the easiest fix would be to use rand_hc on Emscripten.

Unfortunately that would require us to disable many tests since #792 switches the test RNG to ChaCha8 which we depend on for value-reproducibility. We can use a different RNG instead (I chose PCG for rand_distr because it is reasonable quality and easy to embed); this may be the best fix.

[dhardy]

#792 is now merged with full CI compliance! Thanks for the help everyone.

[RalfJung]

FWIW, the tests there passed for Miri because Miri was not available as a component...

We could also, instead of making the test pass when the component is missing, make it fail but mark it as allowed-to-fail on Travis?

[dhardy]

No, because in that case I think GH would not tell us about actual failures on Miri?

We need some use the last nightly with Miri support option to do this properly, unless the Rust team can actually get this in all nightlies. Until then, I think we just continue like usual (often enough new PRs start failing because of something not related to the PR).

[RalfJung]

No, because in that case I think GH would not tell us about actual failures on Miri?

That's accurate.

We need some use the last nightly with Miri support option to do this properly, unless the Rust team can actually get this in all nightlies. Until then, I think we just continue like usual (often enough new PRs start failing because of something not related to the PR).

Yeah, we basically need rust-lang/rustup#1501.

[Nemo157]

The alternative that you can use today is to just pin a specific nightly date for those specific CI builds (given how many years a rustup feature like this has been needed without much movement (I recall discussion around it from well before 1501 was opened), I don't see any chance of getting it soon).

[RalfJung]

But then you won't get Miri updates either, which might become a problem.

Another work-around might be available via rust-lang/rustup-components-history#5.