Merge pull request #437 from lanl/jmm/sanitize

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

Author: Yurlungur

.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;
 }