Build error with clang in case of minimal PSA crypto config

[matetothpal]

Summary

There is a build error when Mbed TLS is build with PSA Crypto support with a minimal configuration. clang reports unreachable code in library/psa_crypto_aead.c, library/psa_crypto_cipher.c and library/psa_crypto.c. When all the algorithms are disabled, some of the functions return immediately, and some code at the end of the functions become unreachable.

Please note that the attached reproducer won't trigger the errors in library/psa_crypto.c. I'm not sure why.

System information

Mbed TLS version: v3.4.0 and developement branch:
Operating system and version: Bare metal and Linux
Configuration (if not default, please attach mbedtls_config.h): my_config.zip
Compiler and options: cmake with clang 13.0.1. See steps to reproduce
Additional environment information: -

Expected behavior

Mbed TLS builds without error

Actual behavior

Compilation fails with error

Steps to reproduce

rm -fr build && mkdir build && pushd build && \
cmake -DCMAKE_C_COMPILER=clang .. && \
cmake --build . ; \
popd

Additional information

/home2/mate/temp/mbedtls/library/psa_crypto_aead.c:126:27: error: code will never be executed [-Werror,-Wunreachable-code]
    operation->key_type = psa_get_key_type(attributes);
                          ^~~~~~~~~~~~~~~~
/home2/mate/temp/mbedtls/library/psa_crypto_aead.c:212:9: error: code will never be executed [-Werror,-Wunreachable-code]
    if (status == PSA_SUCCESS) {
        ^~~~~~
/home2/mate/temp/mbedtls/library/psa_crypto_aead.c:322:9: error: code will never be executed [-Werror,-Wunreachable-code]
    if (status == PSA_SUCCESS) {
        ^~~~~~
/home2/mate/temp/mbedtls/library/psa_crypto_aead.c:557:9: error: code will never be executed [-Werror,-Wunreachable-code]
    if (status == PSA_SUCCESS) {
        ^~~~~~
/home2/mate/temp/mbedtls/library/psa_crypto_aead.c:628:9: error: code will never be executed [-Werror,-Wunreachable-code]
    if (status == PSA_SUCCESS) {
        ^~~~~~
5 errors generated.
library/CMakeFiles/mbedcrypto.dir/build.make:899: recipe for target 'library/CMakeFiles/mbedcrypto.dir/psa_crypto_aead.c.o' failed
make[2]: *** [library/CMakeFiles/mbedcrypto.dir/psa_crypto_aead.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/home2/mate/temp/mbedtls/library/psa_crypto_cipher.c:154:9: error: code will never be executed [-Werror,-Wunreachable-code]
    if (cipher_id != NULL) {
        ^~~~~~~~~
1 error generated.

mbedtls/library/psa_crypto.c:7945:14: error: code will never be executed [-Werror,-Wunreachable-code]
    status = psa_driver_wrapper_pake_input(operation, driver_step,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mbedtls/library/psa_crypto.c:8002:14: error: code will never be executed [-Werror,-Wunreachable-code]
    status = psa_driver_wrapper_pake_get_implicit_key(operation,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[gilles-peskine-arm]

We just merged something that might fix the PAKE issues: #7573 (it would have no influence on the cipher/aead issues). Can you reproduce those with 7f97675?

[matetothpal]

I can confirm that the PAKE issues are fixed on the latest development branch (7f97675)

[gilles-peskine-arm]

Having looked at the code a little, clang's warnings are correct, but annoying: it would be nice if it would just silently optimize the unreachable code away. The code structure is of the form

    switch (type) {
#if FEATURE_1_ENABLED
    case FEATURE_1: do_stuff_1(); break;
#endif
#if FEATURE_2_ENABLED
    case FEATURE_2: do_stuff_2(); break;
#endif
    default: return NOT_SUPPORTED;
}
    cleanup();

And in the special case where none of the features are enabled, the cleanup code is indeed not reachable. I suppose we'll have to manually put the correct conditional directives around the cleanup code.

In the meantime, a workaround is to remove -Wunreachable-code from CFLAGS.

[matetothpal]

Thanks for looking at the issue. So If I'm correct, this would be the workaround:

diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt
index a08f3ff0b..563ead12f 100644
--- a/library/CMakeLists.txt
+++ b/library/CMakeLists.txt
@@ -195,7 +195,7 @@ if(CMAKE_COMPILER_IS_GNUCC)
 endif(CMAKE_COMPILER_IS_GNUCC)
 
 if(CMAKE_COMPILER_IS_CLANG)
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wmissing-declarations -Wmissing-prototypes -Wdocumentation -Wno-documentation-deprecated-sync -Wunreachable-code")
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wmissing-declarations -Wmissing-prototypes -Wdocumentation -Wno-documentation-deprecated-sync")
 endif(CMAKE_COMPILER_IS_CLANG)
 
 if(CMAKE_COMPILER_IS_MSVC)

Shall I create a PR for this?

[gilles-peskine-arm]

That's a possible workaround, but we'd prefer to fix the code so that it doesn't trigger a warning. I'm not sure yet what we'll do.