Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(700)

Issue 6847078: code review 6847078: cmd/gc: do not overflow parser stack on a long chain of... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by remyoudompheng
Modified:
9 years, 6 months ago
Reviewers:
CC:
dfc, minux1, iant, DMorsing, rsc, golang-dev
Visibility:
Public.

Description

cmd/gc: do not overflow parser stack on a long chain of else if. Fixes issue 2615.

Patch Set 1 #

Patch Set 2 : diff -r 1315abc581ed https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 1315abc581ed https://go.googlecode.com/hg/ #

Total comments: 5

Patch Set 4 : diff -r 1315abc581ed https://go.googlecode.com/hg/ #

Total comments: 7

Patch Set 5 : diff -r 1f3ebf9a7548 https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 6 : diff -r bf0ac83d9ab8 https://go.googlecode.com/hg/ #

Patch Set 7 : diff -r d1f2902e0f0d https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2634 lines, -1694 lines) Patch
M src/cmd/gc/go.y View 1 2 3 4 3 chunks +43 lines, -12 lines 0 comments Download
M src/cmd/gc/y.tab.h View 1 3 chunks +46 lines, -22 lines 0 comments Download
M src/cmd/gc/y.tab.c View 1 2 3 4 167 chunks +1997 lines, -1659 lines 0 comments Download
M src/cmd/gc/yerr.h View 1 1 chunk +1 line, -1 line 0 comments Download
A test/fixedbugs/issue2615.go View 1 2 3 4 5 1 chunk +547 lines, -0 lines 0 comments Download

Messages

Total messages: 16
remyoudompheng
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
9 years, 7 months ago (2012-11-21 07:28:00 UTC) #1
dfc
On 2012/11/21 07:28:00, remyoudompheng wrote: > Hello mailto:golang-dev@googlegroups.com (cc: mailto:golang-dev@googlegroups.com), > > I'd like you ...
9 years, 7 months ago (2012-11-21 07:32:13 UTC) #2
minux1
On 2012/11/21 07:32:13, dfc wrote: > Thank you for fixing this longstanding issue but I ...
9 years, 7 months ago (2012-11-21 07:35:40 UTC) #3
iant
I agree that this patch is OK with respect to licensing. The file is already ...
9 years, 7 months ago (2012-11-21 14:28:47 UTC) #4
dfc
Thank you Ian, I withdraw my previous objection. On 22 Nov 2012 01:28, <iant@golang.org> wrote: ...
9 years, 7 months ago (2012-11-21 21:03:43 UTC) #5
remyoudompheng
What about the patch contents?
9 years, 7 months ago (2012-11-21 21:05:58 UTC) #6
DMorsing
https://codereview.appspot.com/6847078/diff/4006/src/cmd/gc/go.y File src/cmd/gc/go.y (right): https://codereview.appspot.com/6847078/diff/4006/src/cmd/gc/go.y#newcode680 src/cmd/gc/go.y:680: else: Reorder the rules so that elseblock_list is first, ...
9 years, 7 months ago (2012-11-22 17:26:21 UTC) #7
remyoudompheng
http://codereview.appspot.com/6847078/diff/4006/src/cmd/gc/go.y File src/cmd/gc/go.y (right): http://codereview.appspot.com/6847078/diff/4006/src/cmd/gc/go.y#newcode680 src/cmd/gc/go.y:680: else: A few lines above we have caseblock and ...
9 years, 7 months ago (2012-11-23 20:44:04 UTC) #8
remyoudompheng
Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com, iant@golang.org, daniel.morsing@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-11-23 21:08:52 UTC) #9
DMorsing
LGTM https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y File src/cmd/gc/go.y (right): https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y#newcode693 src/cmd/gc/go.y:693: elseifblock_list: Name is getting a bit verbose. Shorten ...
9 years, 7 months ago (2012-11-23 22:27:40 UTC) #10
rsc
Thanks for working on this. https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y File src/cmd/gc/go.y (right): https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y#newcode693 src/cmd/gc/go.y:693: elseifblock_list: On 2012/11/23 22:27:40, ...
9 years, 7 months ago (2012-11-26 16:44:22 UTC) #11
remyoudompheng
Hello dave@cheney.net, minux.ma@gmail.com, iant@golang.org, daniel.morsing@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-12-03 13:17:55 UTC) #12
remyoudompheng
https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y File src/cmd/gc/go.y (right): https://codereview.appspot.com/6847078/diff/8002/src/cmd/gc/go.y#newcode693 src/cmd/gc/go.y:693: elseifblock_list: On 2012/11/26 16:44:23, rsc wrote: > On 2012/11/23 ...
9 years, 7 months ago (2012-12-03 13:18:43 UTC) #13
rsc
LGTM w/ the test change passing https://codereview.appspot.com/6847078/diff/14001/test/fixedbugs/issue2615.go File test/fixedbugs/issue2615.go (right): https://codereview.appspot.com/6847078/diff/14001/test/fixedbugs/issue2615.go#newcode534 test/fixedbugs/issue2615.go:534: bits[i/64] &= ^(1 ...
9 years, 6 months ago (2012-12-06 04:50:18 UTC) #14
remyoudompheng
Hello dave@cheney.net, minux.ma@gmail.com, iant@golang.org, daniel.morsing@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 6 months ago (2012-12-06 07:08:04 UTC) #15
remyoudompheng
9 years, 6 months ago (2012-12-06 07:09:24 UTC) #16
*** Submitted as https://code.google.com/p/go/source/detail?r=52c9c412f1f2 ***

cmd/gc: do not overflow parser stack on a long chain of else if.

Fixes issue 2615.

R=dave, minux.ma, iant, daniel.morsing, rsc
CC=golang-dev
https://codereview.appspot.com/6847078
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b