cryfs / cryfs

Cryptographic filesystem for the cloud
https://www.cryfs.org
GNU Lesser General Public License v3.0
2.02k stars 155 forks source link

Build Failure on armel #408

Closed davesteele closed 2 years ago

davesteele commented 2 years ago

Expected Behavior

Successful compile on the armel architecture.

Actual Behavior

Stock build fails.

...
[  3%] Building CXX object vendor/cryptopp/vendor_cryptopp/CMakeFiles/cryptopp-object.dir/aria_simd.cpp.o
In file included from /<<PKGBUILDDIR>>/vendor/cryptopp/vendor_cryptopp/aria_simd.cpp:18:
/usr/lib/gcc/arm-linux-gnueabi/11/include/arm_neon.h:31:2: error: #error "NEON intrinsics not available with the soft-float ABI.          Please use -mfloat-abi=softfp or -mfloat-abi=hard"
   31 | #error "NEON intrinsics not available with the soft-float ABI.  Please use -mfloat-abi=softfp or -mfloat-abi=hard"
      |  ^~~~~
/<<PKGBUILDDIR>>/vendor/cryptopp/vendor_cryptopp/aria_simd.cpp:67:34: error: ‘uint32x4_t’ does not name a type; did you mean ‘uint32_t’?
   67 | inline void ARIA_GSRK_NEON(const uint32x4_t X, const uint32x4_t Y, byte RK[16])
      |                                  ^~~~~~~~~~
      |                                  uint32_t
/<<PKGBUILDDIR>>/vendor/cryptopp/vendor_cryptopp/aria_simd.cpp:67:54: error: ‘uint32x4_t’ does not name a type; did you     mean ‘uint32_t’?
...

Steps to Reproduce the Problem

mkdir -p build
(cd build && cmake .. -DDEPENDENCY_CONFIG=../cmake-utils/DependenciesFromLocalSystem.cmake DBUILD_TESTING=ON -DCRYFS_UPDATE_CHECKS=OFF)

Specifications

smessmer commented 2 years ago

This looks like a bug in the CryptoPP library. Did you try CryFS 0.11.1? It comes with a newer version of that library and the changelog on https://www.cryptopp.com/ mentions something about ARM fixes.

davesteele commented 2 years ago

Same failure on 0.11.1 - https://buildd.debian.org/status/fetch.php?pkg=cryfs&arch=armel&ver=0.11.1-1&stamp=1641316600&raw=0

smessmer commented 2 years ago

Possibly related: https://github.com/cryfs/cryfs/issues/345

davesteele commented 2 years ago

Debian doesn't appear to have armel porter boxes. I don't expect I'll be taking point on this.

smessmer commented 2 years ago

@davesteele can you check if the flag proposed in weidai11/cryptopp#1100 fixes things?

davesteele commented 2 years ago

So is the referenced commit needed for this?

smessmer commented 2 years ago

Not sure, I have unfortunately no way to test on arm machines.

On February 7, 2022 8:17:46 PM David Steele @.***> wrote:

So is the referenced commit needed for this? — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

noloader commented 2 years ago

@davesteele, @smessmer,

It looks like things broke due to some recent Debian changes modulo some GCC changes. I remember Debian discussing changing the minimum arm platform on the Debian ARM mailing list. I don't track GCC, so I don't know what might have changed recently.

The short answer to this problem is, build Crypto++ on armel with the define CRYPTOPP_DISABLE_ARM_NEON. There were some other oddities when adding CRYPTOPP_DISABLE_ARM_NEON, but it did not affect the result. The other oddities were discussed here and fixed with with Commit 66d73d6d8b95. The oddities are, some files get options -march=armv7-a -mfpu=neon even though NEON is disabled. It is benign because the code is removed by the define.

Looking at the CMake files, there is an option to disable NEON:

# These are ARM A-32 options
option(DISABLE_ARM_NEON "Disable NEON" OFF)

