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/packages: panic when NeedImports is used without NeedDeps mode #33077

Closed
fatih opened this issue Jul 12, 2019 · 1 comment
Closed

x/tools/go/packages: panic when NeedImports is used without NeedDeps mode #33077

fatih opened this issue Jul 12, 2019 · 1 comment
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@fatih
Copy link
Member

fatih commented Jul 12, 2019

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

$ go version
go version go1.12.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes. This is from HEAD 9a621aea19f8341c01da59e0d42dd97700f677d0 of the tools repo (https://github.com/golang/tools)

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

go env Output
$ go env
GOARCH="amd64"
GOBIN="/Users/fatih/go/bin"
GOCACHE="/Users/fatih/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/fatih/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/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/k_/87syx3r50m93m72hvqj2qqlw0000gn/T/go-build799516690=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I run the following code:

package main

import (
	"flag"
	"fmt"
	"os"

	"golang.org/x/tools/go/packages"
)

func main() {
	flag.Parse()

	// Many tools pass their command-line arguments (after any flags)
	// uninterpreted to packages.Load so that it can interpret them
	// according to the conventions of the underlying build system.
	cfg := &packages.Config{
		Mode: packages.NeedFiles |
			packages.NeedImports |
			packages.NeedTypes |
			packages.NeedTypesInfo |
			packages.NeedSyntax,
	}
	pkgs, err := packages.Load(cfg, flag.Args()...)
	if err != nil {
		fmt.Fprintf(os.Stderr, "load: %v\n", err)
		os.Exit(1)
	}
	if packages.PrintErrors(pkgs) > 0 {
		os.Exit(1)
	}

	// Print the names of the source files
	// for each package listed on the command line.
	for _, pkg := range pkgs {
		fmt.Println(pkg.ID, pkg.GoFiles)
	}
}

with the following arg:

$ demo.go github.com/fatih/color
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1209c3e]

goroutine 7 [running]:
golang.org/x/tools/go/packages.(*loader).loadRecursive(0xc00010e0b0, 0x0)
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:661 +0x4e
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1(0xc00010e0b0, 0xc00001ca80, 0x0)
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:668 +0x35
created by golang.org/x/tools/go/packages.(*loader).loadRecursive.func1
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:667 +0x13f
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1209c3e]

goroutine 12 [running]:
golang.org/x/tools/go/packages.(*loader).loadRecursive(0xc00010e0b0, 0x0)
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:661 +0x4e
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1(0xc00010e0b0, 0xc00001ca80, 0x0)
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:668 +0x35
created by golang.org/x/tools/go/packages.(*loader).loadRecursive.func1
        /Users/fatih/go/src/golang.org/x/tools/go/packages/packages.go:667 +0x13f
exit status 2

What did you expect to see?

The code should not panic when NeedDeps is not included the config.

What did you see instead?

If NeedDeps is included, it doesn't panic anymore:

cfg := &packages.Config{
	Mode: packages.NeedFiles |
		packages.NeedImports |
		packages.NeedDeps |
		packages.NeedTypes |
		packages.NeedTypesInfo |
		packages.NeedSyntax,
}
$demo.go github.com/fatih/color
github.com/fatih/color [/Users/fatih/go/src/github.com/fatih/color/color.go /Users/fatih/go/src/github.com/fatih/color/doc.go]
@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2019
@fatih fatih changed the title x/tools/go/packages: panic when NeedImports without NeedDeps mode x/tools/go/packages: panic when NeedImports is used without NeedDeps mode Jul 12, 2019
jirfag added a commit to golangci/tools that referenced this issue Jul 16, 2019
@jirfag
go/packages: allow types loading without NeedDeps
0d47e24 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes #golang/go#33077
@jirfag jirfag mentioned this issue Jul 16, 2019
Closed
jirfag added a commit to golangci/tools that referenced this issue Jul 16, 2019
@jirfag
go/packages: allow types loading without NeedDeps
2a85316 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077
@gopherbot
Copy link

Change https://golang.org/cl/186337 mentions this issue: go/packages: allow types loading without NeedDeps

jirfag added a commit to golangci/tools that referenced this issue Sep 9, 2019
@jirfag
go/packages: allow types loading without NeedDeps
3ae24b8 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, golang/go#31930
jirfag added a commit to golangci/tools that referenced this issue Sep 9, 2019
@jirfag
go/packages: allow types loading without NeedDeps
979bdb7 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, fixes golang/go#31930
jirfag added a commit to golangci/tools that referenced this issue Sep 10, 2019
@jirfag
go/packages: allow types loading without NeedDeps
271b1bd 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, fixes golang/go#31930
clintjedwards pushed a commit to clintjedwards/tools that referenced this issue Sep 19, 2019
@jirfag @clintjedwards
go/packages: allow types loading without NeedDeps
bf4e910 
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
          fixes golang/go#31699, fixes golang/go#31930

Change-Id: I416e3c1035d555d67039e45a4fdd1deb9b2184ef
GitHub-Last-Rev: 2e3a46e
GitHub-Pull-Request: golang#139
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186337
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Sep 10, 2020
@rsc rsc unassigned matloob Jun 23, 2022
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
Unreleased
Development

Successfully merging a pull request may close this issue.

go/packages: allow types loading without NeedDeps golangci/tools
3 participants
@fatih @gopherbot @matloob