Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing clang and gnu compiler warnings. #1

Open
wants to merge 60 commits into
base: feature/cmake-updates
Choose a base branch
from

Conversation

phelter
Copy link
Owner

@phelter phelter commented Oct 13, 2022

Fixing majority of the clang compiler warnings.

Description

Fixed majority of clang and gnu compiler warnings. Primary ones are:

  • misuse of doxygen single line comments
  • replaced wrong type usage for Interrupt return and clear interrupt macros.

List of compiler warnings goes from:

target_compile_options( freertos_kernel
  PRIVATE
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-align>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-extra-semi-stmt>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-int-to-pointer-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-noreturn>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pointer-to-int-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-shorten-64-to-32>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-unused-variable>
)
# For linux port
target_compile_options( freertos_kernel_port
  PRIVATE
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-disabled-macro-expansion>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-noreturn>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pointer-to-int-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-reserved-macro-identifier>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-type-limits>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
)

To:

target_compile_options( freertos_kernel
  PRIVATE
    $<$<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-padded>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
)
target_compile_options( freertos_kernel_port
  PRIVATE
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-disabled-macro-expansion>
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-reserved-macro-identifier>
)

Test Steps

Compile with:

add_compile_options(
    ### C Based Language and ID.
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wall>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wextra>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wpedantic>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Werror>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Weverything>
)

Related Issue

Tried to have this list in the CMakeLists.txt originally in PR FreeRTOS#571, but was overruled, so reducing list for future checks.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@phelter phelter force-pushed the feature/fixing-clang-gnu-compiler-warnings branch from 6b9bbe6 to 5b7045c Compare November 4, 2022 23:30
aggarg and others added 16 commits November 8, 2022 14:05
* Fix context switch when time slicing is off

When time slicing is off, context switch should only happen when a
task with priority higher than the currently executing one is unblocked.
Earlier the code was invoking a context switch even when a task with
priority equal the currently executing task was unblocked. This commit
fixes the code to only do a context switch when a higher priority
task is unblocked.

