[Piglit] [PATCH] mingw: use default 2MB stack size instead of 1MB

[Brian Paul]

On 10/12/2017 08:04 AM, Jose Fonseca wrote:
> The intent here was not so much to match the piglti MSVC build, but apps
> build by MSVC in general.
>
> After all, nothing ever prevented us from setting a huge stack size on
> both MinGW and MSVC alike, as both toolchains allow to congifure the
> stack size to whatever we want.
>
>
> The key issue here is that OpenGL driver don't get to pick the apps they
> are loaded, and real OpenGL applications will be likely built with MSVC
> instead of Mingw, and therefore will likely only have the MSVC default
> stack size.  And we should err on the side of caution when testing.
>
>
> Regardless of the compiler used, if we bump the stack size in piglit,
> one is just increasing the chance that wasteful stack allocations go
> undetected on piglit and blow up on real applications.
>
>
> Therefore I suggest we continue to keep 1MB default, and try to fix Mesa
> to be less stack hungry.   If that's not pratical here,

The ir_expression::constant_expression_value() function's local vars are 
pretty minimal.  The two that stand out:

    ir_constant *op[ARRAY_SIZE(this->operands)] = { NULL, };
    ir_constant_data data;

are only 32 and 128 bytes, respectively (on a 32-bit build).  I'm not 
sure what else accounts for the approx 2KB of the activation record.

I don't see an obvious way to fix the problem.  Even if we could reduce 
per-call stack memory, someone could write an evil shader that adds a 
thousand or more terms and we'd overflow the stack again.


> then we should
> try to bump the stack size of specific piglit tests (like those that
> stress the compiler to the extreme, as Cmake allows to set these options
> per executable), and only those tests.  Or just mark the affected tests
> as expected fail/skip.

The op-selection-bool-bvec4-bvec4.frag test is run with 
shader_runner.exe.  I think most of the large/complex shaders in Piglit 
are run with shader_runner so I don't think it makes much difference if 
only shader_runner or all piglit executables are build with a 2MB stack.

In any case, I've got a patch which only sets shader_runner's stack size 
which I'll post next.

I guess another option is to change the 
op-selection-bool-bvec4-bvec4.frag test to be simpler, but that's just 
sweeping the root problem under the rug.  Someone could always craft a 
complex shader which blows out the stack.


>
>
> If we feel that 1MB stack is too restrictive nowadays, then we should
> invest time into moving GLSL compilation into a separate/internal
> thread, which would enable us to pick whatever stack size we want when
> creating that thread.  Another alternative is to do a low-level
> manipulation of stack registers, and force the compilation to happen in
> manually allocated stack.   Neither are easy-peasy though, and severaly
> hamper debugability.

Yeah, those options don't sound too appealing or do-able in the short term.

-Brian

>
> Jose
>
> On 12/10/17 14:43, Brian Paul wrote:
>> The op-selection-bool-bvec4-bvec4.frag test has an expression which
>> adds 512 terms.  This causes 512 levels of recursion in Mesa's
>> ir_expression::constant_expression_value() function.  Since each
>> function activation record is about 2KB in size with MSVC, this
>> causes us to overflow the stack and crash.
>>
>> The crash was only recently exposed with MSVC 2015 debug builds of Mesa.
>>
>> The default MinGW stack size of 2MB is enough to avoid this issue.
>> Since we no longer build Piglit with MSVC, we don't have to match
>> MSVC's 1MB stack default.
>> ---
>>   CMakeLists.txt | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index 4259ec8..cdecca4 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -301,9 +301,6 @@ else ()
>>   endif ()
>>   if (MINGW)
>> -    # Match MSVC default stack size
>> -    set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}
>> -Wl,--stack,1048576")
>> -
>>       # Avoid depending on MinGW runtime DLLs
>>       check_cxx_compiler_flag (-static-libgcc HAVE_STATIC_LIBGCC_FLAG)
>>       if (HAVE_STATIC_LIBGCC_FLAG)
>>
>