Fix undefined behavior flagged by UBSAN
authorKevin Boyd <kevin44boyd@gmail.com>
Wed, 29 Jul 2020 11:42:59 +0000 (11:42 +0000)
committerPaul Bauer <paul.bauer.q@gmail.com>
Wed, 29 Jul 2020 11:42:59 +0000 (11:42 +0000)
Running under the UBSAN sanitizer pointed out a bunch of undefined
behavior - most of them were harmless issues in rarely taken branches,
but should still be avoided.

22 files changed:
CMakeLists.txt
admin/gitlab-ci/gromacs.gitlab-ci.yml
admin/ubsan-suppressions.txt [new file with mode: 0644]
cmake/gmxBuildTypeUBSAN.cmake [new file with mode: 0644]
src/config.h.cmakein
src/gromacs/applied_forces/tests/electricfield.cpp
src/gromacs/domdec/domdec_topology.cpp
src/gromacs/fileio/xdrd.cpp
src/gromacs/gmxpreprocess/gpp_nextnb.cpp
src/gromacs/gmxpreprocess/pdb2gmx.cpp
src/gromacs/mdrun/md.cpp
src/gromacs/mdrun/minimize.cpp
src/gromacs/mdrun/rerun.cpp
src/gromacs/mdrun/runner.cpp
src/gromacs/mdrunutility/threadaffinity.cpp
src/gromacs/nbnxm/pairlist.cpp
src/gromacs/onlinehelp/tests/helpwritercontext.cpp
src/gromacs/selection/indexutil.cpp
src/gromacs/selection/parsetree.cpp
src/gromacs/selection/sm_same.cpp
src/gromacs/utility/arrayref.h
src/gromacs/utility/loggerbuilder.h

index 3e902192dd644b3a3448081b0adcb719a398d43d..cbc6a16be48a01e28662ca701923c0f971cc38a0 100644 (file)
@@ -90,13 +90,14 @@ include(gmxBuildTypeProfile)
 include(gmxBuildTypeTSAN)
 include(gmxBuildTypeASAN)
 include(gmxBuildTypeMSAN)
+include(gmxBuildTypeUBSAN)
 include(gmxBuildTypeReleaseWithAssert)
 
 if(NOT CMAKE_BUILD_TYPE)
-    set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel Reference RelWithAssert Profile TSAN ASAN MSAN." FORCE)
+    set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel Reference RelWithAssert Profile TSAN ASAN MSAN UBSAN." FORCE)
     # Set the possible values of build type for cmake-gui
     set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release"
-        "MinSizeRel" "RelWithDebInfo" "Reference" "RelWithAssert" "Profile" "TSAN" "ASAN" "MSAN")
+        "MinSizeRel" "RelWithDebInfo" "Reference" "RelWithAssert" "Profile" "TSAN" "ASAN" "MSAN" "UBSAN")
 endif()
 if(CMAKE_CONFIGURATION_TYPES)
     # Add appropriate GROMACS-specific build types for the Visual
index a74b8d1aa4a2871b96e021449bed5fc4d354389d..1c688dfa818986cb392a719d1d43e72a57db469b 100644 (file)
@@ -290,6 +290,16 @@ gromacs:clang-ASAN:configure:
     COMPILER_MAJOR_VERSION: 8
     CMAKE_BUILD_TYPE_OPTIONS: "-DCMAKE_BUILD_TYPE=ASAN"
 
+gromacs:clang-UBSAN:configure:
+  extends:
+    - .gromacs:base:configure
+    - .use-clang:base
+    - .rules:merge-requests
+  image: gromacs/cmake-3.13.0-llvm-8-tsan:master
+  variables:
+    COMPILER_MAJOR_VERSION: 8
+    CMAKE_BUILD_TYPE_OPTIONS: "-DCMAKE_BUILD_TYPE=UBSAN"
+
 gromacs:gcc-8-cuda-10.1:release:configure:
   extends:
     - .gromacs:base:release:configure
