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: fi missing from LHS of fi, err := fs.FileInfo(~R0), ~R1 #45606

Closed
nimelehin opened this issue Apr 17, 2021 · 9 comments
Closed

cmd/compile: fi missing from LHS of fi, err := fs.FileInfo(~R0), ~R1 #45606

nimelehin opened this issue Apr 17, 2021 · 9 comments
Assignees
Milestone

Comments

@nimelehin
Copy link
Contributor

@nimelehin nimelehin commented Apr 17, 2021

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

$ go version
go version devel go1.17-0613c748e8 Fri Apr 16 14:15:49 2021 +0000 darwin/amd64

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/nikitamelehin/Library/Caches/go-build"
GOENV="/Users/nikitamelehin/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/nikitamelehin/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/nikitamelehin/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/nikitamelehin/Develop/Compiler/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/nikitamelehin/Develop/Compiler/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel go1.17-0613c748e8 Fri Apr 16 14:15:49 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/nikitamelehin/Develop/Compiler/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0m/jnt6wpss7x50v49qw84c29h40000gn/T/go-build2260580950=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Compiled GO with increased inlineMaxBudged=229. This led to a inlining problem of function func isDir(path string) bool in src/cmd/go/internal/load/pkg.go.

func isDir(path string) bool {
	return isDirCache.Do(path, func() interface{} {
		fi, err := fsys.Stat(path)
		return err == nil && fi.IsDir()
	}).(bool)
}

What did you expect to see?

Successful compilation.

What did you see instead?

Got an error: pkg.go:559:5: internal compiler error: fi missing from LHS of fi, err := fs.FileInfo(~R0), ~R1

@nimelehin
Copy link
Contributor Author

@nimelehin nimelehin commented Apr 17, 2021

Is it reasonable to fix that replacing

- if lhs == n {
+ if nlhs, ok := lhs.(*Name); ok && nlhs.Class == n.Class && nlhs.sym == n.sym {
     rhs = defn.Rhs[i]
     break FindRHS
  }

in func staticValue1(nn Node) Node?

Loading

@ALTree ALTree added this to the Backlog milestone Apr 17, 2021
@ALTree
Copy link
Member

@ALTree ALTree commented Apr 17, 2021

Thanks for reporting this. I'm putting this in the backlog milestone since the crash is only triggered when you change a internal compiler constant (but of course it could be a symptom of a real issue in the inliner code).

cc @dr2chase @mdempsky

Loading

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 17, 2021

/cc @danscales for inlining and closures

Loading

@randall77
Copy link
Contributor

@randall77 randall77 commented Apr 18, 2021

This doesn't appear to be a problem with the new export/import stuff, as setting go117ExportTypes to false still reproduces.

Loading

@mdempsky mdempsky self-assigned this Apr 20, 2021
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 21, 2021

This is the smallest reproducer I've been able to create so far:

package p

func x() {
	var f func()

	func() func() {
		return func() {
			g, _ := f, 0
			g()
		}
	}()()
}

It still requires bumping inlineMaxBudget to at least 83 though.

Loading

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 21, 2021

This reproduces with unmodified cmd/compile:

package p

func x() {
	func() func() {
		return func() {
			f := func() {}
			g, _ := f, 0
			g()
		}
	}()()
}

Loading

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Apr 21, 2021

@mdempsky seems that we are currently creating new name node for g without modifying new name Defn, so Defn.Lhs still refer to old name node instead of new one.

Loading

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 21, 2021

@cuonglm That sounds likely. I took a break from this issue for the moment, so if you want to work on a CL, go for it.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 22, 2021

Change https://golang.org/cl/312630 mentions this issue: cmd/compile: copy definition node for inline vars

Loading

@gopherbot gopherbot closed this in d310b2a Apr 23, 2021
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
6 participants