[PATCH] Explicitly specify CMake MSVC stack size

[Greg_Bedwell at sn.scee.net]

Thanks for looking at this!

> This changes the stack of every tool, not just clang, right? 

Yes and no depending on how you look at it.  It increases the stack size 
for every tool for anybody that is already using CMake 2.8.11, but it 
prevents the stack size from changing for anybody in future that is still 
using a previous version.  According to cmake.org, 2.8.11 was released in 
May 2013, so I'm guessing that a lot of people may not have had reason to 
update yet.  The only reason I updated was because the previous version I 
was using was too old for compiler-rt which I wanted to mess around with.

>Is it possible to change it just for clang?
> 

It's true that the only place I've actually observed us actually requiring 
10MB of stack is in clang.exe, but then I have more exhaustive tests for 
clang.exe than any other tool :).  The primary motivation here is to avoid 
the situation where two people building the exact same revision with 
exactly the same MSVC version get different results with one of the tools 
depending on the version of CMake they happen to have installed.  It just 
feels like it's a horrible day of debugging for someone later on down the 
line.

My idea was that we apply this to every tool for now in order to preserve 
previous behaviour, and then at the point where we mandate a version of 
CMake that supports CMAKE_CXX_STACK_SIZE we can find more appropriate 
values for each tool (most likely, the default 1MB I'll concede for tools 
other than clang).  This will then guarantee that anyone trying to hunt 
down any strange regressions can at least track it down to a single 
revision that changed the stack size.

Does this sound like a reasonable approach?

Thanks,

-Greg

> On 24 September 2013 10:32,  <Greg_Bedwell at sn.scee.net> wrote:
> > Hi,
> >
> > This is my first attempt at submitting a patch, so please go easy on 
me
> > :-).
> >
> > I recently upgraded our version of CMake to 2.8.11.2 and was surprised 
to
> > find that one of our test suites showed a new crash in our MSVC-built
> > clang.  Debugging showed that it was caused by clang overflowing the
> > Windows stack.  I eventually tracked it down to a change to resolve 
the
> > following issue in CMake:
> > http://www.gccxml.org/Bug/view.php?id=12437 [Default link line 
includes
> > massive stack size of 10M with the /STACK:10000000 option ]
> >
> > The effect of this change in CMake is to reduce the Windows stack size 
for
> > CMake built executables from ~10MB to the default value of 1MB.  I've 
only
> > seen this cause an issue in clang when parsing recursive templates so 
I've
> > included a clang test, but in theory could affect any tool built in 
the
> > LLVM tree so my fix is in the top-level LLVM CMake file.  My fix
> > explicitly sets the previous 10000000 bytes value in order to maintain
> > consistent behaviour between builds using different versions of CMake.
> >
> > Unfortunately, due to a bug in previous versions of CMake (
> > http://www.cmake.org/Bug/view.php?id=13968 ), specifying
> > CMAKE_CXX_STACK_SIZE results in a link error of missing "10000000.obj" 
so
> > I've had to guard my change against the version of CMake being greater
> > than or equal to 2.8.11.  My idea is that at some point in the future 
we
> > may set the minimum version of CMake to at least 2.8.11 in which case 
we
> > could consider finding a more appropriate value for 
CMAKE_CXX_STACK_SIZE
> > but for now I think keeping consistency is important (i.e. if the 
process
> > stack size changes, it should be traceable to a specific revision 
where a
> > value was changed, rather than some change external to the LLVM/Clang
> > trees such as differing versions of CMake being used).
> >
> > Without this fix, the attached test will crash due to a stack 
overflow,
> > when generating the project with CMake 2.8.11 (on VS2010 x64 at least) 
and
> > will pass when the project is generated with earlier versions of 
CMake.  I
> > have a slight concern that the test may cause issues for any hosts we
> > support with smaller default stacks.  If this is a real problem then a
> > test may not be appropriate in this case.
> >
> >
> >
> > I appreciate any feedback you may have.  Please commit for me if you 
are
> > happy with the change.
> >
> > Thanks,
> >
> > --
> > Greg Bedwell
> > SN Systems - Sony Computer Entertainment Group
> > http://www.snsys.com


**********************************************************************
This email and any files transmitted with it are confidential and intended 
solely for the use of the individual or entity to whom they are addressed. 
If you have received this email in error please notify postmaster at scee.net
This footnote also confirms that this email message has been checked for 
all known viruses.
Sony Computer Entertainment Europe Limited
Registered Office: 10 Great Marlborough Street, London W1F 7LP, United 
Kingdom
Registered in England: 3277793
**********************************************************************

P Please consider the environment before printing this e-mail