Merge pull request #437 from lanl/jmm/sanitize

Run singularity-eos through clang address sanitizer... and fix the HIP segfault!

.github/workflows/sanitizer.yml

@@ -0,0 +1,40 @@
+name: Sanitizer
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+    sanitizer:
+      name: Run clang sanitizer on minimal code subset
+      runs-on: ubuntu-latest
+
+      steps:
+        - name: Checkout code
+          uses: actions/checkout@v3
+          with:
+            submodules: recursive
+        - name: Set system to non-interactive mode
+          run: export DEBIAN_FRONTEND=noninteractive
+        - name: install dependencies
+          run: |
+            sudo apt-get update -y -qq
+            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential clang llvm
+        - name: build and run tests
+          run: |
+            mkdir -p bin
+            cd bin
+            cmake -DCMAKE_CXX_COMPILER=clang++ \
+                  -DCMAKE_CXX_FLAGS="-fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer" \
+                  -DCMAKE_BUILD_TYPE=Debug \
+                  -DSINGULARITY_STRICT_WARNINGS=ON \
+                  -DSINGULARITY_USE_FORTRAN=OFF \
+                  -DSINGULARITY_BUILD_FORTRAN_BACKEND=ON \
+                  -DSINGULARITY_BUILD_TESTS=ON \
+                  -DSINGULARITY_FORCE_SUBMODULE_MODE=ON \
+                  -DSINGULARITY_USE_KOKKOS=ON \
+                  ..
+            make -j4
+            ctest --output-on-failure

.github/workflows/warnings.yml

@@ -0,0 +1,37 @@
+name: Warnings
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+    warnings-gcc:
+      name: Ensure no warnings from gcc
+      runs-on: ubuntu-latest
+
+      steps:
+        - name: Checkout code
+          uses: actions/checkout@v3
+          with:
+            submodules: recursive
+        - name: Set system to non-interactive mode
+          run: export DEBIAN_FRONTEND=noninteractive
+        - name: install dependencies
+          run: |
+            sudo apt-get update -y -qq
+            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential
+        - name: build and run tests
+          run: |
+            mkdir -p bin
+            cd bin
+            cmake -DCMAKE_BUILD_TYPE=Debug \
+                  -DSINGULARITY_STRICT_WARNINGS=ON \
+                  -DSINGULARITY_USE_FORTRAN=OFF \
+                  -DSINGULARITY_BUILD_FORTRAN_BACKEND=ON \
+                  -DSINGULARITY_BUILD_TESTS=ON \
+                  -DSINGULARITY_FORCE_SUBMODULE_MODE=ON \
+                  -DSINGULARITY_USE_KOKKOS=ON \
+                  ..
+            make -j4

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Added (new features/APIs/variables/...)
 
 ### Fixed (Repair bugs, etc)
