Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-20 Thread Joerg Sonnenberger via cfe-commits 
On Sat, Aug 20, 2016 at 12:45:59AM +, Chandler Carruth wrote:
> I feel like this thread has two very unrelated concerns and it might be
> useful to separate them.
> 
> 1) Should an application be changing the stack limit, or should the system
> be able to enforce this?
> 
> Fortunately, there are two limits. One an application is allowed to change,
> one it isn't. It seems like the mechanism has been provided for system
> administrators to impose firm limits and neither Clang nor another
> application will perturb them. I don't see any reason to preclude an
> application from changing the one it is explicitly permitted to change.
> That seems to be WAI from a permissions and ACL model.

The only application processes that normally change process limits are
the shells or certain network processes that change the file limit
according to the configured number of connections. The soft limit exists
to allow a user to adjust his/her own limits within bounds. That's why
the first category has appropiate syntax for it. The second category on
the other hand knows pretty precisely what it wants to consume in terms
of ressources and plans accordingly. Clang falls into neither category.
The existing bugs are even evidence of that, e.g. 15560 says 8MB is not
enough either. So this is a patchy band aid at most, it doesn't even solve
the problem.

> 2) Should Clang be using the stack to allocate significant portions of its
> memory, or should it be assiduously keeping allocations on the heap and
> removing recursive algorithms?
> 
> I actually think this is the crux of the issue, and #1 is largely coming up
> as a proxy for "Clang should use less stack space".

I fully agree that there would be no request for fiddling with the stack
limit from within clang without clang blowing up the stack.

> However, I'll point out that this commit does not change how much stack
> space Clang uses. It just changes the nature of errors that occur to make
> the experience of today's Clang more friendly for users. So this commit
> seems like a fine thing even if we decide that Clang should work to lower
> this number over time and reduce what it sets it to. In fact, if you want
> to reduce Clang's stack usage, you almost certainly want it to explicitly
> set a limit so that we consistently hit errors when bugs creep into Clang.
> 
> Past that, while I think Clang's current allocation and recursion pattern
> is fine, I don't actually have a terribly strong opinion. But it seems odd
> to suddenly want to make a dramatic change in the design of Clang now... It
> doesn't seem like this was an accidental or unconsidered design decision.

Correct, this commit does nothing about the reducing the stack use by
clang. But let's look at one of the few bug reports that actually seem
to contain some numbers: 20635. This shows 3200 stack frames consuing
the 2MB default stack on 32bit FreeBSD. Similar defaults limits can be
found on most 32bit platforms, 64bit platforms typically have twice that
limit. This means the average stack frame is 600 Bytes for something
that is *intended* to recurse deeply. It seems pretty obvious to me that
this is the real problem and something that should be fixed. The LLVM
are much more conversative here. Many of the deep recursion places
either use very little stack space or follow the more typical work queue
approach. Setting a higher stack limit is I said earlier is just a
bandaid to paper over implementation issues.

Setting an explicit limit in the test suite would be fine by me. The
only problem in this area is that the kind of limits you want to set can
be difficult to do consistently across platforms, i.e. I would certainly
welcome it if no test case is supposed to require more than 1GB of VA
and 1MB/2MB of stack. That's a slightly different question though.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Sean Silva via cfe-commits 
On Fri, Aug 19, 2016 at 4:33 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Fri, Aug 19, 2016 at 03:30:42PM -0700, Richard Smith wrote:
> > It typically does; the default stack ulimit is likely tuned for "normal"
> > applications that are not expected (by an ISO standard) to cope with
> > recursing a thousand levels deep. If the system administrator really
> wants
> > to control the stack space for some stupid reason, they can set a hard
> > ulimit. If people actually do that, and then file bugs on clang crashing
> > (which they do today), we may want to resort to spawning a separate
> thread
> > with a higher ulimit if we detect the problem.
>
> Thread stacks by default are even smaller and for good reason.
>

Could you elaborate on this a bit more? I don't understand how a process
adjusting its own soft stack limit is "abusive" in any way.

Richard's patch is in cc1_main.cpp so it is at the top-level entry point of
the program, so we already know how many threads we intend to spawn etc. so
the issue of VA exhaustion on 32-bit seems moot. (libc might spawn threads
internally though, which could be an issue; Richard mentioned a workaround
though which is spawning the entire compilation onto a separate thread with
sufficient stack; would that work for you?)

