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

Build problems in rust-secp256k1 after #780 #852

Closed
apoelstra opened this issue Aug 10, 2023 · 7 comments · Fixed by #860
Closed

Build problems in rust-secp256k1 after #780 #852

apoelstra opened this issue Aug 10, 2023 · 7 comments · Fixed by #860

Comments

@apoelstra
Copy link

Between cc 1.0.79 and 1.0.80 the "no-std test with ASAN" CI test in rust-secp256k1 stopped working. There was a six-month gap in this release but nothing major or dramatic happened. Bisection tells me that the problem started occurring after the merge of #780.

The error I get is during build, and looks like

   Compiling no_std_test v0.1.0 (/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test)
     Running `/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc --crate-name no_std_test src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=123 --crate-type bin --emit=dep-info,link -C opt-level=3 -C panic=abort -C embed-bitcode=no -C metadata=1293a4e6b7efff78 -C extra-filename=-1293a4e6b7efff78 --out-dir /store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps -L dependency=/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps --extern libc=/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/liblibc-5de6dfcb24ee9f89.rlib --extern secp256k1=/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libsecp256k1-5c90214252d1cafe.rlib --extern serde_cbor=/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libserde_cbor-b92effaf107035cb.rlib -L native=/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/build/secp256k1-sys-cfb84e400e95762c/out`
error: linking with `cc` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:/store/home/apoelstra/.cargo/bin:/store/home/apoelstra/bin:/store/home/apoelstra/.idris2/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/store/home/apoelstra/.local/bin:/store/home/apoelstra/.cabal/bin" VSLANG="1033" "cc" "-m64" "/tmp/rustchhL2Pf/symbols.o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-1293a4e6b7efff78.no_std_test.87cf14f7b6e6638a-cgu.0.rcgu.o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-1293a4e6b7efff78.no_std_test.87cf14f7b6e6638a-cgu.1.rcgu.o" "-Wl,--as-needed" "-L" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps" "-L" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/build/secp256k1-sys-cfb84e400e95762c/out" "-L" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libserde_cbor-b92effaf107035cb.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libhalf-e6655c99ab38f9c5.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libbyteorder-5fc3ce4f7f805d99.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libsecp256k1-5c90214252d1cafe.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/librand-ebdb156e0d3d08a3.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/librand_core-aa5db4de6c0d4a69.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libsecp256k1_sys-a96dfb2f870f90d5.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libserde-084022b6fd934a86.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/liblibc-5de6dfcb24ee9f89.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-cd2f2bc505f56f50.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-ec02dd343723da85.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-00ae71943a8bc9aa.rlib" "-Wl,-Bdynamic" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-1293a4e6b7efff78" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs"
  = note: /usr/bin/ld: /usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../lib/Scrt1.o: in function `_start':
          (.text+0x21): undefined reference to `__libc_start_main'
          /usr/bin/ld: /store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-1293a4e6b7efff78.no_std_test.87cf14f7b6e6638a-cgu.1.rcgu.o: in function `core::fmt::Write::write_char':
          no_std_test.87cf14f7b6e6638a-cgu.1:(.text._ZN4core3fmt5Write10write_char17h3bde70e0b60d9d00E+0xda): undefined reference to `memcpy'
          <snip>

where the snipped output is a pile of more "cannot find memcpy" type errors from various crates, including libcore. If I compile with 57853c4, the previous commit before this one, and force a linker error by adding an unimplemented function (not sure how else to get the exact cc command), the outputt looks like

  = note: LC_ALL="C" PATH="/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:/store/home/apoelstra/.cargo/bin:/store/home/apoelstra/bin:/store/home/apoelstra/.idris2/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/store/home/apoelstra/.local/bin:/store/home/apoelstra/.cabal/bin" VSLANG="1033" "cc" "-m64" "/tmp/rustc3zNS0N/symbols.o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-2f327716e2f5651f.no_std_test.a728628877dda375-cgu.0.rcgu.o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-2f327716e2f5651f.no_std_test.a728628877dda375-cgu.1.rcgu.o" "-Wl,--as-needed" "-L" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps" "-L" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/build/secp256k1-sys-9dd44ba92659f6c7/out" "-L" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libserde_cbor-b92effaf107035cb.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libhalf-e6655c99ab38f9c5.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libbyteorder-5fc3ce4f7f805d99.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libsecp256k1-b0ad93fc3995ce88.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/librand-ebdb156e0d3d08a3.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/librand_core-aa5db4de6c0d4a69.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libsecp256k1_sys-eacd782dd019dfc5.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/libserde-084022b6fd934a86.rlib" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/liblibc-3070fa89807530f5.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-cd2f2bc505f56f50.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-ec02dd343723da85.rlib" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-00ae71943a8bc9aa.rlib" "-Wl,-Bdynamic" "-lc" "-lm" "-lrt" "-lpthread" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/store/home/apoelstra/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/store/home/apoelstra/code/rust-bitcoin/rust-secp256k1/pr-review/no_std_test/target/release/deps/no_std_test-2f327716e2f5651f" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs"