Would it be possible to either (1) define CRYPTOPP_DISABLE_ARM_NEON with Makefile or (2) use DISABLE_ARM_NEON with CMake? Or, you can open config_asm.h and disable the NEON stuff manually.

I tested CMake with cmake -DDISABLE_ARM_NEON=ON and it tested OK on an old RaspberryPi (RP1), which is ARMv6 without NEON.

I'm not sure we will be able to fix this problem. Armel is one of those corner cases, and I am not sure how to test for an armel build. An armel build looks just like an armhf build. But for armel we never need NEON. Any suggestions would be appreciated.

noloader commented 2 years ago

@smessmer,

Not sure, I have unfortunately no way to test on arm machines.

If you are interested, you can test Debian Unstable using their ports and QEMU Chroot's. I used the QEMU Chroot's for armhf and armel problems under Debian Sid with GCC 11.

I documented the procedure I use to setup the QEMU Chroot's at Debian Chroot on the Crypto++ wiki.

I recommend testing on real hardware because the QEMU Chroot's are emulated, so things are slow. But sometimes you can't get your hands on hardware, so you have to suffer the emulation.

And as a free and open software developer, you should get a GCC Compile Farm account. They make a lot of hardware available to you. The Compile Farm lacks armel and armhf, however.

noloader commented 2 years ago

@smessmer ,

I had a look at this under a QEMU/Chroot for armel. Here's the CMakefileList.ext. Here's what I see:

-- CMake version 3.22.1
-- System 
-- Processor 
-- The C compiler identification is GNU 11.2.0
-- The CXX compiler identification is GNU 11.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CRYPTOPP_ARM_NEON_HEADER
-- Performing Test CRYPTOPP_ARM_NEON_HEADER - Failed
-- Performing Test CRYPTOPP_ARMV7A_NEON
-- Performing Test CRYPTOPP_ARMV7A_NEON - Failed
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- Platform: ARM-32
-- Compiler: /usr/bin/c++
-- Compiler options:  -DCRYPTOPP_DISABLE_ARM_NEON
-- Compiler definitions: 
-- Build type: RelWithDebInfo
-- Configuring done
-- Generating done
-- Build files have been written to: /cryptopp/cmake_build

So tests for <arm_neon.h> header failed, and test for -mfpu=neon failed. We are adding -DCRYPTOPP_DISABLE_ARM_NEON as expected, but it is not being used during the compile:

usr/bin/c++   -O2 -g -DNDEBUG -fPIC -MD -MT CMakeFiles/cryptopp-object.dir/aria_simd.cpp.o -MF CMakeFiles/cryptopp-object.dir/aria_simd.cpp.o.d -o CMakeFiles/cryptopp-object.dir/aria_simd.cpp.o -c /cryptopp/aria_simd.cpp
In file included from /cryptopp/aria_simd.cpp:18:
/usr/lib/gcc/arm-linux-gnueabi/11/include/arm_neon.h:31:2: error: #error "NEON intrinsics not available with the soft-float ABI.  Please use -mfloat-abi=softfp or -mfloat-abi=hard"
   31 | #error "NEON intrinsics not available with the soft-float ABI.  Please use -mfloat-abi=softfp or -mfloat-abi=hard"
      |  ^~~~~
/cryptopp/aria_simd.cpp:63:34: error: ‘uint32x4_t’ does not name a type; did you mean ‘uint32_t’?
   63 | inline void ARIA_GSRK_NEON(const uint32x4_t X, const uint32x4_t Y, byte RK[16])
      |                                  ^~~~~~~~~~
      |                                  uint32_t
/cryptopp/aria_simd.cpp:63:54: error: ‘uint32x4_t’ does not name a type; did you mean ‘uint32_t’?
   63 | inline void ARIA_GSRK_NEON(const uint32x4_t X, const uint32x4_t Y, byte RK[16])
      |                                                      ^~~~~~~~~~
      |                                                      uint32_t