-- Sean Silva


> Especially on 32bit platforms, it would be unusable otherwise. To put
> this into perspective: if you need to support a recursion level of 1000
> and can't do that with a 4MB stack, it means you are using more than 4KB
> per recursion level. That's a very high stack use and certainly
> something that qualifies as the kind of abusive behavior the process
> limit is designed for in first place.
>
> > It *is* unreasonable to expect them to fiddle with stack ulimits to get
> the
> > compiler to accept programs that, say, use certain components of boost
> (and
> > fit within the standard's recommended limits).
>
> I fundamentally disagree with this statement. Effectively, this seems to
> me to be papering over excessive stack use and nothing else.
>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Chandler Carruth via cfe-commits 
I feel like this thread has two very unrelated concerns and it might be
useful to separate them.

1) Should an application be changing the stack limit, or should the system
be able to enforce this?

Fortunately, there are two limits. One an application is allowed to change,
one it isn't. It seems like the mechanism has been provided for system
administrators to impose firm limits and neither Clang nor another
application will perturb them. I don't see any reason to preclude an
application from changing the one it is explicitly permitted to change.
That seems to be WAI from a permissions and ACL model.


2) Should Clang be using the stack to allocate significant portions of its
memory, or should it be assiduously keeping allocations on the heap and
removing recursive algorithms?

I actually think this is the crux of the issue, and #1 is largely coming up
as a proxy for "Clang should use less stack space".

However, I'll point out that this commit does not change how much stack
space Clang uses. It just changes the nature of errors that occur to make
the experience of today's Clang more friendly for users. So this commit
seems like a fine thing even if we decide that Clang should work to lower
this number over time and reduce what it sets it to. In fact, if you want
to reduce Clang's stack usage, you almost certainly want it to explicitly
set a limit so that we consistently hit errors when bugs creep into Clang.

Past that, while I think Clang's current allocation and recursion pattern
is fine, I don't actually have a terribly strong opinion. But it seems odd
to suddenly want to make a dramatic change in the design of Clang now... It
doesn't seem like this was an accidental or unconsidered design decision.

