Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/printer, cmd/gofmt: panic on large Go file of length >= 1<<30 #30121

Open
tsenart opened this issue Feb 7, 2019 · 9 comments
Open

go/printer, cmd/gofmt: panic on large Go file of length >= 1<<30 #30121

tsenart opened this issue Feb 7, 2019 · 9 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@tsenart
Copy link

tsenart commented Feb 7, 2019

What version of Go are you using (go version)?

go version go1.11.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/tomas/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tomas/Code/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sp/9jflt4d96wn7bgbq312dswx40000gn/T/go-build135989269=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

cd $(mktemp -d)
curl -LO https://storage.googleapis.com/sourcegraph-public/assets_vfsdata.go.tar.xz
xz -d -T0 -k assets_vfsdata.go.tar.xz
tar -xf assets_vfsdata.go.tar
gofmt -s -w assets_vfsdata.go

What did you expect to see?

Not a panic.

What did you see instead?

A panic.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10e964c]

goroutine 1 [running]:go/printer.(*printer).intersperseComments(0xc141bdf258, 0x7ffeefbff854, 0x11, 0x401750d0, 0x26, 0x401749ec, 0x36, 0x401749ec)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:746 +0x1ccgo/printer.(*printer).flush(0xc141bdf258, 0x7ffeefbff854, 0x11, 0x401750d0, 0x26, 0x401749ec, 0x36, 0x1)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1022 +0xb7go/printer.(*printer).print(0xc141bdf258, 0xc141bde888, 0x2, 0x2)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:982 +0x274go/printer.(*printer).expr1(0xc141bdf258, 0x11738e0, 0xc00000e740, 0x0, 0x1)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:895 +0x1f87go/printer.(*printer).expr(0xc141bdf258, 0x11738e0, 0xc00000e740)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51go/printer.(*printer).expr1(0xc141bdf258, 0x1173e60, 0xc000070840, 0x0, 0x1)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:760 +0x1a20go/printer.(*printer).expr(0xc141bdf258, 0x1173e60, 0xc000070840)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51go/printer.(*printer).printNode(0xc141bdf258, 0x112cf40, 0xc000070840, 0xc0004a43c0, 0x0)        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1128 +0x36b
go/printer.(*Config).fprint(0xc141bdf418, 0x1172200, 0xc00049caf0, 0xc00000e300, 0x112cf40, 0xc000070840, 0xc00048dc20, 0x1253860, 0x112b7e0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1293 +0x170
go/printer.(*printer).nodeSize(0xc141be0a48, 0x1173020, 0xc000070840, 0xf4240, 0xc000070840)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1599 +0x197
go/printer.(*printer).exprList(0xc141be0a48, 0x64c, 0xc00000e700, 0x4, 0x4, 0x1, 0x1, 0x401750d6, 0xc141bdf800)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:212 +0x4d9
go/printer.(*printer).expr1(0xc141be0a48, 0x11739e0, 0xc00000e780, 0x6, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:907 +0x1418
go/printer.(*printer).expr1(0xc141be0a48, 0x1174260, 0xc00000ad20, 0x0, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:790 +0x1880
go/printer.(*printer).expr(0xc141be0a48, 0x1174260, 0xc00000ad20)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51
go/printer.(*printer).expr1(0xc141be0a48, 0x1173e60, 0xc000070870, 0x0, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:760 +0x1a20
go/printer.(*printer).expr(0xc141be0a48, 0x1173e60, 0xc000070870)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51
go/printer.(*printer).printNode(0xc141be0a48, 0x112cf40, 0xc000070870, 0xc0004a4090, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1128 +0x36b
go/printer.(*Config).fprint(0xc141be0c08, 0x1172200, 0xc00049c7e0, 0xc00000e300, 0x112cf40, 0xc000070870, 0xc00048dc20, 0x1253860, 0x112b7e0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1293 +0x170
go/printer.(*printer).nodeSize(0xc141be1690, 0x1173020, 0xc000070870, 0xf4240, 0xc000070870)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1599 +0x197
go/printer.(*printer).exprList(0xc141be1690, 0x117, 0xc00009e200, 0x19, 0x20, 0x1, 0x1, 0x41ea0c87, 0xc000499000)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:212 +0x4d9
go/printer.(*printer).expr1(0xc141be1690, 0x11739e0, 0xc00007e200, 0x0, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:907 +0x1418
go/printer.(*printer).expr(0xc000499690, 0x11739e0, 0xc00007e200)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51
go/printer.(*printer).printNode(0xc000499690, 0x112c640, 0xc00007e200, 0xc0004b1620, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1128 +0x36b
go/printer.(*Config).fprint(0xc141be1850, 0x1172200, 0xc00049c150, 0xc00000e300, 0x112c640, 0xc00007e200, 0xc00048dc20, 0x1253860, 0x112b7e0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1293 +0x170
go/printer.(*printer).nodeSize(0xc141be30c0, 0x1172ae0, 0xc00007e200, 0xf4240, 0xc00007e200)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1599 +0x197
go/printer.(*printer).exprList(0xc141be30c0, 0x10a, 0xc0000507d0, 0x1, 0x1, 0x1, 0x0, 0x0, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:212 +0x4d9
go/printer.(*printer).stmt(0xc00049b0c0, 0x11736e0, 0xc00007e240, 0x100)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1219 +0x17c0
go/printer.(*printer).stmtList(0xc141be30c0, 0xc00007e340, 0x3, 0x4, 0x1, 0xc00049a301)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1027 +0x170
go/printer.(*printer).block(0xc141be30c0, 0xc000080b10, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1050 +0xed
go/printer.(*printer).funcBody(0xc00049b0c0, 0x16, 0x20, 0xc000080b10)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1687 +0x4b4
go/printer.(*printer).expr1(0xc00049b0c0, 0x1173be0, 0xc000050830, 0x7, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:798 +0x11cf
go/printer.(*printer).possibleSelectorExpr(0xc00049b0c0, 0x1173be0, 0xc000050830, 0x7, 0x1, 0xc0000202c8)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:975 +0x9f
go/printer.(*printer).expr1(0xc141be30c0, 0x11738e0, 0xc00007e380, 0x0, 0x1)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:883 +0x2180
go/printer.(*printer).expr(0xc00049b0c0, 0x11738e0, 0xc00007e380)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1001 +0x51
go/printer.(*printer).printNode(0xc00049b0c0, 0x112c3c0, 0xc00007e380, 0xc0004b1590, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1128 +0x36b
go/printer.(*Config).fprint(0xc141be3280, 0x1172200, 0xc00049c0e0, 0xc00000e300, 0x112c3c0, 0xc00007e380, 0xc00048dc20, 0x1253860, 0x112b7e0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1293 +0x170
go/printer.(*printer).nodeSize(0xc141be3ac0, 0x1172960, 0xc00007e380, 0xf4240, 0xc00007e380)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1599 +0x197
go/printer.(*printer).exprList(0xc141be3ac0, 0x0, 0xc000050840, 0x1, 0x1, 0x1, 0x0, 0x0, 0x1151300)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:212 +0x4d9
go/printer.(*printer).spec(0xc141be3ac0, 0x11742a0, 0xc000086140, 0x1, 0x10b2701)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1513 +0x23f
go/printer.(*printer).genDecl(0xc00049bac0, 0xc00007e3c0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1573 +0x79d
go/printer.(*printer).decl(0xc00049bac0, 0x1173c60, 0xc00007e3c0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1719 +0x141
go/printer.(*printer).declList(0xc141be3ac0, 0xc00009e600, 0x1f, 0x20)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1764 +0x12c
go/printer.(*printer).file(0xc141be3ac0, 0xc0004b3100)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/nodes.go:1772 +0x13a
go/printer.(*printer).printNode(0xc00049bac0, 0x11297c0, 0xc0004b3100, 0xc0004b13a0, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1152 +0x59c
go/printer.(*Config).fprint(0xc141be3d80, 0x1172200, 0xc00049c000, 0xc00000e300, 0x11297c0, 0xc0004b3100, 0xc00048dc20, 0xc00000e301, 0xc00049c000)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1293 +0x170
go/printer.(*Config).Fprint(0xc00049bd80, 0x1172200, 0xc00049c000, 0xc00000e300, 0x11297c0, 0xc0004b3100, 0xc0000c5d08, 0x1105e62)
        /usr/local/Cellar/go/1.11.5/libexec/src/go/printer/printer.go:1351 +0x72
main.format(0xc00000e300, 0xc0004b3100, 0x0, 0x0, 0xc0800ec000, 0x41ea2269, 0x7ffffe00, 0x6, 0x8, 0x0, ...)
        /usr/local/Cellar/go/1.11.5/libexec/src/cmd/gofmt/internal.go:105 +0x6ca
main.processFile(0x7ffeefbff854, 0x11, 0x0, 0x0, 0x1172300, 0xc00000c018, 0x0, 0x0, 0x0)
        /usr/local/Cellar/go/1.11.5/libexec/src/cmd/gofmt/gofmt.go:115 +0x22c
main.gofmtMain()
        /usr/local/Cellar/go/1.11.5/libexec/src/cmd/gofmt/gofmt.go:221 +0x185
main.main()
        /usr/local/Cellar/go/1.11.5/libexec/src/cmd/gofmt/gofmt.go:178 +0x22
@mvdan
Copy link
Member

mvdan commented Feb 7, 2019

I can reproduce on go version devel +3fc276ccf8 Mon Feb 4 06:53:49 2019 +0000 linux/amd64. go build succeeds, which confirms that it's only the printer that has issues.

Here is the file after truncating all lines to ~120 bytes, so that we can see what kind of file it is without trying to load 1.1GB: https://play.golang.org/p/i6HApVeq41v

I can't spot an obvious bug or cause, so I guess this is just go/printer breaking when given extremely large amounts of input. In particular, it seems like the original input had extremely long lines:

$ awk '{print length}' assets_vfsdata.go | sort -rn
1075268077
5437233
5430597
4272369
3518345
3517653
[...]

That first length gets close to the maximum value of an int32, so the possible cause here could be an integer overflow somewhere. Though I tested this on 64-bit, so perhaps not.

@tsenart This panic aside, I'd generally suggest to refrain from humongous Go source files, and particularly those that have most of the bytes in a single line. Even if gofmt was fixed, I'm sure other tools could also have issues with these extremely large line lengths. And of course, go build will take a while, and practically no editor will be able to display the file.

If you really do want to include the file as part of the binary, perhaps you could use an assets code generator which set a limit on line lengths, such as 8MiB per line.

@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 7, 2019
@mvdan mvdan added this to the Go1.13 milestone Feb 7, 2019
@mvdan
Copy link
Member

mvdan commented Feb 7, 2019

/cc @griesemer

@tsenart
Copy link
Author

tsenart commented Feb 7, 2019

@tsenart This panic aside, I'd generally suggest to refrain from humongous Go source files, and particularly those that have most of the bytes in a single line. Even if gofmt was fixed, I'm sure other tools could also have issues with these extremely large line lengths. And of course, go build will take a while, and practically no editor will be able to display the file.

Thanks for investigating so quickly and for the feedback. It makes total sense.

If you really do want to include the file as part of the binary, perhaps you could use an assets code generator which set a limit on line lengths, such as 8MiB per line.

Indeed this is an assets file. Do you know of any asset generator project that limits line lengths to 8MiB?

@mvdan
Copy link
Member

mvdan commented Feb 7, 2019

Found the cause, and it is indeed a kind of overflow; the longest line is 1075268077, and go/printer uses infinity = 1 << 30. We can't make that 64-bit, as go/token.Position uses Offset int, so we're limited to 32-bit integers for the sake of 32-bit portability.

I can see a few possible short-term solutions:

  1. Double infinity to math.MaxInt32, which will fix ~1GB files, leaving the breakage for 2GB files and above
  2. Make go/printer error if it encounters a position larger than infinity
  3. Make go/printer avoid panics when dealing with large files, even if that means dropping comments or mis-formatting code

For now, I'll mail 1. I personally believe that 2 should also be done, since go/printer should error if it cannot properly print a well formatted node, but I'll wait for Robert's opinion.

Indeed this is an assets file. Do you know of any asset generator project that limits line lengths to 8MiB?

As it turns out, it's the overall length of the file that causes this panic, and not line length. As long as you have position offsets (from the file's first byte) which are close to 1 << 30, you're going to risk panics. I don't know if Robert has any long-term plans to better support extremely large files, but I wouldn't be surprised if it's not on the roadmap.

@mvdan
Copy link
Member

mvdan commented Feb 7, 2019

  1. Double infinity to math.MaxInt32

Actually, cmd/compile/internal/syntax has const PosMax = 1 << 30 too, so this change would likely not be as trivial as I thought.

@slimsag
Copy link

slimsag commented Feb 8, 2019

(coworker of @tsenart here) FWIW I definitely don't expect most Go tooling (and probably not even the compiler) to support 1GB+ Go source files, that was definitely a mistake / bug on our end.

@odeke-em
Copy link
Member

odeke-em commented Jun 5, 2019

Thanks everyone for the reports, responses and root cause on this bug!

Actually, cmd/compile/internal/syntax has const PosMax = 1 << 30 too, so this change would likely not be as trivial as I thought.

@mvdan, it might be tricky in deed! Perhaps we could negotiate with the compiler team about const PosMax = 1<<30 but aside from that, @tsenart @slimsag and team worked around this issue by fixing the problem on their end with sourcegraph/sourcegraph#2224

I think given that we are this close to the end of the Go1.13 cycle, but also as I believe I've seen the issue about very large source files causing problems, perhaps we should document limits on parsing source files. Maybe later we can revisit using math.MaxInt32? @griesemer @mvdan what do y'all think?

@odeke-em odeke-em changed the title cmd/gofmt: Panic on large asset file go/printer, cmd/gofmt: panic on large Go file of length >= 1<<30 Jun 5, 2019
@mvdan
Copy link
Member

mvdan commented Jun 5, 2019

I think the simplest course of action would be for go/printer, or any other package that could panic on extremely large lines/files, to simply error with a helpful message.

If the issue comes up again in the future, we can always consider better ways of dealing with large files, like doubling the limit or using 64-bit integers.

@griesemer
Copy link
Contributor

I'm not surprised to see problems with 1GB files - it goes to show that no matter how relaxed a restriction is, at some point somebody will run into the restriction.

I think for now the best course of action might be to error for files over a certain size, per @mvdan's suggestion. This may have to happen when the token.File is created for a file (to be investigated). But this doesn't seem urgent, so it can wait for Go 1.14.

@griesemer griesemer modified the milestones: Go1.13, Go1.14 Jun 5, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants