bug#7595: Infinite loop in cc-mode, or the coming of autumn

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 72 bytes --]

In a c-mode buf,
press pound, then back, then M-e
See eternal loop


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Alan Mackenzie]

Hi again, Daniel!

On Wed, Dec 08, 2010 at 03:08:33PM -0800, Daniel Colascione wrote:
> In a c-mode buf,
> press pound, then back, then M-e
> See eternal loop
> 

Would you try out the following patch, please, which I hope fixes the
problem.  (The line numbers will be a little different, but don't worry
about that.)  Let me know how well it works!  Thanks.




*** orig/cc-cmds.el	2010-12-12 21:08:40.000000000 +0000
--- cc-cmds.el	2010-12-16 20:55:30.778431600 +0000
***************
*** 2422,2434 ****
  	  (goto-char last)
  	  (throw 'done '(nil . nil)))
  
! 	 ;; Stop if we encounter a preprocessor line.
! 	 ((and (not macro-end)
  	       (eq (char-after) ?#)
  	       (= (point) (c-point 'boi)))
! 	  (goto-char last)
! 	  ;(throw 'done (cons (eq (point) here) 'macro-boundary))) ; Changed 2003/3/26
! 	  (throw 'done '(t . macro-boundary)))
  
  	 ;; Stop after a ';', '}', or "};"
  	 ((looking-at ";\\|};?")
--- 2422,2435 ----
  	  (goto-char last)
  	  (throw 'done '(nil . nil)))
  
! 	 ;; Stop if we encounter a preprocessor line.  Continue if we hit a naked #
! 	 ((and c-opt-cpp-prefix
! 	       (not macro-end)
  	       (eq (char-after) ?#)
  	       (= (point) (c-point 'boi)))
! 	  (if (= (point) here)		; Not a macro, therefore naked #.
! 	      (forward-char)
! 	    (throw 'done '(t . macro-boundary))))
  
  	 ;; Stop after a ';', '}', or "};"
  	 ((looking-at ";\\|};?")
***************
*** 2627,2640 ****
  		;; Are we about to move forward into or out of a
  		;; preprocessor command?
  		(when (eq (cdr res) 'macro-boundary)
! 		  (save-excursion
! 		    (end-of-line)
! 		    (setq macro-fence
! 			  (and (not (eobp))
! 			       (progn (c-skip-ws-forward)
! 				      (c-beginning-of-macro))
! 			       (progn (c-end-of-macro)
! 				      (point))))))
  		;; Are we about to move forward into a literal?
  		(when (memq (cdr res) '(macro-boundary literal))
  		  (setq range (c-ascertain-following-literal)))
--- 2628,2646 ----
  		;; Are we about to move forward into or out of a
  		;; preprocessor command?
  		(when (eq (cdr res) 'macro-boundary)
! 		  (setq macro-fence
! 			(save-excursion
! 			  (if macro-fence
! 			      (progn
! 				(end-of-line)
! 				(and (not (eobp))
! 				     (progn (c-skip-ws-forward)
! 					    (c-beginning-of-macro))
! 				     (progn (c-end-of-macro)
! 					    (point))))
! 			    (and (not (eobp))
! 				 (c-beginning-of-macro)
! 				 (progn (c-end-of-macro) (point)))))))
  		;; Are we about to move forward into a literal?
  		(when (memq (cdr res) '(macro-boundary literal))
  		  (setq range (c-ascertain-following-literal)))


 
All the best!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Alan,

On 12/16/10 1:39 PM, Alan Mackenzie wrote:
> Would you try out the following patch, please, which I hope fixes the
> problem.  (The line numbers will be a little different, but don't worry
> about that.)  Let me know how well it works!  Thanks.

The patch seems to work well so far. I've replaced my workaround with
it, and I'll let you know whether I see any problems.

Thanks!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)

iEYEARECAAYFAk1F800ACgkQ17c2LVA10Vvx1ACePFyNv1ooW1MYjWBBIOMsGHKK
HtsAoJeHH5UH3YKBs7I89xkJh/lJ3RUZ
=lsup
-----END PGP SIGNATURE-----

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Chong Yidong]

Daniel Colascione <dan.colascione@gmail.com> writes:

> Hi Alan,
>
> On 12/16/10 1:39 PM, Alan Mackenzie wrote:
>> Would you try out the following patch, please, which I hope fixes the
>> problem.  (The line numbers will be a little different, but don't worry
>> about that.)  Let me know how well it works!  Thanks.
>
> The patch seems to work well so far. I've replaced my workaround with
> it, and I'll let you know whether I see any problems.

I've committed Alan's patch to the branch, so that it will be in the
forthcoming pretest.

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

On 1/31/2011 3:08 PM, Chong Yidong wrote:
> Daniel Colascione<dan.colascione@gmail.com>  writes:
>
>> Hi Alan,
>>
>> On 12/16/10 1:39 PM, Alan Mackenzie wrote:
>>> Would you try out the following patch, please, which I hope fixes the
>>> problem.  (The line numbers will be a little different, but don't worry
>>> about that.)  Let me know how well it works!  Thanks.
>>
>> The patch seems to work well so far. I've replaced my workaround with
>> it, and I'll let you know whether I see any problems.
>
> I've committed Alan's patch to the branch, so that it will be in the
> forthcoming pretest.

The following patch repairs a defect in the original that results in an 
infinite loop in c-beginning-of-statement under the following circumstance:

1 #define some_macro value
2
3 [P]
4

Where [P] denotes the point.

=== modified file 'lisp/progmodes/cc-cmds.el'
--- lisp/progmodes/cc-cmds.el   2011-03-04 03:53:00 +0000
+++ lisp/progmodes/cc-cmds.el   2011-03-06 06:20:02 +0000
@@ -2518,6 +2518,8 @@
      (c-save-buffer-state
         ((count (or count 1))
          last ; start point for going back ONE chunk.  Updated each 
chunk movement.
+
+         ;; Beginning of current or previous macro
          (macro-fence
           (save-excursion (and (not (bobp)) (c-beginning-of-macro) 
(point))))
          res                            ; result from sub-function call
@@ -2580,19 +2582,15 @@
                 ;; Are we about to move backwards into or out of a
                 ;; preprocessor command?  If so, locate its beginning.
                 (when (eq (cdr res) 'macro-boundary)
-                 (setq macro-fence
-                       (save-excursion
-                         (if macro-fence
-                             (progn
-                               (end-of-line)
-                               (and (not (eobp))
-                                    (progn (c-skip-ws-forward)
-                                           (c-beginning-of-macro))
-                                    (progn (c-end-of-macro)
-                                           (point))))
-                           (and (not (eobp))
-                                (c-beginning-of-macro)
-                                (progn (c-end-of-macro) (point)))))))
+                  ;; c-back-over-illiterals stopped at the
+                  ;; first non-whitespace character after
+                  ;; the macro end, so the previous nonblank
+                  ;; line should be the macro.
+                  (setq macro-fence
+                        (save-excursion
+                          (c-backward-comments)
+                          (point))))
+
                 ;; Are we about to move backwards into a literal?
                 (when (memq (cdr res) '(macro-boundary literal))
                   (setq range (c-ascertain-preceding-literal)))

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Chong Yidong]

Daniel Colascione <dan.colascione@gmail.com> writes:

> The following patch repairs a defect in the original that results in
> an infinite loop in c-beginning-of-statement under the following
> circumstance:
>
> 1 #define some_macro value
> 2
> 3 [P]
> 4
>
> Where [P] denotes the point.

Le sigh.  We can't keep 23.3 on hold while refining this code, with the
problems in each iteration taking one whole month to surface.

FWIW, in my day to day usage of the 23.3 release candidate, I haven't
encountered any looping problems.  Did you come across this in a real
world situation?  If so, please describe it, and we will shall see if we
just have to bite the bullet and continue pretesting.  But if it does
not trigger for realistic usage, I prefer to leave matters as they are
for 23.3---i.e. swapping one rare infloop in 23.2 (Bug#7595) for a
different one in 23.3 (the above).

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Chong Yidong]

> we will shall see if we just have to bite the bullet and continue
> pretesting.  But if it does not trigger for realistic usage, I prefer
> to leave matters as they are for 23.3---i.e. swapping one rare infloop
> in 23.2 (Bug#7595) for a different one in 23.3 (the above).

On reflection, it's probably better to revert the original fix for 7595,
and go back to the (problematic) prior behavior.  As for the patch you
proposed, it doesn't look obvious and safe enough to include at this
late stage.



------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

On 3/6/2011 9:00 PM, Chong Yidong wrote:
>> we will shall see if we just have to bite the bullet and continue
>> pretesting.  But if it does not trigger for realistic usage, I prefer
>> to leave matters as they are for 23.3---i.e. swapping one rare infloop
>> in 23.2 (Bug#7595) for a different one in 23.3 (the above).
> On reflection, it's probably better to revert the original fix for 7595,
> and go back to the (problematic) prior behavior.  As for the patch you
> proposed, it doesn't look obvious and safe enough to include at this
> late stage.

That's probably the safest course for now.  The original patch adds a 
fair amount of logic, and I'm not entirely convinced myself it's the 
right thing.

This simple workaround fixes the immediate issue raised in bug 7595:

=== modified file 'lisp/progmodes/cc-cmds.el'
--- lisp/progmodes/cc-cmds.el   2010-10-31 11:54:02 +0000
+++ lisp/progmodes/cc-cmds.el   2010-12-09 00:18:26 +0000
@@ -2616,6 +2616,11 @@
               (setq count 0)
               nil)

+             ((looking-at "^\\s-*#\\s-*$")
+              (setq count 0)
+              (end-of-line)
+              nil)
+
              (range                     ; point is within a literal.
               (cond
                ;; sentence-flag is null => skip the entire literal.

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Alan Mackenzie]

Hi, Yidong, Daniel, Stefan!

First thing about Daniel's bug: it doesn't occur in the stand-alone CC
Mode running on Emacs 23.2; this version of CC Mode contains my original
patch from a couple of months ago.

Before applying a fix to the current bug (Daniel, are you sure the
current bug is the "same" (whatever that means) as the original bug?),
I'd like to work out what the difference is between stand-alone CC Mode
and the one in Emacs-23.3/..../lisp/progmodes.

On Mon, Mar 07, 2011 at 12:00:23AM -0500, Chong Yidong wrote:
> > we will shall see if we just have to bite the bullet and continue
> > pretesting.  But if it does not trigger for realistic usage, I
> > prefer to leave matters as they are for 23.3---i.e. swapping one
> > rare infloop in 23.2 (Bug#7595) for a different one in 23.3 (the
> > above).

They're nasty little things which are going to happen occasionally (on
perfectly normal buffer content) when people are frobbing random key
sequences.  Does that count as "realistic usage"?

> On reflection, it's probably better to revert the original fix for
> 7595, and go back to the (problematic) prior behavior.  As for the
> patch you proposed, it doesn't look obvious and safe enough to include
> at this late stage.

Aww!  Are you convinced that the two bugs here really are two aspects of
the same bug?  If not, I'd propose leaving in the 7595 fix.

If my patch for 7595 is taken out, does the most recent bug still
happen?

-- 
Alan Mackenzie (Nuremberg, Germany).



------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> First thing about Daniel's bug: it doesn't occur in the stand-alone CC
> Mode running on Emacs 23.2; this version of CC Mode contains my original
> patch from a couple of months ago.
>
> Before applying a fix to the current bug (Daniel, are you sure the
> current bug is the "same" (whatever that means) as the original bug?),
> I'd like to work out what the difference is between stand-alone CC Mode
> and the one in Emacs-23.3/..../lisp/progmodes.

OK, I see the problem.  The patch you supplied in the email was not
applied correctly to the brach.  The second hunk ended up in
c-beginning-of-statement.

So, it looks like my fault, and I'm not sure how it happened (maybe due
to the line number mismatch).  Sorry.

With this corrected in the branch, the new problem goes away.  I will
make a second release candidate with this fix.



------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d

Re: bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Alan Mackenzie]

Hi, Yidong,

On Mon, Mar 07, 2011 at 03:09:21PM -0500, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > First thing about Daniel's bug: it doesn't occur in the stand-alone CC
> > Mode running on Emacs 23.2; this version of CC Mode contains my original
> > patch from a couple of months ago.

> > Before applying a fix to the current bug (Daniel, are you sure the
> > current bug is the "same" (whatever that means) as the original bug?),
> > I'd like to work out what the difference is between stand-alone CC Mode
> > and the one in Emacs-23.3/..../lisp/progmodes.

> OK, I see the problem.  The patch you supplied in the email was not
> applied correctly to the branch.  The second hunk ended up in
> c-beginning-of-statement.

I think the problem is in the Emacs 24 trunk, too.

> So, it looks like my fault, and I'm not sure how it happened (maybe due
> to the line number mismatch).  Sorry.

> With this corrected in the branch, the new problem goes away.  I will
> make a second release candidate with this fix.

Excellent!  Thanks for finding it.

-- 
Alan Mackenzie (Nuremberg, Germany).

------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

Hi Alan, Yidong,

On 3/7/11 1:02 PM, Alan Mackenzie wrote:
> I think the problem is in the Emacs 24 trunk, too.
> 
>> So, it looks like my fault, and I'm not sure how it happened (maybe due
>> to the line number mismatch).  Sorry.
> 
>> With this corrected in the branch, the new problem goes away.  I will
>> make a second release candidate with this fix.
> 
> Excellent!  Thanks for finding it.
> 

Thanks for looking into this: by the time I get a chance to relax for
the day and check my email, it's already fixed. :-)

It never even occurred to me that the patch wasn't properly applied, or
that I should check against the cc-mode CVS; I usually hew closer to the
Emacs trunk cc-mode.  I've never seen patch botch a modification like
this before.  Out of curiosity, I rolled my branch back to the state it
was in when I applied your patch manually, and it worked fine (albeit
with frighteningly mismatched line numbers).  I wonder how many other
silently misapplied changes are lurking in the code.  Maybe we should
start including more context in our diffs?

Alan, by the way: have you considered keeping cc-mode HEAD in a dvcs?
It'd make synchronizing the various codebases easier, and it'd be
possible to simply issue pull requests instead of emailing patches or
manually making multiple checkins.

Thanks,
Daniel Colascione


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Alan Mackenzie]

Hi, all!

On Mon, Mar 07, 2011 at 12:00:23AM -0500, Chong Yidong wrote:
> > we will shall see if we just have to bite the bullet and continue
> > pretesting.  But if it does not trigger for realistic usage, I prefer
> > to leave matters as they are for 23.3---i.e. swapping one rare infloop
> > in 23.2 (Bug#7595) for a different one in 23.3 (the above).

> On reflection, it's probably better to revert the original fix for 7595,
> and go back to the (problematic) prior behavior.  As for the patch you
> proposed, it doesn't look obvious and safe enough to include at this
> late stage.

Daniel, you correctly identified the wrong bit of code in your proposed
patch.  What has happened is that the code in c-beginning-of-statement
which handles macros has been replaced by the analogous code from
c-END-of-statement.  This is totally crazy and totally wrong.  I've no
record of doing this myself, there's nothing about it I can find in
.../lisp/ChangeLog, so, as yet, I've no idea how it happened.

I think the best solution is to reverse this mysterious change, making
c-b-o-s again identical to the version in Emacs 23.2 (modulo a corrected
typo in a comment).  Here is a patch which does exactly that:



=== modified file 'lisp/progmodes/cc-cmds.el'
*** lisp/progmodes/cc-cmds.el	2011-02-20 18:36:29 +0000
--- lisp/progmodes/cc-cmds.el	2011-03-07 20:39:34 +0000
***************
*** 2564,2582 ****
  		;; Are we about to move backwards into or out of a
  		;; preprocessor command?  If so, locate its beginning.
  		(when (eq (cdr res) 'macro-boundary)
! 		  (setq macro-fence
! 			(save-excursion
! 			  (if macro-fence
! 			      (progn
! 				(end-of-line)
! 				(and (not (eobp))
! 				     (progn (c-skip-ws-forward)
! 					    (c-beginning-of-macro))
! 				     (progn (c-end-of-macro)
! 					    (point))))
! 			    (and (not (eobp))
! 				 (c-beginning-of-macro)
! 				 (progn (c-end-of-macro) (point)))))))
  		;; Are we about to move backwards into a literal?
  		(when (memq (cdr res) '(macro-boundary literal))
  		  (setq range (c-ascertain-preceding-literal)))
--- 2564,2575 ----
  		;; Are we about to move backwards into or out of a
  		;; preprocessor command?  If so, locate its beginning.
  		(when (eq (cdr res) 'macro-boundary)
! 		  (save-excursion
! 		    (beginning-of-line)
! 		    (setq macro-fence
! 			  (and (not (bobp))
! 			       (progn (c-skip-ws-backward) (c-beginning-of-macro))
! 			       (point)))))
  		;; Are we about to move backwards into a literal?
  		(when (memq (cdr res) '(macro-boundary literal))
  		  (setq range (c-ascertain-preceding-literal)))


What does everybody think?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7595: Infinite loop in cc-mode, or the coming of autumn

[Daniel Colascione]

On 1/31/2011 3:08 PM, Chong Yidong wrote:
> Daniel Colascione<dan.colascione@gmail.com>  writes:
>
>> Hi Alan,
>>
>> On 12/16/10 1:39 PM, Alan Mackenzie wrote:
>>> Would you try out the following patch, please, which I hope fixes the
>>> problem.  (The line numbers will be a little different, but don't worry
>>> about that.)  Let me know how well it works!  Thanks.
>>
>> The patch seems to work well so far. I've replaced my workaround with
>> it, and I'll let you know whether I see any problems.
>
> I've committed Alan's patch to the branch, so that it will be in the
> forthcoming pretest.

The following patch repairs a defect in the original that results in an 
infinite loop in c-beginning-of-statement under the following circumstance:

1 #define some_macro value
2
3 [P]
4

Where [P] denotes the point.

=== modified file 'lisp/progmodes/cc-cmds.el'
--- lisp/progmodes/cc-cmds.el   2011-03-04 03:53:00 +0000
+++ lisp/progmodes/cc-cmds.el   2011-03-06 06:20:02 +0000
@@ -2518,6 +2518,8 @@
      (c-save-buffer-state
         ((count (or count 1))
          last ; start point for going back ONE chunk.  Updated each 
chunk movement.
+
+         ;; Beginning of current or previous macro
          (macro-fence
           (save-excursion (and (not (bobp)) (c-beginning-of-macro) 
(point))))
          res                            ; result from sub-function call
@@ -2580,19 +2582,15 @@
                 ;; Are we about to move backwards into or out of a
                 ;; preprocessor command?  If so, locate its beginning.
                 (when (eq (cdr res) 'macro-boundary)
-                 (setq macro-fence
-                       (save-excursion
-                         (if macro-fence
-                             (progn
-                               (end-of-line)
-                               (and (not (eobp))
-                                    (progn (c-skip-ws-forward)
-                                           (c-beginning-of-macro))
-                                    (progn (c-end-of-macro)
-                                           (point))))
-                           (and (not (eobp))
-                                (c-beginning-of-macro)
-                                (progn (c-end-of-macro) (point)))))))
+                  ;; c-back-over-illiterals stopped at the
+                  ;; first non-whitespace character after
+                  ;; the macro end, so the previous nonblank
+                  ;; line should be the macro.
+                  (setq macro-fence
+                        (save-excursion
+                          (c-backward-comments)
+                          (point))))
+
                 ;; Are we about to move backwards into a literal?
                 (when (memq (cdr res) '(macro-boundary literal))
                   (setq range (c-ascertain-preceding-literal)))