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

Jump to bottom

x/tools/go/ast/astutil: panic when given multiple single-import lines #17213

Closed
rhysh opened this issue Sep 23, 2016 · 2 comments
Closed

x/tools/go/ast/astutil: panic when given multiple single-import lines #17213

rhysh opened this issue Sep 23, 2016 · 2 comments
Labels
FrozenDueToAge

Comments

@rhysh
Copy link
Contributor

rhysh commented Sep 23, 2016

Please answer these questions before submitting your issue. Thanks!

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

$ go version
go version go1.7.1 darwin/amd64

golang/tools@3f4088e

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

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rhys/work"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/49/zmds5zsn75z1283vtzxyfr5hj7yjq4/T/go-build054967312=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

I added a test case to golang.org/x/tools/go/ast/astutil's imports_test.go for adding an import of "fmt" to the following file:

package main

import "bufio"
import "bytes"
import "errors"

What did you expect to see?

Test case passes with the following result:

package main

import (
    "bufio"
    "bytes"
    "errors"
    "fmt"
)

What did you see instead?

$ go test golang.org/x/tools/go/ast/astutil
--- FAIL: TestAddImport (0.00s)
panic: runtime error: slice bounds out of range [recovered]
    panic: runtime error: slice bounds out of range

goroutine 34 [running]:
panic(0x146740, 0xc42000a100)
    /usr/local/go/src/runtime/panic.go:500 +0x1a1
testing.tRunner.func1(0xc4200a6180)
    /usr/local/go/src/testing/testing.go:579 +0x25d
panic(0x146740, 0xc42000a100)
    /usr/local/go/src/runtime/panic.go:458 +0x243
golang.org/x/tools/go/ast/astutil.AddNamedImport(0xc42007c540, 0xc42012c600, 0x0, 0x0, 0x16d656, 0x3, 0x0)
    /Users/rhys/work/src/golang.org/x/tools/go/ast/astutil/imports.go:167 +0x99c
golang.org/x/tools/go/ast/astutil.TestAddImport(0xc4200a6180)
    /Users/rhys/work/src/golang.org/x/tools/go/ast/astutil/imports_test.go:540 +0x1f5
testing.tRunner(0xc4200a6180, 0x182ee0)
    /usr/local/go/src/testing/testing.go:610 +0x81
created by testing.(*T).Run
    /usr/local/go/src/testing/testing.go:646 +0x2ec
FAIL    golang.org/x/tools/go/ast/astutil   0.012s

I can work around this by adding a boring AST node after the import section (such as var _ int). In that case the output includes the imports I'd expect, though they're oddly not all in a single block import. This may give a hint as to the cause of #17212... Here's a passing test case:

    {
        name: `issue nnnnn several single-import lines`,
        pkg:  "fmt",
        in: `package main

import "bufio"
import "bytes"
import "errors"

var _ int
`,
        out: `package main

import (
    "bufio"
    "bytes"
    "fmt"
)

import "errors"

var _ int
`,
    },

Removing the var _ int line leads to the panic.
@rhysh
Copy link
Contributor Author

rhysh commented Sep 23, 2016

I've got a fix for this: the range loop at the end of AddNamedImport is buggy, since it modifies the slice as it ranges over it. I'm going to delay mailing a CL to see if I can fix #17212 at the same time.

@gopherbot
Copy link

CL https://golang.org/cl/29681 mentions this issue.

@rhysh rhysh mentioned this issue Sep 24, 2016
Closed
@golang golang locked and limited conversation to collaborators Sep 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rhysh @gopherbot