Compiling with -march=armv8-a+crc+nocrypto results in linking error

[Fugoes]

Expected behavior

The shared library should link without error.

Actual behavior

For the following program named err.cpp:

#include "rocksdb/db.h"

int main() {
        rocksdb::DB* db;
        rocksdb::Options options;
        options.create_if_missing = true;
        rocksdb::Status status = rocksdb::DB::Open(options, "/tmp/testdb", &db);
        return 0;
}

Compile and link it with:

g++ -lrocksdb err.cpp

There are linking errors:

/usr/lib/gcc/aarch64-unknown-linux-gnu/9.3.0/../../../../aarch64-unknown-linux-gnu/bin/ld: /usr/lib/gcc/aarch64-unknown-linux-gnu/9.3.0/../../../../lib64/librocksdb.so: undefined reference to `crc32c_arm64(unsigned int, unsigned char const*, unsigned int)'
/usr/lib/gcc/aarch64-unknown-linux-gnu/9.3.0/../../../../aarch64-unknown-linux-gnu/bin/ld: /usr/lib/gcc/aarch64-unknown-linux-gnu/9.3.0/../../../../lib64/librocksdb.so: undefined reference to `crc32c_runtime_check()'
collect2: error: ld returned 1 exit status

Steps to reproduce the behavior

Compile rocksdb's shared library (version 5.18.4, though I think many versions have this issue) with -march=armv8-a+crc+nocrypto compile option and follow the steps in the 'Actual behavior' section.

---

What's more, the default make shared_lib seems to detect CPU flags wrongly, for a +crc+nocrypto CPU, it produces +crc+crypto options, and lead to SIGILL.

[adamretter]

To decipher armv8-a+crc+nocrypto

1. armv8-a = ARMv8 application architecture profile.
2. crc = With CRC extension.
3. nocrypto Without Cryptographic extension.

[Fugoes]

To decipher armv8-a+crc+nocrypto

1. `armv8-a` = ARMv8 application architecture profile.

2. `crc` = With CRC extension.

3. `nocrypto` Without Cryptographic extension.

I understand the meaning of these flags. I could workaround this issue by using armv8-a+nocrypto+nocrc, though my CPU do have crc capacity. But I think this might be a bug in the build system or the util/crc32c_arm64.h file?

[adamretter]

@Fugoes Sure. I didn't understand the flags though - which is why I added them for my own reference ;-)

[Fugoes]

What's more, most aarch64 server or high ends Android phone should have both crc and crypto I think. I encounter this issue on a raspberry pi 4 b XD.

[siying]

For those who created the file, @HouBingjian , @Zhiwei-Dai and @guyuqi any thought how we should resolve this?

[grooverdan]

In addition to the flags discussion here (that I'm following closely) note that @guyuqi who submitted the same code to mariadb (https://github.com/MariaDB/server/pull/772) missed a getauxval runtime check on HWCAP_PMULL (1 << 4) (wip fix - for maria- https://github.com/mysqlonarm/server/commit/1bd43b613eb5c35fc604b45ccdab4c096717cce9) which will cause Cortex-A72 to SIGILL (ref https://github.com/docker-library/mariadb/issues/318 and https://jira.mariadb.org/browse/MDEV-23030).

[guyuqi]

Yep, HWCAP_PMULL should be also checked in runtime routine like HWCAP_CRC32 does. Sorry for missing it. @grooverdan Thanks for your comments, it's informative. @siying I would like to submit a PR to fix it.

[dr-m]

MariaDB/server#1645 introduces a revised runtime check for the CRC-32C acceleration:

int crc32c_aarch64_available(void)
{
  return !(~getauxval(AT_HWCAP) & (HWCAP_CRC32 | HWCAP_PMULL));
}

I expect to merge it as soon as it has been validated on a (pmull capable) CI system.

[sike-evolve]

@Fugoes I'd like to know how you did compile RocksDB shared library. I'm trying to reproduce the same problem but I couldn't.

[riversand963]

Since #7233 is merged, should we close this one?

[alanpaxton]

@siying Could you clarify for me if @guyuqi fix https://github.com/facebook/rocksdb/pull/7233 resolves this entire issue ? I want to know if I (as the inheritor of sike's work) should pick it up again and try to repro ?

[alanpaxton]

I have acquired a Pi 4 B and explicitly built the shared lib at HEAD with nocrypto (-march=armv8-a+crc+nocrypto) instead of the current default of (-march=armv8-a+crc+crypto). In both cases, the symbols reported missing above are found, and the snippet compiles and runs successfully as err.cpp in ./examples (use nm to check the symbols in a library)

I am using 64bit PiOS.

(cd examples; g++ -fno-rtti err.cpp -oerr -L.. -lrocksdb -I../include -O2 -std=c++17 -lpthread -lrt -ldl -lz -std=c++17)

NOTE ALSO that the 4B DOES support armv8-a crypto instructions, it has a cortex-a72 as described in https://developer.arm.com/documentation/100097/0003/introduction/about-the-cortex-a72-processor-cryptography-engine?lang=en

It does not seem possible to build 5.18 with the current toolchain, so testing whether the problem is reproducible there has not been done. But crc32c_arm64.cc logic has changed significantly, and in particular is based on the definition of __ARM_FEATURE_CRYPTO which is defined when building with crypto; see the following incantations for determining the headers defined by g++

echo | gcc -march=armv8-a+crc+crypto -dM -E - | grep ARM

and undefined without

echo | gcc -march=armv8-a+crc+nocrypto -dM -E - | grep ARM

It seems then, that the reported SIGILL is the one fixed by https://github.com/facebook/rocksdb/pull/7233

tldr; actual behaviour should now be the expected behaviour.

@fugoes @siying I think this can be closed now.