@@ -435,6 +445,19 @@ gromacs:clang-ASAN:build:
   needs:
     - job: gromacs:clang-ASAN:configure
 
+gromacs:clang-UBSAN:build:
+  extends:
+    - .variables:default
+    - .gromacs:base:build
+    - .use-clang:base
+    - .use-ccache
+    - .rules:merge-requests
+  image: gromacs/cmake-3.13.0-llvm-8-tsan:master
+  tags:
+    - k8s-scilifelab
+  needs:
+    - job: gromacs:clang-UBSAN:configure
+
 gromacs:clang-static-analyzer:build:
   extends:
     - .variables:default
@@ -539,6 +562,7 @@ gromacs:clang-8-cuda-10.1:release:build:
     CTEST_RUN_MODE: "ExperimentalTest"
   script:
     - cd $BUILD_DIR
+    - export UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1:suppressions=$CI_PROJECT_DIR/admin/ubsan-suppressions.txt
     # Needed to run MPI enabled code in the docker images, until we set up different users
     - export OMPI_ALLOW_RUN_AS_ROOT=1
     - export OMPI_ALLOW_RUN_AS_ROOT_CONFIRM=1
@@ -649,6 +673,17 @@ gromacs:clang-ASAN:test:
   needs:
     - job: gromacs:clang-ASAN:build
 
+gromacs:clang-UBSAN:test:
+  extends:
+    - .gromacs:base:test
+    - .use-clang:base
+    - .rules:merge-requests
+  image: gromacs/cmake-3.13.0-llvm-8-tsan:master
+  tags:
+    - k8s-scilifelab
+  needs:
+    - job: gromacs:clang-UBSAN:build
+
 gromacs:clang-9-mpi:test:
   extends:
     - .gromacs:base:test
diff --git a/admin/ubsan-suppressions.txt b/admin/ubsan-suppressions.txt
new file mode 100644 (file)
index 0000000..055ba22
--- /dev/null
@@ -0,0 +1,9 @@
+# Add UBSAN suppressions here, with a leading comment about how and
+# why it is appropriate, and any circumstances about when it should be
+# removed.
+
+# This is only a problem when using the built-in XDR implementation of
+# xdr_vector. It supplies an extra argument needed for xdr_string
+# which is unused by other xdr_* primitives like xdr_float. The issue
+# is harmless and only active when not using a system XDR library.
+function:xdr_vector
diff --git a/cmake/gmxBuildTypeUBSAN.cmake b/cmake/gmxBuildTypeUBSAN.cmake
new file mode 100644 (file)
index 0000000..3e26087
--- /dev/null
@@ -0,0 +1,43 @@
+#
+# This file is part of the GROMACS molecular simulation package.
+#
+# Copyright (c) 2020, by the GROMACS development team, led by
+# Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
+# and including many others, as listed in the AUTHORS file in the
+# top-level source directory and at http://www.gromacs.org.
+#
+# GROMACS is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public License
+# as published by the Free Software Foundation; either version 2.1
+# of the License, or (at your option) any later version.
+#
+# GROMACS is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with GROMACS; if not, see
+# http://www.gnu.org/licenses, or write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+#
+# If you want to redistribute modifications to GROMACS, please
+# consider that scientific software is very special. Version
+# control is crucial - bugs must be traceable. We will be happy to
+# consider code for inclusion in the official distribution, but
+# derived work must not be called official GROMACS. Details are found
+# in the README & COPYING files - if they are missing, get the
+# official version at http://www.gromacs.org.
+#
+# To help us fund GROMACS development, we humbly ask that you cite
+# the research papers on the package. Check out http://www.gromacs.org.
+
+# Custom build type "UBSAN", to be used to run the
+# undefined behaviour memory checker.
+
+set(_flags "-O1 -g -fsanitize=undefined -fno-omit-frame-pointer")
+foreach(_language C CXX)
+    string(REPLACE "X" "+" _human_readable_language ${_language})
+    set(CMAKE_${_language}_FLAGS_UBSAN ${_flags} CACHE STRING "${_human_readable_language} flags for undefined behavior sanitizer" FORCE)
+    mark_as_advanced(CMAKE_${_language}_FLAGS_UBSAN)
+endforeach()
index bd831a529bf12fb9163742542c59aaaf4f716f3e..524ff47a535cfa47f62132d3e8e7caf30d035788 100644 (file)
 #define CMAKE_BUILD_TYPE_TSAN 8
 #define CMAKE_BUILD_TYPE_ASAN 9
 #define CMAKE_BUILD_TYPE_MSAN 10
