From e751732c0fc6a6fe7b4543709fc5110d9651d2a2 Mon Sep 17 00:00:00 2001 From: Ryan Burns Date: Thu, 9 Jul 2020 12:17:26 -0700 Subject: [PATCH 1/4] Privatize sardata structs These duplicated names spanning translation units violate ODR. They are only used within their corresponding .c files and thus can be given static linkage. --- .../isceobj/Sensor/src/ALOS_pre_process/read_ALOSE_data.c | 6 +++--- .../isceobj/Sensor/src/ALOS_pre_process/read_ALOS_data.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOSE_data.c b/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOSE_data.c index 77fbfc0..7ca7f1a 100644 --- a/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOSE_data.c +++ b/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOSE_data.c @@ -60,9 +60,9 @@ int reset_params(struct PRM *prm, long *, int *, int *); int fill_shift_data(int, int, int, int, int, char *, char *, FILE *); int handle_prf_change_ALOSE(struct PRM *, FILE *, long *, int); -struct sardata_record r1; -struct sardata_descriptor_ALOSE dfd; -struct sardata_info_ALOSE sdr; +static struct sardata_record r1; +static struct sardata_descriptor_ALOSE dfd; +static struct sardata_info_ALOSE sdr; /* differences in include file from ALOS AUIG diff --git a/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOS_data.c b/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOS_data.c index 87d7465..dd21713 100644 --- a/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOS_data.c +++ b/components/isceobj/Sensor/src/ALOS_pre_process/read_ALOS_data.c @@ -67,9 +67,9 @@ int reset_params(struct PRM *prm, long *, int *, int *); int fill_shift_data(int, int, int, int, int, char *, char *, FILE *); int handle_prf_change(struct PRM *, FILE *, long *, int); -struct sardata_record r1; -struct sardata_descriptor dfd; -struct sardata_info sdr; +static struct sardata_record r1; +static struct sardata_descriptor dfd; +static struct sardata_info sdr; long read_ALOS_data (FILE *imagefile, FILE *outfile, struct PRM *prm, long *byte_offset) { From f65f26e3cc9cd52146f1c479bab188da8b2d4bf8 Mon Sep 17 00:00:00 2001 From: Ryan Burns Date: Thu, 9 Jul 2020 12:23:34 -0700 Subject: [PATCH 2/4] Move global vars from .h to separate TU These global variables were instantiated in the image_sio.h header and included into multiple translation units, violating ODR. They are now marked as 'extern' and declared in a separate TU. --- components/isceobj/Sensor/CMakeLists.txt | 2 +- .../Sensor/src/ALOS_pre_process/SConscript | 5 ++- .../Sensor/src/ALOS_pre_process/image_sio.c | 16 ++++++++++ .../Sensor/src/ALOS_pre_process/image_sio.h | 32 +++++++++---------- .../alosreformat/ALOS_lib/src/image_sio.c | 16 ++++++++++ .../stdproc/alosreformat/CMakeLists.txt | 1 + .../stdproc/alosreformat/include/image_sio.h | 32 +++++++++---------- 7 files changed, 70 insertions(+), 34 deletions(-) create mode 100644 components/isceobj/Sensor/src/ALOS_pre_process/image_sio.c create mode 100644 components/stdproc/alosreformat/ALOS_lib/src/image_sio.c diff --git a/components/isceobj/Sensor/CMakeLists.txt b/components/isceobj/Sensor/CMakeLists.txt index ff6f86c..e651305 100644 --- a/components/isceobj/Sensor/CMakeLists.txt +++ b/components/isceobj/Sensor/CMakeLists.txt @@ -67,7 +67,7 @@ Python_add_library(alos MODULE src/ALOS_pre_process/data_ALOS.h src/ALOS_pre_process/data_ALOSE.h src/ALOS_pre_process/hermite_c.c - src/ALOS_pre_process/image_sio.h + src/ALOS_pre_process/image_sio.c src/ALOS_pre_process/init_from_PRM.c src/ALOS_pre_process/interpolate_ALOS_orbit.c src/ALOS_pre_process/null_sio_struct.c diff --git a/components/isceobj/Sensor/src/ALOS_pre_process/SConscript b/components/isceobj/Sensor/src/ALOS_pre_process/SConscript index 5ed0b70..fb74306 100644 --- a/components/isceobj/Sensor/src/ALOS_pre_process/SConscript +++ b/components/isceobj/Sensor/src/ALOS_pre_process/SConscript @@ -10,7 +10,10 @@ headerFiles = ['data_ALOS.h','data_ALOSE.h','image_sio.h','orbit_ALOS.h','sarlea sourceFiles = ['ALOSE_orbits_utils.c','ALOS_ldr_orbit.c','ALOS_pre_process.c','calc_dop.c','hermite_c.c','init_from_PRM.c', 'interpolate_ALOS_orbit.c','null_sio_struct.c','parse_ALOS_commands.c','polyfit.c','read_ALOSE_data.c', 'read_ALOS_data.c','read_ALOS_sarleader.c','roi_utils.c','set_ALOS_defaults.c','siocomplex.c', - 'swap_ALOS_data_info.c','utils.c','write_ALOS_prm.c','readOrbitPulse.f','readOrbitPulseState.f','readOrbitPulseSetState.f'] + 'swap_ALOS_data_info.c','utils.c','write_ALOS_prm.c', + 'readOrbitPulse.f','readOrbitPulseState.f', + 'readOrbitPulseSetState.f','image_sio.c', + ] lib = envSensorSrc1.Library(target = 'alos', source = sourceFiles) envSensorSrc1.Install(install,lib) envSensorSrc1.Alias('install',install) diff --git a/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.c b/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.c new file mode 100644 index 0000000..8428a4b --- /dev/null +++ b/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.c @@ -0,0 +1,16 @@ +#include "image_sio.h" + +int verbose; +int debug; +int roi; +int swap; +int quad_pol; +int ALOS_format; + +int force_slope; +int dopp; +int quiet_flag; +int SAR_mode; + +double forced_slope; +double tbias; diff --git a/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.h b/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.h index 8437a45..657e4b6 100644 --- a/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.h +++ b/components/isceobj/Sensor/src/ALOS_pre_process/image_sio.h @@ -146,19 +146,19 @@ radar_wavelength lambda rng_spec_wgt rhww */ -int verbose; /* controls minimal level of output */ -int debug; /* more output */ -int roi; /* more output */ -int swap; /* whether to swap bytes */ -int quad_pol; /* quad polarization data */ -int ALOS_format; /* AUIG: ALOS_format = 0 */ - /* ERSDAC: ALOS_format = 1 */ -int force_slope; /* whether to set the slope */ -int dopp; /* whether to calculate doppler */ -int quiet_flag; /* reduce output */ -int SAR_mode; /* 0 => high-res */ - /* 1 => wide obs */ - /* 2 => polarimetry */ - /* from ALOS Product Format 3-2 */ -double forced_slope; /* value to set chirp_slope to */ -double tbias; /* time bias for bad orbit data */ +extern int verbose; /* controls minimal level of output */ +extern int debug; /* more output */ +extern int roi; /* more output */ +extern int swap; /* whether to swap bytes */ +extern int quad_pol; /* quad polarization data */ +extern int ALOS_format; /* AUIG: ALOS_format = 0 */ + /* ERSDAC: ALOS_format = 1 */ +extern int force_slope; /* whether to set the slope */ +extern int dopp; /* whether to calculate doppler */ +extern int quiet_flag; /* reduce output */ +extern int SAR_mode; /* 0 => high-res */ + /* 1 => wide obs */ + /* 2 => polarimetry */ + /* from ALOS Product Format 3-2 */ +extern double forced_slope; /* value to set chirp_slope to */ +extern double tbias; /* time bias for bad orbit data */ diff --git a/components/stdproc/alosreformat/ALOS_lib/src/image_sio.c b/components/stdproc/alosreformat/ALOS_lib/src/image_sio.c new file mode 100644 index 0000000..3f376cb --- /dev/null +++ b/components/stdproc/alosreformat/ALOS_lib/src/image_sio.c @@ -0,0 +1,16 @@ +#include "image_sio.h" + +int verbose; +int debug; +int roi; +int swap; +int quad_pol; +int ALOS_format; + +int force_slope; +int dopp; +int quiet_flag; +int SAR_mode; + +double forced_slope; +double tbias; diff --git a/components/stdproc/alosreformat/CMakeLists.txt b/components/stdproc/alosreformat/CMakeLists.txt index 05e32cc..90232d7 100644 --- a/components/stdproc/alosreformat/CMakeLists.txt +++ b/components/stdproc/alosreformat/CMakeLists.txt @@ -1,6 +1,7 @@ isce2_add_staticlib(alosLib ALOS_lib/src/cfft1d.c ALOS_lib/src/find_fft_length.c + ALOS_lib/src/image_sio.c ALOS_lib/src/utils.c ALOS_lib/src/ALOS_ldr_orbit.c ALOS_lib/src/calc_dop.c diff --git a/components/stdproc/alosreformat/include/image_sio.h b/components/stdproc/alosreformat/include/image_sio.h index 8437a45..3b98e33 100644 --- a/components/stdproc/alosreformat/include/image_sio.h +++ b/components/stdproc/alosreformat/include/image_sio.h @@ -146,19 +146,19 @@ radar_wavelength lambda rng_spec_wgt rhww */ -int verbose; /* controls minimal level of output */ -int debug; /* more output */ -int roi; /* more output */ -int swap; /* whether to swap bytes */ -int quad_pol; /* quad polarization data */ -int ALOS_format; /* AUIG: ALOS_format = 0 */ - /* ERSDAC: ALOS_format = 1 */ -int force_slope; /* whether to set the slope */ -int dopp; /* whether to calculate doppler */ -int quiet_flag; /* reduce output */ -int SAR_mode; /* 0 => high-res */ - /* 1 => wide obs */ - /* 2 => polarimetry */ - /* from ALOS Product Format 3-2 */ -double forced_slope; /* value to set chirp_slope to */ -double tbias; /* time bias for bad orbit data */ +extern int verbose; /* controls minimal level of output */ +extern int debug; /* more output */ +extern int roi; /* more output */ +extern int swap; /* whether to swap bytes */ +extern int quad_pol; /* quad polarization data */ +extern int ALOS_format; /* AUIG: ALOS_format = 0 */ + /* ERSDAC: ALOS_format = 1 */ +extern int force_slope; /* whether to set the slope */ +extern int dopp; /* whether to calculate doppler */ +extern int quiet_flag; /* reduce output */ +extern int SAR_mode; /* 0 => high-res */ + /* 1 => wide obs */ + /* 2 => polarimetry */ + /* from ALOS Product Format 3-2 */ +extern double forced_slope; /* value to set chirp_slope to */ +extern double tbias; /* time bias for bad orbit data */ From a9cc014094eb08c25a248498280a1ee426a1aae9 Mon Sep 17 00:00:00 2001 From: Ryan Burns Date: Tue, 17 Nov 2020 12:25:08 -0800 Subject: [PATCH 3/4] Enable -fno-common by default to catch ODR violations This should prevent ODR-violating code in the future --- .cmake/isce2_buildflags.cmake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.cmake/isce2_buildflags.cmake b/.cmake/isce2_buildflags.cmake index 5570353..255a56a 100644 --- a/.cmake/isce2_buildflags.cmake +++ b/.cmake/isce2_buildflags.cmake @@ -31,3 +31,19 @@ list(FIND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES ${abs_libdir} isSystemDir) if("${isSystemDir}" STREQUAL "-1") list(APPEND CMAKE_INSTALL_RPATH ${abs_libdir}) endif() + +option(ISCE2_STRICT_COMPILATION "Enable strict checks during compilation" ON) +if(ISCE2_STRICT_COMPILATION) + + # Set -fno-common when supported to catch ODR violations + include(CheckCCompilerFlag) + check_c_compiler_flag(-fno-common C_FNO_COMMON) + if(C_FNO_COMMON) + add_compile_options($<$:-fno-common>) + endif() + include(CheckCXXCompilerFlag) + check_cxx_compiler_flag(-fno-common CXX_FNO_COMMON) + if(CXX_FNO_COMMON) + add_compile_options($<$:-fno-common>) + endif() +endif() From 74319b24d5c00d70f87320db7d6a822cf9c17b46 Mon Sep 17 00:00:00 2001 From: Ryan Burns Date: Tue, 17 Nov 2020 16:16:54 -0800 Subject: [PATCH 4/4] Add missing Inertial.py to Orbit module --- components/isceobj/Orbit/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/components/isceobj/Orbit/CMakeLists.txt b/components/isceobj/Orbit/CMakeLists.txt index a16b8b1..0f48d96 100644 --- a/components/isceobj/Orbit/CMakeLists.txt +++ b/components/isceobj/Orbit/CMakeLists.txt @@ -7,6 +7,7 @@ target_include_directories(orbitHermite PUBLIC include) InstallSameDir( orbitHermite __init__.py + Inertial.py ODR.py Orbit.py OrbitExtender.py