Utilize latest GCC/clang's capability in CI

[PengZheng]

GCC is continuing to gain greater static code analysis capabilities these days:
https://developers.redhat.com/articles/2023/05/31/improvements-static-analysis-gcc-13-compiler#

It will be beneficial to add -fanalyzer to the build system.
After eliminating all warnings, we can turn it on in our CI.

Following Rust's trend, I expect more compiler work like this (https://lwn.net/SubscriberLink/936728/382fa9e0e0e2b2ed/) to come.

[moelksasbyahmed]

hello @PengZheng ,
this issue seems to be quite old to be honest
is this issue still available and if it is can you please guide me for what should i do as a beginner as this is my first issue to celix and probably the Apache foundation

[PengZheng]

@moelksasbyahmed I have not explored -fanalyzer for its usefulness yet. You are welcome to try it with Celix and share with us what you've found interesting.

[moelksasbyahmed]

hello @PengZheng its been a while
i have been exploring celix and understand what is bundle and container i had a rough time building with conan i didnt even know what is conan (it turns out to be cpp , c dependency and package manager which is awesome to know that ) and after all i build celix with CMake i was reading in the red hat blog about -fanalyzer and why its important (although not every warning i understand in the article )
also i saw gcc docs for -fanalyzer Options That Control Static Analysis

adding -fanalyzer for local CMake build

i added to CelixProject.cmake

option (ENABLE_GCC_ANALYZER "Enable building with GCC static analyzer." OFF )
if (ENABLE_GCC_ANALYZER)  
    if("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU")  
        set(CMAKE_C_FLAGS "-fanalyzer ${CMAKE_C_FLAGS}")  
        set(CMAKE_CXX_FLAGS "-fanalyzer ${CMAKE_CXX_FLAGS}")  
    else()  
        message(WARNING "ENABLE_GCC_ANALYZER is only supported with GCC compiler 10.0.0 OR higher.")  
    endif()  
endif()

Building

mkdir build && cd build 
cmake  .. -DENABLE_GCC_ANALYZER=ON
make 

Result

-fanalyzer check every path of code to return a warning of multiple of things like possible null reference etc
with -Werror flag in cmake it treated every warning as an error and stops the building

the output

make verbose=1
[  1%] Built target tlib
[  1%] Building C object libs/utils/CMakeFiles/utils.dir/src/hash_map.c.o
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMap_create’:
/home/mohamed/celix/libs/utils/src/hash_map.c:73:19: error: dereference of possibly-NULL ‘map’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
   73 |     map->treshold = (unsigned int) (DEFAULT_INITIAL_CAPACITY * DEFAULT_LOAD_FACTOR);
      |     ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘hashMap_create’: events 1-2
    |
    |   72 |     hash_map_pt map = (hash_map_pt) malloc(sizeof(*map));
    |      |                                     ^~~~~~~~~~~~~~~~~~~~
    |      |                                     |
    |      |                                     (1) this call could return NULL
    |   73 |     map->treshold = (unsigned int) (DEFAULT_INITIAL_CAPACITY * DEFAULT_LOAD_FACTOR);
    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                   |
    |      |                   (2) ‘map’ could be NULL: unchecked value from (1)
    |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMap_addEntry’:
/home/mohamed/celix/libs/utils/src/hash_map.c:337:15: error: dereference of possibly-NULL ‘new’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  337 |     new->hash = hash;
      |     ~~~~~~~~~~^~~~~~
  ‘hashMap_addEntry’: events 1-2
    |
    |  336 |     hash_map_entry_pt new = (hash_map_entry_pt) malloc(sizeof(*new));
    |      |                                                 ^~~~~~~~~~~~~~~~~~~~
    |      |                                                 |
    |      |                                                 (1) this call could return NULL
    |  337 |     new->hash = hash;
    |      |     ~~~~~~~~~~~~~~~~                             
    |      |               |
    |      |               (2) ‘new’ could be NULL: unchecked value from (1)
    |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMapIterator_init’:
/home/mohamed/celix/libs/utils/src/hash_map.c:369:19: error: dereference of possibly-NULL ‘iterator’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  369 |     iterator->map = map;
      |     ~~~~~~~~~~~~~~^~~~~
  ‘hashMapIterator_create’: events 1-2
    |
    |  355 | hash_map_iterator_pt hashMapIterator_create(hash_map_pt map) {
    |      |                      ^~~~~~~~~~~~~~~~~~~~~~
    |      |                      |
    |      |                      (1) entry to ‘hashMapIterator_create’
    |  356 |     hash_map_iterator_pt iterator = hashMapIterator_alloc();
    |      |                                     ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                     |
    |      |                                     (2) calling ‘hashMapIterator_alloc’ from ‘hashMapIterator_create’
    |
    +--> ‘hashMapIterator_alloc’: events 3-4
           |
           |  347 | hash_map_iterator_pt hashMapIterator_alloc(void) {
           |      |                      ^~~~~~~~~~~~~~~~~~~~~
           |      |                      |
           |      |                      (3) entry to ‘hashMapIterator_alloc’
           |  348 |     return calloc(1, sizeof(hash_map_iterator_t));
           |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |            |
           |      |            (4) this call could return NULL
           |
    <------+
    |
  ‘hashMapIterator_create’: events 5-6
    |
    |  356 |     hash_map_iterator_pt iterator = hashMapIterator_alloc();
    |      |                                     ^~~~~~~~~~~~~~~~~~~~~~~
    |      |                                     |
    |      |                                     (5) possible return of NULL to ‘hashMapIterator_create’ from ‘hashMapIterator_alloc’
    |  357 |     hashMapIterator_init(map, iterator);
    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |     |
    |      |     (6) calling ‘hashMapIterator_init’ from ‘hashMapIterator_create’
    |
    +--> ‘hashMapIterator_init’: events 7-8
           |
           |  368 | void hashMapIterator_init(hash_map_pt map, hash_map_iterator_pt iterator) {
           |      |      ^~~~~~~~~~~~~~~~~~~~
           |      |      |
           |      |      (7) entry to ‘hashMapIterator_init’
           |  369 |     iterator->map = map;
           |      |     ~~~~~~~~~~~~~~~~~~~
           |      |                   |
           |      |                   (8) ‘iterator’ could be NULL: unchecked value from (4)
           |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMapKeySet_create’:
/home/mohamed/celix/libs/utils/src/hash_map.c:466:17: error: dereference of possibly-NULL ‘keySet’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  466 |     keySet->map = map;
      |     ~~~~~~~~~~~~^~~~~
  ‘hashMapKeySet_create’: events 1-2
    |
    |  465 |     hash_map_key_set_pt keySet = (hash_map_key_set_pt) malloc(sizeof(*keySet));
    |      |                                                        ^~~~~~~~~~~~~~~~~~~~~~~
    |      |                                                        |
    |      |                                                        (1) this call could return NULL
    |  466 |     keySet->map = map;
    |      |     ~~~~~~~~~~~~~~~~~                                   
    |      |                 |
    |      |                 (2) ‘keySet’ could be NULL: unchecked value from (1)
    |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMapValues_create’:
/home/mohamed/celix/libs/utils/src/hash_map.c:501:17: error: dereference of possibly-NULL ‘values’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  501 |     values->map = map;
      |     ~~~~~~~~~~~~^~~~~
  ‘hashMapValues_create’: events 1-2
    |
    |  500 |     hash_map_values_pt values = (hash_map_values_pt) malloc(sizeof(*values));
    |      |                                                      ^~~~~~~~~~~~~~~~~~~~~~~
    |      |                                                      |
    |      |                                                      (1) this call could return NULL
    |  501 |     values->map = map;
    |      |     ~~~~~~~~~~~~~~~~~                                 
    |      |                 |
    |      |                 (2) ‘values’ could be NULL: unchecked value from (1)
    |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMapValues_toArray’:
/home/mohamed/celix/libs/utils/src/hash_map.c:531:23: error: dereference of possibly-NULL ‘*array’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  531 |         (*array)[i++] = hashMapIterator_nextValue(it);
      |         ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘hashMapValues_toArray’: events 1-3
    |
    |  523 | void hashMapValues_toArray(hash_map_values_pt values, void* *array[], unsigned int *size) {
    |      |      ^~~~~~~~~~~~~~~~~~~~~
    |      |      |
    |      |      (1) entry to ‘hashMapValues_toArray’
    |......
    |  528 |     *array = malloc(vsize * sizeof(**array));
    |      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |              |
    |      |              (2) this call could return NULL
    |  529 |     it = hashMapValues_iterator(values);
    |      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |          |
    |      |          (3) calling ‘hashMapValues_iterator’ from ‘hashMapValues_toArray’
    |
    +--> ‘hashMapValues_iterator’: events 4-5
           |
           |  511 | hash_map_iterator_pt hashMapValues_iterator(hash_map_values_pt values) {
           |      |                      ^~~~~~~~~~~~~~~~~~~~~~
           |      |                      |
           |      |                      (4) entry to ‘hashMapValues_iterator’
           |  512 |     return hashMapIterator_create(values->map);
           |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |            |
           |      |            (5) calling ‘hashMapIterator_create’ from ‘hashMapValues_iterator’
           |
           +--> ‘hashMapIterator_create’: events 6-7
                  |
                  |  355 | hash_map_iterator_pt hashMapIterator_create(hash_map_pt map) {
                  |      |                      ^~~~~~~~~~~~~~~~~~~~~~
                  |      |                      |
                  |      |                      (6) entry to ‘hashMapIterator_create’
                  |  356 |     hash_map_iterator_pt iterator = hashMapIterator_alloc();
                  |  357 |     hashMapIterator_init(map, iterator);
                  |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |     |
                  |      |     (7) calling ‘hashMapIterator_init’ from ‘hashMapIterator_create’
                  |
                  +--> ‘hashMapIterator_init’: events 8-9
                         |
                         |  368 | void hashMapIterator_init(hash_map_pt map, hash_map_iterator_pt iterator) {
                         |      |      ^~~~~~~~~~~~~~~~~~~~
                         |      |      |
                         |      |      (8) entry to ‘hashMapIterator_init’
                         |......
                         |  374 |     if (map->size > 0) {
                         |      |        ~
                         |      |        |
                         |      |        (9) following ‘true’ branch...
                         |
                       ‘hashMapIterator_init’: event 10
                         |
                         |cc1:
                         | (10): ...to here
                         |
                       ‘hashMapIterator_init’: event 11
                         |
                         |cc1:
                         | (11): calling ‘hashMapIterator_init.part.0’ from ‘hashMapIterator_init’
                         |
                         +--> ‘hashMapIterator_init.part.0’: event 12
                                |
                                |  368 | void hashMapIterator_init(hash_map_pt map, hash_map_iterator_pt iterator) {
                                |      |      ^~~~~~~~~~~~~~~~~~~~
                                |      |      |
                                |      |      (12) entry to ‘hashMapIterator_init.part.0’
                                |
                              ‘hashMapIterator_init.part.0’: events 13-14
                                |
                                |  375 |         while (iterator->index < map->tablelength && (iterator->next = map->table[iterator->index++]) == NULL) {
                                |      |                                                   ^                    ~~~~~~~~~~
                                |      |                                                   |                       |
                                |      |                                                   |                       (14) ...to here
                                |      |                                                   (13) following ‘true’ branch...
                                |
                         <------+
                         |
                       ‘hashMapIterator_init’: event 15
                         |
                         |cc1:
                         | (15): returning to ‘hashMapIterator_init’ from ‘hashMapIterator_init.part.0’
                         |
                  <------+
                  |
                ‘hashMapIterator_create’: event 16
                  |
                  |  357 |     hashMapIterator_init(map, iterator);
                  |      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |     |
                  |      |     (16) returning to ‘hashMapIterator_create’ from ‘hashMapIterator_init’
                  |
           <------+
           |
         ‘hashMapValues_iterator’: event 17
           |
           |  512 |     return hashMapIterator_create(values->map);
           |      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |            |
           |      |            (17) returning to ‘hashMapValues_iterator’ from ‘hashMapIterator_create’
           |
    <------+
    |
  ‘hashMapValues_toArray’: events 18-21
    |
    |  529 |     it = hashMapValues_iterator(values);
    |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |          |
    |      |          (18) returning to ‘hashMapValues_toArray’ from ‘hashMapValues_iterator’
    |  530 |     while(hashMapIterator_hasNext(it) && i<vsize){
    |      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                       |
    |      |                                       (19) following ‘true’ branch...
    |  531 |         (*array)[i++] = hashMapIterator_nextValue(it);
    |      |         ~~~~~~~~        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |          |              |
    |      |          |              (21) calling ‘hashMapIterator_nextValue’ from ‘hashMapValues_toArray’
    |      |          (20) ...to here
    |
    +--> ‘hashMapIterator_nextValue’: events 22-26
           |
           |  413 | void * hashMapIterator_nextValue(hash_map_iterator_pt iterator) {
           |      |        ^~~~~~~~~~~~~~~~~~~~~~~~~
           |      |        |
           |      |        (22) entry to ‘hashMapIterator_nextValue’
           |  414 |     hash_map_entry_pt entry;
           |  415 |     if (iterator->expectedModCount != iterator->map->modificationCount) {
           |      |        ~
           |      |        |
           |      |        (23) following ‘false’ branch...
           |......
           |  418 |     entry = iterator->next;
           |      |     ~~~~~~~~~~~~~~~~~~~~~~
           |      |           |
           |      |           (24) ...to here
           |  419 |     if (entry == NULL) {
           |      |        ~
           |      |        |
           |      |        (25) following ‘false’ branch (when ‘entry’ is non-NULL)...
           |......
           |  422 |     if ((iterator->next = entry->next) == NULL) {
           |      |                           ~~~~~~~~~~~
           |      |                                |
           |      |                                (26) ...to here
           |
    <------+
    |
  ‘hashMapValues_toArray’: events 27-28
    |
    |  531 |         (*array)[i++] = hashMapIterator_nextValue(it);
    |      |         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                       | |
    |      |                       | (27) returning to ‘hashMapValues_toArray’ from ‘hashMapIterator_nextValue’
    |      |                       (28) ‘*array + (long unsigned int)i * 8’ could be NULL: unchecked value from (2)
    |
/home/mohamed/celix/libs/utils/src/hash_map.c: In function ‘hashMapEntrySet_create’:
/home/mohamed/celix/libs/utils/src/hash_map.c:569:19: error: dereference of possibly-NULL ‘entrySet’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
  569 |     entrySet->map = map;
      |     ~~~~~~~~~~~~~~^~~~~
  ‘hashMapEntrySet_create’: events 1-2
    |
    |  568 |     hash_map_entry_set_pt entrySet = (hash_map_entry_set_pt) malloc(sizeof(*entrySet));
    |      |                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                                              |
    |      |                                                              (1) this call could return NULL
    |  569 |     entrySet->map = map;
    |      |     ~~~~~~~~~~~~~~~~~~~                                       
    |      |                   |
    |      |                   (2) ‘entrySet’ could be NULL: unchecked value from (1)
    |
cc1: all warnings being treated as errors
make[2]: *** [libs/utils/CMakeFiles/utils.dir/build.make:90: libs/utils/CMakeFiles/utils.dir/src/hash_map.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2175: libs/utils/CMakeFiles/utils.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

now what should i do next ? should i try to resolve these warnings before adding -fanalyzer to the CI workflow ?

[PengZheng]

now what should i do next ? should i try to resolve these warnings before adding -fanalyzer to the CI workflow?

I suggest sending us a PR adding support for the GCC analyzer option. Note that conanfile.py should also be taken care of.

Eliminating these warnings requires some familiarity with the code base.
But fortunately it can be done incrementally afterwards.

[moelksasbyahmed]

done that check #817