+#define CMAKE_BUILD_TYPE_UBSAN 11
 #cmakedefine CMAKE_BUILD_TYPE CMAKE_BUILD_TYPE_@CMAKE_BUILD_TYPE_UPPER@
 
 /* Define relative path to OpenCL kernels */
index cf7ed342eee16e4ec8ea4b37c1a9457594b5590e..8e19955ae37654f42471b1793933e171251ec962 100644 (file)
@@ -48,6 +48,7 @@
 #include "gromacs/gmxlib/network.h"
 #include "gromacs/math/paddedvector.h"
 #include "gromacs/mdlib/forcerec.h"
+#include "gromacs/mdtypes/commrec.h"
 #include "gromacs/mdtypes/enerdata.h"
 #include "gromacs/mdtypes/forceoutput.h"
 #include "gromacs/mdtypes/iforceprovider.h"
@@ -107,11 +108,11 @@ public:
         // Prepare a ForceProviderInput
         t_mdatoms         md;
         std::vector<real> chargeA{ 1 };
-        md.homenr                   = ssize(chargeA);
-        md.chargeA                  = chargeA.data();
-        CommrecHandle      cr       = init_commrec(MPI_COMM_WORLD);
+        md.homenr  = ssize(chargeA);
+        md.chargeA = chargeA.data();
+        t_commrec          cr;
         matrix             boxDummy = { { 0, 0, 0 }, { 0, 0, 0 }, { 0, 0, 0 } };
-        ForceProviderInput forceProviderInput({}, md, 0.0, boxDummy, *cr);
+        ForceProviderInput forceProviderInput({}, md, 0.0, boxDummy, cr);
 
         // Prepare a ForceProviderOutput
         PaddedVector<RVec>  f = { { 0, 0, 0 } };
index 0332a60ec4e267cc71acec83cd7a4e5630ba7534..fe4f8c82ee82a9e53ba9bd4cf05cadd865f0d595 100644 (file)
@@ -121,11 +121,11 @@ struct thread_work_t
      */
     thread_work_t(const gmx_ffparams_t& ffparams) : idef(ffparams) {}
 
-    InteractionDefinitions         idef;       /**< Partial local topology */
-    std::unique_ptr<gmx::VsitePbc> vsitePbc;   /**< vsite PBC structure */
-    int                            nbonded;    /**< The number of bondeds in this struct */
-    ListOfLists<int>               excl;       /**< List of exclusions */
-    int                            excl_count; /**< The total exclusion count for \p excl */
+    InteractionDefinitions         idef;               /**< Partial local topology */
+    std::unique_ptr<gmx::VsitePbc> vsitePbc = nullptr; /**< vsite PBC structure */
+    int                            nbonded  = 0;       /**< The number of bondeds in this struct */
+    ListOfLists<int>               excl;               /**< List of exclusions */
+    int                            excl_count = 0;     /**< The total exclusion count for \p excl */
 };
 
 /*! \brief Struct for the reverse topology: links bonded interactions to atomsx */
index 45175a5d6aeeeb560de6b633c6a87fa16c83a682..d954678b2036b97603c48dcc08ddb0714cd44717 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -114,20 +114,17 @@ int xdr_int64(XDR* xdrs, int64_t* i)
     // requires 64-bit systems.
     static_assert(2 * sizeof(int) >= 8,
                   "XDR handling assumes that an int64_t can be stored in two ints");
-    int imaj, imin;
-    int ret;
 
-    static const int64_t two_p32_m1 = 0xFFFFFFFF;
-    int64_t              imaj64, imin64;
+    static const uint64_t two_p32_m1 = 0xFFFFFFFF;
 
-    imaj64 = ((*i) >> 32) & two_p32_m1;
-    imin64 = (*i) & two_p32_m1;
-    imaj   = static_cast<int>(imaj64);
-    imin   = static_cast<int>(imin64);
-    ret    = xdr_int(xdrs, &imaj);
+    uint64_t imaj64 = ((*i) >> 32) & two_p32_m1;
+    uint64_t imin64 = (*i) & two_p32_m1;
+    int      imaj   = static_cast<int>(imaj64);
+    int      imin   = static_cast<int>(imin64);
+    int      ret    = xdr_int(xdrs, &imaj);
     ret |= xdr_int(xdrs, &imin);
 
-    *i = ((static_cast<int64_t>(imaj) << 32) | (static_cast<int64_t>(imin) & two_p32_m1));
+    *i = ((static_cast<uint64_t>(imaj) << 32) | (static_cast<uint64_t>(imin) & two_p32_m1));
 
     return ret;
 }
index 5f6801a4dc67b7efb80d080c4df3a7f89b8add57..e4b360bd1f4aadfbfe88a97396d107138540be03 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2011,2014,2015,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2011,2014,2015,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -399,8 +399,10 @@ static void sort_and_purge_nnb(t_nextnb* nnb)
         for (n = 0; (n <= nnb->nrex); n++)
         {
             /* Sort atoms in this list */
-            qsort(nnb->a[i][n], nnb->nrexcl[i][n], sizeof(int), compare_int);
-
+            if (nnb->nrexcl[i][n] > 0)
+            {
+                qsort(nnb->a[i][n], nnb->nrexcl[i][n], sizeof(int), compare_int);
+            }
             cnt  = 0;
             prev = -1;
             for (j = 0; j < nnb->nrexcl[i][n]; j++)
index 8a9e08746cb4057121d64c61f63084e7a22173f3..6c76adfd97e088aba830407454bf7966cfb6e1b0 100644 (file)
@@ -1345,6 +1345,10 @@ public:
         itp_file_(nullptr),
         mHmult_(0)
     {
+        gmx::LoggerBuilder builder;
+        builder.addTargetStream(gmx::MDLogger::LogLevel::Info, &gmx::TextOutputFile::standardOutput());
+        builder.addTargetStream(gmx::MDLogger::LogLevel::Warning, &gmx::TextOutputFile::standardError());
+        loggerOwner_ = std::make_unique<LoggerOwner>(builder.build());
     }
 
     // From ICommandLineOptionsModule
@@ -1405,15 +1409,15 @@ private:
     WaterType           waterType_;
     MergeType           mergeType_;
 
-    FILE*                          itp_file_;
-    char                           forcefield_[STRLEN];
-    char                           ffdir_[STRLEN];
-    char*                          ffname_;
-    char*                          watermodel_;
-    std::vector<std::string>       incls_;
-    std::vector<t_mols>            mols_;
-    real                           mHmult_;
-    std::unique_ptr<gmx::MDLogger> loggerPointer_;
+    FILE*                        itp_file_;
+    char                         forcefield_[STRLEN];
+    char                         ffdir_[STRLEN];
+    char*                        ffname_;
+    char*                        watermodel_;
+    std::vector<std::string>     incls_;
+    std::vector<t_mols>          mols_;
+    real                         mHmult_;
+    std::unique_ptr<LoggerOwner> loggerOwner_;
 };
 
 void pdb2gmx::initOptions(IOptionsContainer* options, ICommandLineOptionsModuleSettings* settings)
