Bug #2406

legacy forcefield files with GB trigger segfault

Added by Peter Kasson about 2 years ago. Updated almost 2 years ago.

Target version:
Affected version - extra info:
Affected version:


I just encountered a weird bug working off master:
Using an older toplogy / CHARMM ff file that included gb.itp, I got a segfault in gmx grompp as follows:
0x00000000006ebd44 in DS_Check_Order (DS=0x2f84a40, d=d@entry=d_implicit_genborn_params)
at /home/kasson/gromacs-em/src/gromacs/gmxpreprocess/topdirs.cpp:475
475 if (necessary[d][0] == d_none)
(gdb) print d
$1 = d_implicit_genborn_params

necessary[d] is a null pointer

commenting out the #include "gb" in the CHARMM forcefield.itp fixed this, but it seems we should handle the error more gracefully because people might try to run current gromacs on topologies they prepared previously.

Associated revisions

Revision efd905fd (diff)
Added by Mark Abraham about 2 years ago

Fix reading old force field files

Old force field files that contain fields suitable for explicit
solvent still need to be readable. grompp could already still read the
directives in old force field files and ignore them. However, when
they are read, grompp still needs to understand that they are in a
valid place within the .top file structure. While any place is now
valid (because the contents are ignored), it is simplest to keep the
former requirement, since all files that worked with older GROMACS had
to conform with that requirement.

That requirement was removed in the patch that removed implicit
solvent support Ib241555ff3d8, and it should not have been removed.

I left a comment noting that this is a legacy constraint, not a
necessary one. But probably we'll rewrite topology handling before we
would ever care.

Fixes #2406

Change-Id: Ice11fd34963f7b2a3d5a3b66545c53587d834952


#1 Updated by Peter Kasson about 2 years ago

I should clarify that the simulation in no way used GB; it was having the ff parameters in the topology that caused the segfault.

#2 Updated by Mark Abraham about 2 years ago

Thanks. Certainly we should skip such directives gracefully, but I don't recall whether I tested it explicitly. Perhaps the removals at were overzealous.

#3 Updated by Mark Abraham about 2 years ago

  • Status changed from New to Accepted

reproduced with regressiontests complex/aminoacids using a 2016-era charmm27.ff

#4 Updated by Gerrit Code Review Bot about 2 years ago

Gerrit received a related patchset '1' for Issue #2406.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~Ice11fd34963f7b2a3d5a3b66545c53587d834952
Gerrit URL:

#5 Updated by Mark Abraham about 2 years ago

  • Status changed from Accepted to Fix uploaded

Does that work for your case, Peter?

#6 Updated by Peter Kasson about 2 years ago

Yes, it does. Thanks for the fast patch!

#7 Updated by Mark Abraham about 2 years ago

  • Status changed from Fix uploaded to Resolved

#8 Updated by Mark Abraham almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF