<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 19, 2016 at 1:43 PM, Joerg Sonnenberger via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Aug 19, 2016 at 01:16:59PM -0700, Richard Smith wrote:<br>
> On Fri, Aug 19, 2016 at 1:10 PM, Joerg Sonnenberger via cfe-commits <<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> > On Fri, Aug 19, 2016 at 01:03:51PM -0700, Richard Smith wrote:<br>
> > > On Fri, Aug 19, 2016 at 12:58 PM, Joerg Sonnenberger via cfe-commits <<br>
> > > <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
> > ><br>
> > > > On Thu, Aug 18, 2016 at 11:33:49AM -0700, Richard Smith wrote:<br>
> > > > > On Wed, Aug 17, 2016 at 6:35 AM, Joerg Sonnenberger via cfe-commits <<br>
> > > > > <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
> > > > ><br>
> > > > > > On Wed, Aug 17, 2016 at 01:05:08AM -0000, Richard Smith via<br>
> > cfe-commits<br>
> > > > > > wrote:<br>
> > > > > > > Author: rsmith<br>
> > > > > > > Date: Tue Aug 16 20:05:07 2016<br>
> > > > > > > New Revision: 278882<br>
> > > > > > ><br>
> > > > > > > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=278882&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=278882&view=rev</a><br>
> > > > > > > Log:<br>
> > > > > > > If possible, set the stack rlimit to at least 8MiB on cc1<br>
> > startup,<br>
> > > > and<br>
> > > > > > work<br>
> > > > > > > around a Linux kernel bug where the actual amount of available<br>
> > stack<br>
> > > > may<br>
> > > > > > be a<br>
> > > > > > > *lot* lower than the rlimit.<br>
> > > > > ><br>
> > > > > > Can you please restrict this to Linux? I'm quite opposed to<br>
> > overriding<br>
> > > > > > system default limits, they exist for a reason.<br>
> > > > ><br>
> > > > ><br>
> > > > > No, that wouldn't make any sense. It's not up to the operating<br>
> > system how<br>
> > > > > an application decides to allocate memory (on the heap versus on the<br>
> > > > > stack), and Clang's stack usage isn't going to be significantly<br>
> > lower on<br>
> > > > > other kernels. If some BSD kernel's VM is unable to cope with this,<br>
> > we<br>
> > > > > could spawn a thread with a suitable amount of stack space instead.<br>
> > > ><br>
> > > > This is not about kernel bugs. It is about POLA -- programs are not<br>
> > > > supposed to alter process limits. If GCC does it, it is a GCC bug.<br>
> > > > That's no reason to introduce the same bug here. Using excessive stack<br>
> > > > space due to deep recursion might be a QoI issue, but it is<br>
> > > > fundamentally no different from any other out of memory condition.<br>
> > Those<br>
> > > > kill clang just as easily.<br>
> > ><br>
> > ><br>
> > > Hitting this limit does not imply that memory was exhausted, it instead<br>
> > > means the OS killed a process that was functioning just fine, for no good<br>
> > > reason.<br>
> ><br>
> > You are allocating too much stack space.<br>
><br>
><br>
> Whether we choose to put data on the heap or stack is not up to you or the<br>
> operating system.<br>
<br>
</div></div>How is that relevant to honoring system limits? At the very least your<br>
commit says "clang knows better than the system administrator".</blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> > There is no difference to heap<br>
> > allocations which are bound by different flags. It is just a different<br>
> > allocation mechanism. Even hitting a 4MB stack space limit on 64bit<br>
> > platforms takes quite a bit -- not even boost does that by default. As<br>
> > such, I hardly find it normal.<br>
><br>
> It's not normal; that's why we're explicitly opting into it here, using the<br>
> mechanism that POSIX provides to do so.<br>
<br>
</span>Read again. Input triggering such deep stack case is not normal.</blockquote><div><br></div><div>Nonetheless, it is within the range that the standard expects us to process, and it does happen in practice.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Just<br>
because POSIX provides mechanisms for a shell to implement ulimit<br>
doesn't mean that it is considered bad form for *applications* to<br>
override them, especially increasing them. Shall we raise the address<br>
space limits next, because there is input that easily requires more than<br>
16GB of memory to compile? What about bumping the CPU time limit,<br>
because we have input where clang needs hours to compile a single file?<br></blockquote><div><br></div><div>That's a strawman argument. These situations are not comparable.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
All your commit has done is introduce another funny way to screw users.<br>
There are users of the clang libraries that are not the clang driver.<br>
They don't get this magic stack size boost.</blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Now we have source files<br>
that can be handled by clang but not by library consumers. Lovely edge<br>
case.</blockquote><div><br></div><div>OK, this is a reasonable point, and it seems like the conclusion is that we should document the amount of stack space that clang expects, so that tools using clang can provide that much, and perhaps provide some convenience functions for such tools to use to ensure they get the amount we want. That seems very reasonable to me. There's even a FIXME in this commit to that effect. (Clang's 8MiB expectation is now duplicated in a handful of places; unifying these seems like a very sensible move.)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It all comes back to "raise limits for exceptional programs".<br>
Just like users of compilers for functional languages have learned:<br>
if you write source that triggers such limits, make sure your system is<br>
up for handling them.</blockquote><div><br></div><div>If we were talking about programs that exceeded the recommended limits from the C++ standard, then sure, this seems like a reasonable position to take. But we're not. It's not unreasonable for C++ programmers to expect clang to accept programs that fit within the limits recommended by the C++ standard. 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).</div></div></div></div>