[PATCH] Cygwin: Fixing the math behind rounding down ch.stacklimit to page size

[Tim Adye]

Hi Corinna, Sergejs,

Sorry, I should have followed up in this thread sooner. I can confirm 
that I tested the x86_64 cygwin1-20180607.dll.xz snapshot and it 
resolved my issue with bash with HitmanPro.

Thanks!
Tim.

On 05/06/2018 12:16, Tim Adye wrote:
> Hi Corinna,
>
> On 29 May, 18:44, Corinna Vinschen wrote:
>> Hi Sergejs,
>>
>> On May 25 17:43, Sergejs Lukanihins wrote:
>> > Hello,
>> > > Looks like ch.stacklimit wasn't being page-aligned correctly in
>> > fork.cc; you need to subtract 1 from page_size to do it correctly (see
>> > the attached patch).
>> > > As a result, this was causing stack-overflow exceptions whenever the
>> > stack needed to grow beyond the stacklimit value. When the stack grows
>> > beyond stacklimit value, Windows uses ntdll!_chkstk() function to
>> > check the stack and map in additional stack pages. However, it expects
>> > stacklimit to be page aligned, and the function does not work
>> > correctly if it is not (it triggers STATUS_STACK_OVERFLOW, even if
>> > there is enough stack space).
>> > > Normally, this was not causing any issues, as the stack never really
>> > needs to grow, but it was causing issues when AV software was being
>> > injected into the process (specifically, HitmanPro.Alert being
>> > injected into git’s sh.exe process). Due to function hooks, it lead to
>> > a bigger callstack, and more stack space being required. Making the
>> > change specified in the patch actually resolves the issue.
>> > > I am providing my patches to the Cygwin sources under the 
>> 2-clause BSD license.
>>
>> Good catch!  Patch pushed.
> Thanks for adding this patch, and thanks to Sergejs for providing it!
>
> I think I may have encountered this problem (fork failures for some 
> constructs in bash, only when HitmanPro is enabled). Would it be 
> possible to make a new cygwin1.dll snapshot so I can test that? I 
> tested the current x86_64 snapshot, which was produced just hours 
> before this patch. That still exhibits the problem, but seems to work 
> fine otherwise.
>
> Thanks,
> Tim.