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

Error compiling AESNI in Mbed-TLS with clang on Windows #8372

Closed
sergio-nsk opened this issue Oct 16, 2023 · 4 comments · Fixed by #8374
Closed

Error compiling AESNI in Mbed-TLS with clang on Windows #8372

sergio-nsk opened this issue Oct 16, 2023 · 4 comments · Fixed by #8374
Labels
bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d)

Comments

@sergio-nsk
Copy link
Contributor

sergio-nsk commented Oct 16, 2023

Summary

clang does not have MSVS intrinsics, the pre-processor condition below does not cover this case if clang runs in MSVC compatibility mode

mbedtls/library/aesni.h

Lines 42 to 46 in 1ec6906

#if defined(_MSC_VER)
/* Visual Studio supports AESNI intrinsics since VS 2008 SP1. We only support
* VS 2013 and up for other reasons anyway, so no need to check the version. */
#define MBEDTLS_AESNI_HAVE_INTRINSICS
#endif

It should be

#if defined(_MSC_VER) && !defined(__clang__)
/* Visual Studio supports AESNI intrinsics since VS 2008 SP1. We only support
 * VS 2013 and up for other reasons anyway, so no need to check the version. */
#define MBEDTLS_AESNI_HAVE_INTRINSICS
#endif

System information

Mbed TLS version 2.28.5, 3.5.0
Operating system and version: Windows 10/11
Default configuration.
Compiler: LLVM clang, clang-cl

Expected behavior

Built without errors.

Actual behavior

Got many errors

/mbedtls/library/aesni.c:62:17: error: passing 'unsigned int[4]' to parameter of type 'int *' converts between pointers to integer types with different sign [-Werror,-Wpointer-sign]
        __cpuid(info, 1);
                ^~~~
C:\PROGRA~1\LLVM\lib\clang\16\include\intrin.h:60:17: note: passing argument to parameter here
void __cpuid(int[4], int);
                ^
/mbedtls/library/aesni.c:102:21: error: call to undeclared function '_mm_aesdec_si128'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
            state = _mm_aesdec_si128(state, *rk);
                    ^
/mbedtls/library/aesni.c:102:21: note: did you mean '_mm_and_si128'?
C:\PROGRA~1\LLVM\lib\clang\16\include\emmintrin.h:2611:46: note: '_mm_and_si128' declared here
static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_and_si128(__m128i __a,
                                             ^
<...>

Additional infromation

Fixing PRs will be offered soon.

@gilles-peskine-arm
Copy link
Contributor

This is different, but closely related to #8332, and at first glance I think #8339 should fix this. Can you please check if #8339 works for you?

@gilles-peskine-arm gilles-peskine-arm added bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d) labels Oct 16, 2023
@sergio-nsk
Copy link
Contributor Author

sergio-nsk commented Oct 16, 2023

@gilles-peskine-arm Yes, it looks very similarly. I tried to search existing issues with my errors and could not find them. #8339 will not work for me, because clang does not define the macro __GNUC__ and <cpuid.h> does not exist on Windows.

There is the task Consider removing AESNI assembly #8231, maybe it's more correct that #8339 does not update aesni.c for MinGW but disables easni.c like I want to do it in easni.h.

sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 16, 2023
…indows

It can successfully compile w/ or w/o the clang options -maes -mpclmul.
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 16, 2023
…indows

It can successfully compile w/ or w/o the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
@lpy4105
Copy link
Contributor

lpy4105 commented Oct 17, 2023

#8339 will not work for me, because clang does not define the macro __GNUC__ and <cpuid.h> does not exist on Windows.

I believe it would fix the issue arround __cpuid, but we do have issue of AESNI implementation selection for clang in MSVC compatible mode.

sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 17, 2023
…indows

It can successfully compile w/ the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 17, 2023
…indows

It can successfully compile w/ or w/o the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 19, 2023
…indows

It can successfully compile w/ or w/o the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 19, 2023
…indows

It can successfully compile w/ the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 19, 2023
…indows

It can successfully compile w/ the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
sergio-nsk added a commit to sergio-nsk/mbedtls that referenced this issue Oct 19, 2023
…indows

It can successfully compile w/ or w/o the clang options -maes -mpclmul.

Signed-off-by: Sergey Markelov <sergey@solidstatenetworks.com>
@pengsel
Copy link

pengsel commented Oct 24, 2023

Is there any way to solve this compilation problem without modifying the code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants