[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