Re: libgo patch committed: Update to 1.7rc3

[Ian Lance Taylor <iant at golang dot org>]

On Fri, Aug 5, 2016 at 1:24 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Aug 4, 2016 at 6:48 PM, Ian Lance Taylor <iant@golang.org> wrote:
>> On Thu, Aug 4, 2016 at 1:11 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Thu, Aug 4, 2016 at 12:53 AM, Ian Lance Taylor <iant@golang.org> wrote:
>>>> On Thu, Jul 28, 2016 at 4:24 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>>
>>>>> A new testsuite failure is introduced:
>>>>>
>>>>> FAIL: text/template
>>>>>
>>>>> on both, x86_64-linux-gnu and alpha-linux-gnu.
>>>>>
>>>>> The testcase corrupts stack with a too deep recursion.
>>>>>
>>>>> There is a part in libgo/go/text/template/exec.go that should handle
>>>>> this situaiton:
>>>>>
>>>>> // maxExecDepth specifies the maximum stack depth of templates within
>>>>> // templates. This limit is only practically reached by accidentally
>>>>> // recursive template invocations. This limit allows us to return
>>>>> // an error instead of triggering a stack overflow.
>>>>> const maxExecDepth = 100000
>>>>>
>>>>> but the limit is either set too high, or the error handling code is
>>>>> inefficient on both, split-stack (x86_64) and non-split-stack (alpha)
>>>>> targets. Lowering this value to 10000 "fixes" the testcase on both
>>>>> targets.
>>>>
>>>> I can not recreate this problem on x86 or x86_64.
>>>>
>>>> Does this patch work around the problem on Alpha?
>>>
>>> Yes, the patch "fixes" the problem on alpha, but I still see the
>>> failure on x86_64, even with the unlimited stack.
>>
>> OK, I was able to recreate this by using GNU ld rather than gold.  I
>> have committed the appended patch to reduce the number of recursive
>> template invocations, since you said that 10000 did let the test pass
>> for you, and it works for me using GNU ld.  This number is still high
>> enough to not cut off any reasonable template execution.
>>
>> For this patch bootstrapped and ran Go testsuite on
>> x86_64-pc-linux-gnu, using both GNU ld and gold.  Committed to
>> mainline.
>
> The fixed test now passes reliably on alpha, *but* fails sporadically
> on x86_64-linux-gnu with split-stack (GNU ld).

Good point, I see this too.  I cut the value further, to 1000, and now
the test consistently passes for me.  That still seems adequate for
real word use.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

Attachment: patch.txt
Description: Text document