+- [[OR437]](https://github.com/lanl/singularity-eos/pull/437) Fix segfault on HIP, clean up warnings, add strict sanitizer test
 
 ### Changed (changing behavior/API/variables/...)
 

CMakeLists.txt

@@ -46,6 +46,14 @@ cmake_dependent_option(
   "SINGULARITY_USE_SPINER" OFF)
 
 option(SINGULARITY_USE_FORTRAN "Enable fortran bindings" ON)
+# Optionally build these if you want to build/test the fortran
+# infrastructure without invoking a fortran compiler If fortran is
+# off, you can set this. If fortran is on, it's forced to ON and is
+# not set-able.
+cmake_dependent_option(SINGULARITY_BUILD_FORTRAN_BACKEND
+  "Build the C++ code to which the fortran bindings bind"
+  OFF "NOT SINGULARITY_USE_FORTRAN" ON)
+
 option(SINGULARITY_USE_KOKKOS "Use Kokkos for portability" OFF)
 option(SINGULARITY_USE_EOSPAC "Enable eospac backend" OFF)
 option(SINGULARITY_EOSPAC_ENABLE_SHMEM
@@ -95,6 +103,7 @@ cmake_dependent_option(
 # modify flags options
 option(SINGULARITY_BETTER_DEBUG_FLAGS "Better debug flags for singularity" ON)
 option(SINGULARITY_HIDE_MORE_WARNINGS "hide more warnings" OFF)
+option(SINGULARITY_STRICT_WARNINGS "Make warnings strict" OFF)
 
 # toggle code options
 option(SINGULARITY_USE_TRUE_LOG_GRIDDING
@@ -552,6 +561,10 @@ target_compile_options(
          > # release
          > # cuda
 )
+if (SINGULARITY_STRICT_WARNINGS)
+  target_compile_options(singularity-eos_Interface INTERFACE
+    -Wall -Werror -Wno-unknown-pragmas)
+endif()
 
 if(TARGET singularity-eos_Library)
   target_compile_options(singularity-eos_Library PRIVATE ${xlfix})
@@ -590,6 +603,7 @@ endif()
 
 if(SINGULARITY_BUILD_TESTS)
   include(CTest)
+  enable_testing()
   add_subdirectory(test)
 endif()
 

doc/sphinx/src/building.rst

@@ -118,6 +118,7 @@ The main CMake options to configure building are in the following table:
  ``SINGULARITY_USE_SINGLE_LOGS``         OFF      Use single precision logarithms (may degrade accuracy).
  ``SINGULARITY_NQT_ORDER_1``             OFF      For fast logs, use the less accurate but faster 1st-order version.
  ``SINGULARITY_NQT_PORTABLE``            OFF      For fast logs, use the slower but endianness-independent implementation.
+ ``SINGULARITY_STRICT_WARNINGS``         OFF      For testing. Adds -Wall and -Werror to builds.
 ====================================== ======= ===========================================
 
 More options are available to modify only if certain other options or
@@ -159,6 +160,7 @@ preconditions:
  ``SINGULARITY_TEST_PYTHON``                    ``SINGULARITY_BUILD_TESTS=ON`` ``SINGULARITY_BUILD_PYTHON=ON``                    Test the Python bindings.
  ``SINGULARITY_USE_HELMHOLTZ``                  ``SINGULARITY_USE_SPINER=ON`` ``SINGULARITY_USE_SPINER_WITH_HDF5=ON``             Use Helmholtz equation of state.
  ``SINGULARITY_TEST_HELMHOLTZ``                 ``SINGULARITY_USE_HELMHOLTZ``                                                     Build Helmholtz equation of state tests.
+ ``SINGULARITY_BUILD_FORTRAN_BACKEND``          ``NOT SINGULARITY_USE_FORTRAN``                                                   For testing, you may build the C++ code to which the fortran bindings bind without building the bindings themselves.
 ============================================== ================================================================================= ===========================================
 
 When installing ``singularity-eos``, data files are also installed. The

singularity-eos/CMakeLists.txt

@@ -73,13 +73,13 @@ if (SINGULARITY_BUILD_CLOSURE)
     closure/kinetic_phasetransition_utils.hpp
     closure/kinetic_phasetransition_methods.hpp
   )
-  if (SINGULARITY_USE_FORTRAN OR SINGULARITY_BUILD_TESTS)
+  if (SINGULARITY_BUILD_FORTRAN_BACKEND OR SINGULARITY_BUILD_TESTS)
     # while these are C++ files, they
     # are only needed for the fortran backend or unit testing
     register_srcs(eos/singularity_eos.cpp)
     register_headers(eos/singularity_eos.hpp)
   endif()
-  if (SINGULARITY_USE_FORTRAN)
+  if (SINGULARITY_BUILD_FORTRAN_BACKEND)
     register_srcs(eos/get_sg_eos.cpp)
     if (SINGULARITY_USE_KOKKOS)
       register_srcs(

singularity-eos/base/root-finding-1d/root_finding.hpp

@@ -70,45 +70,45 @@ enum class Status { SUCCESS = 0, FAIL = 1 };
 */
 class RootCounts {
  private:
-  static constexpr int nbins_{15};
+  static constexpr std::size_t nbins_{15};
   mutable Real counts_[nbins_];
 
  public:
   PORTABLE_INLINE_FUNCTION
   RootCounts() {
-    for (int i{0}; i < nbins_; ++i)
+    for (std::size_t i{0}; i < nbins_; ++i)
       counts_[i] = 0;
   }
   PORTABLE_INLINE_FUNCTION void reset() {
-    for (int i{0}; i < nbins_; ++i)
+    for (std::size_t i{0}; i < nbins_; ++i)
       counts_[i] = 0;
   }
-  PORTABLE_INLINE_FUNCTION void increment(int i) const {
+  PORTABLE_INLINE_FUNCTION void increment(std::size_t i) const {
     assert(i < nbins_ && i >= 0);
 #ifdef PORTABILITY_STRATEGY_NONE
     counts_[i] += 1;
 #endif // PORTABILITY_STRATEGY_NONE
   }
   PORTABLE_INLINE_FUNCTION Real total() const {
     Real tot{1.e-20};
-    for (int i{0}; i < nbins_; ++i)
+    for (std::size_t i{0}; i < nbins_; ++i)
       tot += counts_[i];
     return tot;
   }
-  PORTABLE_INLINE_FUNCTION const Real &operator[](const int i) const {
+  PORTABLE_INLINE_FUNCTION const Real &operator[](const std::size_t i) const {
     assert(i < nbins_ && i >= 0);
     return counts_[i];
   }
-  PORTABLE_INLINE_FUNCTION Real &operator[](const int i) {
+  PORTABLE_INLINE_FUNCTION Real &operator[](const std::size_t i) {
     assert(i < nbins_ && i >= 0);
     return counts_[i];
   }
   PORTABLE_INLINE_FUNCTION void print_counts() const {
-    for (int i{0}; i < nbins_; ++i)
+    for (std::size_t i{0}; i < nbins_; ++i)
       printf("%e\n", counts_[i]);
   }
-  PORTABLE_INLINE_FUNCTION int nBins() const { return nbins_; }
-  PORTABLE_INLINE_FUNCTION int more() const { return nbins_ - 1; }
+  PORTABLE_INLINE_FUNCTION std::size_t nBins() const { return nbins_; }
+  PORTABLE_INLINE_FUNCTION std::size_t more() const { return nbins_ - 1; }
 };
 
 PORTABLE_INLINE_FUNCTION bool check_bracket(const Real ya, const Real yb) {
@@ -119,11 +119,11 @@ template <typename T>
 PORTABLE_INLINE_FUNCTION bool set_bracket(const T &f, Real &a, const Real guess, Real &b,
                                           Real &ya, const Real yg, Real &yb,
                                           const bool &verbose = false) {
-  constexpr int max_search_depth = 6;
+  constexpr std::size_t max_search_depth = 6;
   Real dx = b - a;
-  for (int level = 0; level < max_search_depth; level++) {
-    const int nlev = (1 << level);
-    for (int i = 0; i < nlev; i++) {
+  for (std::size_t level = 0; level < max_search_depth; level++) {
+    const std::size_t nlev = (1 << level);
+    for (std::size_t i = 0; i < nlev; i++) {
       const Real x = a + (i + 0.5) * dx;
       const Real yx = f(x);
       if (check_bracket(yx, yg)) {
@@ -158,7 +158,7 @@ PORTABLE_INLINE_FUNCTION Status regula_falsi(const T &f, const Real ytarget,
                                              Real &xroot,
                                              const RootCounts *counts = nullptr,
                                              const bool &verbose = false) {
-  constexpr int max_iter = SECANT_NITER_MAX;
+  constexpr std::size_t max_iter = SECANT_NITER_MAX;
   auto func = [&](const Real x) { return f(x) - ytarget; };
   Real ya = func(a);
   Real yg = func(guess);
@@ -187,9 +187,9 @@ PORTABLE_INLINE_FUNCTION Status regula_falsi(const T &f, const Real ytarget,
   ya *= sign;
   yb *= sign;
 
-  int b1 = 0;
-  int b2 = 0;
-  int iteration_count = 0;
+  std::size_t b1 = 0;
+  std::size_t b2 = 0;
+  std::size_t iteration_count = 0;
   while (b - a > 2.0 * xtol && (std::abs(ya) > ytol || std::abs(yb) > ytol) &&
          iteration_count < max_iter) {
     Real c = (a * yb - b * ya) / (yb - ya);
@@ -251,15 +251,15 @@ PORTABLE_INLINE_FUNCTION Status newton_raphson(const T &f, const Real ytarget,
                                                const bool &verbose = false,
                                                const bool &fail_on_bound_root = true) {
 
-  constexpr int max_iter = NEWTON_RAPHSON_NITER_MAX;
+  constexpr std::size_t max_iter = NEWTON_RAPHSON_NITER_MAX;
   Real _x = guess;
   Real _xold = 0.0;
   auto status = Status::SUCCESS;
 
   Real yg;
   Real dfunc;
 
-  int iter;
+  std::size_t iter;
 
   for (iter = 0; iter < max_iter; iter++) {
     std::tie(yg, dfunc) = f(_x); // C++11 tuple unpacking
@@ -383,7 +383,7 @@ PORTABLE_INLINE_FUNCTION Status secant(const T &f, const Real ytarget, const Rea
   Real x_last, y, yp, ym, dyNum, dyDen, dy;
 
   Real x = xguess;
-  unsigned int iter{0};
+  std::size_t iter{0};
   for (iter = 0; iter < SECANT_NITER_MAX; ++iter) {
     x_last = x;
     dx = fabs(1.e-7 * x) + xtol;
@@ -490,7 +490,7 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea
     x += 2. * xtol;
   }
   // do { // Try to find reasonable region for bisection
-  for (int i{0}; i < BISECT_REG_MAX; ++i) {
+  for (std::size_t i{0}; i < BISECT_REG_MAX; ++i) {
     dx = fabs(grow * x);
     xl = x - dx;
     xr = x + dx;
@@ -547,10 +547,10 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea
                 "\til      = %.10e\n"
                 "\tir      = %.10e\n",
                 xguess, ytarget, xl, xr, fl, fr, il, ir);
-        int nx = 300;
+        std::size_t nx = 300;
         Real dx = (xmax - xmin) / (nx - 1);
         fprintf(stderr, "Area map:\nx\ty\n");
-        for (int i = 0; i < nx; i++) {
+        for (std::size_t i = 0; i < nx; i++) {
           fprintf(stderr, "%.4f\t%.4e\n", x + i * dx, f(x + i * dx));
         }
 #endif
@@ -559,7 +559,7 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea
     }
   }
 
-  for (int i{0}; i < BISECT_NITER_MAX; ++i) {
+  for (std::size_t i{0}; i < BISECT_NITER_MAX; ++i) {
     Real xm = 0.5 * (xl + xr);
     Real fm = f(xm) - ytarget;
     if (fl * fm <= 0) {

singularity-eos/closure/mixed_cell_models.hpp

@@ -209,9 +209,9 @@ class PTESolverBase {
                 const RealIndexer &vfrac_, const RealIndexer &sie_,
                 const RealIndexer &temp_, const RealIndexer &press_, Real *&scratch,
                 Real Tnorm, const MixParams &params = MixParams())
-      : nmat(nmats), neq(neqs), niter(0), eos(eos_), vfrac_total(vfrac_tot),
-        sie_total(sie_tot), rho(rho_), vfrac(vfrac_), sie(sie_), temp(temp_),
-        press(press_), Tnorm(Tnorm), params_(params) {
+      : params_(params), nmat(nmats), neq(neqs), niter(0), vfrac_total(vfrac_tot),
+        sie_total(sie_tot), eos(eos_), rho(rho_), vfrac(vfrac_), sie(sie_), temp(temp_),
+        press(press_), Tnorm(Tnorm) {
     jacobian = AssignIncrement(scratch, neq * neq);
     dx = AssignIncrement(scratch, neq);
     sol_scratch = AssignIncrement(scratch, 2 * neq);

singularity-eos/eos/eos_base.hpp

@@ -41,17 +41,18 @@ constexpr std::size_t MAX_NUM_CHARS = 121;
 // Cuda doesn't have strcat, so we implement it ourselves
 PORTABLE_FORCEINLINE_FUNCTION
 char *StrCat(char *destination, const char *source) {
-  int i, j; // not in loops because they're re-used.
+  std::size_t i, j; // not in loops because they're re-used.
 
   // specifically avoid strlen, which isn't on GPU
   for (i = 0; destination[i] != '\0'; i++) {
   }
   // assumes destination has enough memory allocated
   for (j = 0; source[j] != '\0'; j++) {
     // MAX_NUM_CHARS-1 to leave room for null terminator
-    PORTABLE_REQUIRE((i + j) < MAX_NUM_CHARS - 1,
+    std::size_t ipj = i + j;
+    PORTABLE_REQUIRE(ipj < MAX_NUM_CHARS - 1,
                      "Concat string must be within allowed size");
-    destination[i + j] = source[j];
+    destination[ipj] = source[j];
   }
   // null terminate destination string
   destination[i + j] = '\0';

singularity-eos/eos/eos_gruneisen.hpp

@@ -359,14 +359,14 @@ PORTABLE_INLINE_FUNCTION Real Gruneisen::PressureFromDensityInternalEnergy(
 template <typename Indexer_t>
 PORTABLE_INLINE_FUNCTION Real
 Gruneisen::MinInternalEnergyFromDensity(const Real rho_in, Indexer_t &&lambda) const {
-  const Real rho = std::min(rho_in, _rho_max);
+  // const Real rho = std::min(rho_in, _rho_max);
   MinInternalEnergyIsNotEnabled("Gruneisen");
   return 0.0;
 }
 template <typename Indexer_t>
 PORTABLE_INLINE_FUNCTION Real Gruneisen::EntropyFromDensityInternalEnergy(
     const Real rho_in, const Real sie, Indexer_t &&lambda) const {
-  const Real rho = std::min(rho_in, _rho_max);
+  // const Real rho = std::min(rho_in, _rho_max);
   EntropyIsNotEnabled("Gruneisen");
   return 1.0;
 }

singularity-eos/eos/eos_mgusup.hpp

@@ -370,7 +370,7 @@ MGUsup::FillEos(Real &rho, Real &temp, Real &sie, Real &press, Real &cv, Real &b
   }
   if (output & thermalqs::temperature)
     temp = TemperatureFromDensityInternalEnergy(rho, sie);
-  if (output & thermalqs::specific_internal_energy) sie = sie;
+  // if (output & thermalqs::specific_internal_energy) sie = sie;
   if (output & thermalqs::pressure) press = PressureFromDensityInternalEnergy(rho, sie);
   if (output & thermalqs::specific_heat)
     cv = SpecificHeatFromDensityInternalEnergy(rho, sie);

singularity-eos/eos/eos_powermg.hpp

@@ -452,7 +452,7 @@ PowerMG::FillEos(Real &rho, Real &temp, Real &sie, Real &press, Real &cv, Real &
   }
   if (output & thermalqs::temperature)
     temp = TemperatureFromDensityInternalEnergy(rho, sie);
-  if (output & thermalqs::specific_internal_energy) sie = sie;
+  // if (output & thermalqs::specific_internal_energy) sie = sie;
   if (output & thermalqs::pressure) press = PressureFromDensityInternalEnergy(rho, sie);
   if (output & thermalqs::specific_heat)
     cv = SpecificHeatFromDensityInternalEnergy(rho, sie);

singularity-eos/eos/eos_sap_polynomial.hpp

@@ -40,8 +40,8 @@ class SAP_Polynomial : public EosBase<SAP_Polynomial> {
   SAP_Polynomial(const Real rho0, const Real a0, const Real a1, const Real a2c,
                  const Real a2e, const Real a3, const Real b0, const Real b1,
                  const Real b2c, const Real b2e, const Real b3)
-      : _rho0(rho0), _a0(a0), _a1(a1), _a2c(a2c), _a2e(a2e), _a3(a3), _b0(b0), _b1(b1),
-        _b2c(b2c), _b2e(b2e), _b3(b3) {
+      : _a0(a0), _a1(a1), _a2c(a2c), _a2e(a2e), _a3(a3), _b0(b0), _b1(b1), _b2c(b2c),
+        _b2e(b2e), _b3(b3), _rho0(rho0) {
     CheckParams();
   }