ARMCLANG.cmake: Add option NO_FSHORT_FLAGS#420
ARMCLANG.cmake: Add option NO_FSHORT_FLAGS#420avinaw01-arm merged 1 commit intoARM-software:mainfrom
Conversation
|
This is to fix errors like: |
|
Hi @nicola-mazzucato-arm |
|
hi @avinaw01-arm , I have only seen the above with ARMCLANG among the tests we have in TF-M CI. |
|
Hi Nicola,
I request you to please make the above changes. Thank you. |
nicola-mazzucato-arm
force-pushed
the
fshort
branch
from
52aa654 to
5a7ebdc
Compare
There was a problem hiding this comment.
Pull request overview
Adds a configurable build-time switch for the ARMCLANG toolchain so users can omit -fshort-enums / -fshort-wchar when their secure-side build doesn’t use them, avoiding ABI/link mismatches.
Changes:
- Introduces
NO_FSHORT_FLAGSlogic to conditionally include/exclude-fshort-enumsand-fshort-wcharin ARMCLANGCMAKE_C_FLAGS. - Adds status messages indicating whether the short-type flags are enabled.
- Updates the file header copyright line.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (NO_FSHORT_FLAGS) | ||
| set(FSHORT_C_FLAGS "") | ||
| message(STATUS "Disable -fshort-enums and -fshort-wchar flags") | ||
| else() | ||
| set(FSHORT_C_FLAGS "-fshort-enums -fshort-wchar") | ||
| message(STATUS "Enable -fshort-enums and -fshort-wchar flags") | ||
| endif() |
There was a problem hiding this comment.
NO_FSHORT_FLAGS is treated as a build option, but it’s not declared as a cache variable/option anywhere (no option() / set(... CACHE BOOL ...) in the repo). As a result, it won’t show up in cmake-gui/ccmake and may not be obvious to users how to set it. Consider defining it in this toolchain file (e.g., default OFF as a CACHE BOOL) so it behaves like a real, discoverable build option.
nicola-mazzucato-arm
force-pushed
the
fshort
branch
from
5a7ebdc to
83a46b8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nicola-mazzucato-arm
force-pushed
the
fshort
branch
2 times, most recently
from
d70611b to
4d34742
Compare
The default build flag set for ARMCLANG contains both -fshort-enums -fshort-wchar which can cause linking errors if the secure side is built without them. Add a build option NO_FSHORT_FLAGS so that users can adapt the psa tests with their secure side build options. Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
nicola-mazzucato-arm
force-pushed
the
fshort
branch
from
4d34742 to
50531bf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (NO_FSHORT_FLAGS) | ||
| set(FSHORT_C_FLAGS "") | ||
| message(STATUS "[PSA] : Disable -fshort-enums and -fshort-wchar flags") | ||
| else() | ||
| set(FSHORT_C_FLAGS "-fshort-enums -fshort-wchar") | ||
| message(STATUS "[PSA] : Enable -fshort-enums and -fshort-wchar flags") | ||
| endif() |
There was a problem hiding this comment.
NO_FSHORT_FLAGS is used as a user-facing CMake option here, but it isn't defined as a cache variable anywhere (no option() / set(... CACHE ...) found). That makes the default/intent harder to discover in CMake GUIs and inconsistent with how other build options in this repo are defaulted/validated in api-tests/CMakeLists.txt (e.g. INCLUDE_PANIC_TESTS, WATCHDOG_AVAILABLE). Consider defining NO_FSHORT_FLAGS as a CACHE BOOL with a documented default (0/OFF) and (optionally) validating accepted values alongside the other command-line options.
|
Hi @nicola-mazzucato-arm Best Regards, |
The default build flag set for ARMCLANG contains both -fshort-enums -fshort-wchar which can cause linking errors if the secure side is built without them.
Add a build option NO_FSHORT_FLAGS so that users can adapt the psa tests with their secure side build options.