bug#39075: 28.0.50; Emacs hangs on 100% CPU and grows beyond bounds in shell-mode

[Pieter van Oostrum]

Pieter van Oostrum <pieter-l@vanoostrum.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Thanks, this was very useful.  It turns out to reproduce one must do
>> this at the shell's prompt, after "M-x shell":
>>
>>   $ cd /some/directory/;
>>
>> The /some/directory/ part should be a real directory.  Once one types
>> the semi-colon, Emacs hangs.  Here's the Lisp backtrace:
>>
>>   "Automatic GC" (0x0)
>>   "looking-at" (0x766f5fc8)
>>   "shell--parse-pcomplete-arguments" (0x766f64f8)
>>   "pcomplete-parse-arguments" (0x766f6a90)
>>   "pcomplete-completions" (0x766f6f60)
>>   "pcomplete-completions-at-point" (0x766f7698)
>>   "run-hook-with-args-until-success" (0x766f7690)
>>   "comint-completion-at-point" (0x766f7b10)
>>   0x317f7b0 PVEC_COMPILED
>>   "completion-in-region--postch" (0x766f8450)
>>
>> So I think shell--parse-pcomplete-arguments infloops in this case.
>
> Yes, I have traced it.
>
> shell--parse-pcomplete-arguments splits the line into chunk, each chunk being 
> either
> 1) a sequence of chars not containing white space, \ " ' ;
> 2) a sequence of chars between apostrophes (') not containing ', maybe final 
> one missing
> 3) a sequence of chars between quotes (") not containing unescaped ", maybe 
> final one missing
> 4) a backslash (\) possibly followed by a char
>
> It uses a regexp for that.
> It collects a list of these chunks.
> It skips over white space between the chunks.
>
> The problem is that a semicolon ; is not covered by this regexp. In the case 
> that caused the error the end position was after the semicolon but the match 
> loop stopped just before the semicolon, and would never advance. It kept 
> going in an infinite loop, continually pushing empty strings on the result 
> list, thereby exhausting memory, and using 100% CPU time.
> I don't know why it wouldn't react to C-g, but I guess because after some 
> time it would be mostly in the garbage collector.
>
> Originally case 1) did not have the semicolon.
> It was introduced in commit eaeeece92da51b517097667f13d580aa92ad5d59 on Dec 
> 4, 2018 18:39:47 +0100.
> There was a test case added in test/lisp/shell-tests.el, but it cheated by 
> positioning point before the semicolon.
>
> I think the simplest solution is to add a semicolon to the part where it 
> skips over white space, i.e. treat the semicolon like white space. But I am 
> not wholly sure that the caller doesn't want to see the semicolon. Otherwise 
> the semicolon should be pushed to the result.
>
> diff -u /Users/pieter/TEMP/shell.el.\~1\~ /Users/pieter/TEMP/shell.el
> --- /Users/pieter/TEMP/shell.el.~1~   2020-01-13 14:37:40.000000000 +0100
> +++ /Users/pieter/TEMP/shell.el       2020-01-13 14:38:07.000000000 +0100
> @@ -428,7 +428,7 @@
>      (save-excursion
>        (goto-char begin)
>        (while (< (point) end)
> -     (skip-chars-forward " \t\n")
> +     (skip-chars-forward " \t\n;")
>       (push (point) begins)
>          (let ((arg ()))
>            (while (looking-at
>
> Diff finished.  Mon Jan 13 14:38:22 2020


I propose to amend the test also, so that it would have caught this
error (by hanging). I add both patches here now. I have tested that all
other things worked the same and found no problems.

Is there an official procedure for requesting the change, or is posting it here 
sufficient?

shell.patch2
Description: patch

-- 
Pieter van Oostrum
www: http://pieter.vanoostrum.org/
PGP key: [8DAE142BE17999C4]