Coverity Scan

Fix issues found by Coverity Scan, including infinite loop bugs in the kernels of MLALaplacian. The scan will be done nightly on a local workstation. Add a badge to README.md. Remove the cmake README.md because it's broken.
AMReX-Codes · Jun 10, 2023 · 1de8e1d · 1de8e1d
1 parent cc7a5c1
commit 1de8e1d
Show file tree

Hide file tree

Showing 44 changed files with 215 additions and 159 deletions.
diff --git a/README.md b/README.md
@@ -8,7 +8,9 @@
   <a href="https://doi.org/10.5281/zenodo.2555438">
   <img src="https://zenodo.org/badge/DOI/10.5281/zenodo.2555438.svg" alt="DOI">
   </a>
-  <img src="https://github.com/AMReX-codes/amrex/workflows/cmake/badge.svg?branch=development" alt="CI: CMake on Development">
+  <a href="https://scan.coverity.com/projects/amrex-codes-amrex">
+  <img alt="Coverity Scan Build Status" src="https://scan.coverity.com/projects/28563/badge.svg"/>
+  </a>
 </p>
 
 

diff --git a/Src/Amr/AMReX_Amr.H b/Src/Amr/AMReX_Amr.H
@@ -407,8 +407,8 @@ protected:
     std::string      regrid_grids_file;   //!< Grids file that will bypass regridding.
     std::string      initial_grids_file;  //!< Grids file that will bypass regridding only at initialization.
     Vector<std::unique_ptr<AmrLevel> > amr_level;    //!< Vector of levels
-    Real             cumtime;      //!< Physical time variable.
-    Real             start_time;   //!< Physical time this simulation started.
+    Real             cumtime = std::numeric_limits<Real>::lowest();      //!< Physical time variable.
+    Real             start_time = std::numeric_limits<Real>::lowest();   //!< Physical time this simulation started.
     Vector<Real>      dt_level;     //!< Timestep at this level.
     Vector<int>       level_steps;  //!< Number of time steps at this level.
     Vector<int>       level_count;
@@ -434,7 +434,7 @@ protected:
     std::string      plot_file_root;  //!< Root name of plotfile.
     std::string      small_plot_file_root;  //!< Root name of small plotfile.
 
-    int              which_level_being_advanced; //!< Only >=0 if we are in Amr::timeStep(level,...)
+    int              which_level_being_advanced = -1; //!< Only >=0 if we are in Amr::timeStep(level,...)
 
     int              record_grid_info;
     int              record_run_info;

diff --git a/Src/Amr/AMReX_Amr.cpp b/Src/Amr/AMReX_Amr.cpp
@@ -416,6 +416,7 @@ Amr::InitAmr ()
         int in_finest,ngrid;
 
         is >> in_finest;
+        AMREX_ASSERT(in_finest > 0 && in_finest < std::numeric_limits<int>::max());
         STRIP;
         initial_ba.resize(in_finest);
 
@@ -428,6 +429,7 @@ Amr::InitAmr ()
             BoxList bl;
             is >> ngrid;
             STRIP;