Signed-off-by: Gaurav Aggarwal <[email protected]>
…M_CR5 port (FreeRTOS#584)

* Add support for the configUSE_TASK_FPU_SUPPORT in the GCC/ARM_CR5 port

This is done almost identically as in the GCC/ARM_CA9 port

* Adjust task stack initialitation of the GCC/ARM_CR5 port

Ensure that the task stack initialization is done correctly for the
different options of configUSE_TASK_FPU_SUPPORT.

This is very similar to the GCC/ARM_CA9 port. The only meaningful
difference is, that the FPU of the Cortex-R5 has just sixteen 64-bit
floating point registers as it implements the VFPv3-D16 architecture.
You may also refer to the ARM documentation

* Add support for FPU safe interrupts to the GCC/ARM_CR5 port

Similar to GCC/ARM_CA9 port

* Clarify comment about the size of the FPU registers of Cortex R5
* Updating to version v10.5.1

Co-authored-by: Soren Ptak <[email protected]>
…efault PORT selection for native POSIX and MINGW platforms.
paulbartell and others added 30 commits November 29, 2022 15:38
Fix EOL of .gitattributes
The .git-blame-ignore-revs allows easy filtering out large commits
from calls to git blame.

This can be configured frome the git command line via the following:
git config blame.ignoreRevsFile .git-blame-ignore-revs
listGET_OWNER_OF_NEXT_ENTRY computes `( pxConstList )->pxIndex->pxNext` after
verifying that `( pxConstList )->pxIndex` points to `xListEnd`, which due to
being a MiniListItem_t, can be shorter than a ListItem_t. Thus,
`( pxConstList )->pxIndex` is a `ListItem_t *` that extends past the end of the
`List_t` whose `xListEnd` it points to. This is fixed by accessing `pxNext`
through a `MiniListItem_t` instead.
* vTaskResume and vTaskPrioritySet don't preempt equal priority task

* Update vTaskResumeAll not to preempt task with equal priority

* Fix in xTaskResumeFromISR
This is needed to be compatible with the refactoring done in this
PR - FreeRTOS/FreeRTOS#889

Signed-off-by: Gaurav Aggarwal <[email protected]>

Signed-off-by: Gaurav Aggarwal <[email protected]>
Allow ulTaskGetIdleRunTimeCounter and ulTaskGetIdleRunTimePercent to be
used whenever configGENERATE_RUN_TIME_STATS is enabled, as this is the
only requirement for these functions to work.
The quick start instructions for CMake mention the "master"
git branch which has been replaced by "main" in the current
repo.

The main CMakeLists.txt documents how to integrate a
custom port. Fix a typo in the suggested CMake code.
* Added support of 64bit even

Signed-off-by: Cervenka Dusan <[email protected]>

* Added missing brackets

Signed-off-by: Cervenka Dusan <[email protected]>

* Made proper name for tick macro.

Signed-off-by: Cervenka Dusan <[email protected]>

* Improved macro evaluation

Signed-off-by: Cervenka Dusan <[email protected]>

* Fixed missed port files  + documentation

Signed-off-by: Cervenka Dusan <[email protected]>

* Changes made on PR

Signed-off-by: Cervenka Dusan <[email protected]>

* Fix macro definition.

Signed-off-by: Cervenka Dusan <[email protected]>

* Formatted code with uncrustify

Signed-off-by: Cervenka Dusan <[email protected]>

---------

Signed-off-by: Cervenka Dusan <[email protected]>
The introduction of `portMEMORY_BARRIER` will ensure
the places in the kernel use a barrier will work.
For example, `xTaskResumeAll` has a memory barrier
to ensure its correctness when compiled with optimization
enabled. Without the barrier `xTaskResumeAll` can fail
(e.g. start reading and writing to address 0 and/or
infinite looping) when `xPendingReadyList` contains more
than one task to restore.

In `xTaskResumeAll` the compiler chooses to cache the
`pxTCB` the first time through the loop for use
in every subsequent loop. This is incorrect as the
removal of `pxTCB->xEventListItem` will actually
change the value of `pxTCB` if it was read again
at the top of the loop. The barrier forces the compiler
to read `pxTCB` again at the top of the loop.

The compiler is operating correctly. The removal
`pxTCB->xEventListItem` executes on a `List_t *`
and `ListItem_t *`.  This means that the compiler
can assume that any `MiniListItem_t` values are
unchanged by the loop (i.e. "strict-aliasing").
This allows the compiler to cache `pxTCB` as it
is obtained via a `MiniListItem_t`. This is incorrect
in this case because it is possible for a `ListItem_t *`
to actually alias a `MiniListItem_t`. This is technically
a "violation of aliasing rules" so we use the the barrier
to disable the strict-aliasing optimization in this loop.
…g-tidy instead if you want this level of control.
…TOS#624)

* make port exitable

* correctly set xPortRunning to False

* add suggestions from Review

Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

* add suggestions from Review

Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

---------

Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>
The PR FreeRTOS#597 introduced a new config option configTICK_TYPE_WIDTH_IN_BITS
which can be defined to one of the following:
* TICK_TYPE_WIDTH_16_BITS - Tick type is 16 bit wide.
* TICK_TYPE_WIDTH_32_BITS - Tick type is 32 bit wide.
* TICK_TYPE_WIDTH_64_BITS - Tick type is 64 bit wide.

Earlier we supported 16 and 32 bit width for tick type which was
controlled using the config option configUSE_16_BIT_TICKS. The PR
tried to maintain backward compatibility by honoring
configUSE_16_BIT_TICKS. The backward compatibility did not work as
expected though, as the macro configTICK_TYPE_WIDTH_IN_BITS was used
before it was defined. This PR addresses it by ensuring that the macro
configTICK_TYPE_WIDTH_IN_BITS is defined before it is used.

Testing
1. configUSE_16_BIT_TICKS is defined to 0.

Source (function xTaskIncrementTick in tasks.c):
```
const TickType_t xConstTickCount = xTickCount + ( TickType_t ) 1;
```

Assembly:
```
109e:       4b50            ldr     r3, [pc, FreeRTOS#320]  ; (11e0 <xTaskIncrementTick+0x150>)
10a0:       f8d3 4134       ldr.w   r4, [r3, FreeRTOS#308]  ; 0x134
10a4:       3401            adds    r4, #1
10a6:       f8c3 4134       str.w   r4, [r3, FreeRTOS#308]  ; 0x134
```

It is clear from assembly that the tick type is 32 bit.

2. configUSE_16_BIT_TICKS is defined to 1.

Source (function xTaskIncrementTick in tasks.c):
```
const TickType_t xConstTickCount = xTickCount + ( TickType_t ) 1;
```

Assembly:
```
10e2:       4b53            ldr     r3, [pc, FreeRTOS#332]  ; (1230 <xTaskIncrementTick+0x15c>)
10e4:       f8b3 4134       ldrh.w  r4, [r3, FreeRTOS#308]  ; 0x134
10e8:       b2a4            uxth    r4, r4
10ea:       3401            adds    r4, #1
10ec:       b2a4            uxth    r4, r4
10ee:       f8a3 4134       strh.w  r4, [r3, FreeRTOS#308]  ; 0x134
```

It is clear from assembly that the tick type is 16 bit.

3. configTICK_TYPE_WIDTH_IN_BITS is defined to TICK_TYPE_WIDTH_16_BITS.

Source (function xTaskIncrementTick in tasks.c):
```
const TickType_t xConstTickCount = xTickCount + ( TickType_t ) 1;
```

Assembly:
```
10e2:       4b53            ldr     r3, [pc, FreeRTOS#332]  ; (1230 <xTaskIncrementTick+0x15c>)
10e4:       f8b3 4134       ldrh.w  r4, [r3, FreeRTOS#308]  ; 0x134
10e8:       b2a4            uxth    r4, r4
10ea:       3401            adds    r4, #1
10ec:       b2a4            uxth    r4, r4
10ee:       f8a3 4134       strh.w  r4, [r3, FreeRTOS#308]  ; 0x134
```

It is clear from assembly that the tick type is 16 bit.

4. configTICK_TYPE_WIDTH_IN_BITS is defined to TICK_TYPE_WIDTH_32_BITS.

Source (function xTaskIncrementTick in tasks.c):
```
const TickType_t xConstTickCount = xTickCount + ( TickType_t ) 1;
```

Assembly:
```
109e:       4b50            ldr     r3, [pc, FreeRTOS#320]  ; (11e0 <xTaskIncrementTick+0x150>)
10a0:       f8d3 4134       ldr.w   r4, [r3, FreeRTOS#308]  ; 0x134
10a4:       3401            adds    r4, #1
10a6:       f8c3 4134       str.w   r4, [r3, FreeRTOS#308]  ; 0x134
```

It is clear from assembly that the tick type is 32 bit.

5. configTICK_TYPE_WIDTH_IN_BITS is defined to TICK_TYPE_WIDTH_64_BITS.

```
 #error configTICK_TYPE_WIDTH_IN_BITS set to unsupported tick type width.
```

The testing was done for GCC/ARM_CM3 port which does not support 64 bit
tick type.

6. Neither configUSE_16_BIT_TICKS nor configTICK_TYPE_WIDTH_IN_BITS
defined.

```
 #error Missing definition:  One of configUSE_16_BIT_TICKS and
 configTICK_TYPE_WIDTH_IN_BITS must be defined in FreeRTOSConfig.h.
 See the Configuration section of the FreeRTOS API documentation for
 details.
```

7. Both configUSE_16_BIT_TICKS and configTICK_TYPE_WIDTH_IN_BITS defined.

```
 #error Only one of configUSE_16_BIT_TICKS and
 configTICK_TYPE_WIDTH_IN_BITS must be defined in FreeRTOSConfig.h.
 See the Configuration section of the FreeRTOS API documentation for
 details.
```

Related issue - FreeRTOS#628

Signed-off-by: Gaurav Aggarwal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.