[BUG] Compiler errors/warnings - always have to manually ignore when building the Kernel with Clang

[phelter]

Describe the bug
When compiling with the CMake options:

add_compile_options(
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-fdiagnostics-color=always>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-fcolor-diagnostics>

    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wall>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wextra>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wpedantic> # Note not used in GNU to allow C99 designated initializers.
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Werror>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Weverything>

    # Allow clang and gnu pramgmas.
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-unknown-pragmas>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-typedef-redefinition>

    # Documentation types
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-fcomment-block-commands=retval> # Doesn't recongize doxygen retval
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-fcomment-block-commands=copydetails> # Doesn't recongize doxygen copydetails
)

There are several isssues/warnings that arise when compling with Clang-14

Target

• Development board: N/A - Linux Posix
• Instruction Set Architecture: arm64
• IDE and version: None - cmake build no IDE.
• Toolchain and version: clang-14.0.0-1ubuntu1

Host

• Host OS: Ubuntu
• Version: 22.04

To Reproduce

• Use project - any and configure with above add_compile_options
• Build on ubuntu and could observe the following :

freertos_kernel-src/list.c:35:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/portable/MemMang/heap_4.c:43:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/portable/MemMang/heap_4.c:432:24: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'BlockLink_t *' (aka 'struct A_BLOCK_LINK *') increases required alignment from 1 to 8 [-Werror,-Wcast-align]
freertos_kernel-src/event_groups.c:35:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/event_groups.c:528:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/stream_buffer.c:36:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/stream_buffer.c:235:21: error: padding struct 'struct StreamBufferDef_t' with 7 bytes to align 'uxStreamBufferNumber' [-Werror,-Wpadded]
freertos_kernel-src/stream_buffer.c:834:13: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/stream_buffer.c:1085:13: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/stream_buffer.c:1223:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/stream_buffer.c:1254:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/timers.c:35:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/timers.c:75:20: error: padding size of 'struct tmrTimerControl' with 7 bytes to alignment boundary [-Werror,-Wpadded]
freertos_kernel-src/timers.c:104:20: error: padding size of 'struct tmrCallbackParameters' with 4 bytes to alignment boundary [-Werror,-Wpadded]
freertos_kernel-src/timers.c:593:5: error: function 'prvTimerTask' could be declared with attribute 'noreturn' [-Werror,-Wmissing-noreturn]
freertos_kernel-src/queue.c:35:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/queue.c:126:34: error: padding struct 'struct QueueDefinition' with 6 bytes to align 'pxQueueSetContainer' [-Werror,-Wpadded]
freertos_kernel-src/queue.c:100:16: error: padding size of 'struct QueueDefinition' with 7 bytes to alignment boundary [-Werror,-Wpadded]
freertos_kernel-src/queue.c:1130:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/queue.c:1301:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/queue.c:1958:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/queue.c:2052:30: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/queue.c:164:41: error: no previous extern declaration for non-static variable 'xQueueRegistry' [-Werror,-Wmissing-variable-declarations]
freertos_kernel-src/tasks.c:36:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/tasks.c:333:51: error: no previous extern declaration for non-static variable 'pxCurrentTCB' [-Werror,-Wmissing-variable-declarations]
freertos_kernel-src/tasks.c:380:28: error: no previous extern declaration for non-static variable 'uxTopUsedPriority' [-Werror,-Wmissing-variable-declarations]
freertos_kernel-src/tasks.c:282:21: error: padding struct 'struct tskTaskControlBlock' with 7 bytes to align 'uxTCBNumber' [-Werror,-Wpadded]
freertos_kernel-src/tasks.c:259:16: error: padding size of 'struct tskTaskControlBlock' with 7 bytes to alignment boundary [-Werror,-Wpadded]
freertos_kernel-src/tasks.c:1504:33: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/tasks.c:1919:34: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/include/task.h:160:17: error: padding struct 'struct xTASK_STATUS' with 4 bytes to align 'uxCurrentPriority' [-Werror,-Wpadded]
freertos_kernel-src/include/task.h:163:19: error: padding struct 'struct xTASK_STATUS' with 4 bytes to align 'pxStackBase' [-Werror,-Wpadded]
freertos_kernel-src/include/task.h:154:16: error: padding size of 'struct xTASK_STATUS' with 4 bytes to alignment boundary [-Werror,-Wpadded]
freertos_kernel-src/tasks.c:3490:1: error: function 'prvIdleTask' could be declared with attribute 'noreturn' [-Werror,-Wmissing-noreturn]
freertos_kernel-src/tasks.c:4565:21: error: default label in switch which covers all enumeration values [-Werror,-Wcovered-switch-default]
freertos_kernel-src/tasks.c:4964:17: error: default label in switch which covers all enumeration values [-Werror,-Wcovered-switch-default]
freertos_kernel-src/tasks.c:5063:34: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/tasks.c:5107:17: error: default label in switch which covers all enumeration values [-Werror,-Wcovered-switch-default]
freertos_kernel-src/tasks.c:5198:34: error: implicit conversion changes signedness: 'long' to 'UBaseType_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
freertos_kernel-src/tasks.c:84:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/tasks.c:83:9: error: macro is not used [-Werror,-Wunused-macros]
freertos_kernel-src/tasks.c:82:9: error: macro is not used [-Werror,-Wunused-macros]