+            AMREX_ASSERT(ngrid >= 0 && ngrid < std::numeric_limits<int>::max());
             for (int i = 0; i < ngrid; i++)
             {
                 Box bx;
@@ -458,11 +460,13 @@ Amr::InitAmr ()
 
         is >> in_finest;
         STRIP;
+        AMREX_ASSERT(in_finest >= 0  && in_finest < std::numeric_limits<int>::max());
         regrid_ba.resize(in_finest);
         for (int lev = 1; lev <= in_finest; lev++)
         {
             BoxList bl;
             is >> ngrid;
+            AMREX_ASSERT(ngrid >= 0 && ngrid < std::numeric_limits<int>::max());
             STRIP;
             for (int i = 0; i < ngrid; i++)
             {
@@ -1007,7 +1011,9 @@ Amr::writePlotFileDoit (std::string const& pltfile, bool regular)
             ParallelDescriptor::Barrier("Amr::writePlotFile::end");
             if(ParallelDescriptor::IOProcessor()) {
                 HeaderFile.close();
-                std::rename(pltfileTemp.c_str(), pltfile.c_str());
+                if (std::rename(pltfileTemp.c_str(), pltfile.c_str())) {
+                    amrex::Abort("Amr::writePlotFileDoit: std::rename failed");
+                }
             }
             ParallelDescriptor::Barrier("Renaming temporary plotfile.");
             //
@@ -1460,6 +1466,9 @@ Amr::restart (const std::string& filename)
     is >> mx_lev;
     is >> finest_level;
 
+    AMREX_ASSERT(mx_lev >= 0 && mx_lev < 1000 &&
+                 finest_level >= 0 && finest_level < 1000);
+
     Vector<Box> inputs_domain(max_level+1);
     for (int lev = 0; lev <= max_level; ++lev)
     {
@@ -1837,7 +1846,9 @@ Amr::checkPoint ()
         ParallelDescriptor::Barrier("Amr::checkPoint::end");
         if(ParallelDescriptor::IOProcessor()) {
             HeaderFile.close();
-            std::rename(ckfileTemp.c_str(), ckfile.c_str());
+            if (std::rename(ckfileTemp.c_str(), ckfile.c_str())) {
+                amrex::Abort("Amr::checkPoint: std::rename failed");
+            }
         }
         ParallelDescriptor::Barrier("Renaming temporary checkPoint file.");
     }
@@ -2215,34 +2226,39 @@ Amr::coarseTimeStep (Real stop_time)
             FILE *fp;
             if ((fp=fopen("dump_and_continue","r")) != nullptr) // NOLINT(bugprone-assignment-in-if-condition)
             {
-                remove("dump_and_continue");
+                auto status = remove("dump_and_continue");
+                amrex::ignore_unused(status);
                 to_checkpoint = 1;
                 fclose(fp);
             }
             else if ((fp=fopen("stop_run","r")) != nullptr) // NOLINT(bugprone-assignment-in-if-condition)
             {
-                remove("stop_run");
+                auto status = remove("stop_run");
+                amrex::ignore_unused(status);
                 to_stop = 1;
                 fclose(fp);
             }
             else if ((fp=fopen("dump_and_stop","r")) != nullptr) // NOLINT(bugprone-assignment-in-if-condition)
             {
-                remove("dump_and_stop");
+                auto status = remove("dump_and_stop");
+                amrex::ignore_unused(status);
                 to_checkpoint = 1;
                 to_stop = 1;
                 fclose(fp);
             }
 
             if ((fp=fopen("plot_and_continue","r")) != nullptr) // NOLINT(bugprone-assignment-in-if-condition)
             {
-                remove("plot_and_continue");
+                auto status = remove("plot_and_continue");
+                amrex::ignore_unused(status);
                 to_plot = 1;
                 fclose(fp);
             }
 
             if ((fp=fopen("small_plot_and_continue","r")) != nullptr) // NOLINT(bugprone-assignment-in-if-condition)
             {
-                remove("small_plot_and_continue");
+                auto status = remove("small_plot_and_continue");
+                amrex::ignore_unused(status);
                 to_small_plot = 1;
                 fclose(fp);
             }

diff --git a/Src/Amr/AMReX_AmrLevel.H b/Src/Amr/AMReX_AmrLevel.H
@@ -588,14 +588,14 @@ private:
     MultiFab&                  m_leveldata;
     MultiFabCopyDescriptor     m_mfcd;
     Vector< Vector<MultiFabId> > m_mfid;     // [level][oldnew]
-    Interpolater*              m_map;
+    Interpolater*              m_map = nullptr;
     std::map<int,Box>          m_ba;
-    Real                       m_time;
-    int                        m_growsize;
-    int                        m_index;
-    int                        m_scomp;
-    int                        m_ncomp;
-    bool                       m_FixUpCorners;
+    Real                       m_time = std::numeric_limits<Real>::lowest();
+    int                        m_growsize = std::numeric_limits<int>::lowest();
+    int                        m_index = -1;
+    int                        m_scomp = -1;
+    int                        m_ncomp = -1;
+    bool                       m_FixUpCorners = false;
 
     std::map< int,Vector< Vector<Box> > >                m_fbox; // [grid][level][validregion]
     std::map< int,Vector< Vector<Box> > >                m_cbox; // [grid][level][fillablesubbox]

diff --git a/Src/Amr/AMReX_AmrLevel.cpp b/Src/Amr/AMReX_AmrLevel.cpp
@@ -394,6 +394,8 @@ AmrLevel::restart (Amr&          papa,
     fine_ratio = IntVect::TheUnitVector(); fine_ratio.scale(-1);
     crse_ratio = IntVect::TheUnitVector(); crse_ratio.scale(-1);
 
+    AMREX_ASSERT(level >= 0 && level <= parent->maxLevel());
+
     if (level > 0)
     {
         crse_ratio = parent->refRatio(level-1);
@@ -2074,10 +2076,9 @@ void AmrLevel::constructAreaNotToTag ()
 
         // We disallow tagging in the remaining part of the domain.
         BoxArray tagba = amrex::boxComplement(parent->Geom(level).Domain(),m_AreaToTag);
-        m_AreaNotToTag = tagba;
+        m_AreaNotToTag = std::move(tagba);
 
-        BoxArray bxa(parent->Geom(level).Domain());
-        BL_ASSERT(bxa.contains(m_AreaNotToTag));
+        BL_ASSERT(parent->Geom(level).Domain().contains(m_AreaNotToTag.minimalBox()));
     }
 
     if (parent->useFixedUpToLevel()<level)
@@ -2087,7 +2088,7 @@ void AmrLevel::constructAreaNotToTag ()
         tagarea.grow(-parent->blockingFactor(level));
         m_AreaToTag = tagarea;
         BoxArray tagba = amrex::boxComplement(parent->Geom(level).Domain(),m_AreaToTag);
-        m_AreaNotToTag = tagba;
+        m_AreaNotToTag = std::move(tagba);
     }
 }
 

diff --git a/Src/Amr/AMReX_StateData.cpp b/Src/Amr/AMReX_StateData.cpp
@@ -221,6 +221,7 @@ StateData::restartDoit (std::istream& is, const std::string& chkfile)
 
     int nsets;
     is >> nsets;
+    AMREX_ASSERT(nsets >= 0 && nsets <= 2);
 
     new_data = std::make_unique<MultiFab>(grids,dmap,desc->nComp(),desc->nExtra(),
                                           MFInfo().SetTag("StateData").SetArena(arena),

diff --git a/Src/AmrCore/AMReX_AmrMesh.cpp b/Src/AmrCore/AMReX_AmrMesh.cpp
@@ -78,6 +78,7 @@ AmrMesh::InitAmrMesh (int max_level_in, const Vector<int>& n_cell_in,
     } else {
        max_level = max_level_in;
     }
+    AMREX_ASSERT(max_level >= 0 && max_level < 1000);
 
     int nlev = max_level + 1;
 

diff --git a/Src/Base/AMReX.H b/Src/Base/AMReX.H
@@ -109,7 +109,7 @@ namespace amrex
     //! Print out message to cerr and exit via amrex::Abort().
     void Error (const std::string& msg);
 
-    void Error_host (const char* msg);
+    void Error_host (const char* type, const char* msg);
 
     AMREX_GPU_HOST_DEVICE AMREX_FORCE_INLINE
     void Error (const char* msg = nullptr) {
@@ -121,7 +121,7 @@ namespace amrex
         AMREX_DEVICE_ASSERT(0);
 #endif
 #else
-        Error_host(msg);
+        Error_host("Error", msg);
 #endif
     }
 
@@ -146,8 +146,6 @@ namespace amrex
     //! Print out message to cerr and exit via abort().
     void Abort (const std::string& msg);
 
-    void Abort_host (const char * msg);
-
     AMREX_GPU_HOST_DEVICE AMREX_FORCE_INLINE
     void Abort (const char * msg = nullptr) {
 #if AMREX_DEVICE_COMPILE
@@ -158,7 +156,7 @@ namespace amrex
         AMREX_DEVICE_ASSERT(0);
 #endif
 #else
-        Abort_host(msg);
+        Error_host("Abort", msg);
 #endif
     }
 

diff --git a/Src/Base/AMReX.cpp b/Src/Base/AMReX.cpp
@@ -78,6 +78,16 @@
 #include <vector>
 #include <algorithm>
 
+#ifdef AMREX_USE_COVERITY
+namespace {
+    // coverity[+kill]
+    void amrex_coverity_abort()
+    {
+        std::exit(EXIT_FAILURE);
+    }
+}
+#endif
+
 namespace amrex {
 
 std::vector<std::unique_ptr<AMReX> > AMReX::m_instance;
@@ -206,20 +216,25 @@ amrex::Warning (const std::string& msg)
 }
 
 void
-amrex::Error_host (const char * msg)
+amrex::Error_host (const char* type, const char * msg)
 {
+    amrex::ignore_unused(type);
+#ifdef AMREX_USE_COVERITY
+    amrex_coverity_abort();
+#else
     if (system::error_handler) {
         system::error_handler(msg);
     } else if (system::throw_exception) {
         throw RuntimeError(msg);
     } else {
-        write_lib_id("Error");
+        write_lib_id(type);
         write_to_stderr_without_buffering(msg);
 #ifdef AMREX_USE_OMP
 #pragma omp critical (amrex_abort_omp_critical)
 #endif
         ParallelDescriptor::Abort();
     }
+#endif
 }
 
 void
@@ -230,26 +245,12 @@ amrex::Warning_host (const char * msg)
     }
 }
 
-void
-amrex::Abort_host (const char * msg)
-{
-    if (system::error_handler) {
-        system::error_handler(msg);
-    } else if (system::throw_exception) {
-        throw RuntimeError(msg);
-    } else {
-       write_lib_id("Abort");
-       write_to_stderr_without_buffering(msg);
-#ifdef AMREX_USE_OMP
-#pragma omp critical (amrex_abort_omp_critical)
-#endif
-       ParallelDescriptor::Abort();
-   }
-}
-
 void
 amrex::Assert_host (const char* EX, const char* file, int line, const char* msg)
 {
+#ifdef AMREX_USE_COVERITY
+    amrex_coverity_abort();
+#else
     const int N = 512;
 
     char buf[N];
@@ -282,6 +283,7 @@ amrex::Assert_host (const char* EX, const char* file, int line, const char* msg)
 #endif
        ParallelDescriptor::Abort();
    }
+#endif
 }
 
 namespace

