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

cmd/compile: error line number reported is incorrect if it appears after line 0xFFFFF #36850

Open
icza opened this issue Jan 28, 2020 · 3 comments
Open
Milestone

Comments

@icza
Copy link

@icza icza commented Jan 28, 2020

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

$ go version
go version go1.13.7 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/icza/.cache/go-build"
GOENV="/home/icza/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY="xxx"
GONOSUMDB="xxx"
GOOS="linux"
GOPATH="/home/icza/gows"
GOPRIVATE="xxx"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build104015454=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Create a valid .go source file that has more than 1048574 lines (0xFFFFF). And add some "garbage" to the end of it to make it invalid, to make compilation fail. Attempting to build it (go build big.go) or run it (go run big.go) the go tool detects the error and displays an error message where the line number is capped at 1048574 improperly, e.g.:

./big.go:1048574:2: syntax error: unexpected invalid after top level declaration

Here's a simple Go app that creates a big.go file that has approximately twice as many rows (it has a raw string literal that has 2*0xFFFFF empty lines):

package main

import (
	"bytes"
	"io/ioutil"
	"strings"
)

func main() {
	src := &bytes.Buffer{}
	src.WriteString(`package main
func main() { println(len(s)) }
const s=`)
	src.WriteString("`")
	src.WriteString(strings.Repeat("\n", 2*0xFFFFF))
	src.WriteString("`")

	// This is the garbage to make compilation fail:
	src.WriteString("invalid")

	if err := ioutil.WriteFile("big.go", src.Bytes(), 0666); err != nil {
		panic(err)
	}
}

If we decrease the generated number of lines, e.g.:

src.WriteString(strings.Repeat("\n", 10_000))

We get the proper line number (10003):

./big.go:10003:2: syntax error: unexpected invalid after top level declaration

What did you expect to see?

I expected the proper line number to be printed where the error is detected (2*0xFFFFFF+3 = 2097153):

./big.go:2097153:2: syntax error: unexpected invalid after top level declaration

What did you see instead?

The displayed error line number is improperly capped at 1048574:

./big.go:1048574:2: syntax error: unexpected invalid after top level declaration

Note: This issue was reported on StackOverflow: https://stackoverflow.com/questions/59951140/go-compiler-error-line-number-is-incorrect

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 28, 2020

I think it's fine to cap line numbers, but we shouldn't report the wrong line number. We should say that we don't know.

(I don't think it's necessary to force the compiler to use more memory, slowing down compilations for everyone, in order to report better error messages for extremely large files. Such files are normally machine generated and as such either have no errors or many errors.)

@odeke-em odeke-em changed the title Compiler bug: error line number is capped at 0xFFFFF cmd/compile: error line number is capped at 0xFFFFF Jan 29, 2020
@cagedmantis

This comment has been minimized.

Copy link
Contributor

@cagedmantis cagedmantis commented Feb 3, 2020

@icza I'm going to change the issue title to align with the suggestion that @ianlancetaylor mentioned. Thank you for reporting this.

@cagedmantis cagedmantis changed the title cmd/compile: error line number is capped at 0xFFFFF cmd/compile: error line number reported is incorrect if it appears after line 0xFFFFF Feb 3, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Feb 3, 2020
@tpaschalis

This comment has been minimized.

Copy link
Contributor

@tpaschalis tpaschalis commented Feb 5, 2020

Hey, I did a little digging, with the intention of creating a PR if you agree.

The 1048574 == 1 << 20 - 2 limit comes from here

I did the following change

<<<<<<<
lineBits, lineMax     = 20, 1<<lineBits - 2
=======
lineBits, lineMax     = 23, 1<<lineBits - 2
>>>>>>>

and re-compiled the toolchain using ./make.bash.

The result is that with my Go 1.13, the issue is there.
With the above change and the recompiled toolchain, the correct error line is reported.

➜  build-invalid-file ./make.bash
➜  build-invalid-file go version
go version go1.13.7 darwin/amd64
➜  build-invalid-file go run big.go
# command-line-arguments
./big.go:1048574:2: syntax error: unexpected invalid after top level declaration
➜  build-invalid-file /Users/.../..../go/bin/go run big.go
# command-line-arguments
./big.go:2097153:2: syntax error: unexpected invalid after top level declaration

The makeLico function enforces this limit, which is used in MakePos.

The "after top level declaration" message, originates here, (syntaxErrorAt in disguise) with the line reported from p.pos().

This p.pos() method uses posAt, which uses the original MakePos function.


Sorry for the rambling, long writeup, but I think this outlines how and why this error occurs.

I like Ian's suggestion on not changing the cap, but printing another message when the reported line is at this cap.

I feel that this could be a small change on either errorAt or syntaxErrorAt (?), which would be a simple check before returning. If the position is equal to the limit, it would return a different message.

I'll think of a proper suggestion and return to this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.