bug#27585: segfault when evaluating a file containing only backticks

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

  I've recently started fuzzing GNU Emacs, using the current git sources.
 During the course of that work I stumbled upon this easily reproduced bug:

   deagol ~ $ perl -e 'print "`" x ( 1024 * 1024  * 12);' > t.el
   deagol ~ $ /usr/bin/emacs --batch --script ./t.el
   ..
   Segmentation fault (core dumped)

  (So I'm trying to call "emacs --batch --script $file" where the file
 contains thousands of repeated backtick-characters.)

  Because I've built from source I can see this backtrace:

#5  handle_sigsegv (sig=11, siginfo=<optimized out>, arg=<optimized out>)
    at sysdep.c:1811
#6  <signal handler called>
#7  read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc661e010, first_in_list=first_in_list@entry=false)
    at lread.c:2923
#8  0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#9  read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc66220c0, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#10 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#11 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc6626170, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#12 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#13 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc662a220, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#14 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#15 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc662e2d0, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#16 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#17 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc6632380, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#18 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#19 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc6636430, first_in_list=first_in_list@entry=false)
    at lread.c:3149
#20 0x0000000000ad8cda in read0 (readcharfun=35581829) at lread.c:2220
#21 read1 (readcharfun=readcharfun@entry=35581829, 
    pch=pch@entry=0x7ffcc663a4e0, first_in_list=first_in_list@entry=false)
    at lread.c:3149
....


  I've replicated this upon the package of GNU Emacs as available to
 the old-stable/jessie release of Debian GNU/Linux, which identifies
 itself as:


In GNU Emacs 24.5.1 (x86_64-pc-linux-gnu, GTK+ Version 3.14.5)
 of 2016-03-19 on trouble, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11604000
System Description: Debian GNU/Linux 8.8 (jessie)

Configured using:
 `configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.5/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.5/site-lisp:/usr/share/emacs/site-lisp
 --build x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.5/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.5/site-lisp:/usr/share/emacs/site-lisp
 --with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
 'CFLAGS=-g -O2 -fstack-protector-strong -Wformat
 -Werror=format-security -Wall' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-z,relro'
 

  My current-git build reports as:

In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu)
 of 2017-07-05 built on kernel.default.skx.uk0.bigv.io
Repository revision: 5d62247323f53f3ae9c7d9f51e951635887b2fb6
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...

Configured using:
 'configure --prefix=/tmp/emacs/ --without-makeinfo --with-gnutls=no'

Configured features:
SOUND NOTIFY ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF8
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

  "Obviously" this same bug can be reproduced inside emacs:

    1.  Open Emacs.
    2.  Create a new buffer.
    3.  Fill the buffer with `
    4.  Ctrl-x h
    5.  M-x eval-region



Steve
-- 
https://www.steve.org.uk/

bug#27585: segfault when evaluating a file containing only backticks

[Andreas Schwab]

On Jul 05 2017, Steve Kemp <steve@steve.org.uk> wrote:

>   Because I've built from source I can see this backtrace:
>
> #5  handle_sigsegv (sig=11, siginfo=<optimized out>, arg=<optimized out>)
>     at sysdep.c:1811
> #6  <signal handler called>
> #7  read1 (readcharfun=readcharfun@entry=35581829, 
>     pch=pch@entry=0x7ffcc661e010, first_in_list=first_in_list@entry=false)
>     at lread.c:2923

Most likely just a stack overflow.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

> > #5  handle_sigsegv (sig=11, siginfo=<optimized out>, arg=<optimized out>)
> >     at sysdep.c:1811
> > #6  <signal handler called>
> > #7  read1 (readcharfun=readcharfun@entry=35581829, 
> >     pch=pch@entry=0x7ffcc661e010, first_in_list=first_in_list@entry=false)
> >     at lread.c:2923
> 
> Most likely just a stack overflow.

  Agreed, but still I think a segfault is unexpected and could be
 prevented.

