Mbed-os: Cordio driver test broken on Cypress targets after #12761

[kyle-cypress]

Description of defect

After #12761, the greentea test features-feature_ble-targets-target_cordio-tests-cordio_hci-driver hard faults on Cypress targets. This does not appear to be an expected outcome of #12761, which mentions only the possibility of compilation errors.
The test succeeds if I revert 616b4e7bd8cdd53012f10e6237de494a9178da99.

Target(s) affected by this defect ?

CY8CPROTO_062_4343W
CY8CKIT_062_WIFI_BT
Possibly others.

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM 6.3.1

What version of Mbed-os are you using (tag or sha) ?

af4c8a94f3913ceda9069ec79cba81a793d695fe

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli 1.10.2

How is this defect reproduced ?

mbed test -t GCC_ARM -m CY8CPROTO_062_4343W -n *cordio*driver*

[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/MBOTRIAGE-2652

[0xc0170]

@kjbracey-arm Please review

[kjbracey-arm]

Changing that Callback can break binary compatibility - not mentioned in the PR description, but mentioned in the option description. Does this platform have a binary blob using Callback?

If so, a fix would be to add a target-override for the platform to make that platform have the setting true. Applications would not be able to set it to false to get a saving unless avoiding use of the component with the binary blob.

[0xc0170]

Or if cordio depends on this behavior, could also overwrite it?

cc @ARMmbed/mbed-os-pan

[kjbracey-arm]

It's a platform option, meaning it would have to be done as a target-override in platform/mbed_lib.json.

Alternatively you could have a target option like target.avoid-binary-breakage and then have the Callbackcode switch on the combo of !platform.callback-nontrivial && !target.avoid-binary-breakage. Allow the target to have an option to avoid binary-breaking optimisations, so platform.callback-nontrivial does nothing (or limits itself to optimisations that don't change the binary layout).

[kyle-cypress]

There aren't any binary images on these targets that interact with the mbed APIs. The only blobs are:

• Prebuilt CM0p image. This just starts the CM4 then goes to sleep to wait for IPC calls, which it services independent of anything from mbed.
• Radio firmware. This is not executed on the MCU, just set unmodified down to the radio.

So I can't think of any situations where binary compatibility would be an issue.

[kjbracey-arm]

Thanks for the info. Sounds like it's not a binary thing then. But I just noticed this:

GCC_ARM 6.3.1

We don't support that old a version of GCC any more, and I can believe that Callback might have a problem with it. I don't remember what the issue was, or even if it was Callback, but there was at least one GCC bug that was affecting something I was doing over the last several months, and I was relying on having GCC 8 or later to avoid it.

Can you retest with GCC ARM embedded 9-2019-q4-major?

[kyle-cypress]

Retested with 9-2019-q4-major and the tests passes. So yes, looks like it's a compiler bug. Closing.

[kjbracey-arm]

We should maybe emphasise this in a release note. We do talk about supported compiler versions a lot, but it's generally routine defensiveness about "we don't test it". But if we know there is a real failure, we should shout a bit louder to get people to take it seriously.

Maybe even strengthen the build tool warning? Have a minimum version of a toolchain beyond which it's actually a build-stopping-error rather than letting it proceed?

Or maybe not a tool thing - actually put a #error for version into Callback.h itself (inside the #if for the trivial option, which is what I think it is). That would imply putting effort into figuring out the limit and failure condition though, which maybe doesn't make sense.