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

reflect: call of reflect.Value.Type on zero Value #70156

Closed
mauri870 opened this issue Nov 1, 2024 · 7 comments
Closed

reflect: call of reflect.Value.Type on zero Value #70156

mauri870 opened this issue Nov 1, 2024 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.

Comments

@mauri870
Copy link
Member

mauri870 commented Nov 1, 2024

Go version

gotip

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/mauri870/gopath/bin'
GOCACHE='/home/mauri870/.cache/go-build'
GOENV='/home/mauri870/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mauri870/gopath/pkg/mod'
GOOS='linux'
GOPATH='/home/mauri870/gopath'
GOPROXY='https://proxy.golang.org'
GOROOT='/home/mauri870/git/go'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/mauri870/git/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-3b96eebcbd Wed Aug 7 16:07:33 2024 +0000'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/mauri870/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/mauri870/git/elastic/go-structform/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3704686524=/tmp/go-build -gno-record-gcc-switches'

What did you do?

$ git clone https://github.com/elastic/go-structform.git
$ cd go-structform
$ gotip test -run ^TestFoldUnfoldConsistent$ ./gotype

What did you see happen?

The test fails with gotip but succeeds with go1.23.2

--- FAIL: TestFoldUnfoldConsistent (0.04s)
    --- FAIL: TestFoldUnfoldConsistent/0:_null_(<nil>_->_*interface_{}) (0.00s)
panic: reflect: call of reflect.Value.Type on zero Value [recovered]
        panic: reflect: call of reflect.Value.Type on zero Value

goroutine 90 [running]:
testing.tRunner.func1.2({0x6ad800, 0xc0000102e8})
        /home/mauri870/git/go/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
        /home/mauri870/git/go/src/testing/testing.go:1635 +0x35e
panic({0x6ad800?, 0xc0000102e8?})
        /home/mauri870/git/go/src/runtime/panic.go:785 +0x132
reflect.Value.typeSlow({0x0?, 0x0?, 0x9c82a0?})
        /home/mauri870/git/go/src/reflect/value.go:2376 +0x10e
reflect.Value.Type(...)
        /home/mauri870/git/go/src/reflect/value.go:2371
github.com/elastic/go-structform/gotype.foldAnyReflect(0xc0000d48d0, {0x0?, 0x0?, 0xc000028f70?})
        /home/mauri870/git/elastic/go-structform/gotype/fold_reflect.go:565 +0xe8
github.com/elastic/go-structform/gotype.foldInterfaceElem(0xc0000d48d0, {0x6ab620?, 0xc000028f70?, 0xc0000102d0?})
        /home/mauri870/git/elastic/go-structform/gotype/fold_reflect.go:140 +0x58
github.com/elastic/go-structform/gotype.getFoldPointer.makePointerFold.func1(0xc0000d48d0, {0x696b20?, 0xc000028f70?, 0x696b20?})
        /home/mauri870/git/elastic/go-structform/gotype/fold_reflect.go:124 +0xb1
github.com/elastic/go-structform/gotype.foldAnyReflect(0xc0000d48d0, {0x696b20?, 0xc000028f70?, 0x1006b87c0?})
        /home/mauri870/git/elastic/go-structform/gotype/fold_reflect.go:569 +0x130
github.com/elastic/go-structform/gotype.foldInterfaceValue(0xc0000d48d0, {0x696b20, 0xc000028f70})
        /home/mauri870/git/elastic/go-structform/gotype/fold.go:117 +0x230
github.com/elastic/go-structform/gotype.(*Iterator).Fold(...)
        /home/mauri870/git/elastic/go-structform/gotype/fold.go:94
github.com/elastic/go-structform/gotype.Fold({0x696b20, 0xc000028f70}, {0x78c8c0?, 0xc0000e6210?}, {0x0?, 0x2c?, 0x605?})
        /home/mauri870/git/elastic/go-structform/gotype/fold.go:58 +0x65
github.com/elastic/go-structform/gotype.TestFoldUnfoldConsistent.func1(0xc0000da340)
        /home/mauri870/git/elastic/go-structform/gotype/unfold_test.go:81 +0x269
testing.tRunner(0xc0000da340, 0xc0000bf280)
        /home/mauri870/git/go/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 7
        /home/mauri870/git/go/src/testing/testing.go:1743 +0x390
FAIL    github.com/elastic/go-structform/gotype 0.046s
FAIL

What did you expect to see?

I expect the test to pass as it does with older Go versions. This package uses some reflection trickery, so I’m not sure if we were relying on incorrect reflection usage.

A bisect suggest the offending commit to be CL 599096.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 1, 2024
@mauri870
Copy link
Member Author

mauri870 commented Nov 1, 2024

cc @golang/compiler (because of the bisect result, sorry if it turns out unrelated)

@ianlancetaylor
Copy link
Member

Definitely seems to be a miscompilation. Here is a small reproducer. This program should not panic, but it does at HEAD.

package main

import (
	"reflect"
)

func main() {
	pi := new(interface{})
	v := reflect.ValueOf(pi).Elem()
	if v.Kind() != reflect.Interface {
		panic(0)
	}
	if (v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface) && v.IsNil() {
		return
	}
	panic(1)
}

@qiulaidongfeng
Copy link
Member

Definitely seems to be a miscompilation. Here is a small reproducer. This program should not panic, but it does at HEAD.

package main

import (
	"reflect"
)

func main() {
	pi := new(interface{})
	v := reflect.ValueOf(pi).Elem()
	if v.Kind() != reflect.Interface {
		panic(0)
	}
	if (v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface) && v.IsNil() {
		return
	}
	panic(1)
}

use go run -gcflags="-N" . don`t panic
use go run . happen panic

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/624695 mentions this issue: cmd/compile: init limit for newly created value in prove pass

@fengyoulin
Copy link
Contributor

I think, since the constant parts of the prove pass was rewritten in CL 599096, the limits for the newly created values need to be initialized before use.

I have a fix, PTAL. @randall77

Change https://go.dev/cl/624695 mentions this issue: cmd/compile: init limit for newly created value in prove pass

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/624855 mentions this issue: cmd/compile: update comment for initLimit in prove pass

gopherbot pushed a commit that referenced this issue Nov 9, 2024
For: #70156

Change-Id: Ie39a88130f27b4b210ddbcf396cc0ddd2713d58b
Reviewed-on: https://go-review.googlesource.com/c/go/+/624855
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: David Chase <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Mauri de Souza Meneguzzo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

6 participants