@@ -1693,7 +1697,7 @@ void pdb2gmx::optionsFinished()
 
     /* Force field selection, interactive or direct */
     choose_ff(strcmp(ff_.c_str(), "select") == 0 ? nullptr : ff_.c_str(), forcefield_,
-              sizeof(forcefield_), ffdir_, sizeof(ffdir_), *loggerPointer_);
+              sizeof(forcefield_), ffdir_, sizeof(ffdir_), loggerOwner_->logger());
 
     if (strlen(forcefield_) > 0)
     {
@@ -1726,12 +1730,7 @@ int pdb2gmx::run()
     int         this_chainstart;
     int         prev_chainstart;
 
-    gmx::LoggerBuilder builder;
-    builder.addTargetStream(gmx::MDLogger::LogLevel::Info, &gmx::TextOutputFile::standardOutput());
-    builder.addTargetStream(gmx::MDLogger::LogLevel::Warning, &gmx::TextOutputFile::standardError());
-    gmx::LoggerOwner logOwner(builder.build());
-    loggerPointer_              = std::make_unique<gmx::MDLogger>(logOwner.logger());
-    const gmx::MDLogger& logger = *loggerPointer_;
+    const gmx::MDLogger& logger = loggerOwner_->logger();
 
     GMX_LOG(logger.info)
             .asParagraph()
index bae77f14de0df5ec7eb6da270b28d16b275d77d6..3f563e201633143afcf33ee34406687caa0848bc 100644 (file)
@@ -253,7 +253,9 @@ void gmx::LegacySimulator::do_md()
                   "Either specify the -ei option to mdrun, or do not use this checkpoint file.");
     }
 
-    initialize_lambdas(fplog, *ir, MASTER(cr), &state_global->fep_state, state_global->lambda, lam0);
+    int*                fep_state = MASTER(cr) ? &state_global->fep_state : nullptr;
+    gmx::ArrayRef<real> lambda    = MASTER(cr) ? state_global->lambda : gmx::ArrayRef<real>();
+    initialize_lambdas(fplog, *ir, MASTER(cr), fep_state, lambda, lam0);
     Update     upd(*ir, deform);
     const bool doSimulatedAnnealing = initSimulatedAnnealing(ir, &upd);
     const bool useReplicaExchange   = (replExParams.exchangeInterval > 0);