I'm guessing we are doing something wrong in the CMakefileList.txt file. Do you know why this is not working (around line 760-800 of CMakefileList.txt):

if (CRYPTOPP_ARMV7A_NEON) // Around line 760
    // This is skipped as expected
    ...
else() // Around line 800
    // This is executed as expected
    list(APPEND CRYPTOPP_COMPILE_OPTIONS "-DCRYPTOPP_DISABLE_ARM_NEON")
endif()
smessmer commented 2 years ago

This seems to be threatening CryFS inclusion in the API software repositories now, see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006820

I'm not familiar with ARM so I don't think there's much I can do here. @noloader Did you figure out what is causing this? If not, @davesteele did you try @noloader 's proposal of building with NEON disabled?

noloader commented 2 years ago

@smessmer,

Crypto++ 8.6 appears to be building for armel and armhf. See https://buildd.debian.org/status/package.php?p=libcrypto%2B%2B .

Can you provide a build result for CryptFS that shows what is going wrong now?

smessmer commented 2 years ago

Wait so the experiments you were doing above were with CryptoPP 8.5? Or did you change something in 8.6 to make it work? I thought you had been able to reproduce and confirm the issue?

On March 6, 2022 6:19:59 PM Jeffrey Walton @.***> wrote:

@smessmer, Crypto++ 8.6 appears to be building for armel and armhf. See https://buildd.debian.org/status/package.php?p=libcrypto%2B%2B . Can you provide a build result for CryptFS that shows what is going wrong now? — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.***>

noloader commented 2 years ago

@smessmer,

I thought you had been able to reproduce and confirm the issue?

Yes, confirmed.

Or did you change something in 8.6 to make it work?

The change for Debian was provided about 3 weeks ago. It started in Debian Unstable. It should be in Debian Testing by now (or entering Testing soon).

You can also add -DCRYPTOPP_DISABLE_ARM_NEON to CPPFLAGS for armel builds if the fixes have not been picked up yet.

(I updated the comment. Updates enter into Debian Unstable, and then head to Debian Testing. See here on the Debian wiki).

smessmer commented 2 years ago

Or did you change something in 8.6 to make it work?

The change for Debian was provided about 3 weeks ago. It started in Debian Testing. It should be in Debian Unstable by now (or entering Unstable soon).

Ah you re-released CryptoPP 8.6 or is there 8.6.1 or something like that with the fix? The CryFS build actually doesn't use the debian package of CryptoPP but has a copy of CryptoPP in it's repository. I'll have to copy over any fixes or new released versions of CryptoPP so that the cryfs build is fixed

noloader commented 2 years ago

Ah you re-released CryptoPP 8.6 or is there 8.6.1 or something like that with the fix?

For Debian, they carry patches against the current release. So in their numbering scheme, they are at Crypto++ 8.6.0-3 now.

I'll have to copy over any fixes or new released versions of CryptoPP so that the cryfs build is fixed

Yeah, so the big one is at GH #1094, and the zip file in the last comment.

But it may be easier to set -DCRYPTOPP_DISABLE_ARM_NEON for armel.

I want to release a new Crypto++ soon. We just fixed a bug on Cygwin due to recent changes, so I would like to let the code simmer another week or two.

davesteele commented 2 years ago

If not, @davesteele did you try @noloader 's proposal of building with NEON disabled?

This is in the current release.

noloader commented 2 years ago

Thanks @davesteele,

That should work for you. If it does not work, then please ping me so I can ensure Cmake is in order for you. (My testing of Cmake is Ok on armel and armhf using cryptest-cmake.sh).

At Crypto++ 8.7 release, the build problems should go away. But you can keep -DCRYPTOPP_DISABLE_ARM_NEON for NEON since it does not affect amrel. Armel does not use NEON, so it does not matter if you supply DISABLE_ASM or DISABLE_NEON.

davesteele commented 2 years ago

@noloader, it does not. It leads to the current failure.

noloader commented 2 years ago

@davesteele,

