GNU bug report logs - #53203
Comment with lots of color codes crashes or hangs emacs in scss-mode

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 53203 in the body.
You can then email your comments to 53203 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Colin <my.old.email.sucked <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Comment with lots of color codes crashes or hangs emacs in scss-mode
Date: Wed, 12 Jan 2022 17:20:24 +1000

Hi,


I have the following snippet in example.scss file:

```

/*
#ffffff #ffffff
#ffffff #ffffff
#ffffff #ffffff
#ffffff #ffffff
#ffffff #ffffff
*/
```

On opening the file with emacs -Q (27.1 and a fresh build of git master 
(29.0.50), emacs hangs.

If I remove one/two lines, it appears to burn some CPU and then work, so 
it looks like an exponential search or something.

For now I'll remove the snippet from my code, but an interesting bug 
nonetheless ;)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Colin <my.old.email.sucked <at> gmail.com>
Cc: 53203 <at> debbugs.gnu.org
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Wed, 12 Jan 2022 15:16:57 +0200

> Date: Wed, 12 Jan 2022 17:20:24 +1000
> From: Colin <my.old.email.sucked <at> gmail.com>
> 
> I have the following snippet in example.scss file:
> 
> ```
> 
> /*
> #ffffff #ffffff
> #ffffff #ffffff
> #ffffff #ffffff
> #ffffff #ffffff
> #ffffff #ffffff
> */
> ```
> 
> On opening the file with emacs -Q (27.1 and a fresh build of git master 
> (29.0.50), emacs hangs.
> 
> If I remove one/two lines, it appears to burn some CPU and then work, so 
> it looks like an exponential search or something.
> 
> For now I'll remove the snippet from my code, but an interesting bug 
> nonetheless ;)

It seems to infloop in JIT font-lock, and the culprit seems to be this
part of font-lock-keywords:

       ;; Even though pseudo-elements should be prefixed by ::, a
       ;; single colon is accepted for backward compatibility.
       "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                       css-pseudo-element-ids)
                               t)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 53203 <at> debbugs.gnu.org, Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Thu, 13 Jan 2022 08:00:18 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> It seems to infloop in JIT font-lock, and the culprit seems to be this
> part of font-lock-keywords:
>
>        ;; Even though pseudo-elements should be prefixed by ::, a
>        ;; single colon is accepted for backward compatibility.
>        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
>                                        css-pseudo-element-ids)
>                                t)

Trying to understand the regexp used for scss here, I think that bit is
somewhat innocuous -- it just matches those words.

       ;; Even though pseudo-elements should be prefixed by ::, a
       ;; single colon is accepted for backward compatibility.
       "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                       css-pseudo-element-ids)
                               t)
       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"

But then we get:

       "\\(?:([^)]+)\\)?"
       (if (not sassy)
           "[^:{}()\n]*"
         (concat "[^:{}()\n#]*\\(?:" scss--hash-re "[^:{}()\n#]*\\)*"))
       "\\)*"

Which is a whole lot of backtracking, presumably exacerbated by the
previous ids bit of the regexp.

But I've repressed all I once knew about the scss language -- what is it
really trying to match here?  Anybody?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 53203 <at> debbugs.gnu.org, my.old.email.sucked <at> gmail.com
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Thu, 13 Jan 2022 10:58:34 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Colin <my.old.email.sucked <at> gmail.com>,  53203 <at> debbugs.gnu.org
> Date: Thu, 13 Jan 2022 08:00:18 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > It seems to infloop in JIT font-lock, and the culprit seems to be this
> > part of font-lock-keywords:
> >
> >        ;; Even though pseudo-elements should be prefixed by ::, a
> >        ;; single colon is accepted for backward compatibility.
> >        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
> >                                        css-pseudo-element-ids)
> >                                t)
> 
> Trying to understand the regexp used for scss here, I think that bit is
> somewhat innocuous -- it just matches those words.
> 
>        ;; Even though pseudo-elements should be prefixed by ::, a
>        ;; single colon is accepted for backward compatibility.
>        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
>                                        css-pseudo-element-ids)
>                                t)
>        "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"

I posted that because I saw this regexp in the backtrace obtained by
interrupting the infloop.

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

From: Simen Heggestøyl <simenheg <at> runbox.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sat, 14 May 2022 12:34:25 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> But then we get:
>
>        "\\(?:([^)]+)\\)?"
>        (if (not sassy)
>            "[^:{}()\n]*"
>          (concat "[^:{}()\n#]*\\(?:" scss--hash-re "[^:{}()\n#]*\\)*"))
>        "\\)*"
>
> Which is a whole lot of backtracking, presumably exacerbated by the
> previous ids bit of the regexp.

