cmd/yacc: generated code panics with index out of range.

[gopherbot]

by [email protected]:

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. Download vitess: go get github.com/youtube/vitess/go/vt/sqlparser, I am using commit
2fe039e2e6385d154a36bf66397e70568620d668
2. Create a file named github.com/youtube/vitess/go/vt/sqlparser/nested_test.go with the
contents of http://play.golang.org/p/xMDjv9I95E
3. go test

This should panic in the sql.go which is generated by yacc. You can do 

1. rm sql.go
2. go tool yacc sql.y
3. go test

It will produce the same panic.

What is the expected output?

Nothing

What do you see instead?

panic index out of range in yyParse

Which version are you using?  (run 'go version')

go version devel +f9e8a970798c Wed Jan 22 16:39:39 2014 -0500 linux/amd64

Please provide any additional information below.

I also reported this bug at vitessio/vitess#33

[minux]

Comment 1:

This is not a bug per se, it's similar to a stack overflow with unbounded recursion.
yacc has a limit on the stack size it uses.
this panic is simply because the input is nested too deep.
increase the variable stacksize in cmd/yacc/yacc.go is the only way if you want to
solve this (or rewrite your grammar).
we have several alternatives, for example:
1. allocate a larger stack when it's about to overflow.
2. make the stacksize configurable.
3. return syntax error when the stack overflows.

Labels changed: added release-none, repo-main.

[minux]

Comment 2:

Owner changed to @robpike.

[robpike]

Comment 3:

This is a known issue with this kind of grammar. You want to recur the right way so the
grammar reduces rather than pushes state onto the stack.
Minux's suggestions are the way to fix this.

Status changed to WontFix.

[sougou]

Comment 4:

I posted similar comments for the vitess bug.
We anyway don't want to allow this much nesting in a SQL statement. I'll just catch the
panic and return a syntax error to prevent the crash.

[robpike]

Comment 5:

Instead of catching the panic, you could count the recursion depth.

[sougou]

Comment 6:

Counting the recursion depth would only catch this use case. OTOH, catching the panic
could leave the parser in an inconsistent state.
Is there a better way to count all recursion depths? All I can think of is to use a
common variable that I can inc/dec for every rule fired.

[robpike]

Comment 7:

I don't know of one. The right fix is to design the grammar so the recursion is the
right way around.