index c67ff09cc27e580a5f39da5d5f009f9b4bc64143..c96ef39acd12aa0c80e94a612e5d1b26d77fc633 100644 (file)
@@ -389,7 +389,9 @@ static void init_em(FILE*                fplog,
     {
         state_global->ngtc = 0;
     }
-    initialize_lambdas(fplog, *ir, MASTER(cr), &(state_global->fep_state), state_global->lambda, nullptr);
+    int*                fep_state = MASTER(cr) ? &state_global->fep_state : nullptr;
+    gmx::ArrayRef<real> lambda    = MASTER(cr) ? state_global->lambda : gmx::ArrayRef<real>();
+    initialize_lambdas(fplog, *ir, MASTER(cr), fep_state, lambda, nullptr);
 
     if (ir->eI == eiNM)
     {
@@ -2543,7 +2545,7 @@ void LegacySimulator::do_steep()
         }
 
         /* Send IMD energies and positions, if bIMD is TRUE. */
-        if (imdSession->run(count, TRUE, state_global->box,
+        if (imdSession->run(count, TRUE, MASTER(cr) ? state_global->box : nullptr,
                             MASTER(cr) ? state_global->x.rvec_array() : nullptr, 0)
             && MASTER(cr))
         {
index 2d12115cf5da075fa51dabcb8469a2ce771e2acd..c241a6b4993c8ad70982b9e9035bc8f055fce99a 100644 (file)
@@ -277,8 +277,9 @@ void gmx::LegacySimulator::do_rerun()
         auto nonConstGlobalTopology                          = const_cast<gmx_mtop_t*>(top_global);
         nonConstGlobalTopology->intermolecularExclusionGroup = genQmmmIndices(*top_global);
     }
-
-    initialize_lambdas(fplog, *ir, MASTER(cr), &state_global->fep_state, state_global->lambda, lam0);
+    int*                fep_state = MASTER(cr) ? &state_global->fep_state : nullptr;
+    gmx::ArrayRef<real> lambda    = MASTER(cr) ? state_global->lambda : gmx::ArrayRef<real>();
+    initialize_lambdas(fplog, *ir, MASTER(cr), fep_state, lambda, lam0);
     const bool        simulationsShareState = false;
     gmx_mdoutf*       outf = init_mdoutf(fplog, nfile, fnm, mdrunOptions, cr, outputProvider,
                                    mdModulesNotifier, ir, top_global, oenv, wcycle,
index b90bc9aea50cf0cafa4a174e934731c015eaf20a..a32dc97a706f6bc91da652cc99a623b190b4b816 100644 (file)
@@ -1051,9 +1051,9 @@ int Mdrunner::mdrunner()
     snew(oriresdata, 1);
     init_orires(fplog, &mtop, inputrec, cr, ms, globalState.get(), oriresdata);
 
-    auto deform = prepareBoxDeformation(globalState->box, MASTER(cr) ? DDRole::Master : DDRole::Agent,
-                                        PAR(cr) ? NumRanks::Multiple : NumRanks::Single,
-                                        cr->mpi_comm_mygroup, *inputrec);
+    auto deform = prepareBoxDeformation(
+            globalState != nullptr ? globalState->box : box, MASTER(cr) ? DDRole::Master : DDRole::Agent,
+            PAR(cr) ? NumRanks::Multiple : NumRanks::Single, cr->mpi_comm_mygroup, *inputrec);
 
     ObservablesHistory observablesHistory = {};
 
@@ -1098,7 +1098,7 @@ int Mdrunner::mdrunner()
     /* override nsteps with value set on the commandline */
     override_nsteps_cmdline(mdlog, mdrunOptions.numStepsCommandline, inputrec);
 
-    if (SIMMASTER(cr))
+    if (isSimulationMasterRank)
     {
         copy_mat(globalState->box, box);
     }
@@ -1776,7 +1776,7 @@ int Mdrunner::mdrunner()
     /* we need to join all threads. The sub-threads join when they
        exit this function, but the master thread needs to be told to
        wait for that. */
-    if (PAR(cr) && MASTER(cr))
+    if (MASTER(cr))
     {
         tMPI_Finalize();
     }
index a2d08a0f59656a514ef0bf749fa8636b722b3bfb..8cf377acbf160ab0ee2c720f86bea938eecc880b 100644 (file)
@@ -541,10 +541,8 @@ static bool detectDefaultAffinityMask(const int nthreads_hw_avail)
     MPI_Initialized(&mpiIsInitialized);
     if (mpiIsInitialized)
     {
-        bool detectedDefaultAffinityMaskOnAllRanks;
-        MPI_Allreduce(&detectedDefaultAffinityMask, &detectedDefaultAffinityMaskOnAllRanks, 1,
-                      MPI_C_BOOL, MPI_LAND, MPI_COMM_WORLD);
-        detectedDefaultAffinityMask = detectedDefaultAffinityMaskOnAllRanks;
+        bool maskToReduce = detectedDefaultAffinityMask;
+        MPI_Allreduce(&maskToReduce, &detectedDefaultAffinityMask, 1, MPI_C_BOOL, MPI_LAND, MPI_COMM_WORLD);
     }
 #endif
 
index 253a4cd97ae770259d47d1b839b2afaf83c0bab2..ad09daed08485454e8bc888c9dc98cd60b7ab1ad 100644 (file)
@@ -3894,7 +3894,7 @@ static Range<int> getIZoneRange(const Nbnxm::GridSet::DomainSetup& domainSetup,
 }
 
 /* Returns the j-zone range for pairlist construction for the give locality and i-zone */
-static Range<int> getJZoneRange(const gmx_domdec_zones_t& ddZones,
+static Range<int> getJZoneRange(const gmx_domdec_zones_t* ddZones,
                                 const InteractionLocality locality,
                                 const int                 iZone)
 {
@@ -3906,12 +3906,12 @@ static Range<int> getJZoneRange(const gmx_domdec_zones_t& ddZones,
     else if (iZone == 0)
     {
         /* Non-local: we need to avoid the local (zone 0 vs 0) interactions */
-        return { 1, *ddZones.iZones[iZone].jZoneRange.end() };
+        return { 1, *ddZones->iZones[iZone].jZoneRange.end() };
     }
     else
     {
         /* Non-local with non-local i-zone: use all j-zones */
-        return ddZones.iZones[iZone].jZoneRange;
+        return ddZones->iZones[iZone].jZoneRange;
     }
 }
 
@@ -3977,7 +3977,9 @@ void PairlistSet::constructPairlists(const Nbnxm::GridSet&         gridSet,
         }
     }
 
-    const gmx_domdec_zones_t& ddZones = *gridSet.domainSetup().zones;
+    const gmx_domdec_zones_t* ddZones = gridSet.domainSetup().zones;
+    GMX_ASSERT(locality_ == InteractionLocality::Local || ddZones != nullptr,
+               "Nonlocal interaction locality with null ddZones.");
 
     const auto iZoneRange = getIZoneRange(gridSet.domainSetup(), locality_);
 
@@ -4003,7 +4005,7 @@ void PairlistSet::constructPairlists(const Nbnxm::GridSet&         gridSet,
             /* With GPU: generate progressively smaller lists for
              * load balancing for local only or non-local with 2 zones.
              */
-            progBal = (locality_ == InteractionLocality::Local || ddZones.n <= 2);
+            progBal = (locality_ == InteractionLocality::Local || ddZones->n <= 2);
 
 #pragma omp parallel for num_threads(numLists) schedule(static)
             for (int th = 0; th < numLists; th++)
index eba2f1abf6f0313fa40df9b8b10c756c3b88dac7..bab07ea99ce1572e864de0a957fcaf52ef7dcb74 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2017,2019, by the GROMACS development team, led by
+ * Copyright (c) 2015,2017,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -50,6 +50,7 @@
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/stringutil.h"
+#include "gromacs/utility/textwriter.h"
 
 #include "testutils/stringtest.h"
 
@@ -65,7 +66,9 @@ class HelpWriterContextTest : public gmx::test::StringTestBase
 public:
     void testFormatting(const std::string& text, gmx::HelpOutputFormat format, const char* id)
     {
-        gmx::HelpWriterContext context(nullptr, format);
+        FILE*                  fp = nullptr;
+        gmx::TextWriter        writerStub(fp);
+        gmx::HelpWriterContext context(&writerStub, format);
         std::string            result = context.substituteMarkupAndWrapToString(settings_, text);
         if (id == nullptr)
         {
index 55a38ee9616d1f472cb38e09be5f6231b8034222..5a6a4507bc3845ce3d261a1d3933d56d8f3eb94c 100644 (file)
@@ -849,7 +849,10 @@ void gmx_ana_index_make_block(t_blocka* t, const gmx_mtop_t* top, gmx_ana_index_
             srenew(t->a, g->isize);
             t->nalloc_a = g->isize;
         }
-        std::memcpy(t->a, g->index, g->isize * sizeof(*(t->a)));
+        if (t->nra > 0)
+        {
+            std::memcpy(t->a, g->index, g->isize * sizeof(*(t->a)));
+        }
     }
 
     /* Allocate memory for the block index. We don't know in advance
@@ -1295,7 +1298,10 @@ void gmx_ana_indexmap_copy(gmx_ana_indexmap_t* dest, gmx_ana_indexmap_t* src, bo
         dest->b.nra = src->b.nra;
         std::memcpy(dest->orgid, src->orgid, dest->b.nr * sizeof(*dest->orgid));
         std::memcpy(dest->b.index, src->b.index, (dest->b.nr + 1) * sizeof(*dest->b.index));
-        std::memcpy(dest->b.a, src->b.a, dest->b.nra * sizeof(*dest->b.a));
+        if (dest->b.nra > 0)
+        {
+            std::memcpy(dest->b.a, src->b.a, dest->b.nra * sizeof(*dest->b.a));
+        }
     }
     dest->mapb.nr  = src->mapb.nr;
     dest->mapb.nra = src->mapb.nra;
index 1294a138013bc7ff562fb3135781450c58d35b6e..d7e536358e8a8d8225215d90d8b38fb2fa002973 100644 (file)
@@ -497,7 +497,10 @@ void _gmx_selelem_init_method_params(const gmx::SelectionTreeElementPointer& sel
     nparams  = sel->u.expr.method->nparams;
     orgparam = sel->u.expr.method->param;
     snew(param, nparams);
-    memcpy(param, orgparam, nparams * sizeof(gmx_ana_selparam_t));
+    if (nparams > 0)
+    {
+        memcpy(param, orgparam, nparams * sizeof(gmx_ana_selparam_t));
+    }
     for (i = 0; i < nparams; ++i)
     {
         param[i].flags &= ~SPAR_SET;
index c3aa2123da4bb75642ab4f45ae8016ef65ca601b..abcd1745cdd29eceaa1c86a89ba7321cec6d753d 100644 (file)
@@ -513,8 +513,11 @@ static void evaluate_same_str(const gmx::SelMethodEvalContext& /*context*/,
     while (j < g->isize)
     {
         /* Do a binary search of the strings. */
-        void* ptr;
-        ptr = bsearch(&d->val.s[j], d->as_s_sorted, d->nas, sizeof(d->as_s_sorted[0]), &cmp_str);
+        void* ptr = nullptr;
+        if (d->nas > 0)
+        {
+            ptr = bsearch(&d->val.s[j], d->as_s_sorted, d->nas, sizeof(d->as_s_sorted[0]), &cmp_str);
+        }
         /* Check whether the value was found in the as list. */
         if (ptr == nullptr)
         {
index 419450c5c3f1ca5460ebde17b3b2f82b0a2cb94c..f2c852b9703b40d14507149d376ffd3767fc6c10 100644 (file)
@@ -78,7 +78,7 @@ struct ArrayRefIter :
     constexpr ArrayRefIter(ArrayRefIter<std::remove_const_t<T2>> it) noexcept : it_(&*it)
     {
     }
-
+    constexpr T*            data() const noexcept { return it_; }
     constexpr T&            operator*() const noexcept { return *it_; }
     constexpr ArrayRefIter& operator+=(std::ptrdiff_t i) noexcept
     {
@@ -270,12 +270,12 @@ public:
         return begin_[n];
     }
     //! Returns the first element.
-    reference front() const { return *begin_; }
+    reference front() const { return *(begin_); }
     //! Returns the first element.
     reference back() const { return *(end_ - 1); }
 
     //! Returns a raw pointer to the contents of the array.
-    pointer data() const { return &*begin_; }
+    pointer data() const { return begin_.data(); }
 
     /*! \brief
      * Swaps referenced memory with the other object.
index 1cc1e8a7adddc5e6121ed22837a9b3afbde88c7d..5948a01e6c5c21ef1b6459ab302fee613ce60f9d 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2016,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2016,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -111,7 +111,7 @@ private:
  *
  * This class is responsible of managing all memory allocated by LoggerBuilder
  * that is needed for operation of the actual logger.  Also the actual logger
- * instance is owned by this class.  This allows keeing the actual logger simple
+ * instance is owned by this class.  This allows keeping the actual logger simple
  * and streamlined.
  *
  * This class supports move construction and assignment, which allows