Mbed-os: [Warning] psa_client.h@25,0: "PSA_SUCCESS" redefined

[teetak01]

Description

Mbed OS 5.12.0 produces a lot of compiler warnings from PSA_SUCCESS.

target: LPC55S69_NS
toolchain: GCC_ARM, probably all.

14:16:29 Compile [ 73.8%]: EthernetInterface.cpp
14:16:29 [Warning] psa_client.h@25,0: "PSA_SUCCESS" redefined
14:16:29 Compile [ 74.0%]: NetStackMemoryManager.cpp
14:16:29 [Warning] psa_client.h@25,0: "PSA_SUCCESS" redefined
14:16:29 Compile [ 74.1%]: DTLSSocket.cpp
14:16:29 [Warning] psa_client.h@25,0: "PSA_SUCCESS" redefined
14:16:29 Compile [ 74.3%]: EMACInterface.cpp
14:16:29 [Warning] psa_client.h@25,0: "PSA_SUCCESS" redefined

To reproduce:

mbed import mbed-cloud-client-example
<add certificate>
cp configs-psa/wifi_esp8266_v4.json mbed_app.json
mbed compile -t ARM -m LPC55S69_NS --profile release

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

[cmonr]

@ARMmbed/mbed-os-psa

[orenc17]

Please supply one of .d files so we could find the root cause of the issue

[teetak01]

@orenc17 you can use the latest client release to verify this.

[orenc17]

@ARMmbed/mbed-os-maintainers please move the assignee to crypto team
they should use psa/error.h instead of redefining all the status codes

[0xc0170]

@ARMmbed/mbed-os-maintainers please move the assignee to crypto team

Done

[0xc0170]

@ARMmbed/mbed-os-crypto Please fix

[gilles-peskine-arm]

I'm taking a look. psa/crypto_values.h contains some definitions that are duplicated from psa/error.h because PSA crypto also exists as a standalone library on non-PSA platforms. Is the problem actually the duplicated definition between psa/error.h and psa/crypto_values.h? In the build log I see references to psa_client.h. psa_client.h in 5.12.0 is aligned with an older beta version of the PSA specifications which had a different definition of this and other macros.

So is the request here that Mbed Crypto includes psa/error.h in Mbed OS, or is the request to provide some backward compatibility with an older version of the definitions?

[orenc17]

CC @athoelke

[alzix]

I think the request is to deduplicate the definitions in order to solve compilation warnings.

One possible almost immediate solution, but suboptimal from my perspective, is to remove the duplicates (sed/patch) during importer script phase.

As for long term solution Mbed Crypto error codes should be reviewed and perhaps aligned with psa/error.h

[gilles-peskine-arm]

Crypto has already aligned the error codes with psa/error.h (spec discussion (internal link): https://github.com/ARMmbed/PSA-IPC-doc/issues/133 ; implemented in crypto in https://github.com/ARMmbed/mbed-crypto/pull/55 which is present in Mbed OS since 5.12.0). TFM's error codes are the ones that are not up to date with psa/error.h. So my question stands: does crypto need to provide a patch to make it compatible with the older error codes, or merely arrange to include psa/error.h?

[kjbracey-arm]

Repeated #defines are permitted in C, as long as they're identical.

A quick GCC test confirms it doesn't warn if they are identical. So you have a mismatch somewhere.

Ah, I think maybe Gilles is a little ahead of me.

[gilles-peskine-arm]

@kjbracey-arm There are 5 files in Mbed OS 5.12.0 that define PSA_SUCCESS. TARGET_PSA via psa/error.h and mbed-crypto via psa/crypto.h contain the new definition.
TFM via psa_client.h and PAL via pal_common.h and
pal_client_api_intf.h contain the old definition.

If we need to live with both definitions, then all the headers probably need patching to support the redefinition (if we only patch one then the patch will only work if the inclusion is in a certain order). Furthermore it's likely that there are other mismatches that the compiler wouldn't catch, e.g. if one part of the system emits PSA_CONNECTION_REFUSED = INT32_MIN + 1 and another part expects PSA_ERROR_CONNECTION_REFUSED = -130. So it's not just about crypto. Who's using what version of the PSA framework? What functional tests would catch such a mismatch?

[alzix]

cc @Patater

[gilles-peskine-arm]

@alzix We (crypto team) can't fix anything until we understand what's required. It's an incompatibility between Mbed Crypto and something else in Mbed OS, not a bug of Mbed Crypto itself, and we don't know what to change, or even if Mbed Crypto is the one that needs to change.

[jeromecoutant]

Hi

Issue is still valid :-(

components/TARGET_PSA/TARGET_TFM/interface/include/psa_client.h:#define PSA_SUCCESS (0)
components/TARGET_PSA/inc/psa/error.h:#define PSA_SUCCESS ((psa_status_t)0)
features/frameworks/TARGET_PSA/pal/pal_client_api_intf.h:#define PSA_SUCCESS (0)
features/frameworks/TARGET_PSA/pal/pal_common.h:#define PSA_SUCCESS (0)
features/mbedtls/mbed-crypto/inc/psa/crypto_values.h:#define PSA_SUCCESS ((psa_status_t)0)

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2119