By the ancient scientific method of commenting out code, it seems to me
that the culprit is rather the last group of the regexp starting on
line 955:

  (concat "\\(?:" scss--hash-re
          "\\|[^@/:{}() \t\n#]\\)"
          "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))

That is, this part:

  \\(?:" scss--hash-re "[^:{}()#]*\\)*

Though I'm rather clueless on how proceed debugging/optimizing it. 😕

> But I've repressed all I once knew about the scss language -- what is
> it really trying to match here?  Anybody?

It's supposed to match selectors like the one on line 39 in
test/manual/indent/scss-mode.scss:

  p.#{$name} var

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sat, 14 May 2022 14:02:54 +0200

Simen Heggestøyl <simenheg <at> runbox.com> writes:

>> But I've repressed all I once knew about the scss language -- what is
>> it really trying to match here?  Anybody?
>
> It's supposed to match selectors like the one on line 39 in
> test/manual/indent/scss-mode.scss:
>
>   p.#{$name} var

Ah, I see -- it wants to match a selector followed by braces.  I.e.,
these things:

p.#{$name} var
{
}

p.#{$name}, f.#{$bar}:active, f.bar::after {
}

p.#{$name} f.#{$bar} k.var #{$bar}  #{$bar}
{
}

p.#{$name}
{
}

Hm...


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sat, 14 May 2022 14:38:21 +0200

OK, I think I understand most of the regexp now, but it's a strange mix
of strictness and sloppiness.

    (,(concat
       "^[ \t]*\\("
       (if (not sassy)
           ;; We don't allow / as first char, so as not to
           ;; take a comment as the beginning of a selector.
           "[^@/:{}() \t\n][^:{}()]*"
         ;; Same as for non-sassy except we do want to allow { and }
         ;; chars in selectors in the case of #{$foo}
         ;; variable interpolation!
         (concat "\\(?:" scss--hash-re
                 "\\|[^@/:{}() \t\n#]\\)"
                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))
       ;; Even though pseudo-elements should be prefixed by ::, a
       ;; single colon is accepted for backward compatibility.
       "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                       css-pseudo-element-ids)
                               t)
       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
       "\\(?:([^)]+)\\)?"

First we work really hard to see whether we have something that looks
like a selector, i.e. a line that starts with,

p
p::after
p:active

or in the scss case

p#{foo}:active


and then we allow any kind of junk for the rest of the line, like

p.a *[line noise]* p.b {
}

with this bit:

     (if (not sassy)
           "[^:{}()\n]*"
         (concat "[^:{}()\n#]*\\(?:" scss--hash-re "[^:{}()\n#]*\\)*"))
       "\\)*"
       "\\)\\(?:\n[ \t]*\\)*{")
     (1 'css-selector keep))


Anyway, I still don't grok this bit:

         (concat "\\(?:" scss--hash-re
                 "\\|[^@/:{}() \t\n#]\\)"
                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))

That is, we're matching an scss/css selector first, and then...
possibly another one?  Which we're doing later, anyway.

So just getting rid of that bit seems to lead to the same highlighting
(and gets rid of the original reported problem).  But like I said, I
don't understand quite what it's trying to do there.  Can anybody come
up with an scss example this breaks?

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 1139fd1976..fd47a7b83b 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -953,14 +953,13 @@ css--font-lock-keywords
          ;; chars in selectors in the case of #{$foo}
          ;; variable interpolation!
          (concat "\\(?:" scss--hash-re
-                 "\\|[^@/:{}() \t\n#]\\)"
-                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))
+                 "\\|[^@/:{}() \t\n][^:{}()]\\)*"))
        ;; Even though pseudo-elements should be prefixed by ::, a
        ;; single colon is accepted for backward compatibility.
        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                        css-pseudo-element-ids)
                                t)
