gh-Mbed-TLS/TF-PSA-Crypto

Watch

Fork 0

Fix module-import-in-extern-c compiler error #557

Closed

zacharykeyessonos wants to merge 2 commits from fix-module-import-in-extern-c-compiler-error into development

zacharykeyessonos commented

2025-10-31 14:41:44 -05:00

(Migrated from github.com)

Description

This fixes https://github.com/Mbed-TLS/mbedtls/issues/10490.

This fixes a compiler error when building C++ projects that use the 'modules' C++ language feature consume Mbed TLS. The specific error is module-import-in-extern-c, which occurs when #includes occur within extern C blocks.

Example:

$ clang-17 -std=c++20 -fmodules -Itf-psa-crypto/include -Itf-psa-crypto/drivers/builtin/include -o sample sample.cpp
In file included from sample.cpp:8:
In file included from tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:44:
tf-psa-crypto/drivers/builtin/include/mbedtls/private/chacha20.h:26:1: error: import of C++ module '_Builtin_stdint' appears within extern "C" language linkage specification [-Wmodule-import-in-extern-c]
   26 | #include <stdint.h>
      | ^
tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:35:1: note: extern "C" language linkage specification begins here
   35 | extern "C" {
      | ^
1 error generated.

This seems to primarily be an issue with clang. See https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fmodules

PR checklist

changelog entry added
framework PR not required
mbedtls development PR provided Mbed-TLS/mbedtls#10488
mbedtls 3.6 PR provided Mbed-TLS/mbedtls#10489
tests Sample C++ file that includes all affected headers compiles

## Description This fixes https://github.com/Mbed-TLS/mbedtls/issues/10490. This fixes a compiler error when building C++ projects that use the 'modules' C++ language feature consume Mbed TLS. The specific error is module-import-in-extern-c, which occurs when `#include`s occur within `extern C` blocks. Example: ``` $ clang-17 -std=c++20 -fmodules -Itf-psa-crypto/include -Itf-psa-crypto/drivers/builtin/include -o sample sample.cpp In file included from sample.cpp:8: In file included from tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:44: tf-psa-crypto/drivers/builtin/include/mbedtls/private/chacha20.h:26:1: error: import of C++ module '_Builtin_stdint' appears within extern "C" language linkage specification [-Wmodule-import-in-extern-c] 26 | #include <stdint.h> | ^ tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:35:1: note: extern "C" language linkage specification begins here 35 | extern "C" { | ^ 1 error generated. ``` This seems to primarily be an issue with clang. See https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fmodules ## PR checklist - [X] **changelog** entry added - [ ] **framework PR** not required - [x] **mbedtls development PR** provided Mbed-TLS/mbedtls#10488 - [X] **mbedtls 3.6 PR** provided Mbed-TLS/mbedtls#10489 - [X] **tests** Sample C++ file that includes all affected headers compiles

gilles-peskine-arm commented

2025-10-31 15:36:52 -05:00

(Migrated from github.com)

Thank you very much for raising and fixing this!

Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already.

changelog not required because: ChangeLog.d/00README.md says fixes for compiler warnings are not generally required.

We often don't add changelog entries about compiler warnings because compilers are sometimes weirdly picky, and we don't want to create the expectation that we'll treat every warning from every version of every compiler as a bug. But this is a different, as far as I understand: our header files are not compatible with modern C++, it's not just something you can silence with -Wno-error=..., right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules).

Many of the maintainers of Mbed TLS and TF-PSA-Crypto, me included, are not very familiar with C++. For example, I know you need to put extern "C" around declarations of functions, but I don't know the precise rules about what else must, can or cannot be wrapped with extern "C". So it would be good to have a test for that. We have a test that tries to build a trivial C++ programs that includes our headers: component_test_full_clang, and the program is cpp_dummy_build.cpp. Can we adapt this test to catch this problem? I guess by changing the build flags in CMakeLists.txt? By default we use an older compiler, but I guess we would need a more recent compiler there?

Thank you very much for raising and fixing this! Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already. > changelog not required because: ChangeLog.d/00README.md says fixes for compiler warnings are not generally required. We often don't add changelog entries about compiler warnings because compilers are sometimes weirdly picky, and we don't want to create the expectation that we'll treat every warning from every version of every compiler as a bug. But this is a different, as far as I understand: our header files are not compatible with modern C++, it's not just something you can silence with `-Wno-error=...`, right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules). Many of the maintainers of Mbed TLS and TF-PSA-Crypto, me included, are not very familiar with C++. For example, I know you need to put `extern "C"` around declarations of functions, but I don't know the precise rules about what else must, can or cannot be wrapped with `extern "C"`. So it would be good to have a test for that. We have a test that tries to build a trivial C++ programs that includes our headers: [`component_test_full_clang`](https://github.com/Mbed-TLS/mbedtls/blob/mbedtls-4.0.0/tests/scripts/components-configuration.sh#L94), and the program is [`cpp_dummy_build.cpp`](https://github.com/Mbed-TLS/mbedtls/blob/mbedtls-4.0.0/programs/test/generate_cpp_dummy_build.sh). Can we adapt this test to catch this problem? I guess by changing the build flags in `CMakeLists.txt`? By default we use an older compiler, but I guess we would need a more recent compiler there?

zacharykeyessonos commented

2025-10-31 20:10:39 -05:00

(Migrated from github.com)

Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already.

Done! https://github.com/Mbed-TLS/mbedtls/issues/10490

it's not just something you can silence with -Wno-error=..., right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules).

I think this actually can be ignored by adding -Wno-module-import-in-extern-c. I'm happy to add a changelog entry for this though.

Can we adapt this test to catch this problem? I guess by changing the build flags in CMakeLists.txt? By default we use an older compiler, but I guess we would need a more recent compiler there?

It looks like simply adding target_compile_options(cpp_dummy_build PRIVATE -fmodules) to the cpp_dummy_build_cpp build exposes the bug. I hit this bug with clang-13, so I think older versions of clang will hit this.

> Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already. Done! https://github.com/Mbed-TLS/mbedtls/issues/10490 > it's not just something you can silence with `-Wno-error=...`, right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules). I think this actually can be ignored by adding `-Wno-module-import-in-extern-c`. I'm happy to add a changelog entry for this though. > Can we adapt this test to catch this problem? I guess by changing the build flags in `CMakeLists.txt`? By default we use an older compiler, but I guess we would need a more recent compiler there? It looks like simply adding `target_compile_options(cpp_dummy_build PRIVATE -fmodules)` to the `cpp_dummy_build_cpp` build exposes the bug. I hit this bug with `clang-13`, so I think older versions of `clang` will hit this.

zacharykeyessonos commented

2025-11-04 11:47:48 -06:00

(Migrated from github.com)

Hi @gilles-peskine-arm — Let me know if there are other changes you thing I should make. Thanks!

Pull request closed

This pull request cannot be reopened because the branch was deleted.

No reviewers

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto!557

No description provided.

Rows
Columns