stevengj / nlopt

library for nonlinear optimization, wrapping many algorithms for global and local, constrained or unconstrained, optimization
Other
1.81k stars 562 forks source link

Enabling some compiler warnings produce many that could be fixed #519

Open Jaaaky opened 1 year ago

Jaaaky commented 1 year ago

I'm trying to port your library, enabling compiler warnings produces many that could be fixed and actually I think fixed them would enhance the quality of the lib too much.

Here are most of them

../../libraries/nlopt/plis.c: In function 'plis_':
../../libraries/nlopt/plis.c:189:5: error: floating constant exceeds range of 'float' [-Werror=overflow]
  189 |     eta9 = 1e120;
      |     ^~~~
compilation terminated due to -Wfatal-errors.
cc1: some warnings being treated as errors

../../libraries/nlopt/newuoa.c: In function 'trsapp_':
../../libraries/nlopt/newuoa.c:245:12: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
  245 |     if (dd == zero) {
      |            ^~
../../libraries/nlopt/newuoa.c:406:24: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
  406 |     if ((double) isave == zero) {
      |                        ^~
../../libraries/nlopt/newuoa.c:413:15: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
  413 |     if (tempa != tempb) {
      |               ^~
../../libraries/nlopt/newuoa.c: In function 'bigden_':
../../libraries/nlopt/newuoa.c:643:14: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
  643 |   if (sstemp == 0) return NLOPT_ROUNDOFF_LIMITED;
      |              ^~
../../libraries/nlopt/newuoa.c:897:15: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
  897 |     if (tempa != tempb) {
      |               ^~
../../libraries/nlopt/newuoa.c: In function 'biglag_':
../../libraries/nlopt/newuoa.c:1362:15: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
 1362 |     if (tempa != tempb) {
      |               ^~
../../libraries/nlopt/newuoa.c: In function 'update_':
../../libraries/nlopt/newuoa.c:1443:41: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
 1443 |  } else if (zmat[*knew + j * zmat_dim1] != zero) {
      |                                         ^~
../../libraries/nlopt/newuoa.c: In function 'newuob_':
../../libraries/nlopt/newuoa.c:2090:11: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
 2090 |  if (d__1 == 0) { rc = NLOPT_ROUNDOFF_LIMITED; goto L530; }
      |           ^~
../../libraries/nlopt/newuoa.c:2325:29: warning: comparing floating-point with '==' or '!=' is unsafe [-Wfloat-equal]
 2325 |     if (ksave == 0 && delta == rho) {
      |                             ^~
Jaaaky commented 1 year ago

Here are some compiler flags to enable to show dozen of possible problems:

 -Wall -Wextra  -Werror=overflow -Wtype-limits -Woverflow -Wunused-value -Wmaybe-uninitialized -Werror=type-limits -Werror=maybe-uninitialized -Wfloat-equal -Wmissing-declarations -Werror=deprecated-declarations -Wincompatible-pointer-types -Wint-conversion

Solving those can help avoid dozen of bugs and undefined behaviors

Jaaaky commented 1 year ago

Some more warning that could be fixed, but needs logic checking that I could not do

ccsa_quadratic.c: In function 'ccsa_quadratic_minimize':
ccsa_quadratic.c:365:4: warning: floating constant exceeds range of 'float' [-Woverflow]
  365 |    for (i = 0; i < m; ++i) dual_ub[i] = 1e40;
      |    ^~~

plip.c: In function 'plip_':
plip.c:204:5: warning: floating constant exceeds range of 'float' [-Woverflow]
  204 |     eta9 = 1e120;
      |     ^~~~

pnet.c: In function 'pnet_':
pnet.c:224:5: warning: floating constant exceeds range of 'float' [-Woverflow]
  224 |     eta9 = 1e120;
      |     ^~~~

mma.c: In function 'mma_minimize':
mma.c:241:4: warning: floating constant exceeds range of 'float' [-Woverflow]
  241 |    for (i = 0; i < m; ++i) dual_ub[i] = 1e40;
      |    ^~~

DIRsubrout.c: In function 'direct_dirheader_':
DIRsubrout.c:1490:2: warning: floating constant exceeds range of 'float' [-Woverflow]
 1490 |  *epsfix = 1e100;
      |  ^
DIRsubrout.c: In function 'direct_dirsummary_':
DIRsubrout.c:1586:3: warning: floating constant exceeds range of 'float' [-Woverflow]
 1586 |   if (*fglobal > -1e99)
      |   ^~

plis.c: In function 'plis_':
plis.c:189:5: warning: floating constant exceeds range of 'float' [-Woverflow]
  189 |     eta9 = 1e120;
      |     ^~~~
Jaaaky commented 1 year ago

This looks like serious one too:

nldrmd.c:205:11: warning: ‘memset’ specified bound between 18446744056529682432 and 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
  205 |           memset(xcur, 0, sizeof(double)*n);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Jaaaky commented 1 year ago

Another interesting one

bobyqa.c:1037:19: error: 'adelt' may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1037 |     temp = *adelt * *adelt - wfixsq;
      |            ~~~~~~~^~~~~~~~
compilation terminated due to -Wfatal-errors.
cc1: some warnings being treated as errors
stevengj commented 1 year ago

Are you compiling on a platform where float is the same as double? The floating constant exceeds range of 'float' make no sense otherwise, since the indicated constants are within the range of double precision.

stevengj commented 1 year ago
warning: ‘memset’ specified bound between 18446744056529682432 and 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
  205 |           memset(xcur, 0, sizeof(double)*n);

The code looks correct to me. I have no idea what this warning means — if n is the size of the array, as it is in this code, then this is setting the correct number of bytes to zero.