-       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
+       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)?"
        "\\(?:([^)]+)\\)?"
        (if (not sassy)
            "[^:{}()\n]*"



-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sat, 14 May 2022 15:30:26 +0200

But how about just trying to make it stricter?  That is, just allow any
number of selectors, with the separators between.  The following seems
to work fine in the test cases.

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 1139fd1976..4db3ae82f0 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -944,29 +944,29 @@ css--font-lock-keywords
     ;; I use `keep' this should work better).  But really the part of the
     ;; selector between [...] should simply not be highlighted.
     (,(concat
-       "^[ \t]*\\("
+       "^[ \t]*\\(\\(?:"
        (if (not sassy)
            ;; We don't allow / as first char, so as not to
            ;; take a comment as the beginning of a selector.
-           "[^@/:{}() \t\n][^:{}()]*"
+           "[[:alnum:]%*#.>]+"
          ;; Same as for non-sassy except we do want to allow { and }
          ;; chars in selectors in the case of #{$foo}
          ;; variable interpolation!
          (concat "\\(?:" scss--hash-re
-                 "\\|[^@/:{}() \t\n#]\\)"
-                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))
+                 "\\|[[:alnum:]%*#.>]+\\)"))
        ;; Even though pseudo-elements should be prefixed by ::, a
        ;; single colon is accepted for backward compatibility.
        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                        css-pseudo-element-ids)
                                t)
-       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
+       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)\\)?"
+       ;; Braces after selectors.
+       "\\(?:\\[[^]]+\\]\\)?"
+       ;; Parentheses after selectors.
        "\\(?:([^)]+)\\)?"
-       (if (not sassy)
-           "[^:{}()\n]*"
-         (concat "[^:{}()\n#]*\\(?:" scss--hash-re "[^:{}()\n#]*\\)*"))
-       "\\)*"
-       "\\)\\(?:\n[ \t]*\\)*{")
+       ;; Separators between selectors.
+       "[ \t,+~>]*"
+       "\\)+\\)\\(?:\n[ \t]*\\)*{")
      (1 'css-selector keep))
     ;; In the above rule, we allow the open-brace to be on some subsequent
     ;; line.  This will only work if we properly mark the intervening text


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sat, 14 May 2022 17:33:11 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> But how about just trying to make it stricter?  That is, just allow any
> number of selectors, with the separators between.  The following seems
> to work fine in the test cases.

I've now done something like this in Emacs 29, and added a bunch of tests.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Simen Heggestøyl <simenheg <at> runbox.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sun, 15 May 2022 12:45:11 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> But how about just trying to make it stricter?  That is, just allow any
>> number of selectors, with the separators between.  The following seems
>> to work fine in the test cases.
>
> I've now done something like this in Emacs 29, and added a bunch of tests.

Hm, I can't seem to find it, did you push it yet?

I tested a bit with the patch you posted 15:30 yesterday, but it seems
to have some problems:

Pasting this into a CSS mode buffer now freezes up Emacs:

  /*
   * xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
   */

Selectors with hyphens are no longer highlighted, e.g.:

  .foo-bar {

Only the last part of multi line selectors are now highlighted,
e.g. only `body` here:

  div,
  body {

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Simen Heggestøyl <simenheg <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sun, 15 May 2022 14:14:30 +0200

Simen Heggestøyl <simenheg <at> runbox.com> writes:

> Hm, I can't seem to find it, did you push it yet?

Nope; forgot to, and it seems like a good thing.  :-)

> I tested a bit with the patch you posted 15:30 yesterday, but it seems
> to have some problems:
>
> Pasting this into a CSS mode buffer now freezes up Emacs:
>
>   /*
>    * xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>    */
>
> Selectors with hyphens are no longer highlighted, e.g.:
>
>   .foo-bar {
>
> Only the last part of multi line selectors are now highlighted,
> e.g. only `body` here:
>
>   div,
>   body {

I've now fixed these and pushed the change.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Simen Heggestøyl <simenheg <at> runbox.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 53203 <at> debbugs.gnu.org,
 Colin <my.old.email.sucked <at> gmail.com>
Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs
 emacs in scss-mode
Date: Sun, 15 May 2022 16:46:57 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Simen Heggestøyl <simenheg <at> runbox.com> writes:
>
>> I tested a bit with the patch you posted 15:30 yesterday, but it seems
>> to have some problems:
>>
>> Pasting this into a CSS mode buffer now freezes up Emacs:
>>
>>   /*
>>    * xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>>    */
>>
>> Selectors with hyphens are no longer highlighted, e.g.:
>>
>>   .foo-bar {
>>
>> Only the last part of multi line selectors are now highlighted,
>> e.g. only `body` here:
>>
>>   div,
>>   body {
>
> I've now fixed these and pushed the change.

Thanks! Looks good.

The new selector tests makes it much less scary to tweak the regexp.

Previous Next

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