On Fri, Aug 5, 2016 at 1:24 AM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Thu, Aug 4, 2016 at 6:48 PM, Ian Lance Taylor <i...@golang.org> wrote: >> On Thu, Aug 4, 2016 at 1:11 AM, Uros Bizjak <ubiz...@gmail.com> wrote: >>> On Thu, Aug 4, 2016 at 12:53 AM, Ian Lance Taylor <i...@golang.org> wrote: >>>> On Thu, Jul 28, 2016 at 4:24 AM, Uros Bizjak <ubiz...@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
Index: gcc/go/gofrontend/MERGE =================================================================== --- gcc/go/gofrontend/MERGE (revision 239258) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -672db63f342c99bdc7ed46f040038440f429e600 +3b9c57a35370f26e6cf5839084e367e75e45ec97 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/text/template/exec.go =================================================================== --- libgo/go/text/template/exec.go (revision 239141) +++ libgo/go/text/template/exec.go (working copy) @@ -19,9 +19,9 @@ import ( // 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. -// For gccgo we make this 10000 rather than 100000 to avoid stack overflow +// For gccgo we make this 1000 rather than 100000 to avoid stack overflow // on non-split-stack systems. -const maxExecDepth = 10000 +const maxExecDepth = 1000 // state represents the state of an execution. It's not part of the // template so that multiple executions of the same template