search

briansmith / ring

Safe, fast, small crypto using Rust
Other
3.73k stars 703 forks source link

Version 0.17 __ARM_ARCH error cross-compiling via manylinux2014-cross:aarch64 #1728

Closed bobzilladev closed 1 year ago

bobzilladev commented 1 year ago

Seeing this behavior change when webpki started including 0.17 of ring, an error about __ARM_ARCH missing when cross-compiling from x86_64 Ubuntu to aarch64 manylinux2014. Guessing the included GCC does not have ARM_ARCH defined, and the newer BoringSSL requires it.

Adding this as a workaround gets compilation working again: export CFLAGS_aarch64_unknown_linux_gnu="-D__ARM_ARCH=8"

The error:

warning: In file included from /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ring-0.17.3/pregenerated/aesv8-armx-linux64.S:4:0:
warning: include/ring-core/asm_base.h:73:2: error: #error "ARM assembler must define __ARM_ARCH"
warning:  #error "ARM assembler must define __ARM_ARCH"
warning:   ^
warning: In file included from /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ring-0.17.3/pregenerated/aesv8-armx-linux64.S:7:0:
warning: include/ring-core/arm_arch.h:82:2: error: #error "ARM assembler must define __ARM_ARCH"
warning:  #error "ARM assembler must define __ARM_ARCH"

Don't know if this is something that should have a fix put in for, or if ring-poc is holding it wrong, but it's a change from 0.16 to 0.17 so wanted to bring it up. The project where this was found is using manylinux2014 to create highly-compatible distributable pre-compiled artifacts.

briansmith commented 1 year ago

The BoringSSL code says:

// We require the ARM assembler provide |__ARM_ARCH| from Arm C Language
// Extensions (ACLE). This is supported in GCC 4.8+ and Clang 3.2+. MSVC does
// not implement ACLE, but we require Clang's assembler on Windows.

So I think it is surprising to everybody that GCC 11 wouldn't define __ARM_ARCH here.

briansmith commented 1 year ago

Guessing the included GCC does not have ARM_ARCH defined, and the newer BoringSSL requires it.

It seems like some versions of GCC define __ARM_ARCH but with the wrong value. But it is surprising that you aren't seeing it defined at all. I wonder if your sysroot is correct.

bobzilladev commented 1 year ago

Grabbing the right GCC version, it is a lot older but still in the range of that BoringSSL comment:

# aarch64-unknown-linux-gnu-gcc --version
aarch64-unknown-linux-gnu-gcc (GCC) 4.8.5
Copyright (C) 2015 Free Software Foundation, Inc.

Env:

> /usr/bin/docker run -it --net=host -v .:/build -w /build ghcr.io/rust-cross/manylinux2014-cross:aarch64 env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/aarch64-unknown-linux-gnu/bin
HOSTNAME=ip-10-0-0-245
TERM=xterm
DEBIAN_FRONTEND=noninteractive
CC_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-gcc
AR_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-ar
CXX_aarch64_unknown_linux_gnu=aarch64-unknown-linux-gnu-g++
TARGET_CC=aarch64-unknown-linux-gnu-gcc
TARGET_AR=aarch64-unknown-linux-gnu-ar
TARGET_RANLIB=aarch64-unknown-linux-gnu-ranlib
TARGET_CXX=aarch64-unknown-linux-gnu-g++
TARGET_READELF=aarch64-unknown-linux-gnu-readelf
TARGET_SYSROOT=/usr/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/sysroot/
TARGET_C_INCLUDE_PATH=/usr/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/sysroot/usr/include/
CARGO_BUILD_TARGET=aarch64-unknown-linux-gnu
CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-unknown-linux-gnu-gcc
TARGET_CMAKE_TOOLCHAIN_FILE_PATH=/usr/aarch64-unknown-linux-gnu/cmake-toolchain.cmake
AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_DIR=/usr/aarch64-unknown-linux-gnu/
HOME=/root
briansmith commented 1 year ago

:sigh: gcc (GCC) 4.8.5 is very old and I'm surprised it's worked for so long up to now. I don't think manylinux's strategy of using ancient decade-old GCC is the right way of solving the problem you have. It would be better to find a solution that uses modern compilers (Clang 17 or GCC 13) with old sysroots, which should have the same effect in supporting old systems but with modern performance. Especially ECC code in ring really needs a good, modern, compiler to work well.

briansmith commented 1 year ago

export CFLAGS_aarch64_unknown_linux_gnu="-D__ARM_ARCH=8"

Perhaps instead pass the correct -march in CFLAGS. See https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/ARM-Options.html#ARM-Options

bobzilladev commented 1 year ago

:sigh: gcc (GCC) 4.8.5 is very old and I'm surprised it's worked for so long up to now.

Yeah, looks like manylinux_2_28 based on GCC 7.5.0 works fine out of the box. Feel free to close this issue out, wanted to make the behavior change known, just in case.

Perhaps instead pass the correct -march in CFLAGS

Just tried with the only v8 option (-march=armv8-a) on 4.8.5, doesn't seem to set ARM_ARCH, fwiw:

running: "aarch64-unknown-linux-gnu-gcc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-gdwarf-4" "-fno-omit-frame-pointer" "-march=armv8-a" "-I" "include" "-I" "/build/target/aarch64-unknown-linux-gnu/debug/build/ring-6694976b207fa307/out" "-std=c1x" "-pedantic" "-Wall" "-Wextra" "-Wbad-function-cast" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wenum-compare" "-Wfloat-equal" "-Wformat=2" "-Winline" "-Winvalid-pch" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wnested-externs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wstrict-prototypes" "-Wundef" "-Wuninitialized" "-Wwrite-strings" "-fvisibility=hidden" "-fstack-protector" "-g3" "-DNDEBUG" "-o" "/build/target/aarch64-unknown-linux-gnu/debug/build/ring-6694976b207fa307/out/crypto/curve25519/curve25519.o" "-c" "crypto/curve25519/curve25519.c" ... cargo:warning=include/ring-core/asm_base.h:73:2: error: #error "ARM assembler must define __ARM_ARCH"

briansmith commented 1 year ago

Thanks, @bobzilladev. My guess is that the older manylinux probably has a sysroot that's older than GCC 4.8 itself, probably to aid in portability/backward compatibility. Unfortunately, it looks like it will be impractical to support compilers that don't support ACLE (i.e. that don't support __ARM_ARCH), so I am going to close this. I appreciate you bringing this to everybody's attention though.