From: Mark Abraham Date: Tue, 13 Aug 2019 14:48:40 +0000 (+0200) Subject: Fix issues for clang-analyzer-8 X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=2cafe3dbdf335f76750d6ab90a03b0d58ab6a3a9;p=alexxy%2Fgromacs.git Fix issues for clang-analyzer-8 pdb2gmx can no longer attempt to add a vsite after issuing a warning that it cannot convert an atom, instead exiting with a fatal error. Fixed bug in gmx order -calcdist Suppressed some analyzer leak warnings of gmock objects. If the leaks are real, ASAN will really find it, and if not we aren't going to fix it in gmock. Change-Id: Ib3290d7722c1e719ce5291cf3db68a87bc7ce89a --- diff --git a/admin/builds/clang-analyzer.py b/admin/builds/clang-analyzer.py index 345ebb4015..1b1129ef95 100644 --- a/admin/builds/clang-analyzer.py +++ b/admin/builds/clang-analyzer.py @@ -35,7 +35,7 @@ # These options need to match the node label in Jenkins and the # capabilities in releng/agents.py for the agent where the analysis is # intended to run. -build_options = ['clang-6', 'clang-static-analyzer-6'] +build_options = ['clang-8', 'clang-static-analyzer-8'] # Policy global variables use_stdlib_through_env_vars = False diff --git a/docs/release-notes/2020/major/tools.rst b/docs/release-notes/2020/major/tools.rst index 3451957b9c..f07f762460 100644 --- a/docs/release-notes/2020/major/tools.rst +++ b/docs/release-notes/2020/major/tools.rst @@ -7,3 +7,8 @@ Improvements to |Gromacs| tools Also, please use the syntax :issue:`number` to reference issues on redmine, without the a space between the colon and number! +Fixed bug in gmx order -calcdist +"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" +The reference position for the distance calculation was calculated +wrongly. + diff --git a/src/external/thread_mpi/src/comm.cpp b/src/external/thread_mpi/src/comm.cpp index 28464fa1ef..86089ba12a 100644 --- a/src/external/thread_mpi/src/comm.cpp +++ b/src/external/thread_mpi/src/comm.cpp @@ -555,8 +555,8 @@ int tMPI_Comm_split(tMPI_Comm comm, int color, int key, tMPI_Comm *newcomm) int i, j; int N = tMPI_Comm_N(comm); volatile tMPI_Comm *newcomm_list; - volatile int colors[MAX_PREALLOC_THREADS]; /* array with the colors - of each thread */ + volatile int colors[MAX_PREALLOC_THREADS] = { 0 }; /* array with the colors + of each thread */ volatile int keys[MAX_PREALLOC_THREADS]; /* same for keys (only one of the threads actually suplies these arrays to the comm diff --git a/src/gromacs/analysisdata/tests/mock_datamodule.cpp b/src/gromacs/analysisdata/tests/mock_datamodule.cpp index 7fdd647eb6..b2551ca655 100644 --- a/src/gromacs/analysisdata/tests/mock_datamodule.cpp +++ b/src/gromacs/analysisdata/tests/mock_datamodule.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2011,2012,2013,2014,2015,2017, by the GROMACS development team, led by + * Copyright (c) 2011,2012,2013,2014,2015,2017,2019, 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. @@ -536,6 +536,7 @@ MockAnalysisDataModule::setupStaticCheck(const AnalysisDataTestInput &data, if (bParallel) { +#ifndef __clang_analyzer__ ::testing::Expectation init = EXPECT_CALL(*this, parallelDataStarted(source, _)) .WillOnce(Return(true)); @@ -573,6 +574,7 @@ MockAnalysisDataModule::setupStaticCheck(const AnalysisDataTestInput &data, } EXPECT_CALL(*this, dataFinished()) .After(framesFinished); +#endif } else { diff --git a/src/gromacs/awh/awh.cpp b/src/gromacs/awh/awh.cpp index 3d035c3ff8..98cfaf04a7 100644 --- a/src/gromacs/awh/awh.cpp +++ b/src/gromacs/awh/awh.cpp @@ -100,19 +100,19 @@ struct BiasCoupledToSystem BiasCoupledToSystem(Bias bias, const std::vector &pullCoordIndex); - Bias bias; /**< The bias. */ - const std::vector pullCoordIndex; /**< The pull coordinates this bias acts on. */ + Bias bias_; /**< The bias. */ + const std::vector pullCoordIndex_; /**< The pull coordinates this bias acts on. */ /* Here AWH can be extended to work on other coordinates than pull. */ }; BiasCoupledToSystem::BiasCoupledToSystem(Bias bias, const std::vector &pullCoordIndex) : - bias(std::move(bias)), - pullCoordIndex(pullCoordIndex) + bias_(std::move(bias)), + pullCoordIndex_(pullCoordIndex) { /* We already checked for this in grompp, but check again here. */ - GMX_RELEASE_ASSERT(static_cast(bias.ndim()) == pullCoordIndex.size(), "The bias dimensionality should match the number of pull coordinates."); + GMX_RELEASE_ASSERT(static_cast(bias_.ndim()) == pullCoordIndex_.size(), "The bias dimensionality should match the number of pull coordinates."); } Awh::Awh(FILE *fplog, @@ -175,7 +175,7 @@ Awh::Awh(FILE *fplog, biasCoupledToSystem_.emplace_back(Bias(k, awhParams, awhParams.awhBiasParams[k], dimParams, beta, inputRecord.delta_t, numSharingSimulations, biasInitFilename, thisRankWillDoIO), pullCoordIndex); - biasCoupledToSystem_.back().bias.printInitializationToLog(fplog); + biasCoupledToSystem_.back().bias_.printInitializationToLog(fplog); } /* Need to register the AWH coordinates to be allowed to apply forces to the pull coordinates. */ @@ -186,7 +186,7 @@ Awh::Awh(FILE *fplog, std::vector pointSize; for (auto const &biasCts : biasCoupledToSystem_) { - pointSize.push_back(biasCts.bias.state().points().size()); + pointSize.push_back(biasCts.bias_.state().points().size()); } /* Ensure that the shared biased are compatible between simulations */ biasesAreCompatibleForSharingBetweenSimulations(awhParams, pointSize, multiSimRecord_); @@ -227,9 +227,9 @@ real Awh::applyBiasForcesAndUpdateBias(int ePBC, * pull coordinates. */ awh_dvec coordValue = { 0, 0, 0, 0 }; - for (int d = 0; d < biasCts.bias.ndim(); d++) + for (int d = 0; d < biasCts.bias_.ndim(); d++) { - coordValue[d] = get_pull_coord_value(pull_, biasCts.pullCoordIndex[d], &pbc); + coordValue[d] = get_pull_coord_value(pull_, biasCts.pullCoordIndex_[d], &pbc); } /* Perform an AWH biasing step: this means, at regular intervals, @@ -242,11 +242,11 @@ real Awh::applyBiasForcesAndUpdateBias(int ePBC, * to supports bias sharing within a single simulation. */ gmx::ArrayRef biasForce = - biasCts.bias.calcForceAndUpdateBias(coordValue, - &biasPotential, &biasPotentialJump, - commRecord_, - multiSimRecord_, - t, step, seed_, fplog); + biasCts.bias_.calcForceAndUpdateBias(coordValue, + &biasPotential, &biasPotentialJump, + commRecord_, + multiSimRecord_, + t, step, seed_, fplog); awhPotential += biasPotential; @@ -257,9 +257,9 @@ real Awh::applyBiasForcesAndUpdateBias(int ePBC, * The bias potential is returned at the end of this function, * so that it can be added externally to the correct energy data block. */ - for (int d = 0; d < biasCts.bias.ndim(); d++) + for (int d = 0; d < biasCts.bias_.ndim(); d++) { - apply_external_pull_coord_force(pull_, biasCts.pullCoordIndex[d], + apply_external_pull_coord_force(pull_, biasCts.pullCoordIndex_[d], biasForce[d], &mdatoms, forceWithVirial); } @@ -269,7 +269,7 @@ real Awh::applyBiasForcesAndUpdateBias(int ePBC, /* We might have skipped updates for part of the grid points. * Ensure all points are updated before writing out their data. */ - biasCts.bias.doSkippedUpdatesForAllPoints(); + biasCts.bias_.doSkippedUpdatesForAllPoints(); } } @@ -288,7 +288,7 @@ std::shared_ptr Awh::initHistoryFromState() const for (size_t k = 0; k < awhHistory->bias.size(); k++) { - biasCoupledToSystem_[k].bias.initHistoryFromState(&awhHistory->bias[k]); + biasCoupledToSystem_[k].bias_.initHistoryFromState(&awhHistory->bias[k]); } return awhHistory; @@ -321,7 +321,7 @@ void Awh::restoreStateFromHistory(const AwhHistory *awhHistory) for (size_t k = 0; k < biasCoupledToSystem_.size(); k++) { - biasCoupledToSystem_[k].bias.restoreStateFromHistory(awhHistory ? &awhHistory->bias[k] : nullptr, commRecord_); + biasCoupledToSystem_[k].bias_.restoreStateFromHistory(awhHistory ? &awhHistory->bias[k] : nullptr, commRecord_); } } @@ -339,7 +339,7 @@ void Awh::updateHistory(AwhHistory *awhHistory) const for (size_t k = 0; k < awhHistory->bias.size(); k++) { - biasCoupledToSystem_[k].bias.updateHistory(&awhHistory->bias[k]); + biasCoupledToSystem_[k].bias_.updateHistory(&awhHistory->bias[k]); } } @@ -381,7 +381,7 @@ void Awh::writeToEnergyFrame(int64_t step, int numSubblocks = 0; for (auto &biasCoupledToSystem : biasCoupledToSystem_) { - numSubblocks += biasCoupledToSystem.bias.numEnergySubblocksToWrite(); + numSubblocks += biasCoupledToSystem.bias_.numEnergySubblocksToWrite(); } GMX_ASSERT(numSubblocks > 0, "We should always have data to write"); @@ -399,7 +399,7 @@ void Awh::writeToEnergyFrame(int64_t step, int energySubblockCount = 0; for (auto &biasCoupledToSystem : biasCoupledToSystem_) { - energySubblockCount += biasCoupledToSystem.bias.writeToEnergySubblocks(&(awhEnergyBlock->sub[energySubblockCount])); + energySubblockCount += biasCoupledToSystem.bias_.writeToEnergySubblocks(&(awhEnergyBlock->sub[energySubblockCount])); } } diff --git a/src/gromacs/domdec/domdec.cpp b/src/gromacs/domdec/domdec.cpp index 8ec5ce80e5..f9d642e16c 100644 --- a/src/gromacs/domdec/domdec.cpp +++ b/src/gromacs/domdec/domdec.cpp @@ -1288,6 +1288,7 @@ static void setup_neighbor_relations(gmx_domdec_t *dd) ivec tmp, s; gmx_domdec_zones_t *zones; gmx_domdec_ns_ranges_t *izone; + GMX_ASSERT(dd->ndim >= 0, "Must have non-negative number of dimensions for DD"); for (d = 0; d < dd->ndim; d++) { diff --git a/src/gromacs/domdec/partition.cpp b/src/gromacs/domdec/partition.cpp index e36f49cf45..e789d80160 100644 --- a/src/gromacs/domdec/partition.cpp +++ b/src/gromacs/domdec/partition.cpp @@ -1744,6 +1744,7 @@ get_zone_pulse_cgs(gmx_domdec_t *dd, } if (dim_ind == 2 && (zonei == 2 || zonei == 3)) { + GMX_ASSERT(dim1 >= 0 && dim1 < DIM, "Must have a valid dimension index"); rn[dim1] += cg_cm[cg][dim1] - c->cr1[zone]; tric_sh = 0; for (i = dim1+1; i < DIM; i++) @@ -1809,6 +1810,7 @@ get_zone_pulse_cgs(gmx_domdec_t *dd, if (bDistMB_pulse) { clear_rvec(rb); + GMX_ASSERT(dim >= 0 && dim < DIM, "Must have a valid dimension index"); rb[dim] += cg_cm[cg][dim] - c->bc[dim_ind] + tric_sh; if (rb[dim] > 0) { diff --git a/src/gromacs/ewald/pme_spread.cpp b/src/gromacs/ewald/pme_spread.cpp index d21f4c6a24..8e3b31be21 100644 --- a/src/gromacs/ewald/pme_spread.cpp +++ b/src/gromacs/ewald/pme_spread.cpp @@ -50,6 +50,7 @@ #include "gromacs/utility/basedefinitions.h" #include "gromacs/utility/exceptions.h" #include "gromacs/utility/fatalerror.h" +#include "gromacs/utility/gmxassert.h" #include "gromacs/utility/smalloc.h" #include "pme_grid.h" @@ -874,6 +875,7 @@ void spread_on_grid(const gmx_pme_t *pme, nthread = pme->nthread; assert(nthread > 0); + GMX_ASSERT(grids != nullptr || !bSpread, "If there's no grid, we cannot be spreading"); #ifdef PME_TIME_THREADS c1 = omp_cyc_start(); diff --git a/src/gromacs/fft/fft5d.cpp b/src/gromacs/fft/fft5d.cpp index f1ca537d7b..6392d3b093 100644 --- a/src/gromacs/fft/fft5d.cpp +++ b/src/gromacs/fft/fft5d.cpp @@ -273,7 +273,7 @@ fft5d_plan fft5d_plan_3d(int NG, int MG, int KG, MPI_Comm comm[2], int flags, t_ oK1[i] = (KG*i)/P[1]; #endif } - for (i = 0; i < P[0]-1; i++) + for (i = 0; P[0] > 0 && i < P[0]-1; i++) { N0[i] = oN0[i+1]-oN0[i]; M0[i] = oM0[i+1]-oM0[i]; @@ -282,7 +282,7 @@ fft5d_plan fft5d_plan_3d(int NG, int MG, int KG, MPI_Comm comm[2], int flags, t_ N0[P[0]-1] = NG-oN0[P[0]-1]; M0[P[0]-1] = MG-oM0[P[0]-1]; K0[P[0]-1] = KG-oK0[P[0]-1]; - for (i = 0; i < P[1]-1; i++) + for (i = 0; P[1] > 0 && i < P[1]-1; i++) { N1[i] = oN1[i+1]-oN1[i]; M1[i] = oM1[i+1]-oM1[i]; @@ -296,7 +296,8 @@ fft5d_plan fft5d_plan_3d(int NG, int MG, int KG, MPI_Comm comm[2], int flags, t_ C: contiguous dimension, and nP: number of processor in subcommunicator for that step */ - + GMX_ASSERT(prank[0] < P[0], "Must have valid rank within communicator size"); + GMX_ASSERT(prank[1] < P[1], "Must have valid rank within communicator size"); pM[0] = M0[prank[0]]; oM[0] = oM0[prank[0]]; pK[0] = K1[prank[1]]; diff --git a/src/gromacs/fileio/matio.cpp b/src/gromacs/fileio/matio.cpp index 2eb0897281..0dcf881104 100644 --- a/src/gromacs/fileio/matio.cpp +++ b/src/gromacs/fileio/matio.cpp @@ -61,7 +61,7 @@ using namespace gmx; static const char mapper[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*()-_=+{}|;:',<.>/?"; -#define NMAP static_cast(strlen(mapper)) +#define NMAP static_cast(sizeof(mapper)/sizeof(mapper[0])) #define MAX_XPM_LINELENGTH 4096 diff --git a/src/gromacs/gmxana/gmx_analyze.cpp b/src/gromacs/gmxana/gmx_analyze.cpp index 4e98ff3d98..1dc3769df1 100644 --- a/src/gromacs/gmxana/gmx_analyze.cpp +++ b/src/gromacs/gmxana/gmx_analyze.cpp @@ -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,2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, 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. @@ -398,6 +398,11 @@ static void average(const char *avfile, int avbar_opt, */ static real optimal_error_estimate(double sigma, const double fitparm[], real tTotal) { + // When sigma is zero, the fitparm data can be uninitialized + if (sigma == 0.0) + { + return 0; + } double ss = fitparm[1]*fitparm[0]+(1-fitparm[1])*fitparm[2]; if ((tTotal <= 0) || (ss <= 0)) { @@ -484,10 +489,13 @@ static void estimate_error(const char *eefile, int nb_min, int resol, int n, } if (sig[s] == 0) { - ee = 0; - a = 1; - tau1 = 0; - tau2 = 0; + ee = 0; + a = 1; + tau1 = 0; + tau2 = 0; + fitparm[0] = 0; + fitparm[1] = 0; + fitparm[2] = 0; } else { diff --git a/src/gromacs/gmxana/gmx_bar.cpp b/src/gromacs/gmxana/gmx_bar.cpp index eca6efd295..16d70d70c5 100644 --- a/src/gromacs/gmxana/gmx_bar.cpp +++ b/src/gromacs/gmxana/gmx_bar.cpp @@ -265,11 +265,19 @@ static gmx_bool lambda_components_check(const lambda_components_t *lc, { return FALSE; } + GMX_RELEASE_ASSERT((name != nullptr) || (name_length == 0), + "If name is empty, the length of the substring to examine within it must be zero"); len = std::strlen(lc->names[index]); if (len != name_length) { return FALSE; } + if (name_length == 0) + { + // Everything matches a zero-length substring. This branch is + // needed because name could in principle be nullptr. + return TRUE; + } return std::strncmp(lc->names[index], name, name_length) == 0; } diff --git a/src/gromacs/gmxana/gmx_dipoles.cpp b/src/gromacs/gmxana/gmx_dipoles.cpp index 984d9287ce..6601111115 100644 --- a/src/gromacs/gmxana/gmx_dipoles.cpp +++ b/src/gromacs/gmxana/gmx_dipoles.cpp @@ -70,6 +70,7 @@ #include "gromacs/utility/exceptions.h" #include "gromacs/utility/fatalerror.h" #include "gromacs/utility/futil.h" +#include "gromacs/utility/gmxassert.h" #include "gromacs/utility/smalloc.h" #define e2d(x) ENM2DEBYE*(x) @@ -810,10 +811,11 @@ static void do_dip(const t_topology *top, int ePBC, real volume, gmx_rmpbc_t gpbc = nullptr; gnx_tot = gnx[0]; - if (ncos > 1) + if (ncos == 2) { gnx_tot += gnx[1]; } + GMX_RELEASE_ASSERT(ncos == 1 || ncos == 2, "Invalid number of groups used with -ncos"); vol_aver = 0.0; diff --git a/src/gromacs/gmxana/gmx_mindist.cpp b/src/gromacs/gmxana/gmx_mindist.cpp index 52cd5148f0..1c4752e8a0 100644 --- a/src/gromacs/gmxana/gmx_mindist.cpp +++ b/src/gromacs/gmxana/gmx_mindist.cpp @@ -254,6 +254,7 @@ static void calc_dist(real rcut, gmx_bool bPBC, int ePBC, matrix box, rvec x[], j1 = nx1; index3 = index1; } + GMX_RELEASE_ASSERT(index1 != nullptr, "Need a valid index for plotting distances"); rmin2 = 1e12; rmax2 = -1e12; @@ -376,6 +377,7 @@ static void dist_plot(const char *fn, const char *afile, const char *dfile, } else { + GMX_RELEASE_ASSERT(ng > 1, "Must have more than one group with bMat"); snew(leg, (ng*(ng-1))/2); for (i = j = 0; (i < ng-1); i++) { @@ -492,6 +494,7 @@ static void dist_plot(const char *fn, const char *afile, const char *dfile, } else { + GMX_RELEASE_ASSERT(ng > 1, "Must have more than one group when not using -matrix"); for (i = 1; (i < ng); i++) { calc_dist(rcut, bPBC, ePBC, box, x0, gnx[0], gnx[i], index[0], index[i], bGroup, @@ -799,6 +802,7 @@ int gmx_mindist(int argc, char *argv[]) } gnx[0] = 1; } + GMX_RELEASE_ASSERT(!bMat || ng > 1, "Must have more than one group with bMat"); if (resfnm) { diff --git a/src/gromacs/gmxana/gmx_msd.cpp b/src/gromacs/gmxana/gmx_msd.cpp index f31fd08355..5e6c1d4a64 100644 --- a/src/gromacs/gmxana/gmx_msd.cpp +++ b/src/gromacs/gmxana/gmx_msd.cpp @@ -800,6 +800,7 @@ static int corr_loop(t_corr *curr, const char *fn, const t_topology *top, int eP /* calculate the center of mass */ if (!gnx_com.empty()) { + GMX_RELEASE_ASSERT(index_com != nullptr, "Center-of-mass removal must have valid index group"); calc_com(bMol, gnx_com[0], index_com[0], xa[cur], xa[prev], box, &top->atoms, com); } diff --git a/src/gromacs/gmxana/gmx_order.cpp b/src/gromacs/gmxana/gmx_order.cpp index 1c0ee1af42..87dda9fbc1 100644 --- a/src/gromacs/gmxana/gmx_order.cpp +++ b/src/gromacs/gmxana/gmx_order.cpp @@ -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,2016,2017,2018, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, 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. @@ -381,9 +381,8 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, { /* if permolecule = TRUE, order parameters will be calculed per molecule * and stored in slOrder with #slices = # molecules */ - rvec *x0, /* coordinates with pbc */ - *x1, /* coordinates without pbc */ - dist; /* vector between two atoms */ + rvec *x0, /* coordinates with pbc */ + *x1; /* coordinates without pbc */ matrix box; /* box (3x3) */ t_trxstatus *status; rvec cossum, /* sum of vector angles for three axes */ @@ -403,7 +402,7 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, int *slCount; /* nr. of atoms in one slice */ real sdbangle = 0; /* sum of these angles */ gmx_bool use_unitvector = FALSE; /* use a specified unit vector instead of axis to specify unit normal*/ - rvec direction, com, dref, dvec; + rvec direction, com; int comsize, distsize; int *comidx = nullptr, *distidx = nullptr; char *grpname = nullptr; @@ -521,18 +520,19 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, } svmul(1.0/comsize, com, com); } + rvec displacementFromReference; if (distcalc) { - dref[XX] = 0.0; dref[YY] = 0.0; dref[ZZ] = 0.0; + rvec dref = { 0.0, 0.0, 0.0 }; for (j = 0; j < distsize; j++) { - rvec_inc(dist, x1[distidx[j]]); + rvec_inc(dref, x1[distidx[j]]); } svmul(1.0/distsize, dref, dref); if (radial) { - pbc_dx(&pbc, dref, com, dvec); - unitv(dvec, dvec); + pbc_dx(&pbc, dref, com, displacementFromReference); + unitv(displacementFromReference, displacementFromReference); } } @@ -562,6 +562,7 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, if (bUnsat) { + rvec dist; /* Using convention for unsaturated carbons */ /* first get Sz, the vector from Cn to Cn+1 */ rvec_sub(x1[a[index[i+1]+j]], x1[a[index[i]+j]], dist); @@ -583,6 +584,7 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, } else { + rvec dist; /* get vector dist(Cn-1,Cn+1) for tail atoms */ rvec_sub(x1[a[index[i+1]+j]], x1[a[index[i-1]+j]], dist); length = norm(dist); /* determine distance between two atoms */ @@ -660,7 +662,7 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, if (radial) { /* bin order parameter by arc distance from reference group*/ - arcdist = gmx_angle(dvec, direction); + arcdist = gmx_angle(displacementFromReference, direction); (*distvals)[j][i] += arcdist; } else if (i == 1) @@ -669,10 +671,11 @@ static void calc_order(const char *fn, const int *index, int *a, rvec **order, tmpdist = trace(box); /* should be max value */ for (k = 0; k < distsize; k++) { - pbc_dx(&pbc, x1[distidx[k]], x1[a[index[i]+j]], dvec); - /* at the moment, just remove dvec[axis] */ - dvec[axis] = 0; - tmpdist = std::min(tmpdist, norm2(dvec)); + rvec displacement; + pbc_dx(&pbc, x1[distidx[k]], x1[a[index[i]+j]], displacement); + /* at the moment, just remove displacement[axis] */ + displacement[axis] = 0; + tmpdist = std::min(tmpdist, norm2(displacement)); } //fprintf(stderr, "Min dist %f; trace %f\n", tmpdist, trace(box)); (*distvals)[j][i] += std::sqrt(tmpdist); diff --git a/src/gromacs/gmxana/gmx_wham.cpp b/src/gromacs/gmxana/gmx_wham.cpp index c8175ac1b8..79900c51a2 100644 --- a/src/gromacs/gmxana/gmx_wham.cpp +++ b/src/gromacs/gmxana/gmx_wham.cpp @@ -1232,8 +1232,6 @@ static void getRandomIntArray(int nPull, int blockLength, int* randomArray, gmx: } randomArray[ipull] = ipullRandom; } - /*for (ipull=0; ipullbsMethod) { case bsMethod_hist: - snew(randomArray, nAllPull); printf("\n\nWhen computing statistical errors by bootstrapping entire histograms:\n"); please_cite(stdout, "Hub2006"); break; @@ -1674,6 +1671,7 @@ static void do_bootstrapping(const char *fnres, const char* fnprof, const char * { case bsMethod_hist: /* bootstrap complete histograms from given histograms */ + srenew(randomArray, nAllPull); getRandomIntArray(nAllPull, opt->histBootStrapBlockLength, randomArray, &opt->rng); for (i = 0; i < nAllPull; i++) { diff --git a/src/gromacs/gmxpreprocess/gen_vsite.cpp b/src/gromacs/gmxpreprocess/gen_vsite.cpp index 38537e1a4c..dacc317552 100644 --- a/src/gromacs/gmxpreprocess/gen_vsite.cpp +++ b/src/gromacs/gmxpreprocess/gen_vsite.cpp @@ -1558,7 +1558,7 @@ void do_vsites(gmx::ArrayRef rtpFFDB, PreprocessingAtom const char *ffdir) { #define MAXATOMSPERRESIDUE 16 - int k, m, i0, ni0, whatres, resind, add_shift, nvsite, nadd; + int k, m, i0, ni0, whatres, add_shift, nvsite, nadd; int ai, aj, ak, al; int nrfound = 0, needed, nrbonds, nrHatoms, Heavy, nrheavies, tpM, tpHeavy; int Hatoms[4], heavies[4]; @@ -1570,7 +1570,6 @@ void do_vsites(gmx::ArrayRef rtpFFDB, PreprocessingAtom int *o2n, *newvsite_type, *newcgnr, ats[MAXATOMSPERRESIDUE]; t_atom *newatom; char ***newatomname; - char *resnm = nullptr; int cmplength; bool isN, planarN, bFound; @@ -1661,14 +1660,14 @@ void do_vsites(gmx::ArrayRef rtpFFDB, PreprocessingAtom /* generate vsite constructions */ /* loop over all atoms */ - resind = -1; + int resind = -1; for (int i = 0; (i < at->nr); i++) { if (at->atom[i].resind != resind) { resind = at->atom[i].resind; - resnm = *(at->resinfo[resind].name); } + const char *resnm = *(at->resinfo[resind].name); /* first check for aromatics to virtualize */ /* don't waste our effort on DNA, water etc. */ /* Only do the vsite aromatic stuff when we reach the @@ -2028,11 +2027,10 @@ void do_vsites(gmx::ArrayRef rtpFFDB, PreprocessingAtom } if (bWARNING) { - fprintf(stderr, - "Warning: cannot convert atom %d %s (bound to a heavy atom " - "%s with \n" - " %d bonds and %d bound hydrogens atoms) to virtual site\n", - i+1, *(at->atomname[i]), tpname, nrbonds, nrHatoms); + gmx_fatal(FARGS, "Cannot convert atom %d %s (bound to a heavy atom " + "%s with \n" + " %d bonds and %d bound hydrogens atoms) to virtual site\n", + i+1, *(at->atomname[i]), tpname, nrbonds, nrHatoms); } if (bAddVsiteParam) { diff --git a/src/gromacs/gmxpreprocess/grompp.cpp b/src/gromacs/gmxpreprocess/grompp.cpp index 10cb4d9875..0b30620662 100644 --- a/src/gromacs/gmxpreprocess/grompp.cpp +++ b/src/gromacs/gmxpreprocess/grompp.cpp @@ -960,6 +960,7 @@ static void read_posres(gmx_mtop_t *mtop, } npbcdim = ePBC2npbcdim(ePBC); + GMX_RELEASE_ASSERT(npbcdim <= DIM, "Invalid npbcdim"); clear_rvec(com); if (rc_scaling != erscNO) { diff --git a/src/gromacs/gmxpreprocess/pdb2gmx.cpp b/src/gromacs/gmxpreprocess/pdb2gmx.cpp index 7657d6171f..db8a24d4c0 100644 --- a/src/gromacs/gmxpreprocess/pdb2gmx.cpp +++ b/src/gromacs/gmxpreprocess/pdb2gmx.cpp @@ -1750,6 +1750,7 @@ int pdb2gmx::run() if ((i == 0) || (this_chainnumber != prev_chainnumber) || (bWat_ != bPrevWat_)) { + GMX_RELEASE_ASSERT(pdba_all.pdbinfo, "Must have pdbinfo from reading a PDB file if chain number is changing"); this_chainstart = pdba_all.atom[i].resind; bMerged = false; if (i > 0 && !bWat_) diff --git a/src/gromacs/gmxpreprocess/readir.cpp b/src/gromacs/gmxpreprocess/readir.cpp index c2a54c3f21..be95157da7 100644 --- a/src/gromacs/gmxpreprocess/readir.cpp +++ b/src/gromacs/gmxpreprocess/readir.cpp @@ -809,9 +809,13 @@ void check_ir(const char *mdparin, t_inputrec *ir, t_gromppopts *opts, { sprintf(err_buf, "nstlog must be non-zero"); CHECK(ir->nstlog == 0); - sprintf(err_buf, "nst-transition-matrix (%d) must be an integer multiple of nstlog (%d)", - expand->nstTij, ir->nstlog); - CHECK((expand->nstTij % ir->nstlog) != 0); + // Avoid modulus by zero in the case that already triggered an error exit. + if (ir->nstlog != 0) + { + sprintf(err_buf, "nst-transition-matrix (%d) must be an integer multiple of nstlog (%d)", + expand->nstTij, ir->nstlog); + CHECK((expand->nstTij % ir->nstlog) != 0); + } } } diff --git a/src/gromacs/listed_forces/manage_threading.cpp b/src/gromacs/listed_forces/manage_threading.cpp index d10dd20f60..5546f1f040 100644 --- a/src/gromacs/listed_forces/manage_threading.cpp +++ b/src/gromacs/listed_forces/manage_threading.cpp @@ -208,7 +208,8 @@ static void divide_bondeds_over_threads(bonded_threading_t *bt, { ilist_data_t ild[F_NRE]; - assert(bt->nthreads > 0); + GMX_ASSERT(bt->nthreads > 0, "Must have positive number of threads"); + const int numThreads = bt->nthreads; bt->haveBondeds = false; int numType = 0; @@ -248,12 +249,12 @@ static void divide_bondeds_over_threads(bonded_threading_t *bt, if (nrToAssignToCpuThreads == 0) { /* No interactions, avoid all the integer math below */ - for (int t = 0; t <= bt->nthreads; t++) + for (int t = 0; t <= numThreads; t++) { bt->workDivision.setBound(fType, t, 0); } } - else if (bt->nthreads <= bt->max_nthread_uniform || fType == F_DISRES) + else if (numThreads <= bt->max_nthread_uniform || fType == F_DISRES) { /* On up to 4 threads, load balancing the bonded work * is more important than minimizing the reduction cost. @@ -261,10 +262,10 @@ static void divide_bondeds_over_threads(bonded_threading_t *bt, const int stride = 1 + NRAL(fType); - for (int t = 0; t <= bt->nthreads; t++) + for (int t = 0; t <= numThreads; t++) { /* Divide equally over the threads */ - int nr_t = (((nrToAssignToCpuThreads/stride)*t)/bt->nthreads)*stride; + int nr_t = (((nrToAssignToCpuThreads/stride)*t)/numThreads)*stride; if (fType == F_DISRES) { @@ -314,7 +315,7 @@ static void divide_bondeds_over_threads(bonded_threading_t *bt, int t; fprintf(debug, "%16s", interaction_function[f].name); - for (t = 0; t < bt->nthreads; t++) + for (t = 0; t < numThreads; t++) { fprintf(debug, " %4d", (bt->workDivision.bound(f, t + 1) - diff --git a/src/gromacs/mdlib/lincs.cpp b/src/gromacs/mdlib/lincs.cpp index a9b65cdf8f..b67723674d 100644 --- a/src/gromacs/mdlib/lincs.cpp +++ b/src/gromacs/mdlib/lincs.cpp @@ -2356,10 +2356,10 @@ bool constrain_lincs(bool computeRmsd, } } - int p_imax; - real ncons_loc; - real p_ssd; - real p_max = 0; + int p_imax = 0; + real ncons_loc = 0; + real p_ssd = 0; + real p_max = 0; if (debug) { cconerr(*lincsd, xprime, pbc, diff --git a/src/gromacs/mdlib/mdatoms.cpp b/src/gromacs/mdlib/mdatoms.cpp index 3ff5a699ae..45202fe9ed 100644 --- a/src/gromacs/mdlib/mdatoms.cpp +++ b/src/gromacs/mdlib/mdatoms.cpp @@ -395,6 +395,7 @@ void atoms2md(const gmx_mtop_t *mtop, const t_inputrec *ir, else if (md->cFREEZE) { g = md->cFREEZE[i]; + GMX_ASSERT(opts->nFreeze != nullptr, "Must have freeze groups to initialize masses"); if (opts->nFreeze[g][XX] && opts->nFreeze[g][YY] && opts->nFreeze[g][ZZ]) { /* Set the mass of completely frozen particles to ALMOST_ZERO diff --git a/src/gromacs/mdrun/md.cpp b/src/gromacs/mdrun/md.cpp index 316668d615..f5c247089a 100644 --- a/src/gromacs/mdrun/md.cpp +++ b/src/gromacs/mdrun/md.cpp @@ -182,8 +182,7 @@ void gmx::LegacySimulator::do_md() gmx_bool bSumEkinhOld, bDoReplEx, bExchanged, bNeedRepartition; gmx_bool bTemp, bPres, bTrotter; real dvdl_constr; - rvec *cbuf = nullptr; - int cbuf_nalloc = 0; + std::vector cbuf; matrix lastbox; int lamnew = 0; /* for FEP */ @@ -1213,13 +1212,8 @@ void gmx::LegacySimulator::do_md() if (ir->eI == eiVVAK) { - /* We probably only need md->homenr, not state->natoms */ - if (state->natoms > cbuf_nalloc) - { - cbuf_nalloc = state->natoms; - srenew(cbuf, cbuf_nalloc); - } - copy_rvecn(as_rvec_array(state->x.data()), cbuf, 0, state->natoms); + cbuf.resize(state->x.size()); + std::copy(state->x.begin(), state->x.end(), cbuf.begin()); } if (c_useGpuUpdateConstrain) @@ -1279,8 +1273,8 @@ void gmx::LegacySimulator::do_md() ); wallcycle_start(wcycle, ewcUPDATE); trotter_update(ir, step, ekind, enerd, state, total_vir, mdatoms, &MassQ, trotter_seq, ettTSEQ4); - /* now we know the scaling, we can compute the positions again again */ - copy_rvecn(cbuf, as_rvec_array(state->x.data()), 0, state->natoms); + /* now we know the scaling, we can compute the positions again */ + std::copy(cbuf.begin(), cbuf.end(), state->x.begin()); update_coords(step, ir, mdatoms, state, f.arrayRefWithPadding(), fcd, ekind, M, &upd, etrtPOSITION, cr, constr); diff --git a/src/gromacs/mdrun/rerun.cpp b/src/gromacs/mdrun/rerun.cpp index d3937bf09b..b55e456eba 100644 --- a/src/gromacs/mdrun/rerun.cpp +++ b/src/gromacs/mdrun/rerun.cpp @@ -201,7 +201,7 @@ void gmx::LegacySimulator::do_rerun() bool doFreeEnergyPerturbation = false; unsigned int force_flags; tensor force_vir, shake_vir, total_vir, pres; - t_trxstatus *status; + t_trxstatus *status = nullptr; rvec mu_tot; t_trxframe rerun_fr; gmx_localtop_t top; diff --git a/src/gromacs/mdrun/shellfc.cpp b/src/gromacs/mdrun/shellfc.cpp index 564dc42fbc..8c4e39a6e5 100644 --- a/src/gromacs/mdrun/shellfc.cpp +++ b/src/gromacs/mdrun/shellfc.cpp @@ -78,6 +78,7 @@ #include "gromacs/utility/arraysize.h" #include "gromacs/utility/cstringutil.h" #include "gromacs/utility/fatalerror.h" +#include "gromacs/utility/gmxassert.h" #include "gromacs/utility/smalloc.h" typedef struct { @@ -161,6 +162,8 @@ static void predict_shells(FILE *fplog, rvec x[], rvec v[], real dt, real dt_1, fudge, tm, m1, m2, m3; rvec *ptr; + GMX_RELEASE_ASSERT(mass || mtop, "Must have masses or a way to look them up"); + /* We introduce a fudge factor for performance reasons: with this choice * the initial force on the shells is about a factor of two lower than * without diff --git a/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp b/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp index bb672c0962..49965e17ce 100644 --- a/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp +++ b/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp @@ -58,8 +58,10 @@ MockThreadAffinityAccess::MockThreadAffinityAccess() { using ::testing::_; using ::testing::Return; +#ifndef __clang_analyzer__ ON_CALL(*this, setCurrentThreadAffinityToCore(_)) .WillByDefault(Return(true)); +#endif } MockThreadAffinityAccess::~MockThreadAffinityAccess() diff --git a/src/gromacs/mdrunutility/tests/threadaffinitytest.h b/src/gromacs/mdrunutility/tests/threadaffinitytest.h index 38ca39ccc9..501e2a8ee8 100644 --- a/src/gromacs/mdrunutility/tests/threadaffinitytest.h +++ b/src/gromacs/mdrunutility/tests/threadaffinitytest.h @@ -42,6 +42,7 @@ #include "gromacs/hardware/hw_info.h" #include "gromacs/mdrunutility/threadaffinity.h" +#include "gromacs/utility/basedefinitions.h" #include "gromacs/utility/logger.h" #include "gromacs/utility/physicalnodecommunicator.h" #include "gromacs/utility/stringutil.h" @@ -119,8 +120,12 @@ class ThreadAffinityTestHelper void expectAffinitySetThatFails(int core) { using ::testing::Return; +#ifndef __clang_analyzer__ EXPECT_CALL(affinityAccess_, setCurrentThreadAffinityToCore(core)) .WillOnce(Return(false)); +#else + GMX_UNUSED_VALUE(core); +#endif } void expectWarningMatchingRegex(const char *re) diff --git a/src/gromacs/nbnxm/grid.cpp b/src/gromacs/nbnxm/grid.cpp index a20cf1249e..fbc52c4bd8 100644 --- a/src/gromacs/nbnxm/grid.cpp +++ b/src/gromacs/nbnxm/grid.cpp @@ -944,6 +944,7 @@ void Grid::fillCell(GridSetData *gridSetData, #if NBNXN_SEARCH_SIMD4_FLOAT_X_BB if (nbat->XFormat == nbatXYZQ) { + GMX_ASSERT(bb_work_aligned != nullptr, "Must have valid aligned work structure"); calc_bounding_box_xxxx_simd4(numAtoms, nbat->x().data() + atomStart*nbat->xstride, bb_work_aligned, pbb_ptr); } diff --git a/src/gromacs/pulling/tests/pull.cpp b/src/gromacs/pulling/tests/pull.cpp index b56fad0b88..ebf966ea08 100644 --- a/src/gromacs/pulling/tests/pull.cpp +++ b/src/gromacs/pulling/tests/pull.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2018, by the GROMACS development team, led by + * Copyright (c) 2016,2018,2019, 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. @@ -77,7 +77,7 @@ class PullTest : public ::testing::Test // PBC stuff set_pbc(&pbc, epbc, box); - GMX_ASSERT(pbc.ndim_ePBC >= 1, "Tests only support PBC along at least x"); + GMX_ASSERT(pbc.ndim_ePBC >= 1 && pbc.ndim_ePBC <= DIM, "Tests only support PBC along at least x and at most x, y, and z"); real boxSizeZSquared; if (pbc.ndim_ePBC > ZZ) diff --git a/src/gromacs/selection/compiler.cpp b/src/gromacs/selection/compiler.cpp index 5e35fc407e..7d43e2f103 100644 --- a/src/gromacs/selection/compiler.cpp +++ b/src/gromacs/selection/compiler.cpp @@ -282,6 +282,7 @@ #include "gromacs/selection/indexutil.h" #include "gromacs/selection/selection.h" #include "gromacs/utility/exceptions.h" +#include "gromacs/utility/gmxassert.h" #include "gromacs/utility/smalloc.h" #include "gromacs/utility/stringutil.h" @@ -2010,6 +2011,7 @@ evaluate_boolean_minmax_grps(const SelectionTreeElementPointer &sel, switch (sel->u.boolt) { case BOOL_NOT: + GMX_ASSERT(g != nullptr, "Need a valid group"); gmx_ana_index_reserve(gmin, g->isize); gmx_ana_index_reserve(gmax, g->isize); gmx_ana_index_difference(gmax, g, sel->child->cdata->gmin); @@ -2043,6 +2045,7 @@ evaluate_boolean_minmax_grps(const SelectionTreeElementPointer &sel, case BOOL_OR: /* We can assume here that the gmin of children do not overlap * because of the way _gmx_sel_evaluate_or() works. */ + GMX_ASSERT(g != nullptr, "Need a valid group"); gmx_ana_index_reserve(gmin, g->isize); gmx_ana_index_reserve(gmax, g->isize); gmx_ana_index_copy(gmin, sel->child->cdata->gmin, false); diff --git a/src/gromacs/tools/pme_error.cpp b/src/gromacs/tools/pme_error.cpp index 781b7b99cb..a4334cef25 100644 --- a/src/gromacs/tools/pme_error.cpp +++ b/src/gromacs/tools/pme_error.cpp @@ -458,6 +458,8 @@ static real estimate_reciprocal( double t1 = 0.0; #endif + GMX_RELEASE_ASSERT(q != nullptr, "Must have charges"); + if (seed == 0) { seed = static_cast(gmx::makeRandomSeed()); diff --git a/src/gromacs/trajectoryanalysis/modules/surfacearea.cpp b/src/gromacs/trajectoryanalysis/modules/surfacearea.cpp index c6fb7776df..dade2e781c 100644 --- a/src/gromacs/trajectoryanalysis/modules/surfacearea.cpp +++ b/src/gromacs/trajectoryanalysis/modules/surfacearea.cpp @@ -708,11 +708,14 @@ nsc_dclm_pbc(const rvec *coords, const ArrayRef &radius, int nat, } if (mode & FLAG_DOTS) { + GMX_RELEASE_ASSERT(nu_dots != nullptr, "Must have valid nu_dots pointer"); *nu_dots = lfnr; + GMX_RELEASE_ASSERT(lidots != nullptr, "Must have valid lidots pointer"); *lidots = dots; } if (mode & FLAG_ATOM_AREA) { + GMX_RELEASE_ASSERT(at_area != nullptr, "Must have valid at_area pointer"); *at_area = atom_area; } *value_of_area = area;