If you compare these two invocations you will see that the latter has -lc -lm -lrt -lpthread while the former does not. I guess that -lc is what's causing errors related to memcpy being undefined.

I've read the code in this PR a few times and can't see any way that it would cause these linker flags to get dropped, but empirically that's what appears to be happening.

Originally posted by @apoelstra in #780 (comment)

@NobodyXu
Copy link
Collaborator

NobodyXu commented Aug 11, 2023

Can you provide the Cargo.toml and the src for minimum reproduction of this?
I'm really not sure what is going on.

@apoelstra
Copy link
Author

apoelstra commented Aug 11, 2023

Sure. So, the repo is here: https://github.com/rust-bitcoin/rust-secp256k1/tree/14e82186d1f9d6a0054d0c8982ce28b30c76b874 -- the cc dependency appears in secp256k1-sys/Cargo.toml and the problem is exhibited when running cargo +nightly build in the no_std_test directory. (You need nightly because this no-std test uses a bunch of compiler features. Or it used to, and we left the feature()s in over the years..)

The dependency pattern is that no_std_test depends on secp256k1 (the main crate) which depends on secp256k1-sys (which has vendored C bindings and uses cc to build them).

Specifically, if you clone that repo, to reproduce:

cargo update -p cc --precise 1.0.80 # will trigger the bug; use 1.0.79 to not trigger it
cd no_std_test
cargo +nightly build

The relevant Cargo.tomls are:

Cargo.toml

[package]
name = "secp256k1"
version = "0.27.0"
authors = [ "Dawid Ciężarkiewicz <dpc@ucore.info>",
            "Andrew Poelstra <apoelstra@wpsoftware.net>" ]
license = "CC0-1.0"
homepage = "https://github.com/rust-bitcoin/rust-secp256k1/"
repository = "https://github.com/rust-bitcoin/rust-secp256k1/"
documentation = "https://docs.rs/secp256k1/"
description = "Rust wrapper library for Pieter Wuille's `libsecp256k1`. Implements ECDSA and BIP 340 signatures for the SECG elliptic curve group secp256k1 and related utilities."
keywords = [ "crypto", "ECDSA", "secp256k1", "libsecp256k1", "bitcoin" ]
readme = "README.md"
edition = "2018"

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[features]
default = ["std"]
std = ["alloc", "secp256k1-sys/std"]
# allow use of Secp256k1::new and related API that requires an allocator
alloc = ["secp256k1-sys/alloc"]
bitcoin-hashes = ["bitcoin_hashes"] # Feature alias because of the underscore.
bitcoin-hashes-std = ["std", "bitcoin_hashes", "bitcoin_hashes/std"]
rand-std = ["std", "rand", "rand/std", "rand/std_rng"]
recovery = ["secp256k1-sys/recovery"]
lowmemory = ["secp256k1-sys/lowmemory"]
global-context = ["std"]
# disable re-randomization of the global context, which provides some
# defense-in-depth against sidechannel attacks. You should only use
# this feature if you expect the `rand` crate's thread_rng to panic.
# (If you are sure the `rand-std` feature will not be enabled, e.g.
# if you are doing a no-std build, then this feature does nothing
# and is not necessary.)
global-context-less-secure = ["global-context"]

[dependencies]
secp256k1-sys = { version = "0.8.1", default-features = false, path = "./secp256k1-sys" }
serde = { version = "1.0.103", default-features = false, optional = true }

