Bill-Gray / find_orb

Orbit determination from observations
https://www.projectpluto.com/find_orb.htm
GNU General Public License v2.0
97 stars 45 forks source link

g++ compile error due to warning in runge.cpp #43

Closed Brensom closed 1 year ago

Brensom commented 1 year ago

Building fails to me on the current version:

ror  -I ~/include -O3 runge.cpp
runge.cpp: In function ‘int calc_derivativesl(long double, const long double*, long double*, int)’:
runge.cpp:993:28: error: ‘jupiter_loc[0]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[0]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
runge.cpp:993:28: error: ‘jupiter_loc[1]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[1]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
runge.cpp:993:28: error: ‘jupiter_loc[2]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[2]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
cc1plus: all warnings being treated as errors
make: *** [makefile:357: runge.o] Error 1
==> ERROR: A failure occurred in build().
    Aborting...

My building system:

COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/13.1.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --enable-languages=ada,c,c++,d,fortran,go,lto,objc,obj-c++ --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.1.1 20230429 (GCC) 
Bill-Gray commented 1 year ago

Thank you. I am fairly certain that this is a spurious warning, where gcc/g++ isn't quite bright enough to follow the logic through to see that jupiter_loc[0...2] really will be initialized. It does seem odd that it's not noticing that, by its logic, saturn_loc[0...2] might also be uninitialized.

Does the following patch (which says "always copy Jupiter's location into jupiter_loc, whether we're within the Galilean limits or not") cause the warnings/errors to go away?

@@ -1025,11 +1025,9 @@ int calc_derivativesl( const ldouble jd, const ldouble *ival, ldouble *oval,

             if( i == IDX_JUPITER)
                {
+               memcpy( jupiter_loc, planet_loc + 12, 3 * sizeof( double));
                if( r < GALILEAN_LIMIT)
-                  {
-                  memcpy( jupiter_loc, planet_loc + 12, 3 * sizeof( double));
                   local_perturbers |= (15 << 11);
-                  }
                else     /* "throw" Galileans into Jupiter: */
                   mass_to_use = MASS_JUPITER_SYSTEM;
                }

Failing that, we may just have to do a for( i = 0; i < 3; i++) jupiter_loc[i] = 0.; redundant (and incorrect) initialization. But I'd prefer the above patch.

Bill-Gray commented 1 year ago

I've made the above patch (see commit aee89e961ed33d62d52b76daace662815edc50). I don't see that it does any harm, and may appease the compiler into not giving us spurious warnings... please let me know if I'm wrong about that.

Brensom commented 1 year ago

I don't see any changes. I build from Master the latest version:

g++ -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection -Wp,-D_GLIBCXX_ASSERTIONS -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 miscell.cpp
runge.cpp: In function ‘int calc_derivativesl(long double, const long double*, long double*, int)’:
runge.cpp:993:28: error: ‘jupiter_loc[0]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[0]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
runge.cpp:993:28: error: ‘jupiter_loc[1]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[1]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
runge.cpp:993:28: error: ‘jupiter_loc[2]’ may be used uninitialized [-Werror=maybe-uninitialized]
  993 |                      coord = jupiter_loc[j] + planet_loc[12 + j] * JUPITER_R;
      |                            ^
runge.cpp:839:25: note: ‘jupiter_loc[2]’ was declared here
  839 |    double lunar_loc[3], jupiter_loc[3], saturn_loc[3];
      |                         ^~~~~~~~~~~
g++ -o eph2tle eph2tle.o conv_ele.o elem2tle.o simplex.o lsquare.o -L ~/lib -lm -llunar -ljpl -lsatell
cc1plus: all warnings being treated as errors
make: *** [makefile:357: runge.o] Error 1
==> ERROR: A failure occurred in build().
Brensom commented 1 year ago

The program is still not building...

Bill-Gray commented 1 year ago

Sorry, I'm still baffled by this one. For the nonce, I'd suggest adding these lines :

   for( i = 0; i < 3; i++)       /* redundant initialization */
      jupiter_loc[i] = 0.;       /* to avoid gcc-13 warning  */

just above line 959 (if( perturbers).) This should do exactly what the comment suggests : tell gcc/g++-13 that those values really have been initialized.

If that fails, and/or if you encounter further such errors, I'd suggest doing the build with make -DNO_ERRORS. This will cause the -Werror flag to be dropped, and the error message will go back to being an "ordinary" warning.

I regard both of these as stopgap measures. At some point, I need to spin up a more bleeding-edge distro and compile with gcc/g++-13 (latest version I have is 12, which doesn't exhibit this behavior.)

Generally speaking, I'm a strong proponent of -Werror. Some people complain that the compiler emits bogus warnings that are difficult to remove. I've mostly been able to dismiss such concerns. But this is one stubborn warning...

Please let me know what you find!