[AC21.5] Fix assert in reverse search

Stephen J. Turnbull turnbull at sk.tsukuba.ac.jp
Mon Dec 10 04:29:32 EST 2007


APPROVE COMMIT 21.5

Thanks to Klaus Reim for the excellent test case and Aidan for
noticing the connection and verifying the fix.

I decided to put the test in tests/reproduce-bugs.el.  I'll probably
be renaming that to reproduce-crashes.el, since non-fatal bugs should
be tested using test-harness.

People who write tests might be interested to see the technique used:
a vector of keystrokes is executed using `execute-kbd-macro'.  This is
convenient here because the test case involves an interactive feature
of isearch.  It probably could be refined to a more primitive test
case, but why bother?  A crash is a pretty coarse criterion!

Stephen J. Turnbull writes:
 > 21.5
 > 
 > In search.c simple_search(), a reverse search can underrun the buffer.
 > In a debug build, this will assert in DEC_BYTEBPOS.  This doesn't
 > happen on most reverse searches because you need the initial substring
 > of the buffer to match the tail of the pattern, or the buffer pointer
 > won't get to where it tries to decrement from 1.
 > 
 > At least, that's what I think is going on.  I will commit when I've
 > constructed a test case and verified the theory.
 > 
 > This patch also includes the usual marginal doc "IMHO improvements".
 > 
 > This patch is informational only, I used cvs diff -w to generate it.
 > Do not apply this patch.
 > 
 > Index: src/ChangeLog
 > ===================================================================
 > RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v
 > retrieving revision 1.1112
 > diff -u -U0 -r1.1112 ChangeLog
 > --- src/ChangeLog	5 Dec 2007 08:26:00 -0000	1.1112
 > +++ src/ChangeLog	5 Dec 2007 09:08:30 -0000
 > @@ -0,0 +1,5 @@
 > +2007-12-05  Stephen J. Turnbull  <stephen at xemacs.org>
 > +
 > +	* search.c (simple_search): Fix underrun in reverse search.
 > +	(search_buffer): Clarify decision to use boyer_moore or not.
 > +
 > 
 > Index: src/search.c
 > ===================================================================
 > RCS file: /pack/xemacscvs/XEmacs/xemacs/src/search.c,v
 > retrieving revision 1.47
 > diff -u -w -r1.47 search.c
 > --- src/search.c	1 Oct 2007 08:07:53 -0000	1.47
 > +++ src/search.c	5 Dec 2007 09:08:34 -0000
 > @@ -1371,14 +1371,17 @@
 >  	    boyer_moore_ok = 0;
 >  	  if (translated != c || inverse != c)
 >  	    {
 > -	      /* Keep track of which character set row
 > -		 contains the characters that need translation.  */
 > +	      /* Keep track of which charset and character set row
 > +		 contains the characters that need translation.
 > +		 Zero out the bits corresponding to the last byte.
 > +	      */
 >  	      int charset_base_code = c & ~ICHAR_FIELD3_MASK;
 >  	      if (charset_base == -1)
 >  		charset_base = charset_base_code;
 >  	      else if (charset_base != charset_base_code)
 > -		/* If two different rows appear, needing translation,
 > -		   then we cannot use boyer_moore search.  */
 > +		/* If two different rows appear, needing translation, then
 > +		   we cannot use boyer_moore search.  See the comment at the
 > +		   head of boyer_moore(). */
 >  		boyer_moore_ok = 0;
 >  	    }
 >  	  memcpy (pat, tmp_str, new_bytelen);
 > @@ -1468,6 +1471,13 @@
 >  	n--;
 >        }
 >    else
 > +    {
 > +      /* If lim < len, then there are too few buffer positions to hold the
 > +	 pattern between the beginning of the buffer and lim.  Adjust to
 > +	 ensure pattern fits.  If we don't do this, we can assert in the
 > +	 DEC_BYTEBPOS below. */
 > +      if (lim < len)
 > +	lim = len;
 >      while (n < 0)
 >        {
 >  	while (1)
 > @@ -1505,6 +1515,7 @@
 >  	  }
 >  	n++;
 >        }
 > +    }
 >   stop:
 >    if (n == 0)
 >      {
 > 
 > _______________________________________________
 > XEmacs-Patches mailing list
 > XEmacs-Patches at xemacs.org
 > http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-patches




More information about the XEmacs-Patches mailing list