Steve
-- 
https://www.steve.org.uk/

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Steve Kemp <steve@steve.org.uk>
> Date: Wed, 05 Jul 2017 06:21:10 +0000
> 
> 
>   I've recently started fuzzing GNU Emacs, using the current git sources.
>  During the course of that work I stumbled upon this easily reproduced bug:
> 
>    deagol ~ $ perl -e 'print "`" x ( 1024 * 1024  * 12);' > t.el
>    deagol ~ $ /usr/bin/emacs --batch --script ./t.el
>    ..
>    Segmentation fault (core dumped)

Here it says:

  Re-entering top level after C stack overflow

and doesn't crash.

> > Most likely just a stack overflow.
> 
>   Agreed, but still I think a segfault is unexpected and could be
>  prevented.

See above: the machinery to try and prevent it exists, but it doesn't
always succeed.  And it really can't be 100% reliable.  So I'm unsure
what did you expect, and why.  Emacs generally gives you enough rope
to hang yourself; it's up to you not to be tempted to do so...

IOW: why would someone want to run such a silly "program"?

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

> >    deagol ~ $ perl -e 'print "`" x ( 1024 * 1024  * 12);' > t.el
> >    deagol ~ $ /usr/bin/emacs --batch --script ./t.el
> >    ..
> >    Segmentation fault (core dumped)
> 
> Here it says:
> 
>   Re-entering top level after C stack overflow
> 
> and doesn't crash.

  That's great.  For me it crashes on all the versions I've tested.
 Sometimes with a shorter input, others need longer, but it had seemed
 universal.

> See above: the machinery to try and prevent it exists, but it doesn't
> always succeed.  And it really can't be 100% reliable.  So I'm unsure
> what did you expect, and why.

  Honestly?  I expect Emacs to not crash.  The mechanics might be
 simple, or they might be complex but as a mere user I shouldn't
 be expected to know or care about the details.  I expect evaluating
 lisp to not kill the editor - although I appreciate that executing
 unknown could code do all kinds of nasty things, from uploading
 my images to the internet, to running "rm -rf $HOME".

  The fine manual, in "54.2 When Is There a Bug", agrees.  When
 paraphrased as "segfault == bug".

> IOW: why would someone want to run such a silly "program"?

  In the real world?  Nobody.  It was just the first crash that
 came out of fuzz-testing.  Perhaps there will be more subtle
 and interesting ones to report in the future when the fuzzer
 has ran for a few more days/weeks.  Perhaps not.  Either way
 I'd regard it as a bug that should be fixed, even if it is
 not a security hole, and not something that is going to surprise
 users in practice.

Steve
-- 
https://steve.fi/

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Steve Kemp <steve@steve.org.uk>
> Date: Wed, 05 Jul 2017 18:55:31 +0000
> 
> > See above: the machinery to try and prevent it exists, but it doesn't
> > always succeed.  And it really can't be 100% reliable.  So I'm unsure
> > what did you expect, and why.
> 
>   Honestly?  I expect Emacs to not crash.

You wrote a program that triggers infinite recursion.  Such programs
will crash in most, if not all, languages.  So your expectations are
unrealistic.

>  I expect evaluating lisp to not kill the editor

Valid Lisp, I agree.  But yours isn't.

