Avoid crash with empty reference for 'same as' selection

Make 'same ... as none' selections not segfault.  If there were no atoms
in the reference group, the code that sorted the values and removed
duplicates incorrectly set the number of values as one, resulting in
incorrect memory access later.

Fixes #1742 (segfault part).

Change-Id: I9d14c30404121356ee3abf1a5575fb0baa82fb7b

diff --git a/src/gromacs/selection/sm_same.cpp b/src/gromacs/selection/sm_same.cpp
index 71192b842d9c01eece0ef71422dcae99891840aa..cef245c5d928991e510e7436cefdbd856aa16fd7 100644 (file)
--- a/src/gromacs/selection/sm_same.cpp
+++ b/src/gromacs/selection/sm_same.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015, 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.
@@ -343,6 +343,10 @@ init_frame_same_int(t_topology * /* top */, t_trxframe * /* fr */, t_pbc * /* pb
 
     /* Collapse adjacent values, and check whether the array is sorted. */
     d->bSorted = true;
+    if (d->nas == 0)
+    {
+        return;
+    }
     for (i = 1, j = 0; i < d->nas; ++i)
     {
         if (d->as.i[i] != d->as.i[j])
@@ -469,6 +473,10 @@ init_frame_same_str(t_topology * /* top */, t_trxframe * /* fr */, t_pbc * /* pb
      * For strings, it's unlikely that the values would be sorted originally,
      * so set bSorted always to false. */
     d->bSorted        = false;
+    if (d->nas == 0)
+    {
+        return;
+    }
     d->as_s_sorted[0] = d->as.s[0];
     for (i = 1, j = 0; i < d->nas; ++i)
     {

diff --git a/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesEmptyReferenceForSame.xml b/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesEmptyReferenceForSame.xml
new file mode 100644 (file)
index 0000000..49fb847
--- /dev/null
+++ b/src/gromacs/selection/tests/refdata/SelectionCollectionDataTest_HandlesEmptyReferenceForSame.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <ParsedSelections Name="Parsed">
+    <ParsedSelection Name="Selection1">
+      <String Name="Input">same residue as none</String>
+      <String Name="Text">same residue as none</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+    <ParsedSelection Name="Selection2">
+      <String Name="Input">same resname as none</String>
+      <String Name="Text">same resname as none</String>
+      <Bool Name="Dynamic">false</Bool>
+    </ParsedSelection>
+  </ParsedSelections>
+  <CompiledSelections Name="Compiled">
+    <Selection Name="Selection1">
+      <Sequence Name="Atoms">
+        <Int Name="Length">0</Int>
+      </Sequence>
+    </Selection>
+    <Selection Name="Selection2">
+      <Sequence Name="Atoms">
+        <Int Name="Length">0</Int>
+      </Sequence>
+    </Selection>
+  </CompiledSelections>
+</ReferenceData>

diff --git a/src/gromacs/selection/tests/selectioncollection.cpp b/src/gromacs/selection/tests/selectioncollection.cpp
index 66369c1a7d6293a5865c7bd7ffc311c4c0b74ab1..6a40ad7462b8411f47554e27e0a9c69022b0d634 100644 (file)
--- a/src/gromacs/selection/tests/selectioncollection.cpp
+++ b/src/gromacs/selection/tests/selectioncollection.cpp
@@ -1087,6 +1087,16 @@ TEST_F(SelectionCollectionDataTest, HandlesEmptySelectionWithUnevaluatedExpressi
 }
 
 
+TEST_F(SelectionCollectionDataTest, HandlesEmptyReferenceForSame)
+{
+    static const char * const selections[] = {
+        "same residue as none",
+        "same resname as none"
+    };
+    runTest("simple.gro", selections);
+}
+
+
 TEST_F(SelectionCollectionDataTest, HandlesNumericComparisons)
 {
     static const char * const selections[] = {