search

nasa / CryptoLib

Provide a software-only solution using the CCSDS Space Data Link Security Protocol - Extended Procedures (SDLS-EP) to secure communications between a spacecraft running the core Flight System (cFS) and a ground station.
Other
66 stars 25 forks source link

Address comparison warnings (-> errors) when compiling with GCC 13.2 #246

Closed Niautanor closed 3 weeks ago

Niautanor commented 1 month ago

I get the following warnings (which escalate to errors due to -Werror) when building with GCC 13.2

[1/3] Building C object src/CMakeFiles/crypto.dir/core/crypto_mc.c.o
FAILED: src/CMakeFiles/crypto.dir/core/crypto_mc.c.o 
/usr/bin/cc -Dcrypto_EXPORTS -I/home/luna/git/CryptoLib/include -I/home/luna/git/CryptoLib/src/../include -Wall -Wextra -Werror -g -O0 -g -fPIC -MD -MT src/CMakeFiles/crypto.dir/core/crypto_mc.c.o -MF src/CMakeFiles/crypto.dir/core/crypto_mc.c.o.d -o src/CMakeFiles/crypto.dir/core/crypto_mc.c.o -c /home/luna/git/CryptoLib/src/core/crypto_mc.c
/home/luna/git/CryptoLib/src/core/crypto_mc.c: In function ‘Crypto_SA_readARSN’:
/home/luna/git/CryptoLib/src/core/crypto_mc.c:212:27: error: the comparison will always evaluate as ‘false’ for the address of ‘iv’ will never be NULL [-Werror=address]
  212 |             if(sa_ptr->iv == NULL)
      |                           ^~
In file included from /home/luna/git/CryptoLib/include/crypto_print.h:26,
                 from /home/luna/git/CryptoLib/include/crypto.h:38,
                 from /home/luna/git/CryptoLib/src/core/crypto_mc.c:22:
/home/luna/git/CryptoLib/include/crypto_structs.h:71:13: note: ‘iv’ declared here
   71 |     uint8_t iv[IV_SIZE];    // Initialization Vector
      |             ^~
cc1: all warnings being treated as errors
[2/3] Building C object src/CMakeFiles/crypto.dir/sa/internal/sa_interface_inmemory.template.c.o
FAILED: src/CMakeFiles/crypto.dir/sa/internal/sa_interface_inmemory.template.c.o 
/usr/bin/cc -Dcrypto_EXPORTS -I/home/luna/git/CryptoLib/include -I/home/luna/git/CryptoLib/src/../include -Wall -Wextra -Werror -g -O0 -g -fPIC -MD -MT src/CMakeFiles/crypto.dir/sa/internal/sa_interface_inmemory.template.c.o -MF src/CMakeFiles/crypto.dir/sa/internal/sa_interface_inmemory.template.c.o.d -o src/CMakeFiles/crypto.dir/sa/internal/sa_interface_inmemory.template.c.o -c /home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c: In function ‘sa_get_from_spi’:
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:438:12: error: the comparison will always evaluate as ‘false’ for the address of ‘sa’ will never be NULL [-Werror=address]
  438 |     if (sa == NULL)
      |            ^~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:41:30: note: ‘sa’ declared here
   41 | static SecurityAssociation_t sa[NUM_SA];
      |                              ^~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:443:20: error: the comparison will always evaluate as ‘false’ for the address of ‘iv’ will never be NULL [-Werror=address]
  443 |     if (sa[spi].iv == NULL && (sa[spi].shivf_len > 0) && crypto_config.cryptography_type != CRYPTOGRAPHY_TYPE_KMCCRYPTO)
      |                    ^~
In file included from /home/luna/git/CryptoLib/include/crypto_print.h:26,
                 from /home/luna/git/CryptoLib/include/crypto.h:38,
                 from /home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:15:
/home/luna/git/CryptoLib/include/crypto_structs.h:71:13: note: ‘iv’ declared here
   71 |     uint8_t iv[IV_SIZE];    // Initialization Vector
      |             ^~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:447:21: error: the comparison will always evaluate as ‘false’ for the address of ‘abm’ will never be NULL [-Werror=address]
  447 |     if (sa[spi].abm == NULL && sa[spi].ast)
      |                     ^~
/home/luna/git/CryptoLib/include/crypto_structs.h:76:13: note: ‘abm’ declared here
   76 |     uint8_t abm[ABM_SIZE];  // Authentication Bit Mask (Primary Hdr. through Security Hdr.)
      |             ^~~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c: In function ‘sa_get_operational_sa_from_gvcid_find_iv’:
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:476:26: error: the comparison will always evaluate as ‘false’ for the address of ‘iv’ will never be NULL [-Werror=address]
  476 |             if (sa[i].iv == NULL && (sa[i].ast == 1 || sa[i].est == 1) && crypto_config.cryptography_type != CRYPTOGRAPHY_TYPE_KMCCRYPTO)
      |                          ^~
/home/luna/git/CryptoLib/include/crypto_structs.h:71:13: note: ‘iv’ declared here
   71 |     uint8_t iv[IV_SIZE];    // Initialization Vector
      |             ^~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:482:27: error: the comparison will always evaluate as ‘false’ for the address of ‘abm’ will never be NULL [-Werror=address]
  482 |             if (sa[i].abm == NULL && sa[i].ast)
      |                           ^~
/home/luna/git/CryptoLib/include/crypto_structs.h:76:13: note: ‘abm’ declared here
   76 |     uint8_t abm[ABM_SIZE];  // Authentication Bit Mask (Primary Hdr. through Security Hdr.)
      |             ^~~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c: In function ‘sa_get_operational_sa_from_gvcid’:
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:663:12: error: the comparison will always evaluate as ‘false’ for the address of ‘sa’ will never be NULL [-Werror=address]
  663 |     if (sa == NULL)
      |            ^~
/home/luna/git/CryptoLib/src/sa/internal/sa_interface_inmemory.template.c:41:30: note: ‘sa’ declared here
   41 | static SecurityAssociation_t sa[NUM_SA];
      |                              ^~
cc1: all warnings being treated as errors

GCC is saying "Since sa is a statically allocated array the standard says that it's address is and always will be different from NULL.". I initially assumed these checks were added following a certain coding standard that requires all pointer accesses to be checked but e.g. key_interface_internal_template.c doesn't have them either so maybe they were intended to be meaningful (or are remnants from an old version where they were meaningful).

Please have a look if these checks still have some reason for being there that is currently unfulfilled (i.e. the intention of the if (sa) seems to be to check that sa_init() was called: maybe sa_init() should set some variable to indicate that that could be checked instead) or if they can be removed and I'll be happy to provide a merge request.

rjbrown2 commented 3 weeks ago

Thank you for bringing this to our attention. Many of these are no longer necessary. They are in the process of being removed.