Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Stefan Monnier]

>   * frame.h (FRAME_PARAMETER):
>   Prefer scoped_list1 to local_list1 where either would do.

Do we really need those scoped_* thingies?
I'm already doubtful that the local_* thingies will be useful, so
I think distinguishing between local_* and scoped_* is way beyond the
limit of diminishing returns.


        Stefan

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Paul Eggert]

Stefan Monnier wrote:

> I'm already doubtful that the local_* thingies will be useful, so
> I think distinguishing between local_* and scoped_* is way beyond the
> limit of diminishing returns.

The more I look at it the more I think local_cons and local_list* are 
mistakes and should be discarded.

For example, build_desired_tool_bar_string has a loop, each iteration of 
which creates temporaries; here local_list4 could exhaust the stack 
whereas scoped_list4 is fine, and trunk bzr 117912 fixed that by 
replacing local_list4 with scoped_list4.  There appear to be similar 
problems in every function that currently uses local_cons or 
local_list*, so I'm thinking we should remove these macros and go back 
to Fcons and list*.

We should also audit uses of make_local_vector, make_local_string, and 
build_local_string to make sure that none of them are in loops that 
could blow the stack.  I think most uses of these are OK but some are 
questionable.

If there's significant doubt whether the local_* and scoped_* thingies 
are useful perhaps we should simply remove all of them....

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Dmitry Antipov]

On 09/22/2014 09:40 AM, Paul Eggert wrote:

> For example, build_desired_tool_bar_string has a loop, each iteration of which creates temporaries;
> here local_list4 could exhaust the stack whereas scoped_list4 is fine, and trunk bzr 117912 fixed
> that by replacing local_list4 with scoped_list4.  There appear to be similar problems in every function
> that currently uses local_cons or local_list*, so I'm thinking we should remove these macros and
> go back to Fcons and list*.
>
> We should also audit uses of make_local_vector, make_local_string, and build_local_string to make sure
> that none of them are in loops that could blow the stack.  I think most uses of these are OK but
> some are questionable.

SAFE_ALLOCA and friends has exactly the same problems, and I don't see the way to make it
absolutely safe with an existing implementations of alloca.  This applies to VLAs as well.

Someday we will be able to utilize -fsplit-stack; it may be reasonable to define
USE_LOCAL_LISP_OBJECTS only if -fsplit-stack is also supported.

Dmitry

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Dmitry Antipov]

On 09/22/2014 10:42 AM, Dmitry Antipov wrote:

> Someday we will be able to utilize -fsplit-stack; it may be reasonable to define
> USE_LOCAL_LISP_OBJECTS only if -fsplit-stack is also supported.
       ^^^^^
       STACK

Just for the record: as of gcc 4.8.3 and trunk revision 117913, I have 143 "stack
usage might be unbounded" messages when compiling with -Wstack-usage=2048, and
182 when USE_STACK_LISP_OBJECTS is used.

Dmitry

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Stefan Monnier]

>> For example, build_desired_tool_bar_string has a loop, each iteration
>> of which creates temporaries; here local_list4 could exhaust the
>> stack whereas scoped_list4 is fine, and trunk bzr 117912 fixed that
>> by replacing local_list4 with scoped_list4.

Yuck!

> SAFE_ALLOCA and friends has exactly the same problems, and I don't see
> the way to make it absolutely safe with an existing implementations of
> alloca.

We don't need it to be absolutely safe (it's a given that using pointers
into stack-allocated objects is generally unsafe, unless we use some
code analysis to try and enforce some discipline).

But the rules that have to be followed need to be *very* clear and
stated in the most obvious (or in your face) place.

[ And even if we do that, someone unfamiliar with those rules will come
  up and modify "unrelated" code which end up breaking them.  ]


        Stefan

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Eli Zaretskii]

> Date: Mon, 22 Sep 2014 10:42:35 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > We should also audit uses of make_local_vector, make_local_string, and build_local_string to make sure
> > that none of them are in loops that could blow the stack.  I think most uses of these are OK but
> > some are questionable.
> 
> SAFE_ALLOCA and friends has exactly the same problems, and I don't see the way to make it
> absolutely safe with an existing implementations of alloca.  This applies to VLAs as well.

We indeed have a few calls to SAFE_ALLOCA and SAFE_NALLOCA inside loops.

> Someday we will be able to utilize -fsplit-stack; it may be reasonable to define
> USE_LOCAL_LISP_OBJECTS only if -fsplit-stack is also supported.

AFAIU, -fsplit-stack is a mixed blessing at best when calling code
that was not compiled with that switch (a.k.a. "external libraries").

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 22 Sep 2014 08:57:13 -0400
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel@gnu.org
> 
> > SAFE_ALLOCA and friends has exactly the same problems, and I don't see
> > the way to make it absolutely safe with an existing implementations of
> > alloca.
> 
> We don't need it to be absolutely safe (it's a given that using pointers
> into stack-allocated objects is generally unsafe, unless we use some
> code analysis to try and enforce some discipline).

That's not the issue here, AFAIU.  The issue here is that if you use
SAFE_ALLOCA inside a loop, each call to it might decide to call
alloca, because the size is small enough, but the sum of all
allocations could easily exceed the stack space, certainly the 16KB
limit we want to impose on such allocations.

We don't have a lot of these in the sources, but we do have a few.

> But the rules that have to be followed need to be *very* clear and
> stated in the most obvious (or in your face) place.

Where?

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Paul Eggert]

Dmitry Antipov wrote:
> SAFE_ALLOCA and friends has exactly the same problems

Their uses need to be audited or fixed too, as we shouldn't have 
repeated alloca inside an unbounded loop.  In the meantime we shouldn't 
make things worse.

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Dmitry Antipov]

On 09/22/2014 07:32 PM, Paul Eggert wrote:

> Their uses need to be audited or fixed too, as we shouldn't have repeated
> alloca inside an unbounded loop.  In the meantime we shouldn't make things worse.

Let's wait for replies on https://gcc.gnu.org/ml/gcc/2014-09/msg00308.html :-)...

Dmitry

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Eli Zaretskii]

> Date: Mon, 22 Sep 2014 20:04:01 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> On 09/22/2014 07:32 PM, Paul Eggert wrote:
> 
> > Their uses need to be audited or fixed too, as we shouldn't have repeated
> > alloca inside an unbounded loop.  In the meantime we shouldn't make things worse.
> 
> Let's wait for replies on https://gcc.gnu.org/ml/gcc/2014-09/msg00308.html :-)...

Not really helpful, what they replied.  We are left to our own
devices, as always.

Re: [Emacs-diffs] trunk r117912: Minor improvements to new stack-allocated Lisp objects.

[Paul Eggert]

Eli Zaretskii wrote:
> We are left to our own devices, as always.

In trunk bzr 117924 I installed a fix for this problem as it relates to 
the longstanding macros SAFE_ALLOCA, SAFE_NALLOCA, and SAFE_ALLOCA_LISP.

The problem remains unfixed for the experimental alloca-based macros 
enabled by USE_LOCAL_ALLOCATORS.  We could fix these macros in a similar 
way, although we'd need to add an sa_avail local to each function that 
uses these macros without also using USE_SAFE_ALLOCA.