ARM g++ compile error due to warning on signed char in bias.cpp

[spenczar]

I'm working on an Apple Macbook M1 ("Apple Silicon") which is an ARM platform. I'm running inside a Docker container so my OS is Ubuntu, but the architecture is 64 bit ARM.

Building fails to me on the current version:

$ make
g++ -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 fo.cpp
g++ -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 ades_out.cpp
g++ -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 b32_eph.cpp
g++ -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 bc405.cpp
g++ -c -Wall -pedantic -Wextra -Werror  -I ~/include -O3 bias.cpp
bias.cpp: In function 'int find_fcct_biases(double, double, char, double, double*, double*)':
bias.cpp:85:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
   85 |    if( catalog == -1)    /* free up internal data */
      |        ~~~~~~~~^~~~~
cc1plus: all warnings being treated as errors
make: *** [makefile:348: bias.o] Error 1

This is using g++ 11.3.0 on Ubuntu 22.04.

Here's the script I ran in a fresh ubuntu:22.04 Docker container:

apt update -y
apt install -y \
    ncurses-dev \
    libcurl4-openssl-dev \
    build-essential \
    g++ \
    gcc \
    git

git clone https://github.com/Bill-Gray/lunar.git
git clone https://github.com/Bill-Gray/sat_code.git
git clone https://github.com/Bill-Gray/jpl_eph.git
git clone https://github.com/Bill-Gray/find_orb.git
git clone https://github.com/Bill-Gray/miscell.git

cd lunar
make
make install
cd -

cd jpl_eph
make
make install
cd -

cd sat_code
make
make install
cd -

cd miscell
make
make install
cd -

cd find_orb
make
make install
cd -

This errors

[spenczar]

If I label the type as a signed character, then the compiler no longer complains:

--- a/bias.cpp
+++ b/bias.cpp
@@ -68,7 +68,7 @@ const char *fcct14_bias_file_name = NULL;

 FILE *fopen_ext( const char *filename, const char *permits);   /* miscell.cpp */

-int find_fcct_biases( const double ra, const double dec, const char catalog,
+int find_fcct_biases( const double ra, const double dec, const signed char catalog,
                  const double jd, double *bias_ra, double *bias_dec)
 {
    static int16_t *bias_data = NULL;

[Bill-Gray]

Well, that's interesting. This code has been compiled on ARM before; we ran into problems with alignment (you can't read, say, an integer from an unaligned address on ARM) and the absence of __float128. Seems to me we should have encountered this warning before, since ARM characters default to being unsigned. My guess is that this has been buggy code all along, and that only now has the compiler been bright enough to recognize it.

Please let me know of anything else you run into. While the code has been built on ARM devices (Raspberry Pi, at least, and probably others), I don't know how thoroughly it's gotten exercised on them. Apparently, not thoroughly enough to have caused me to hear about this bug.

(Though I'm not entirely sure it's a bug. I think the compiler may have interpreted this as if( catalog == (char)-1)' in which case the comparison would have been done properly.)

[Bill-Gray]

I added variants of the following :

ifdef UCHAR
       CXXFLAGS += -funsigned-char
endif

to the makefiles for lunar, jpl_eph, sat_code, and find_orb, and rebuilt everything using unsigned chars. The only warning I got was the one you've reported, with bias.cpp, now fixed. Running it for a bit, loading up a few objects and generating ephemerides, didn't reveal any problems. I'll stick with the unsigned char version and see if any horrors emerge, but it looks good thus far.

[Bill-Gray]

Well, I sort of expected some other problem(s) to appear with the switch to unsigned characters. But aside from this one issue, it appears to have been a non-event, so I think we can close this...