# You likely only want to enable these if you explicitly do not want to use "std", otherwise enable
# the respective -std feature e.g., bitcoin-hashes-std
bitcoin_hashes = { version = "0.12", default-features = false, optional = true }
rand = { version = "0.8", default-features = false, optional = true }

[dev-dependencies]
rand_core = "0.6"
serde_cbor = "0.10.0"
serde_test = "1.0.19"
bincode = "1.3.3"

[target.wasm32-unknown-unknown.dev-dependencies]
wasm-bindgen-test = "0.3"
getrandom = { version = "0.2", features = ["js"] }


[[example]]
name = "sign_verify_recovery"
required-features = ["recovery", "bitcoin-hashes-std"]

[[example]]
name = "sign_verify"
required-features = ["bitcoin-hashes-std"]

[[example]]
name = "generate_keys"
required-features = ["rand-std"]

[workspace]
members = ["secp256k1-sys"]
exclude = ["no_std_test"]

secp256k1-sys/Cargo.toml

[package]
name = "secp256k1-sys"
version = "0.8.1"
authors = [ "Dawid Ciężarkiewicz <dpc@ucore.info>",
            "Andrew Poelstra <apoelstra@wpsoftware.net>",
            "Steven Roose <steven@stevenroose.org>" ]
license = "CC0-1.0"
homepage = "https://github.com/rust-bitcoin/rust-secp256k1/"
repository = "https://github.com/rust-bitcoin/rust-secp256k1/"
documentation = "https://docs.rs/secp256k1-sys/"
description = "FFI for Pieter Wuille's `libsecp256k1` library."
keywords = [ "secp256k1", "libsecp256k1", "ffi" ]
readme = "README.md"
build = "build.rs"
links = "rustsecp256k1_v0_8_1"
edition = "2018"

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[build-dependencies]
cc = "1.0.28"

[dev-dependencies]
libc = "0.2"

[features]
default = ["std"]
recovery = []
lowmemory = []
std = ["alloc"]
alloc = []

no_std_test/Cargo.toml

[package]
name = "no_std_test"
version = "0.1.0"
authors = ["Elichai Turkel <elichai.turkel@gmail.com>"]

[features]
alloc = ["secp256k1/alloc", "wee_alloc"]

[dependencies]
wee_alloc = { version = "0.4.5", optional = true }
secp256k1 = { path = "../", default-features = false, features = ["serde", "rand", "recovery"] }
libc = { version = "0.2", default-features = false }
serde_cbor = { version = "0.10", default-features = false } # A random serializer that supports no-std.


[profile.release]
panic = "abort"

[profile.dev]
panic = "abort"

I will try to create a minimum reproducable case in the coming week, though I've got some (happy) family stuff that is likely to distract me for the next little while.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Aug 11, 2023

Thanks, can reproduce on aarch64-apple-darwin on cc v1.0.82 with cargo 1.73.0-nightly (d78bbf4bd 2023-08-03).

@NobodyXu
Copy link
Collaborator

It's still unclear to me how #780 causes this problem though, given that it doesn't modify how the command is created and what args/envs are passed https://github.com/rust-lang/cc-rs/pull/780/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R1344

@NobodyXu
Copy link
Collaborator

P.S. for the build.rs of secp256k1-sys, I don't think you can use cfg!(feature = "lowmemory") in build.rs.

AFAIK you would have to do env::var("CARGO_FEATURE_<name>").

@kadiwa4
Copy link
Contributor

kadiwa4 commented Aug 20, 2023

cc introduced a dependency to libc in 1.0.80 and has default features enabled for it. If you are using Cargo's resolver v1, you will face this issue: rust-lang/cargo#4866 (it combines the features enabled for build-dependencies with those for dependencies), so updating the resolver version is necessary.

That means you need to update your Edition to 2021 (or, if you use a workspace, add resolver = "2"). I haven't actually tried if that fixes the problem for your crate tho.

@thomcc
Copy link
Member

thomcc commented Aug 20, 2023

We could perhaps turn default features off for libc. And if that doesn't work, we could manually write the externs ourselves I suppose.

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 a pull request may close this issue.

4 participants