Fix recently introduced bool vector overrun.

This was due to an optimization that went awry.
Reported by Glenn Morris in
<http://lists.gnu.org/archive/html/emacs-devel/2013-11/msg00622.html>.
* alloc.c (make_uninit_bool_vector): Don't allocate a dummy word
for empty vectors, undoing the 2013-11-18 change.
* data.c (bool_vector_binop_driver): Rely on this.
Fix bug that occasionally overran the destination.
* lisp.h (struct Lisp_Bool_vector): Document this.

diff --git a/src/ChangeLog b/src/ChangeLog
index e0ac7f298c7353eea74146ecb6f35bfe2b2f1ab1..925f6389bf8b5649b5fedf4e9c3971cd3a5a47ce 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,15 @@
+2013-11-21  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Fix recently introduced bool vector overrun.
+       This was due to an optimization that went awry.
+       Reported by Glenn Morris in
+       <http://lists.gnu.org/archive/html/emacs-devel/2013-11/msg00622.html>.
+       * alloc.c (make_uninit_bool_vector): Don't allocate a dummy word
+       for empty vectors, undoing the 2013-11-18 change.
+       * data.c (bool_vector_binop_driver): Rely on this.
+       Fix bug that occasionally overran the destination.
+       * lisp.h (struct Lisp_Bool_vector): Document this.
+
 2013-11-20  Jan Djärv  <jan.h.d@swipnet.se>
 
        * nsterm.m (init, run, stop:): Enable again. stop calls super stop

diff --git a/src/alloc.c b/src/alloc.c
index 7c560fd0f0d2a85e9c791c0fc25aa0ac7fe16c45..283bc613c82c4a97281dcc7dff17229a2d4b4ed9 100644 (file)
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -2066,8 +2066,7 @@ Lisp_Object
 make_uninit_bool_vector (EMACS_INT nbits)
 {
   Lisp_Object val;
-  EMACS_INT words0 = bool_vector_words (nbits);
-  EMACS_INT words = words0 + !words0;  /* Allocate at least one word.  */
+  EMACS_INT words = bool_vector_words (nbits);
   EMACS_INT word_bytes = words * sizeof (bits_word);
   EMACS_INT needed_elements = ((bool_header_size - header_size + word_bytes
                                + word_size - 1)
@@ -2078,9 +2077,9 @@ make_uninit_bool_vector (EMACS_INT nbits)
   XSETPVECTYPESIZE (XVECTOR (val), PVEC_BOOL_VECTOR, 0, 0);
   p->size = nbits;
 
-  /* Clear padding at the end.  If NBITS != 0 this initializes more
-     than it needs to, but that's OK.  */
-  p->data[words - 1] = 0;
+  /* Clear padding at the end.  */
+  if (words)
+    p->data[words - 1] = 0;
 
   return val;
 }

diff --git a/src/data.c b/src/data.c
index 33dd11049ec839f8902253c9cd53cd4048679cc0..bdd56bf0f62ef0b919c1d44f3a46fba09bdcb7a4 100644 (file)
--- a/src/data.c
+++ b/src/data.c
@@ -3054,60 +3054,64 @@ bool_vector_binop_driver (Lisp_Object a,
       switch (op)
        {
        case bool_vector_exclusive_or:
-         while (destdata[i] == (adata[i] ^ bdata[i]))
-           if (! (++i < nr_words))
-             return Qnil;
+         for (; i < nr_words; i++)
+           if (destdata[i] != (adata[i] ^ bdata[i]))
+             goto set_dest;
          break;
 
        case bool_vector_subsetp:
-       case bool_vector_union:
-         while (destdata[i] == (adata[i] | bdata[i]))
-           if (! (++i < nr_words))
+         for (; i < nr_words; i++)
+           if (adata[i] &~ bdata[i])
              return Qnil;
+         return Qt;
+
+       case bool_vector_union:
+         for (; i < nr_words; i++)
+           if (destdata[i] != (adata[i] | bdata[i]))
+             goto set_dest;
          break;
 
        case bool_vector_intersection:
-         while (destdata[i] == (adata[i] & bdata[i]))
-           if (! (++i < nr_words))
-             return Qnil;
+         for (; i < nr_words; i++)
+           if (destdata[i] != (adata[i] & bdata[i]))
+             goto set_dest;
          break;
 
        case bool_vector_set_difference:
-         while (destdata[i] == (adata[i] &~ bdata[i]))
-           if (! (++i < nr_words))
-             return Qnil;
+         for (; i < nr_words; i++)
+           if (destdata[i] != (adata[i] &~ bdata[i]))
+             goto set_dest;
          break;
        }
+
+      return Qnil;
     }
 
+ set_dest:
   switch (op)
     {
     case bool_vector_exclusive_or:
-      do
+      for (; i < nr_words; i++)
        destdata[i] = adata[i] ^ bdata[i];
-      while (++i < nr_words);
-      break;
-
-    case bool_vector_subsetp:
       break;
 
     case bool_vector_union:
-      do
+      for (; i < nr_words; i++)
        destdata[i] = adata[i] | bdata[i];
-      while (++i < nr_words);
       break;
 
     case bool_vector_intersection:
-      do
+      for (; i < nr_words; i++)
        destdata[i] = adata[i] & bdata[i];
-      while (++i < nr_words);
       break;
 
     case bool_vector_set_difference:
-      do
+      for (; i < nr_words; i++)
        destdata[i] = adata[i] &~ bdata[i];
-      while (++i < nr_words);
       break;
+
+    default:
+      eassume (0);
     }
 
   return dest;

diff --git a/src/lisp.h b/src/lisp.h
index 8521c87e5d7fb618e344ba4b7bcf023c9a306344..a9aac2cc0bbc0a744bbcc273a1159627d8e6163e 100644 (file)
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1213,7 +1213,7 @@ struct Lisp_Bool_Vector
     /* This is the size in bits.  */
     EMACS_INT size;
     /* The actual bits, packed into bytes.
-       Zeros fill out the last word as needed; there's always at least one word.
+       Zeros fill out the last word if needed.
        The bits are in little-endian order in the bytes, and
        the bytes are in little-endian order in the words.  */
     bits_word data[FLEXIBLE_ARRAY_MEMBER];