IAR build fixes after 4.1.0#10635
IAR build fixes after 4.1.0#10635gilles-peskine-arm wants to merge 4 commits intoMbed-TLS:developmentfrom
Conversation
gilles-peskine-arm
added
needs-ci
gilles-peskine-arm
added
priority-high
github-project-automation
Bot
moved this to In Development
in Roadmap pull requests (new board)
There was a problem hiding this comment.
Pull request overview
Fixes IAR (ICCARM) build issues for the library and unit tests by adjusting includes, suppressing an IAR-specific diagnostic, and adding explicit casts to satisfy stricter type checking.
Changes:
- Update
tf-psa-cryptoandframeworksubmodules to revisions that support the IAR build. - Make test-suite headers conditional on
MBEDTLS_FS_IOso baremetal/no-FS builds don’t pull POSIX headers. - Silence IAR warnings and tighten type conversions in SSL/unit-test helper code paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tf-psa-crypto | Bumps submodule revision to bring in IAR-related fixes. |
| framework | Bumps submodule revision to keep framework aligned with the build fixes. |
| tests/suites/test_suite_pkcs7.function | Gates POSIX includes behind MBEDTLS_FS_IO for IAR/baremetal friendliness. |
| tests/src/test_helpers/ssl_helpers.c | Adds IAR diagnostic suppression and makes enum conversions explicit. |
| library/ssl_tls.c | Adds an explicit cast for IAR compatibility in key export path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| #if defined(__IAR_SYSTEMS_ICC__) | ||
| /* Suppress a very overeager warning from IAR: it dislikes a forward goto | ||
| * that bypasses the initialization of a variable, even if that variable | ||
| * is not used after the jump. (This is perfectly valid C; it would only | ||
| * be invalid C if jumping into a block from outside that block.) | ||
| */ | ||
| #pragma diag_suppress=Pe546 // transfer of control bypasses initialization | ||
| #endif |
There was a problem hiding this comment.
This disables warning Pe546 for the remainder of the translation unit, which can mask real issues introduced later in the file. Prefer to scope the suppression as tightly as possible (e.g., use IAR’s diagnostic push/pop if available, or re-enable the diagnostic after the specific code section with the corresponding restore pragma).
| int cipher_type_arg, int md_type_arg, | ||
| int etm, int tag_mode, | ||
| mbedtls_ssl_protocol_version tls_version, | ||
| size_t cid0_len, | ||
| size_t cid1_len) | ||
| { | ||
| mbedtls_md_type_t md_type = (mbedtls_md_type_t) md_type_arg; | ||
| mbedtls_cipher_type_t cipher_type = (mbedtls_cipher_type_t) cipher_type_arg; | ||
|
|
There was a problem hiding this comment.
Converting from int to enum types inside the function removes compile-time type checking and can hide invalid values flowing into the helper. If feasible within the test-helper API, consider changing the function parameters to mbedtls_cipher_type_t cipher_type and mbedtls_md_type_t md_type (and adjusting callers accordingly) so the compiler enforces correct usage without relying on casts.
| int cipher_type_arg, int md_type_arg, | |
| int etm, int tag_mode, | |
| mbedtls_ssl_protocol_version tls_version, | |
| size_t cid0_len, | |
| size_t cid1_len) | |
| { | |
| mbedtls_md_type_t md_type = (mbedtls_md_type_t) md_type_arg; | |
| mbedtls_cipher_type_t cipher_type = (mbedtls_cipher_type_t) cipher_type_arg; | |
| mbedtls_cipher_type_t cipher_type, | |
| mbedtls_md_type_t md_type, | |
| int etm, int tag_mode, | |
| mbedtls_ssl_protocol_version tls_version, | |
| size_t cid0_len, | |
| size_t cid1_len) | |
| { |
| int ciphersuite_id = mbedtls_ssl_get_ciphersuite_id_from_ssl(ssl); | ||
| const mbedtls_ssl_ciphersuite_t *ciphersuite = mbedtls_ssl_ciphersuite_from_id(ciphersuite_id); | ||
| const mbedtls_md_type_t hash_alg = ciphersuite->mac; | ||
| const mbedtls_md_type_t hash_alg = (mbedtls_md_type_t) ciphersuite->mac; |
There was a problem hiding this comment.
The explicit cast looks like it’s compensating for a type mismatch between ciphersuite->mac and mbedtls_md_type_t. Where practical, it’s better to address this at the source (e.g., ensure the mac field is typed as mbedtls_md_type_t, or introduce a clearly named conversion helper) so the codebase doesn’t accumulate casts that can obscure real type issues.
mpg
added
needs-work
and removed
needs-preceding-pr
gilles-peskine-arm
changed the title
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Don't try to include a Unix header when we don't need one. The tests will still fail to build if `MBEDTLS_FS_IO` is enabled and the platform has stdio but not Unix extensions. (This is also the case for some of our X.509 library code.) Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Even without CI validation, we can promise that we've made the situation better, and an IAR warning has been reported in Mbed-TLS#10648 Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
iar-4.1.0
branch
from
893d371 to
91350be
Compare
gilles-peskine-arm
added
needs-review
|
This is ready for review. The CI will pass once the framework and crypto prerequisites are merged and the submodule pointers are updated here. |
gilles-peskine-arm
added
the
needs-preceding-pr
Fix the build of the library and the unit tests with IAR. Fixes #10648.
Needs preceding PR:
check_names)PR checklist