I think one of the problems with our Cmake file was, Cmake was not using the CPPFLAGS and CXXFLAGS we were setting for the project. We would add -DCRYPTOPP_DISABLE_ARM_NEON but Cmake would not use it. I think you need Commit fae7769fbe00.

Humorously in a morbid sort of way, the Cmake folks are trying to undo the change at PR #79.


Put another way, you need this around CmakefileList.txt, line 1035:

# add_compile_definitions added in CMake 3.12
if (${CMAKE_VERSION} VERSION_LESS "3.12")
  # https://stackoverflow.com/q/61250087
  add_definitions("${CMAKE_CPP_FLAGS}" "${CMAKE_CXX_FLAGS}" "${CRYPTOPP_COMPILE_DEFINITIONS}" "${CRYPTOPP_COMPILE_OPTIONS}")
else()
  add_compile_definitions("${CMAKE_CPP_FLAGS}" "${CRYPTOPP_COMPILE_DEFINITIONS}")
  add_compile_options("${CMAKE_CXX_FLAGS}" "${CRYPTOPP_COMPILE_OPTIONS}")
endif()
davesteele commented 2 years ago

0.11.1-4, with https://github.com/noloader/cryptopp-cmake/pull/79:

/usr/bin/ld: ../../vendor/cryptopp/vendor_cryptopp/libcryptopp.a(rijndael.cpp.o): in function `CryptoPP::CRYPTOGAMS_decrypt(unsigned char const*, unsigned char const*, unsigned char*, unsigned int const*)':

./build/vendor/cryptopp/vendor_cryptopp/./vendor/cryptopp/vendor_cryptopp/rijndael.cpp:366: undefined reference to `cryptogams_AES_decrypt_block'

The full log is worth inspecting.

Once this is addressed, I'll do the next run against 0.11.2.

AdrianBunk commented 2 years ago

0.11.1-4, with noloader/cryptopp-cmake#79:

/usr/bin/ld: ../../vendor/cryptopp/vendor_cryptopp/libcryptopp.a(rijndael.cpp.o): in function `CryptoPP::CRYPTOGAMS_decrypt(unsigned char const*, unsigned char const*, unsigned char*, unsigned int const*)':

./build/vendor/cryptopp/vendor_cryptopp/./vendor/cryptopp/vendor_cryptopp/rijndael.cpp:366: undefined reference to `cryptogams_AES_decrypt_block'

The full log is worth inspecting.

I'm currently testing a patch for this issue.

noloader commented 2 years ago

@davesteele, @AdrianBunk,

... undefined reference to `cryptogams_AES_decrypt_block'

My bad... Instead of -DCRYPTOPP_DISABLE_ARM_NEON, use -DCRYPTOPP_DISABLE_ASM.

What is happening is, the Cryptogams code is being used because we pivot the Cryptogams code on CRYPTOPP_DISABLE_ASM, and not CRYPTOPP_DISABLE_ARM_NEON.

Or, you can continue to use -DCRYPTOPP_DISABLE_ARM_NEON but you need this from config_asm.h, line 375. Notice the outer !defined(CRYPTOPP_DISABLE_ARM_NEON), which was added due to the break:

#if !defined(CRYPTOPP_DISABLE_ARM_NEON)
# if defined(__arm__) && defined(__linux__)
#  if defined(__GNUC__) || defined(__clang__)
#   define CRYPTOGAMS_ARM_AES      1
#   define CRYPTOGAMS_ARM_SHA1     1
#   define CRYPTOGAMS_ARM_SHA256   1
#   define CRYPTOGAMS_ARM_SHA512   1
#  endif
# endif
#endif

The outer !defined(CRYPTOPP_DISABLE_ARM_NEON) was added at Commit 4473b50803d0 because of this problem.

I am so sorry this is causing so much grief for Cryptfs.

davesteele commented 2 years ago

Build is passing for 0.11.2-2. Other arch problems remain, which I will log in a new ticket.