Expected behavior
Should be warning clean

• cast-align - suggest explicitly stating this is okay in heap if it is - otherwise fix. can remove with

#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wcast-align"
#endif
... Your code here
#ifdef __clang__
#pragma clang diagnostic pop
#endif

• covered-switch-default - either allow or fix.
• missing-variable-declarations - fix these
• missing-noreturn - either allow or fix.
• padded - pad the various types so that they are using memory efficiently when used in arrays or lists.
• sign conversion - fix via explicit cast when expected - or fix used types.
• unused macros - remove or if they are only used due to your config - wrap with the config that uses those macros as well.

If you are fine with these then please add them to your definition of the CMakeLists.txt for freertos_kernel via:

target_compile_options(freertos_kernel
  PRIVATE
   # $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-align> # Suggest fixing this one so that other files in here are checked.
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-noreturn>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
)

Screenshots
N/A
Additional context
N/A

[jputcu]

Besides failed builds on FreeBSD with clang, I'm also having failures on Windows with gcc, all due to -Werror.
I Think it is generally advised not to use said compiler option because an upgrade to the compiler could break existing builds due to stricter warnings.

[jputcu]

This is maybe partially fixed by #694

[phelter]

@jputcu

I Think it is generally advised not to use said compiler option because an upgrade to the compiler could break existing builds due to stricter warnings.

I take the stance that you must opt-out of particular checks and not opt-in. That way when a compiler gets better and an upgrade to that compiler adds additional checks, I know that the code I write is passing said checks. All of the code I write for customers comes with a known and pre-defined set of tools. If they decide to upgrade themselves then they only need to ignore those checks in their respective CMakeLists.txt for the targets that fail the new checks until such time they decide to fix them. But at least then they are documented that it was a conscious decision to ignore them and not have the stance of ignorance is bliss .

I am only requesting that FreeRTOS-Kernel either fixes the above issues or makes the conscious decision to ignore them by documenting this decision in their CMakeLists.txt .

[n9wxu]

This is a difficult issue because of the many compilers we are supporting.

1. We try to avoid compiler specific flags in the common code. These clutter the code base and make it more difficult to support. Pragmas are certainly compiler specific.
2. Our rule is -Wall and -Wextra. Adding -Wpedantic is a path to chaos with multiple compilers. Richard Barry's experience with multiple compilers and FreeRTOS over the years is you can't please all the compilers all of the time with these options so the all/extra choice is our compromise.

There are certainly pitfalls with opt-in and opt-out. In either case it is easy for future improvements to be masked by the past choice.

That aside, the issues caught in the report above deserve study.

[phelter]

@n9wxu - I have successfully pleased all of the compilers all of the time. with the method outlined above. Based on the ports you have a limited list of compilers that are supported by FreeRTOS. It would be in your best interest to have at least the big 3 - MSVC, GNU, and Clang - fully supported, and then treat other compilers as build at your own risk. That way majority of the uses - probably higher than 80% will be able to take advantage of not having to deal with masking these out every time for the sake of 20% of the use cases using a different compiler.

Yes I agree it's a preference thing, but cmake allows you to only specify the compilers that you guarantee that level of checks on - see add_compile_options() above which only enables the -Wall -Wextra and -Wpedantic for the Clang and GNU compilers at this moment. I find those two (don't use MSVC much) will ensure that the documentation is correctly defined along with correctness for memory efficiency and speed (eg -Wpadded). Which is very useful.

[aggarg]

I am able to build clean with these suppressions -

    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-align>

I did not need -Wno-missing-noreturn and -Wno-sign-conversion. I am using the latest code from main. Which version are you using? Would you please share the lines that produce these 2 warnings?

[phelter]

I was using:
GIT_REPOSITORY https://github.com/phelter/FreeRTOS-Kernel.git
GIT_TAG 1169b3b

With clang-14 - if you use an older version of clang then you might not see those warnings.

[aggarg]

Thanks I was using clang-14 only. Here is the PR to address these issues - #711

With these changes I am able to build clean.