On Fri, Aug 19, 2016 at 4:33 PM Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Fri, Aug 19, 2016 at 03:30:42PM -0700, Richard Smith wrote:
> > It typically does; the default stack ulimit is likely tuned for "normal"
> > applications that are not expected (by an ISO standard) to cope with
> > recursing a thousand levels deep. If the system administrator really
> wants
> > to control the stack space for some stupid reason, they can set a hard
> > ulimit. If people actually do that, and then file bugs on clang crashing
> > (which they do today), we may want to resort to spawning a separate
> thread
> > with a higher ulimit if we detect the problem.
>
> Thread stacks by default are even smaller and for good reason.
> Especially on 32bit platforms, it would be unusable otherwise. To put
> this into perspective: if you need to support a recursion level of 1000
> and can't do that with a 4MB stack, it means you are using more than 4KB
> per recursion level. That's a very high stack use and certainly
> something that qualifies as the kind of abusive behavior the process
> limit is designed for in first place.
>
> > It *is* unreasonable to expect them to fiddle with stack ulimits to get
> the
> > compiler to accept programs that, say, use certain components of boost
> (and
> > fit within the standard's recommended limits).
>
> I fundamentally disagree with this statement. Effectively, this seems to
> me to be papering over excessive stack use and nothing else.
>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Joerg Sonnenberger via cfe-commits 
On Fri, Aug 19, 2016 at 03:30:42PM -0700, Richard Smith wrote:
> It typically does; the default stack ulimit is likely tuned for "normal"
> applications that are not expected (by an ISO standard) to cope with
> recursing a thousand levels deep. If the system administrator really wants
> to control the stack space for some stupid reason, they can set a hard
> ulimit. If people actually do that, and then file bugs on clang crashing
> (which they do today), we may want to resort to spawning a separate thread
> with a higher ulimit if we detect the problem.

Thread stacks by default are even smaller and for good reason.
Especially on 32bit platforms, it would be unusable otherwise. To put
this into perspective: if you need to support a recursion level of 1000
and can't do that with a 4MB stack, it means you are using more than 4KB
per recursion level. That's a very high stack use and certainly
something that qualifies as the kind of abusive behavior the process
limit is designed for in first place.

> It *is* unreasonable to expect them to fiddle with stack ulimits to get the
> compiler to accept programs that, say, use certain components of boost (and
> fit within the standard's recommended limits).

I fundamentally disagree with this statement. Effectively, this seems to
me to be papering over excessive stack use and nothing else.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Richard Smith via cfe-commits 
On Fri, Aug 19, 2016 at 1:43 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Fri, Aug 19, 2016 at 01:16:59PM -0700, Richard Smith wrote:
> > On Fri, Aug 19, 2016 at 1:10 PM, Joerg Sonnenberger via cfe-commits <
> > cfe-commits@lists.llvm.org> wrote:
> >
> > > On Fri, Aug 19, 2016 at 01:03:51PM -0700, Richard Smith wrote:
> > > > On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits
> <
> > > > cfe-commits@lists.llvm.org> wrote:
> > > >
> > > > > On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> > > > > > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via
> cfe-commits <
> > > > > > cfe-commits@lists.llvm.org> wrote:
> > > > > >
> > > > > > > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via
> > > cfe-commits
> > > > > > > wrote:
> > > > > > > > Author: rsmith
> > > > > > > > Date: Tue Aug 16 20:05:07 2016
> > > > > > > > New Revision: 278882
> > > > > > > >
> > > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > > > > > > Log:
> > > > > > > > If possible, set the stack rlimit to at least 8MiB on cc1
> > > startup,
> > > > > and
> > > > > > > work
> > > > > > > > around a Linux kernel bug where the actual amount of
> available
> > > stack
> > > > > may
> > > > > > > be a
> > > > > > > > *lot* lower than the rlimit.
> > > > > > >
> > > > > > > Can you please restrict this to Linux? I'm quite opposed to
> > > overriding
> > > > > > > system default limits, they exist for a reason.
> > > > > >
> > > > > >
> > > > > > No, that wouldn't make any sense. It's not up to the operating
> > > system how
> > > > > > an application decides to allocate memory (on the heap versus on
> the
> > > > > > stack), and Clang's stack usage isn't going to be significantly
> > > lower on
> > > > > > other kernels. If some BSD kernel's VM is unable to cope with
> this,
> > > we
> > > > > > could spawn a thread with a suitable amount of stack space
> instead.
> > > > >
> > > > > This is not about kernel bugs. It is about POLA -- programs are not
> > > > > supposed to alter process limits. If GCC does it, it is a GCC bug.
> > > > > That's no reason to introduce the same bug here. Using excessive
> stack
> > > > > space due to deep recursion might be a QoI issue, but it is
> > > > > fundamentally no different from any other out of memory condition.
> > > Those
> > > > > kill clang just as easily.
> > > >
> > > >
> > > > Hitting this limit does not imply that memory was exhausted, it
> instead
> > > > means the OS killed a process that was functioning just fine, for no
> good
> > > > reason.
> > >
> > > You are allocating too much stack space.
> >
> >
> > Whether we choose to put data on the heap or stack is not up to you or
> the
> > operating system.
>
> How is that relevant to honoring system limits? At the very least your
> commit says "clang knows better than the system administrator".


It typically does; the default stack ulimit is likely tuned for "normal"
applications that are not expected (by an ISO standard) to cope with
recursing a thousand levels deep. If the system administrator really wants
to control the stack space for some stupid reason, they can set a hard
ulimit. If people actually do that, and then file bugs on clang crashing
(which they do today), we may want to resort to spawning a separate thread
with a higher ulimit if we detect the problem.

> > There is no difference to heap
> > > allocations which are bound by different flags. It is just a different
> > > allocation mechanism. Even hitting a 4MB stack space limit on 64bit
> > > platforms takes quite a bit -- not even boost does that by default. As
> > > such, I hardly find it normal.
> >
> > It's not normal; that's why we're explicitly opting into it here, using
> the
> > mechanism that POSIX provides to do so.
>
> Read again. Input triggering such deep stack case is not normal.


Nonetheless, it is within the range that the standard expects us to
process, and it does happen in practice.

Just
> because POSIX provides mechanisms for a shell to implement ulimit
> doesn't mean that it is considered bad form for *applications* to
> override them, especially increasing them. Shall we raise the address
> space limits next, because there is input that easily requires more than
> 16GB of memory to compile? What about bumping the CPU time limit,
> because we have input where clang needs hours to compile a single file?
>

That's a strawman argument. These situations are not comparable.

All your commit has done is introduce another funny way to screw users.
> There are users of the clang libraries that are not the clang driver.
> They don't get this magic stack size boost.


Actually, they do in a lot of cases. libclang parses code in a separate
thread with at least this much stack space by default. Implicit module
compilations run in a separate thread with at least this much stack space.

Now we have source files
> that can be handled by clang but not by library

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Joerg Sonnenberger via cfe-commits 
On Fri, Aug 19, 2016 at 01:16:59PM -0700, Richard Smith wrote:
> On Fri, Aug 19, 2016 at 1:10 PM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > On Fri, Aug 19, 2016 at 01:03:51PM -0700, Richard Smith wrote:
> > > On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits <
> > > cfe-commits@lists.llvm.org> wrote:
> > >
> > > > On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> > > > > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
> > > > > cfe-commits@lists.llvm.org> wrote:
> > > > >
> > > > > > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via
> > cfe-commits
> > > > > > wrote:
> > > > > > > Author: rsmith
> > > > > > > Date: Tue Aug 16 20:05:07 2016
> > > > > > > New Revision: 278882
> > > > > > >
> > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > > > > > Log:
> > > > > > > If possible, set the stack rlimit to at least 8MiB on cc1
> > startup,
> > > > and
> > > > > > work
> > > > > > > around a Linux kernel bug where the actual amount of available
> > stack
> > > > may
> > > > > > be a
> > > > > > > *lot* lower than the rlimit.
> > > > > >
> > > > > > Can you please restrict this to Linux? I'm quite opposed to
> > overriding
> > > > > > system default limits, they exist for a reason.
> > > > >
> > > > >
> > > > > No, that wouldn't make any sense. It's not up to the operating
> > system how
> > > > > an application decides to allocate memory (on the heap versus on the
> > > > > stack), and Clang's stack usage isn't going to be significantly
> > lower on
> > > > > other kernels. If some BSD kernel's VM is unable to cope with this,
> > we
> > > > > could spawn a thread with a suitable amount of stack space instead.
> > > >
> > > > This is not about kernel bugs. It is about POLA -- programs are not
> > > > supposed to alter process limits. If GCC does it, it is a GCC bug.
> > > > That's no reason to introduce the same bug here. Using excessive stack
> > > > space due to deep recursion might be a QoI issue, but it is
> > > > fundamentally no different from any other out of memory condition.
> > Those
> > > > kill clang just as easily.
> > >
> > >
> > > Hitting this limit does not imply that memory was exhausted, it instead
> > > means the OS killed a process that was functioning just fine, for no good
> > > reason.
> >
> > You are allocating too much stack space.
> 
> 
> Whether we choose to put data on the heap or stack is not up to you or the
> operating system.

How is that relevant to honoring system limits? At the very least your
commit says "clang knows better than the system administrator".

> > There is no difference to heap
> > allocations which are bound by different flags. It is just a different
> > allocation mechanism. Even hitting a 4MB stack space limit on 64bit
> > platforms takes quite a bit -- not even boost does that by default. As
> > such, I hardly find it normal.
> 
> It's not normal; that's why we're explicitly opting into it here, using the
> mechanism that POSIX provides to do so.

Read again. Input triggering such deep stack case is not normal. Just
because POSIX provides mechanisms for a shell to implement ulimit
doesn't mean that it is considered bad form for *applications* to
override them, especially increasing them. Shall we raise the address
space limits next, because there is input that easily requires more than
16GB of memory to compile? What about bumping the CPU time limit,
because we have input where clang needs hours to compile a single file?

All your commit has done is introduce another funny way to screw users.
There are users of the clang libraries that are not the clang driver.
They don't get this magic stack size boost. Now we have source files
that can be handled by clang but not by library consumers. Lovely edge
case. It all comes back to "raise limits for exceptional programs".
Just like users of compilers for functional languages have learned:
if you write source that triggers such limits, make sure your system is
up for handling them.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Richard Smith via cfe-commits 
On Fri, Aug 19, 2016 at 1:10 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Fri, Aug 19, 2016 at 01:03:51PM -0700, Richard Smith wrote:
> > On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits <
> > cfe-commits@lists.llvm.org> wrote:
> >
> > > On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> > > > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
> > > > cfe-commits@lists.llvm.org> wrote:
> > > >
> > > > > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via
> cfe-commits
> > > > > wrote:
> > > > > > Author: rsmith
> > > > > > Date: Tue Aug 16 20:05:07 2016
> > > > > > New Revision: 278882
> > > > > >
> > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > > > > Log:
> > > > > > If possible, set the stack rlimit to at least 8MiB on cc1
> startup,
> > > and
> > > > > work
> > > > > > around a Linux kernel bug where the actual amount of available
> stack
> > > may
> > > > > be a
> > > > > > *lot* lower than the rlimit.
> > > > >
> > > > > Can you please restrict this to Linux? I'm quite opposed to
> overriding
> > > > > system default limits, they exist for a reason.
> > > >
> > > >
> > > > No, that wouldn't make any sense. It's not up to the operating
> system how
> > > > an application decides to allocate memory (on the heap versus on the
> > > > stack), and Clang's stack usage isn't going to be significantly
> lower on
> > > > other kernels. If some BSD kernel's VM is unable to cope with this,
> we
> > > > could spawn a thread with a suitable amount of stack space instead.
> > >
> > > This is not about kernel bugs. It is about POLA -- programs are not
> > > supposed to alter process limits. If GCC does it, it is a GCC bug.
> > > That's no reason to introduce the same bug here. Using excessive stack
> > > space due to deep recursion might be a QoI issue, but it is
> > > fundamentally no different from any other out of memory condition.
> Those
> > > kill clang just as easily.
> >
> >
> > Hitting this limit does not imply that memory was exhausted, it instead
> > means the OS killed a process that was functioning just fine, for no good
> > reason.
>
> You are allocating too much stack space.


Whether we choose to put data on the heap or stack is not up to you or the
operating system.

There is no difference to heap
> allocations which are bound by different flags. It is just a different
> allocation mechanism. Even hitting a 4MB stack space limit on 64bit
> platforms takes quite a bit -- not even boost does that by default. As
> such, I hardly find it normal.


It's not normal; that's why we're explicitly opting into it here, using the
mechanism that POSIX provides to do so.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Joerg Sonnenberger via cfe-commits 
On Fri, Aug 19, 2016 at 01:03:51PM -0700, Richard Smith wrote:
> On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> > > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
> > > cfe-commits@lists.llvm.org> wrote:
> > >
> > > > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> > > > wrote:
> > > > > Author: rsmith
> > > > > Date: Tue Aug 16 20:05:07 2016
> > > > > New Revision: 278882
> > > > >
> > > > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > > > Log:
> > > > > If possible, set the stack rlimit to at least 8MiB on cc1 startup,
> > and
> > > > work
> > > > > around a Linux kernel bug where the actual amount of available stack
> > may
> > > > be a
> > > > > *lot* lower than the rlimit.
> > > >
> > > > Can you please restrict this to Linux? I'm quite opposed to overriding
> > > > system default limits, they exist for a reason.
> > >
> > >
> > > No, that wouldn't make any sense. It's not up to the operating system how
> > > an application decides to allocate memory (on the heap versus on the
> > > stack), and Clang's stack usage isn't going to be significantly lower on
> > > other kernels. If some BSD kernel's VM is unable to cope with this, we
> > > could spawn a thread with a suitable amount of stack space instead.
> >
> > This is not about kernel bugs. It is about POLA -- programs are not
> > supposed to alter process limits. If GCC does it, it is a GCC bug.
> > That's no reason to introduce the same bug here. Using excessive stack
> > space due to deep recursion might be a QoI issue, but it is
> > fundamentally no different from any other out of memory condition. Those
> > kill clang just as easily.
> 
> 
> Hitting this limit does not imply that memory was exhausted, it instead
> means the OS killed a process that was functioning just fine, for no good
> reason.

You are allocating too much stack space. There is no difference to heap
allocations which are bound by different flags. It is just a different
allocation mechanism. Even hitting a 4MB stack space limit on 64bit
platforms takes quite a bit -- not even boost does that by default. As
such, I hardly find it normal.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Richard Smith via cfe-commits 
On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
> > cfe-commits@lists.llvm.org> wrote:
> >
> > > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> > > wrote:
> > > > Author: rsmith
> > > > Date: Tue Aug 16 20:05:07 2016
> > > > New Revision: 278882
> > > >
> > > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > > Log:
> > > > If possible, set the stack rlimit to at least 8MiB on cc1 startup,
> and
> > > work
> > > > around a Linux kernel bug where the actual amount of available stack
> may
> > > be a
> > > > *lot* lower than the rlimit.
> > >
> > > Can you please restrict this to Linux? I'm quite opposed to overriding
> > > system default limits, they exist for a reason.
> >
> >
> > No, that wouldn't make any sense. It's not up to the operating system how
> > an application decides to allocate memory (on the heap versus on the
> > stack), and Clang's stack usage isn't going to be significantly lower on
> > other kernels. If some BSD kernel's VM is unable to cope with this, we
> > could spawn a thread with a suitable amount of stack space instead.
>
> This is not about kernel bugs. It is about POLA -- programs are not
> supposed to alter process limits. If GCC does it, it is a GCC bug.
> That's no reason to introduce the same bug here. Using excessive stack
> space due to deep recursion might be a QoI issue, but it is
> fundamentally no different from any other out of memory condition. Those
> kill clang just as easily.


Hitting this limit does not imply that memory was exhausted, it instead
means the OS killed a process that was functioning just fine, for no good
reason. As you say, this limit exists for a reason -- and the reason is to
catch programs that are recursing infinitely or using an unexpected amount
of stack space. In Clang's case, using a lot of stack is not unexpected and
does not suggest infinite recursion. So the appropriate thing to do is to
relax or otherwise avoid the arbitrary and harmful limit.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-19 Thread Joerg Sonnenberger via cfe-commits 
On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:
> On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> > wrote:
> > > Author: rsmith
> > > Date: Tue Aug 16 20:05:07 2016
> > > New Revision: 278882
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > Log:
> > > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and
> > work
> > > around a Linux kernel bug where the actual amount of available stack may
> > be a
> > > *lot* lower than the rlimit.
> >
> > Can you please restrict this to Linux? I'm quite opposed to overriding
> > system default limits, they exist for a reason.
> 
> 
> No, that wouldn't make any sense. It's not up to the operating system how
> an application decides to allocate memory (on the heap versus on the
> stack), and Clang's stack usage isn't going to be significantly lower on
> other kernels. If some BSD kernel's VM is unable to cope with this, we
> could spawn a thread with a suitable amount of stack space instead.

This is not about kernel bugs. It is about POLA -- programs are not
supposed to alter process limits. If GCC does it, it is a GCC bug.
That's no reason to introduce the same bug here. Using excessive stack
space due to deep recursion might be a QoI issue, but it is
fundamentally no different from any other out of memory condition. Those
kill clang just as easily.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-18 Thread Richard Smith via cfe-commits 
On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> wrote:
> > Author: rsmith
> > Date: Tue Aug 16 20:05:07 2016
> > New Revision: 278882
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > Log:
> > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and
> work
> > around a Linux kernel bug where the actual amount of available stack may
> be a
> > *lot* lower than the rlimit.
>
> Can you please restrict this to Linux? I'm quite opposed to overriding
> system default limits, they exist for a reason.


No, that wouldn't make any sense. It's not up to the operating system how
an application decides to allocate memory (on the heap versus on the
stack), and Clang's stack usage isn't going to be significantly lower on
other kernels. If some BSD kernel's VM is unable to cope with this, we
could spawn a thread with a suitable amount of stack space instead.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-18 Thread Richard Smith via cfe-commits 
llvm-config.h doesn't provide the necessary macros. I've applied a
different fix for out-of-tree builds in r279112.

On Wed, Aug 17, 2016 at 11:51 PM, Vedant Kumar via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Done in clang/r279035.
>
> thanks,
> vedant
>
> > On Aug 17, 2016, at 7:43 AM, Will Dietz via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > (Seems to fix the build here, FWIW.  If sounds reasonable can someone
> commit this? Thanks!)
> >
> > ~Will
> >
> > On Wed, Aug 17, 2016 at 9:39 AM Will Dietz  wrote:
> > This is the first use of "llvm/Config/config.h" in the clang tree, which
> breaks out-of-tree builds for me (since llvm's config.h isn't installed).
> >
> > Would using "llvm/Config/llvm-config.h" suffice instead? I'm trying that
> now...
> >
> > ~Will
> >
> > On Wed, Aug 17, 2016 at 8:36 AM Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> > On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> wrote:
> > > Author: rsmith
> > > Date: Tue Aug 16 20:05:07 2016
> > > New Revision: 278882
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > > Log:
> > > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and
> work
> > > around a Linux kernel bug where the actual amount of available stack
> may be a
> > > *lot* lower than the rlimit.
> >
> > Can you please restrict this to Linux? I'm quite opposed to overriding
> > system default limits, they exist for a reason.
> >
> > Joerg
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-18 Thread Vedant Kumar via cfe-commits 
Done in clang/r279035.

thanks,
vedant

> On Aug 17, 2016, at 7:43 AM, Will Dietz via cfe-commits 
>  wrote:
> 
> (Seems to fix the build here, FWIW.  If sounds reasonable can someone commit 
> this? Thanks!)
> 
> ~Will
> 
> On Wed, Aug 17, 2016 at 9:39 AM Will Dietz  wrote:
> This is the first use of "llvm/Config/config.h" in the clang tree, which 
> breaks out-of-tree builds for me (since llvm's config.h isn't installed).
> 
> Would using "llvm/Config/llvm-config.h" suffice instead? I'm trying that 
> now...
> 
> ~Will
> 
> On Wed, Aug 17, 2016 at 8:36 AM Joerg Sonnenberger via cfe-commits 
>  wrote:
> On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits wrote:
> > Author: rsmith
> > Date: Tue Aug 16 20:05:07 2016
> > New Revision: 278882
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > Log:
> > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work
> > around a Linux kernel bug where the actual amount of available stack may be 
> > a
> > *lot* lower than the rlimit.
> 
> Can you please restrict this to Linux? I'm quite opposed to overriding
> system default limits, they exist for a reason.
> 
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-17 Thread Will Dietz via cfe-commits 
(Seems to fix the build here, FWIW.  If sounds reasonable can someone
commit this? Thanks!)

~Will

On Wed, Aug 17, 2016 at 9:39 AM Will Dietz  wrote:

> This is the first use of "llvm/Config/config.h" in the clang tree, which
> breaks out-of-tree builds for me (since llvm's config.h isn't installed).
>
> Would using "llvm/Config/llvm-config.h" suffice instead? I'm trying that
> now...
>
> ~Will
>
> On Wed, Aug 17, 2016 at 8:36 AM Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
>> wrote:
>> > Author: rsmith
>> > Date: Tue Aug 16 20:05:07 2016
>> > New Revision: 278882
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
>> > Log:
>> > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and
>> work
>> > around a Linux kernel bug where the actual amount of available stack
>> may be a
>> > *lot* lower than the rlimit.
>>
>> Can you please restrict this to Linux? I'm quite opposed to overriding
>> system default limits, they exist for a reason.
>>
>> Joerg
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-17 Thread Will Dietz via cfe-commits 
This is the first use of "llvm/Config/config.h" in the clang tree, which
breaks out-of-tree builds for me (since llvm's config.h isn't installed).

Would using "llvm/Config/llvm-config.h" suffice instead? I'm trying that
now...

~Will

On Wed, Aug 17, 2016 at 8:36 AM Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits
> wrote:
> > Author: rsmith
> > Date: Tue Aug 16 20:05:07 2016
> > New Revision: 278882
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> > Log:
> > If possible, set the stack rlimit to at least 8MiB on cc1 startup, and
> work
> > around a Linux kernel bug where the actual amount of available stack may
> be a
> > *lot* lower than the rlimit.
>
> Can you please restrict this to Linux? I'm quite opposed to overriding
> system default limits, they exist for a reason.
>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-17 Thread Joerg Sonnenberger via cfe-commits 
On Wed, Aug 17, 2016 at 01:05:08AM -, Richard Smith via cfe-commits wrote:
> Author: rsmith
> Date: Tue Aug 16 20:05:07 2016
> New Revision: 278882
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
> Log:
> If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work
> around a Linux kernel bug where the actual amount of available stack may be a
> *lot* lower than the rlimit.

Can you please restrict this to Linux? I'm quite opposed to overriding
system default limits, they exist for a reason.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r278882 - If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work

2016-08-16 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Tue Aug 16 20:05:07 2016
New Revision: 278882

URL: http://llvm.org/viewvc/llvm-project?rev=278882=rev
Log:
If possible, set the stack rlimit to at least 8MiB on cc1 startup, and work
around a Linux kernel bug where the actual amount of available stack may be a
*lot* lower than the rlimit.

GCC also sets a higher stack rlimit on startup, but it goes all the way to
64MiB. We can increase this limit if it proves necessary.

The kernel bug is as follows: Linux kernels prior to version 4.1 may choose to
map the process's heap as little as 128MiB before the process's stack for a PIE
binary, even in a 64-bit virtual address space. This means that allocating more
than 128MiB before you reach the process's stack high water mark can lead to
crashes, even if you don't recurse particularly deeply.

We work around the kernel bug by touching a page deep within the stack (after
ensuring that we know how big it is), to preallocate virtual address space for
the stack so that the kernel doesn't allow the brk() area to wander into it,
when building clang as a Linux PIE binary.

Modified:
cfe/trunk/tools/driver/cc1_main.cpp

Modified: cfe/trunk/tools/driver/cc1_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1_main.cpp?rev=278882=278881=278882=diff
==
--- cfe/trunk/tools/driver/cc1_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1_main.cpp Tue Aug 16 20:05:07 2016
@@ -25,9 +25,11 @@
 #include "clang/Frontend/Utils.h"
 #include "clang/FrontendTool/Utils.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Config/config.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Signals.h"
@@ -35,6 +37,9 @@
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#if HAVE_SYS_RESOURCE_H
+#include 
+#endif
 using namespace clang;
 using namespace llvm::opt;
 
@@ -64,7 +69,67 @@ void initializePollyPasses(llvm::PassReg
 }
 #endif
 
+#if HAVE_SYS_RESOURCE_H && HAVE_GETRLIMIT && HAVE_SETRLIMIT
+// The amount of stack we think is "sufficient". If less than this much is
+// available, we may be unable to reach our template instantiation depth
+// limit and other similar limits.
+// FIXME: Unify this with the stack we request when spawning a thread to build
+// a module.
+static const int kSufficientStack = 8 << 20;
+
+#if defined(__linux__) && defined(__PIE__)
+LLVM_ATTRIBUTE_NOINLINE
+static void ensureStackAddressSpace() {
+  // Linux kernels prior to 4.1 will sometimes locate the heap of a PIE binary
+  // relatively close to the stack (they are only guaranteed to be 128MiB
+  // apart). This results in crashes if we happen to heap-allocate more than
+  // 128MiB before we reach our stack high-water mark.
+  //
+  // To avoid these crashes, ensure that we have sufficient virtual memory
+  // pages allocated before we start running by touching an early page. (We
+  // allow 512KiB for kernel/libc-provided data such as command-line arguments
+  // and environment variables, and for main and cc1_main)
+  volatile char ReservedStack[kSufficientStack - 512 * 1024];
+  volatile int N = 0;
+  (void)+ReservedStack[N];
+}
+#else
+static void ensureStackAddressSpace() {}
+#endif
+
+/// Attempt to ensure that we have at least 8MiB of usable stack space.
+static void ensureSufficientStack() {
+  struct rlimit rlim;
+  if (getrlimit(RLIMIT_STACK, ) != 0)
+return;
+
+  // Increase the soft stack limit to our desired level, if necessary and
+  // possible.
+  if (rlim.rlim_cur != RLIM_INFINITY && rlim.rlim_cur < kSufficientStack) {
+// Try to allocate sufficient stack.
+if (rlim.rlim_max == RLIM_INFINITY || rlim.rlim_max >= kSufficientStack)
+  rlim.rlim_cur = kSufficientStack;
+else if (rlim.rlim_cur == rlim.rlim_max)
+  return;
+else
+  rlim.rlim_cur = rlim.rlim_max;
+
+if (setrlimit(RLIMIT_STACK, ) != 0 ||
+rlim.rlim_cur != kSufficientStack)
+  return;
+  }
+
+  // We should now have a stack of size at least kSufficientStack. Ensure
+  // that we can actually use that much, if necessary.
+  ensureStackAddressSpace();
+}
+#else
+static void ensureSufficientStack() {
+#endif
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
+  ensureSufficientStack();
+
   std::unique_ptr Clang(new CompilerInstance());
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits