Add nostd support #42

xoloki · 2019-05-15T19:05:37Z

This change implements nostd support for the merlin library. It mostly consists of config directives, and some remapping of std:: to core::.

isislovecruft

Looks good! So we don't have a bunch of small changes getting reversed back and forth, maybe we should squash this down into one or a couple commits? Either I can do a squash merge or you can do it first.

isislovecruft · 2019-05-20T20:00:38Z

src/strobe.rs

@@ -1,7 +1,7 @@
 //! Minimal implementation of (parts of) Strobe.

 use keccak;
-use std::ops::{Deref, DerefMut};
+use ::core::ops::{Deref, DerefMut};


rustfmt seems to want this line above use keccak and without the leading ::

Done @isislovecruft. I did this locally while on vacation, but failed to push it :O

hdevalence · 2019-05-21T14:20:07Z

Thanks for the PR! I am a little hesitant about merging it without having some reliable way to test that the no_std support actually works. The tooling around no_std is very awkward and it's easy for no_std to silently break. So I would prefer not to promise no_std support without being able to ensure that it will continue to work in the future.

isislovecruft · 2019-05-29T01:23:26Z

For the testing we can do what subtle currently does with cross-compile on nightly. I'll add that to the .travis.yml.

Note that this branch currently does not pass the test, and isn't yet no_std compliant. Part of the fix for this is going to be switching to the zeroize crate or else fixing clear_on_drop to be no_std.

first pass nostd, turn off default features and add std/alloc features Update Cargo.toml Update Cargo.toml use core ops when in alloc mode chain features remove std::ops entirely since we always have core anyway remove hex and debug transcript to get rid of std errors alloc default set no_std config attribute remove core crate extern import core crate before use don't add extern crate core; use global namespace when referring to core include core explicitly if we're in std mode re-enable debug-transcript feature re-enable debug-transcript feature fmt fixes

cf. dalek-cryptography#42

xoloki · 2019-05-29T21:36:49Z

For the testing we can do what subtle currently does with cross-compile on nightly. I'll add that to the .travis.yml.

I put your .travil.yml changes onto my branch.

Note that this branch currently does not pass the test, and isn't yet no_std compliant. Part of the fix for this is going to be switching to the zeroize crate or else fixing clear_on_drop to be no_std.

I didn't actually have any problems using my branch in no_std mode inside MC code. But it's definitely failing the test after the travis change:

  Downloaded byteorder v1.3.1
  Downloaded keccak v0.1.0
  Downloaded clear_on_drop v0.2.3
  Downloaded rand_core v0.3.1
  Downloaded rand_core v0.4.0
   Compiling cc v1.0.37
   Compiling byteorder v1.3.1
   Compiling rand_core v0.4.0
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7em-none-eabi` target may not be installed
error: aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: Could not compile `rand_core`.

It doesn't make a lot of sense that rand_core would be pulling in std here; Cargo.toml is pretty explicit:

[features]
default = ["std"]
nightly = ["clear_on_drop/nightly"]
debug-transcript = ["hex"]
std = ["rand_core/std", "byteorder/std"]
alloc = ["rand_core/alloc"]

I tried running the xargo build --no-default-features command locally, and I got failures in byteorder and clear_on_drop in addition to rand_core:

   Compiling byteorder v1.3.1
error: failed to run custom build command for `clear_on_drop v0.2.3`
process didn't exit successfully: `/usr/src/myapp/target/debug/build/clear_on_drop-d06ae577f6602215/build-script-build` (exit code: 101)
--- stdout
TARGET = Some("thumbv7em-none-eabi")
OPT_LEVEL = Some("0")
HOST = Some("x86_64-unknown-linux-gnu")
CC_thumbv7em-none-eabi = None
CC_thumbv7em_none_eabi = None
TARGET_CC = None
CC = None
CROSS_COMPILE = None
CFLAGS_thumbv7em-none-eabi = None
CFLAGS_thumbv7em_none_eabi = None
TARGET_CFLAGS = None
CFLAGS = None
CRATE_CC_NO_DEFAULTS = None
DEBUG = Some("true")
CARGO_CFG_TARGET_FEATURE = Some("dsp,mclass,v5te,v6k,v6t2,v7")
running: "arm-none-eabi-gcc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-mthumb" "-march=armv7e-m" "-Wall" "-Wextra" "-o" "/usr/src/myapp/target/thumbv7em-none-eabi/debug/build/clear_on_drop-260f6a6672d2b8cb/out/src/hide.o" "-c" "src/hide.c"

--- stderr
thread 'main' panicked at '

Internal error occurred: Failed to find tool. Is `arm-none-eabi-gcc` installed?

', /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cc-1.0.32/src/lib.rs:2367:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

warning: build failed, waiting for other jobs to finish...
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7em-none-eabi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `rand_core`.
warning: build failed, waiting for other jobs to finish...
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv7em-none-eabi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `byteorder`.

I explicitly tried installing the target to make sure that wasn't the problem:

root@85e99ad4605e:/usr/src/myapp# rustup target install thumbv7em-none-eabi
info: component 'rust-std' for target 'thumbv7em-none-eabi' is up to date

So, really not sure what's going on here. Do you have a working example of a crate that builds in no_std mode using the xargo command above?

xoloki · 2019-05-29T21:49:27Z

I tried cargo nono, which didn't think byteorder was a problem, but agreed that rand_core was:

root@85e99ad4605e:/usr/src/myapp# cargo nono check --no-default-features
merlin: SUCCESS
byteorder: SUCCESS
rand_core: FAILURE
  - Source code contains an explicit `use std::` statement.

Checking rand_core sources now...

cf. dalek-cryptography#42

xoloki · 2019-05-29T22:17:42Z

Ok, I was able to fix the cargo nono rand_core error by upgrading to 0.4:

root@85e99ad4605e:/usr/src/myapp# cargo nono check --no-default-features
merlin: SUCCESS
rand_core: SUCCESS
byteorder: SUCCESS
keccak: SUCCESS
clear_on_drop: FAILURE
  - Did not find a #![no_std] attribute or a simple conditional attribute like #[cfg_attr(not(feature = "std"), no_std)] in the crate source. Crate most likely doesn't support no_std without changes.

The clear_on_drop failure appears to be spurious; the first line of the lib.rs defines no_std in all non-test modes:

#![cfg_attr(not(test), no_std)]

I'll see how the xargo command works now with rand_core 0.4.

xoloki · 2019-05-29T22:29:10Z

xargo is still failing. So I checked out subtle, to see if it was working there. Looks like it was turned off because it no longer works:

    # Test if crate can be truly built without std
    # Disabled because this tooling started to error, feel free to fix it
    #- rust: nightly
    #  env: TARGET=thumbv7em-none-eabi
    #  script: xargo build --no-default-features --target $TARGET
    #  install:
    #    - cargo install xargo || true
    #    - rustup target install armv7-unknown-linux-gnueabihf
    #    - rustup component add rust-src

So cargo nono now works, modulo the bogus clear_on_drop error. Can I comment out the xargo test from .travis.yml, and we'll add it back later once subtle does as well?

xoloki · 2019-06-13T22:53:48Z

Okay @isislovecruft @hdevalence I have a PR in for cargo-nono to fix the spurious error from clear_on_drop: hobofan/cargo-nono#38

I've updated this PR to use cargo nono in .travis.yml. It's currently failing in CI but it will work once the other PR gets pushed and there is a new cargo-nono release.

hdevalence · 2019-06-22T07:42:22Z

Is this PR superseded by #43?

xoloki · 2019-06-24T19:29:33Z

Is this PR superseded by #43?

PR #43 uses xargo to implement a no_std CI check, whereas this PR uses cargo-nono. Unfortunately both tools appear to be broken at this time @hdevalence, so neither can be merged as-is.

I haven't heard anything about my cargo-nono PR in almost three weeks, and @isislovecruft says that xargo is no longer being maintained, so we're kind of at an impasse for now.

hdevalence · 2019-06-27T12:15:23Z

Would using something like the script in this README work: https://github.com/hdevalence/no_std_test_lib ?

xoloki · 2019-06-27T20:29:09Z

My cargo-nono changes were merged and released last night @hdevalence so I'm running a new build to see if it's working. If it's still broken I'll check out your no_std_test_lib.

xoloki · 2019-06-28T19:00:24Z

The crates.io index is still returning the old 1.1.4 cargo-nono, I'm trying to get it updated now.

I tried the script, but it's giving the same errors as xargo.

…e set when default features turned off

xoloki · 2019-07-01T21:15:49Z

I managed to get cargo-nono v1.1.5 available via crates.io @hdevalence, and CI is now passing. Let me know if there's anything else you need from me.

hdevalence · 2019-07-02T18:40:42Z

Cargo.toml

 nightly = ["clear_on_drop/nightly"]
 debug-transcript = ["hex"]
+std = ["rand_core/std", "byteorder/std"]
+alloc = ["rand_core/alloc"]


Does the alloc feature actually get used for anything? I don't think Merlin allocates, so if this feature is only being used to enable a corresponding feature in rand_core, I'm not sure we need it -- since Cargo features are additive, anything else that needs rand_core/alloc can enable it themselves.

hdevalence · 2019-07-02T18:40:59Z

Cargo.toml

 nightly = ["clear_on_drop/nightly"]
 debug-transcript = ["hex"]
+std = ["rand_core/std", "byteorder/std"]
+alloc = ["rand_core/alloc"]


Suggested change

alloc = ["rand_core/alloc"]

hdevalence · 2019-07-02T18:41:08Z

src/lib.rs

@@ -1,3 +1,5 @@
+#![cfg_attr(not(feature = "std"), no_std)]
+#![cfg_attr(feature = "alloc", feature(alloc))]


Suggested change

#![cfg_attr(feature = "alloc", feature(alloc))]

hdevalence · 2019-07-02T18:41:56Z

Hi, this looks good aside from the alloc comments; let me know if I missed something and they are actually needed inside Merlin.

xoloki · 2019-07-03T19:13:59Z

Nope, I removed the alloc feature and everything still works perfectly. PR is updated, CI is still working.

I think we're good now, let me know if you need anything else. And thanks for all your help with this!

hdevalence · 2019-07-03T19:32:55Z

Merged into develop; I'll do a new release soon! Thanks for the PR.

xoloki · 2019-07-10T18:53:40Z

Excellent, thanks @hdevalence. Looking forward to the release so I can make my bulletproofs PR, which was the end goal all along :)

hdevalence · 2019-07-11T16:57:21Z

Released 1.2.0!

xoloki added 16 commits April 1, 2019 14:31

first pass nostd, turn off default features and add std/alloc features

c74e2f6

Update Cargo.toml

9a4c220

Update Cargo.toml

25f0f78

use core ops when in alloc mode

27592d5

chain features

8f08d91

remove std::ops entirely since we always have core anyway

123b446

remove hex and debug transcript to get rid of std errors

e482d75

alloc default

860e841

set no_std config attribute

16493a8

remove core crate extern

2481ab1

import core crate before use

6512e9f

don't add extern crate core; use global namespace when referring to core

354346b

include core explicitly if we're in std mode

14da0b5

re-enable debug-transcript feature

9414bba

re-enable debug-transcript feature

abf29bd

fmt fixes

92618c1

isislovecruft self-requested a review May 15, 2019 20:17

isislovecruft changed the base branch from master to develop May 20, 2019 19:58

isislovecruft requested changes May 20, 2019

View reviewed changes

isislovecruft reviewed May 20, 2019

View reviewed changes

isislovecruft added a commit to isislovecruft/merlin that referenced this pull request May 29, 2019

Add testing for no_std support in Travis builds.

99d8051

cf. dalek-cryptography#42

xoloki added 2 commits May 29, 2019 12:30

fmt fix in use statements

d6d5f87

test for nostd compatibility

d09b6d8

isislovecruft added a commit to isislovecruft/merlin that referenced this pull request May 29, 2019

Add testing for no_std support in Travis builds.

46d91fa

cf. dalek-cryptography#42

upgrade rand_core to avoid no_std test failure

e13de3f

isislovecruft mentioned this pull request May 29, 2019

no_std is broken, doesn't play with sgx w3f/schnorrkel#31

Open

use cargo-nono rather than xargo to test no_std support

6c2a73a

Update .travis.yml

6c520ab

xoloki added 3 commits July 1, 2019 13:49

Update .travis.yml

1dc0037

force reinstall of cargo-nono to pick up v1.1.5; specify valid featur…

49fcc7d

…e set when default features turned off

remove date tag from nightly version in second test

bc3b3fd

hdevalence reviewed Jul 2, 2019

View reviewed changes

remove alloc feature since it isn't actually used by merlin

6e07e07

hdevalence merged commit 6165fc4 into dalek-cryptography:develop Jul 3, 2019

burdges mentioned this pull request Aug 5, 2019

Add external RNG input #47

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nostd support #42

Add nostd support #42

xoloki commented May 15, 2019

isislovecruft left a comment

isislovecruft May 20, 2019

xoloki May 29, 2019

hdevalence commented May 21, 2019

isislovecruft commented May 29, 2019 •

edited

Loading

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented Jun 13, 2019

hdevalence commented Jun 22, 2019

xoloki commented Jun 24, 2019

hdevalence commented Jun 27, 2019

xoloki commented Jun 27, 2019

xoloki commented Jun 28, 2019

xoloki commented Jul 1, 2019

hdevalence Jul 2, 2019

hdevalence Jul 2, 2019

hdevalence Jul 2, 2019

hdevalence commented Jul 2, 2019

xoloki commented Jul 3, 2019

hdevalence commented Jul 3, 2019

xoloki commented Jul 10, 2019

hdevalence commented Jul 11, 2019

		@@ -1,3 +1,5 @@
		#![cfg_attr(not(feature = "std"), no_std)]
		#![cfg_attr(feature = "alloc", feature(alloc))]

Add nostd support #42

Add nostd support #42

Conversation

xoloki commented May 15, 2019

isislovecruft left a comment

Choose a reason for hiding this comment

isislovecruft May 20, 2019

Choose a reason for hiding this comment

xoloki May 29, 2019

Choose a reason for hiding this comment

hdevalence commented May 21, 2019

isislovecruft commented May 29, 2019 • edited Loading

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented May 29, 2019

xoloki commented Jun 13, 2019

hdevalence commented Jun 22, 2019

xoloki commented Jun 24, 2019

hdevalence commented Jun 27, 2019

xoloki commented Jun 27, 2019

xoloki commented Jun 28, 2019

xoloki commented Jul 1, 2019

hdevalence Jul 2, 2019

Choose a reason for hiding this comment

hdevalence Jul 2, 2019

Choose a reason for hiding this comment

hdevalence Jul 2, 2019

Choose a reason for hiding this comment

hdevalence commented Jul 2, 2019

xoloki commented Jul 3, 2019

hdevalence commented Jul 3, 2019

xoloki commented Jul 10, 2019

hdevalence commented Jul 11, 2019

isislovecruft commented May 29, 2019 •

edited

Loading