GNU bug report logs - #43499
27.1; It is possible for (forward-comment -1) to crash emacs

Previous Next

Full log

Message #8 received at 43499 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jeff Norden <jnorden <at> tntech.edu>
Cc: 43499 <at> debbugs.gnu.org
Subject: Re: bug#43499: 27.1;
 It is possible for (forward-comment -1) to crash emacs
Date: Sat, 19 Sep 2020 12:08:51 +0300

> From: Jeff Norden <jnorden <at> tntech.edu>
> Date: Fri, 18 Sep 2020 20:25:33 -0500
> 
> In an unusual circumstance, (forward-comment -1) can move the point before the
> accessible buffer text.  This can even result in the point becoming negative.
> In the worst-case scenario, emacs becomes completely unresponsive, and it
> might even be necessary to reboot the computer.

Thanks.  In my case, I get a segfault in DEC_BOTH (because it attempts
to dereference a pointer outside of buffer text).

> The loop should, I think, be changed to the following.  The only change is how
> from and stop are compared.
> 
>               while (from > stop)
>                 {
>                   DEC_BOTH (from, from_byte);
>                   UPDATE_SYNTAX_TABLE_BACKWARD (from);
>                   c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
>                   if (SYNTAX (c) == Scomment_fence
>                       && !char_quoted (from, from_byte))
>                     {
>                       fence_found = 1;
>                       break;
>                     }
>                   rarely_quit (++quit_count);
>                 }

Thanks.  I propose a slightly different change below.  I think it's
somewhat better, because it does the comparison only once, and the
while loop can then run at full speed without testing on each
iteration.  (It looks like a large change, but almost all of it is
just whitespace changes due to re-indentation of the loop.)  Do you
agree?

diff --git a/src/syntax.c b/src/syntax.c
index a79ab86..e8b32f5 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -2545,20 +2545,23 @@ DEFUN ("forward-comment", Fforward_comment, Sforward_comment, 1, 1, 0,
 	      bool fence_found = 0;
 	      ptrdiff_t ini = from, ini_byte = from_byte;
 
-	      while (1)
+	      if (from > stop)
 		{
-		  DEC_BOTH (from, from_byte);
-		  UPDATE_SYNTAX_TABLE_BACKWARD (from);
-		  c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
-		  if (SYNTAX (c) == Scomment_fence
-		      && !char_quoted (from, from_byte))
+		  while (1)
 		    {
-		      fence_found = 1;
-		      break;
+		      DEC_BOTH (from, from_byte);
+		      UPDATE_SYNTAX_TABLE_BACKWARD (from);
+		      c = FETCH_CHAR_AS_MULTIBYTE (from_byte);
+		      if (SYNTAX (c) == Scomment_fence
+			  && !char_quoted (from, from_byte))
+			{
+			  fence_found = 1;
+			  break;
+			}
+		      else if (from == stop)
+			break;
+		      rarely_quit (++quit_count);
 		    }
-		  else if (from == stop)
-		    break;
-		  rarely_quit (++quit_count);
 		}
 	      if (fence_found == 0)
 		{

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.