diff --git a/Src/Base/AMReX_BoxArray.cpp b/Src/Base/AMReX_BoxArray.cpp
@@ -100,6 +100,7 @@ BARef::define (std::istream& is, int& ndims)
     int   maxbox;
     ULong tmphash;
     is.ignore(bl_ignore_max, '(') >> maxbox >> tmphash;
+    AMREX_ASSERT(maxbox >= 0 && maxbox < std::numeric_limits<int>::max());
     resize(maxbox);
     auto pos = is.tellg();
     {
@@ -1440,9 +1441,7 @@ BoxArray::removeOverlap (bool simplify)
         bl.simplify();
     }
 
-    BoxArray nba(std::move(bl));
-
-    *this = nba;
+    *this = BoxArray(std::move(bl));
 
 #ifdef AMREX_MEM_PROFILING
     m_ref->total_hash_bytes = total_hash_bytes_save;
@@ -1813,6 +1812,7 @@ readBoxArray (BoxArray&     ba,
         int maxbox;
         ULong in_hash; // will be ignored
         is.ignore(bl_ignore_max, '(') >> maxbox >> in_hash;
+        AMREX_ASSERT(maxbox >= 0 && maxbox < std::numeric_limits<int>::max());
         ba.resize(maxbox);
         for (int i = 0; i < maxbox; i++)
         {

diff --git a/Src/Base/AMReX_DistributionMapping.cpp b/Src/Base/AMReX_DistributionMapping.cpp
@@ -1990,6 +1990,7 @@ DistributionMapping::readFrom (std::istream& is)
 
     int n;
     is.ignore(100000, '(') >> n;
+    AMREX_ASSERT(n >= 0 && n < std::numeric_limits<int>::max());
     pmap.resize(n);
     for (auto& x : pmap) {
         is >> x;
@@ -2049,7 +2050,7 @@ DistributionMapping MakeSimilarDM (const BoxArray& ba, const BoxArray& src_ba,
                     max_overlap_index = gid;
                 }
             }
-            AMREX_ASSERT(max_overlap > 0);
+            AMREX_ASSERT(max_overlap > 0 && max_overlap_index >= 0);
             pmap[i] = src_dm[max_overlap_index];
         }
     }