Moreover, there are those among us (I'm not one of them) who thinks
Emacs shouldn't even try to recover from stack overflow, they say it
should crash hard right there and then.

So your expectations are not necessarily shared, even as aspirations,
by some developers.

> > IOW: why would someone want to run such a silly "program"?
> 
>   In the real world?  Nobody.

Then why are we discussing this use case?  Let's talk about
more practical and interesting cases.

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

> >  I expect evaluating lisp to not kill the editor
>
> Valid Lisp, I agree.  But yours isn't.

  Here we disagree.

  I enjoy reporting bugs when invalid things are tried, because I
 expect software to be robust, or as much as possible.  If you feed
 invalid script to gawk, perl, emacs, etc, etc, I expect an error
 not a segfault.

  I appreciate there are often difficult cases, such as infinite
 recursion, division by zero, and memory exhaustion, but the idea
 that ones programming environment is free to crash if your code
 is wrong is .. unpleasant.

> >   In the real world?  Nobody.
>
> Then why are we discussing this use case?  Let's talk about
> more practical and interesting cases.

  I read from this that I should not bother running any more
 fuzzing, after all if it does result in any crashes they're a result
 of bogus-coding that would never be hit in the real world.

  That's unfortunate, but I've no particular wish to argue
 in public.    I will follow your suggestion.

  Bug left open because it crashes for me, if it fails to crash
 for others with a somewhat reasonable error message I guess
 that is good.

Steve
--

bug#27585: segfault when evaluating a file containing only backticks

[Andreas Schwab]

On Jul 05 2017, Eli Zaretskii <eliz@gnu.org> wrote:

> You wrote a program that triggers infinite recursion.

This isn't infinite recursion since the input is finite.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Steve Kemp <steve@steve.org.uk>
> Date: Thu, 06 Jul 2017 03:46:35 +0000
> 
>   I enjoy reporting bugs when invalid things are tried, because I
>  expect software to be robust, or as much as possible.  If you feed
>  invalid script to gawk, perl, emacs, etc, etc, I expect an error
>  not a segfault.

I enjoy seeing _any_ bugs reported about Emacs, because we want to
make Emacs as robust as possible.  Emacs tries very hard not to crash,
but sometimes the techniques we use cannot work reliably.  As in this
case.  There's a difference between "as robust as possible" and "100%
robust", and expecting not to see segfault _at_all_ from a system with
a full-fledged programming language built into it is unrealistic,
especially when a program deliberately tries to cause Emacs to run out
of memory.

>   I appreciate there are often difficult cases, such as infinite
>  recursion, division by zero, and memory exhaustion, but the idea
>  that ones programming environment is free to crash if your code
>  is wrong is .. unpleasant.

Emacs is not a programming environment.  Emacs is an editor and a
text-processing environment with a built-in extension language.  The
extension language is provided for extending the editor, not for
running arbitrary programs that have no relation to the editor.  You
can, of course, do the latter if you wish, but that is not the main
use case for Emacs, and not the one for which it is optimized.  Making
such a marginal use case more important than it is will tax the much
more important uses of Emacs as an editor and text-processing tool.

Emacs is required not to crash when used in its main roles, or when it
runs one of the packages provided with it.  It should also try very
hard not to crash in other cases as well, but it is not _required_ to
do so, not at all costs anyway.

What that means is that each crash should first be carefully analyzed
in order to understand the underlying reasons and factors which led to
the crash.  Having found the reasons, if there are reasonable ways of
resolving them to prevent the crash, that shall be done.  A crash that
happens during some use case that belongs to the main Emacs use
patterns increases our motivation to seek for a solution, even if the
solutions are hard to find or require complicated changes.  Crashes
outside of the patterns we care about don't produce such an added
motivation, but are still solved if the solution is reasonably
practical and doesn't adversely affects other important uses.

I'm sorry I have to describe all this, which I have no doubt you
understand very well.  It's just that this discussion until now seems
to somehow ignore these simple and clear-cut considerations, without
which I feel the discussion loses its important context, and you
somehow feel that this project is not interested in hearing about
bugs, which is simply not true.

> > >   In the real world?  Nobody.
> >
> > Then why are we discussing this use case?  Let's talk about
> > more practical and interesting cases.
> 
>   I read from this that I should not bother running any more
>  fuzzing, after all if it does result in any crashes they're a result
>  of bogus-coding that would never be hit in the real world.

I suggested nothing of the kind.  Fuzzing can uncover any number of
problems of different nature.  Each one of those should be analyzed
first, before the decision is made whether it should be fixed and with
what priority.  I presume that the above is due to some offense you
took from what I wrote, which is why I thought it was important to
explain what I think should be the way of handling bug reports -- any
bug reports -- submitted against Emacs.

>   That's unfortunate, but I've no particular wish to argue
>  in public.    I will follow your suggestion.

Once again, there was no suggestion from me that fuzzing will never
uncover any bugs we will consider important to fix, or even
unimportant but easy enough to fix.  Each bug report is analyzed on
its own right, and the decision whether to fix it and at what priority
is made independently for each one of them.  It doesn't matter what
method was used to trigger the problem, the analysis and the
conclusion consider the problem itself, not the method used to find
it.

In this case, Emacs already does everything we knew was possible with
modern operating systems (and some of us think we do too much).  Emacs
tries to avoid stack overflow both on the Lisp level and on the C
level, with some non-negligible degree of success.  It is entirely
clear to us that a cleverly constructed Lisp program could circumvent
these defenses and cause a segfault anyway, but Emacs is not supposed
to be a 100% safe environment for running such malicious programs;
crashing for them is IMO way better than some other possible outcomes,
like wiping out the filesystem or wedging the OS kernel.

In sum, I hope you will continue trying to break Emacs and will report
any "successes", so we could improve Emacs in the future.

Thanks.

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: Steve Kemp <steve@steve.org.uk>,  27585@debbugs.gnu.org
> Date: Thu, 06 Jul 2017 08:46:14 +0200
> 
> On Jul 05 2017, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > You wrote a program that triggers infinite recursion.
> 
> This isn't infinite recursion since the input is finite.

If we want to be pedantically accurate, then no "infinite" recursion
is ever truly infinite, since it will always stop when the system is
shut down, or the power fails, or the world ends, whichever happens
first.

So we are actually always talking about recursions deep enough to blow
up the stack.

bug#27585: segfault when evaluating a file containing only backticks

[Andreas Schwab]

On Jul 06 2017, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@suse.de>
>> Cc: Steve Kemp <steve@steve.org.uk>,  27585@debbugs.gnu.org
>> Date: Thu, 06 Jul 2017 08:46:14 +0200
>> 
>> On Jul 05 2017, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> > You wrote a program that triggers infinite recursion.
>> 
>> This isn't infinite recursion since the input is finite.
>
> If we want to be pedantically accurate, then no "infinite" recursion
> is ever truly infinite, since it will always stop when the system is
> shut down, or the power fails, or the world ends, whichever happens
> first.

This is not a useful way to categorize infinite recursion.  In this
case, the recursion is always bounded by the size of the input.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

> I presume that the above is due to some offense you
> took from what I wrote, which is why I thought it was important to
> explain what I think should be the way of handling bug reports -- any
> bug reports -- submitted against Emacs.

  I appreciate the time you took to do so, but no, no offense was
 taken.

  I only thought stopping was sensible because my experience is
 that fuzzing tends to discover are things that are malformed, broken,
 and almost never the kind of things that a user would intend to do.

  In short your initial response to this particular case seems
 like it would apply to any similar issue which  is liable to
 be discovered - they'd be genuine bugs, but they'd also be things
 that would never happen "in the wild", so while they wouldn't
 be ignored, it would be easy to push them to the back of the queue.

> In sum, I hope you will continue trying to break Emacs and will report
> any "successes", so we could improve Emacs in the future.

  THanks.  I'm sitting on a couple of hundred crashing cases, just
 trying to simplify them and see if they share the same cause (they
 probably do).

Steve
-- 
https://steve.fi/

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: steve@steve.org.uk,  27585@debbugs.gnu.org
> Date: Thu, 06 Jul 2017 17:31:35 +0200
> 
> > If we want to be pedantically accurate, then no "infinite" recursion
> > is ever truly infinite, since it will always stop when the system is
> > shut down, or the power fails, or the world ends, whichever happens
> > first.
> 
> This is not a useful way to categorize infinite recursion.

Neither is this hair-splitting.

bug#27585: segfault when evaluating a file containing only backticks

[Andreas Schwab]

On Jul 06 2017, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@suse.de>
>> Cc: steve@steve.org.uk,  27585@debbugs.gnu.org
>> Date: Thu, 06 Jul 2017 17:31:35 +0200
>> 
>> > If we want to be pedantically accurate, then no "infinite" recursion
>> > is ever truly infinite, since it will always stop when the system is
>> > shut down, or the power fails, or the world ends, whichever happens
>> > first.
>> 
>> This is not a useful way to categorize infinite recursion.
>
> Neither is this hair-splitting.

Nothing of this is hair-splitting.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#27585: segfault when evaluating a file containing only backticks

[Daniel Colascione]

On July 5, 2017 11:41:45 AM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Steve Kemp <steve@steve.org.uk>
>> Date: Wed, 05 Jul 2017 06:21:10 +0000
>> 
>> 
>>   I've recently started fuzzing GNU Emacs, using the current git
>sources.
>>  During the course of that work I stumbled upon this easily
>reproduced bug:
>> 
>>    deagol ~ $ perl -e 'print "`" x ( 1024 * 1024  * 12);' > t.el
>>    deagol ~ $ /usr/bin/emacs --batch --script ./t.el
>>    ..
>>    Segmentation fault (core dumped)
>
>Here it says:
>
>  Re-entering top level after C stack overflow
>
>and doesn't crash.
>
>> > Most likely just a stack overflow.
>> 
>>   Agreed, but still I think a segfault is unexpected and could be
>>  prevented.
>
>See above: the machinery to try and prevent it exists, but it doesn't
>always succeed.  And it really can't be 100% reliable.  So I'm unsure
>what did you expect, and why.  Emacs generally gives you enough rope
>to hang yourself; it's up to you not to be tempted to do so...

This argument doesn't make sense to me. If we're happy letting elisp segfault, why bounds check AREF? 

Other managed runtimes --- Java, C# --- are perfectly capable of reliably detecting and recovering from stack exhaustion. There is absolutely no reason, aside from an implementation defect, for the elisp runtime not to do the same.

Stack overflow detection could be made perfectly reliable.


>
>IOW: why would someone want to run such a silly "program"?

bug#27585: segfault when evaluating a file containing only backticks

[Daniel Colascione]

On July 5, 2017 12:47:22 PM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Steve Kemp <steve@steve.org.uk>
>> Date: Wed, 05 Jul 2017 18:55:31 +0000
>> 
>> > See above: the machinery to try and prevent it exists, but it
>doesn't
>> > always succeed.  And it really can't be 100% reliable.  So I'm
>unsure
>> > what did you expect, and why.
>> 
>>   Honestly?  I expect Emacs to not crash.
>
>You wrote a program that triggers infinite recursion.  Such programs
>will crash in most, if not all, languages.  So your expectations are
>unrealistic.
>
>>  I expect evaluating lisp to not kill the editor
>
>Valid Lisp, I agree.  But yours isn't.
>
>Moreover, there are those among us (I'm not one of them) who thinks
>Emacs shouldn't even try to recover from stack overflow, they say it
>should crash hard right there and then.

Native stack? Certainly. The current approach, a signal handler that longjmps to top-level, cannot possibly work reliability, since it interrupts and abandons whatever the code is doing. If it has some kind of lock held and you try to take that lock again, you deadlock. Data structures might be in completely incoherent states. The last time we had this discussion, someone asserted that the worst that could happen might be a "memory leak". That's very wrong.

This signal handler is a huge, ticking time bomb, and I completely turn it off in my Emacs. Everyone else should too.

Recovering when elisp blows the stack is a different matter.


>
>So your expectations are not necessarily shared, even as aspirations,
>by some developers.
>
>> > IOW: why would someone want to run such a silly "program"?
>> 
>>   In the real world?  Nobody.
>
>Then why are we discussing this use case?  Let's talk about
>more practical and interesting cases.

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> Date: Thu, 06 Jul 2017 08:52:44 -0700
> CC: 27585@debbugs.gnu.org
> From: Daniel Colascione <dancol@dancol.org>
> 
> >Moreover, there are those among us (I'm not one of them) who thinks
> >Emacs shouldn't even try to recover from stack overflow, they say it
> >should crash hard right there and then.
> 
> Native stack? Certainly.

Yes, this discussion is about the native stack, not the ELisp stack.

> Recovering when elisp blows the stack is a different matter.

I believe we already do that.

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Steve Kemp <steve@steve.org.uk>
> Date: Thu, 06 Jul 2017 15:33:42 +0000
> 
>   In short your initial response to this particular case seems
>  like it would apply to any similar issue which  is liable to
>  be discovered - they'd be genuine bugs, but they'd also be things
>  that would never happen "in the wild", so while they wouldn't
>  be ignored, it would be easy to push them to the back of the queue.

That wasn't my intent, and I apologize if my wording somehow implied
that.  I was only talking about the particular crash you reported.
Any other crash should be analyzed separately, and the conclusion
could very well be different.

>   THanks.  I'm sitting on a couple of hundred crashing cases, just
>  trying to simplify them and see if they share the same cause (they
>  probably do).

Well, I hope some of them are for different reasons.

It is strange you cannot trigger the stack-overflow protection in your
build, though.  If you write a trivial infinite-recursion Lisp
function, and then run it after lifting max-lisp-eval-depth and
max-specpdl-size to the largest positive number, don't you see what I
saw on my system?

bug#27585: segfault when evaluating a file containing only backticks

[Daniel Colascione]

On July 6, 2017 9:19:40 AM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 06 Jul 2017 08:52:44 -0700
>> CC: 27585@debbugs.gnu.org
>> From: Daniel Colascione <dancol@dancol.org>
>> 
>> >Moreover, there are those among us (I'm not one of them) who thinks
>> >Emacs shouldn't even try to recover from stack overflow, they say it
>> >should crash hard right there and then.
>> 
>> Native stack? Certainly.
>
>Yes, this discussion is about the native stack, not the ELisp stack.

Sort of --- an elisp file is driving the native code to blow its stack. I really don't think that any valid syntax should cause Emacs to segfault.

In this instance, couldn't we add a hard threshold to limit recursion?


>
>> Recovering when elisp blows the stack is a different matter.
>
>I believe we already do that.

We have a threshold. It's conservative, but it works most of the time. IMHO, explicit stack probing would be both less conservative and note robust.

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> Date: Thu, 06 Jul 2017 09:37:30 -0700
> CC: steve@steve.org.uk,27585@debbugs.gnu.org
> From: Daniel Colascione <dancol@dancol.org>
> 
> In this instance, couldn't we add a hard threshold to limit recursion?

I'm not sure I understand the proposal.  Could you elaborate?

> >> Recovering when elisp blows the stack is a different matter.
> >
> >I believe we already do that.
> 
> We have a threshold. It's conservative, but it works most of the time. IMHO, explicit stack probing would be both less conservative and note robust.

Won't it be expensive?  Or maybe I don't have a clear idea what you
meant by "stack probing".

bug#27585: segfault when evaluating a file containing only backticks

[Paul Eggert]

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

Thanks for reporting the bug. I reproduced the problem on Fedora 26 x86-64, 
fixed it in master by applying the attached patch, and am boldly marking the bug 
as fixed.

As Eli and Daniel mentioned, this area of Emacs cannot be 100% reliable and to 
some extent is indeed a "ticking time bomb". That being said, the problem in 
this particular case was that Emacs had a bad heuristic for guessing whether a 
segmentation violation address was due to stack overflow on GNU/Linux. This bad 
heuristic has been in place for years without anybody reporting it. It's good 
that we fixed this bug (though I hope "normal" users never notice the bug fix :-).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-stack-overflow-heuristic-on-GNU-Linux.patch --]
[-- Type: text/x-patch; name="0001-Improve-stack-overflow-heuristic-on-GNU-Linux.patch", Size: 5692 bytes --]

From 9dee1c884eb50ba282eb9dd2495c5269add25963 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 14 Jul 2017 04:54:05 -0700
Subject: [PATCH] Improve stack-overflow heuristic on GNU/Linux

Problem reported by Steve Kemp (Bug#27585).
* src/eval.c (near_C_stack_top): Remove.  All uses replaced
by current_thread->stack_top.
(record_in_backtrace): Set current_thread->stack_top.
This is for when the Lisp interpreter calls itself.
* src/lread.c (read1): Set current_thread->stack_top.
This is for recursive s-expression reads.
* src/print.c (print_object): Set current_thread->stack_top.
This is for recursive s-expression printing.
* src/thread.c (mark_one_thread): Get stack top first.
* src/thread.h (struct thread_state.stack_top): Now void *, not char *.
---
 src/eval.c   |  9 +--------
 src/lisp.h   |  1 -
 src/lread.c  |  1 +
 src/print.c  |  2 +-
 src/sysdep.c |  2 +-
 src/thread.c | 10 ++++++----
 src/thread.h | 10 ++++++++--
 7 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 8f293c9..e590038 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -213,13 +213,6 @@ backtrace_next (union specbinding *pdl)
   return pdl;
 }
 
-/* Return a pointer to somewhere near the top of the C stack.  */
-void *
-near_C_stack_top (void)
-{
-  return backtrace_args (backtrace_top ());
-}
-
 void
 init_eval_once (void)
 {
@@ -2090,7 +2083,7 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs)
   specpdl_ptr->bt.kind = SPECPDL_BACKTRACE;
   specpdl_ptr->bt.debug_on_exit = false;
   specpdl_ptr->bt.function = function;
-  specpdl_ptr->bt.args = args;
+  current_thread->stack_top = specpdl_ptr->bt.args = args;
   specpdl_ptr->bt.nargs = nargs;
   grow_specpdl ();
 
diff --git a/src/lisp.h b/src/lisp.h
index f5cb6c7..1e8ef7a 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3874,7 +3874,6 @@ extern Lisp_Object vformat_string (const char *, va_list)
   ATTRIBUTE_FORMAT_PRINTF (1, 0);
 extern void un_autoload (Lisp_Object);
 extern Lisp_Object call_debugger (Lisp_Object arg);
-extern void *near_C_stack_top (void);
 extern void init_eval_once (void);
 extern Lisp_Object safe_call (ptrdiff_t, Lisp_Object, ...);
 extern Lisp_Object safe_call1 (Lisp_Object, Lisp_Object);
diff --git a/src/lread.c b/src/lread.c
index fe5de38..901e40b 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2676,6 +2676,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
   bool uninterned_symbol = false;
   bool multibyte;
   char stackbuf[MAX_ALLOCA];
+  current_thread->stack_top = stackbuf;
 
   *pch = 0;
 
diff --git a/src/print.c b/src/print.c
index b6ea3ff..12edf01 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1748,7 +1748,7 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
   char buf[max (sizeof "from..to..in " + 2 * INT_STRLEN_BOUND (EMACS_INT),
 		max (sizeof " . #" + INT_STRLEN_BOUND (printmax_t),
 		     40))];
-
+  current_thread->stack_top = buf;
   maybe_quit ();
 
   /* Detect circularities and truncate them.  */
diff --git a/src/sysdep.c b/src/sysdep.c
index b522367..db99f53 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -1772,7 +1772,7 @@ stack_overflow (siginfo_t *siginfo)
   /* The known top and bottom of the stack.  The actual stack may
      extend a bit beyond these boundaries.  */
   char *bot = stack_bottom;
-  char *top = near_C_stack_top ();
+  char *top = current_thread->stack_top;
 
   /* Log base 2 of the stack heuristic ratio.  This ratio is the size
      of the known stack divided by the size of the guard area past the
diff --git a/src/thread.c b/src/thread.c
index e378797..1f7ced3 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -595,14 +595,15 @@ thread_select (select_func *func, int max_fds, fd_set *rfds,
 static void
 mark_one_thread (struct thread_state *thread)
 {
-  struct handler *handler;
-  Lisp_Object tem;
+  /* Get the stack top now, in case mark_specpdl changes it.  */
+  void *stack_top = thread->stack_top;
 
   mark_specpdl (thread->m_specpdl, thread->m_specpdl_ptr);
 
-  mark_stack (thread->m_stack_bottom, thread->stack_top);
+  mark_stack (thread->m_stack_bottom, stack_top);
 
-  for (handler = thread->m_handlerlist; handler; handler = handler->next)
+  for (struct handler *handler = thread->m_handlerlist;
+       handler; handler = handler->next)
     {
       mark_object (handler->tag_or_ch);
       mark_object (handler->val);
@@ -610,6 +611,7 @@ mark_one_thread (struct thread_state *thread)
 
   if (thread->m_current_buffer)
     {
+      Lisp_Object tem;
       XSETBUFFER (tem, thread->m_current_buffer);
       mark_object (tem);
     }
diff --git a/src/thread.h b/src/thread.h
index 9e94de5..52b16f1 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -62,8 +62,14 @@ struct thread_state
   char *m_stack_bottom;
 #define stack_bottom (current_thread->m_stack_bottom)
 
-  /* An address near the top of the stack.  */
-  char *stack_top;
+  /* The address of an object near the C stack top, used to determine
+     which words need to be scanned by the garbage collector.  This is
+     also used to detect heuristically whether segmentation violation
+     address indicates stack overflow, as opposed to some internal
+     error in Emacs.  If the C function F calls G which calls H which
+     calls ... F, then at least one of the functions in the chain
+     should set this to the address of a local variable.  */
+  void *stack_top;
 
   struct catchtag *m_catchlist;
 #define catchlist (current_thread->m_catchlist)
-- 
2.7.4

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 14 Jul 2017 05:09:34 -0700
> Cc: 27585-done@debbugs.gnu.org
> 
> It's good that we fixed this bug (though I hope "normal" users never
> notice the bug fix :-).

Indeed, thanks.

bug#27585: segfault when evaluating a file containing only backticks

[Steve Kemp]

On Fri Jul 14, 2017 at 16:30:16 +0300, Eli Zaretskii wrote:

> > It's good that we fixed this bug (though I hope "normal" users never
> > notice the bug fix :-).
> 
> Indeed, thanks.

  Thanks again.  I  do note one behaviour change though, which is
 perhaps worth noting.

  Previously this segfaulted:

    $ perl -e 'print "," x ( 1024 * 1024  * 12);' > t.el
    $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el

  Now it does not, as the bug is fixed, but I'd expect the
 emacs process to terminate when the script is loaded. 

  That doesn't happen though:

    $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
    Re-entering top level after C stack overflow
    [emacs still running]

  Compare that with the normal example:

    $ echo "(defun foo() "Test")" > t.el
    $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
    $ 

  Perhaps not a huge problem, but it is a change I think?

Steve
-- 
https://steve.fi/

bug#27585: segfault when evaluating a file containing only backticks

[Paul Eggert]

Steve Kemp wrote:
>      $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
>      Re-entering top level after C stack overflow
>      [emacs still running]
> 
>    Compare that with the normal example:
> 
>      $ echo "(defun foo() "Test")" > t.el
>      $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
>      $
> 
>    Perhaps not a huge problem, but it is a change I think?

After a stack overflow, Emacs drops what it's doing and goes to the top level, 
regardless of its command-line arguments. Perhaps it would be better if Emacs 
did something else for this particular case. It's low priority, though.

bug#27585: segfault when evaluating a file containing only backticks

[Eli Zaretskii]

> Cc: 27585@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 14 Jul 2017 22:12:08 -0700
> 
> Steve Kemp wrote:
> >      $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
> >      Re-entering top level after C stack overflow
> >      [emacs still running]
> > 
> >    Compare that with the normal example:
> > 
> >      $ echo "(defun foo() "Test")" > t.el
> >      $ /tmp/emacs/bin/emacs -Q --batch --script ./t.el
> >      $
> > 
> >    Perhaps not a huge problem, but it is a change I think?
> 
> After a stack overflow, Emacs drops what it's doing and goes to the top level, 
> regardless of its command-line arguments. Perhaps it would be better if Emacs 
> did something else for this particular case. It's low priority, though.

I think the idea is to let the user do whatever is necessary to end
the session as quickly as possible.  Granted, it is optimized for
interactive usage, since the goal of the stack-overflow protection is
to avoid losing unrelated edits due to some code that's gone awry, and
that makes little sense in non-interactive sessions.  So batch
invocations might indeed do something else, like exit.  I agree that
the priority is not too high, because again, stack overflow is most
harmful in interactive sessions.