unique: panic for Make(struct{}{})

haruyama480 commented 1 month ago

Go version

go version go1.23.1 darwin/arm64

Output of `go env` in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xxx/Library/Caches/go-build'
GOENV='/Users/xxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xxx/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/xxx/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/xxx/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.1.darwin-arm64'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='go1.23.1'
GOTOOLDIR='/Users/xxx/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.1.darwin-arm64/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/xxx/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/xxx/ghq/github.com/xxx/go-learning/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/7c/vn1g1x4n4nqclvb5dnjkf8q80000gn/T/go-build85467120=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

https://go.dev/play/p/esaCHP90Kzk

output

``` fatal error: getWeakHandle on invalid pointer goroutine 1 gp=0xc0000061c0 m=0 mp=0x5003e0 [running]: runtime.throw({0x4857e3?, 0x500301?}) /usr/local/go-faketime/src/runtime/panic.go:1067 +0x48 fp=0xc0000485c0 sp=0xc000048590 pc=0x45d948 runtime.getWeakHandle(0x51eec0) /usr/local/go-faketime/src/runtime/mheap.go:2135 +0x113 fp=0xc000048608 sp=0xc0000485c0 pc=0x422f93 runtime.getOrAddWeakHandle(0x51eec0) /usr/local/go-faketime/src/runtime/mheap.go:2083 +0x1c fp=0xc000048650 sp=0xc000048608 pc=0x422cbc internal/weak.runtime_registerWeakPointer(0x0?) /usr/local/go-faketime/src/runtime/mheap.go:2045 +0x13 fp=0xc000048668 sp=0xc000048650 pc=0x45d233 internal/weak.Make[...](0x51eec0?) /usr/local/go-faketime/src/internal/weak/pointer.go:62 +0x5d fp=0xc000048680 sp=0xc000048668 pc=0x46a63d unique.Make[...].func1() /usr/local/go-faketime/src/unique/handle.go:57 +0x88 fp=0xc0000486b8 sp=0xc000048680 pc=0x46a588 unique.Make[...]({}) /usr/local/go-faketime/src/unique/handle.go:67 +0x135 fp=0xc000048738 sp=0xc0000486b8 pc=0x46a475 main.main() /tmp/sandbox2127609540/prog.go:8 +0x1a fp=0xc000048750 sp=0xc000048738 pc=0x46941a runtime.main() /usr/local/go-faketime/src/runtime/proc.go:272 +0x28b fp=0xc0000487e0 sp=0xc000048750 pc=0x4315eb runtime.goexit({}) /usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000487e8 sp=0xc0000487e0 pc=0x463ea1 goroutine 2 gp=0xc000006700 m=nil [force gc (idle)]: runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?) /usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000048fa8 sp=0xc000048f88 pc=0x45da2e runtime.goparkunlock(...) /usr/local/go-faketime/src/runtime/proc.go:430 runtime.forcegchelper() /usr/local/go-faketime/src/runtime/proc.go:337 +0xa5 fp=0xc000048fe0 sp=0xc000048fa8 pc=0x431925 runtime.goexit({}) /usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000048fe8 sp=0xc000048fe0 pc=0x463ea1 created by runtime.init.7 in goroutine 1 /usr/local/go-faketime/src/runtime/proc.go:325 +0x1a goroutine 3 gp=0xc000006c40 m=nil [GC sweep wait]: runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?) /usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000049780 sp=0xc000049760 pc=0x45da2e runtime.goparkunlock(...) /usr/local/go-faketime/src/runtime/proc.go:430 runtime.bgsweep(0xc00006a000) /usr/local/go-faketime/src/runtime/mgcsweep.go:277 +0x94 fp=0xc0000497c8 sp=0xc000049780 pc=0x41d814 runtime.gcenable.gowrap1() /usr/local/go-faketime/src/runtime/mgc.go:203 +0x25 fp=0xc0000497e0 sp=0xc0000497c8 pc=0x412285 runtime.goexit({}) /usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000497e8 sp=0xc0000497e0 pc=0x463ea1 created by runtime.gcenable in goroutine 1 /usr/local/go-faketime/src/runtime/mgc.go:203 +0x66 goroutine 4 gp=0xc000006e00 m=nil [GC scavenge wait]: runtime.gopark(0xc00006a000?, 0x49eab8?, 0x1?, 0x0?, 0xc000006e00?) /usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000049f78 sp=0xc000049f58 pc=0x45da2e runtime.goparkunlock(...) /usr/local/go-faketime/src/runtime/proc.go:430 runtime.(*scavengerState).park(0x4ff640) /usr/local/go-faketime/src/runtime/mgcscavenge.go:425 +0x49 fp=0xc000049fa8 sp=0xc000049f78 pc=0x41b289 runtime.bgscavenge(0xc00006a000) /usr/local/go-faketime/src/runtime/mgcscavenge.go:653 +0x3c fp=0xc000049fc8 sp=0xc000049fa8 pc=0x41b7bc runtime.gcenable.gowrap2() /usr/local/go-faketime/src/runtime/mgc.go:204 +0x25 fp=0xc000049fe0 sp=0xc000049fc8 pc=0x412225 runtime.goexit({}) /usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000049fe8 sp=0xc000049fe0 pc=0x463ea1 created by runtime.gcenable in goroutine 1 /usr/local/go-faketime/src/runtime/mgc.go:204 +0xa5 goroutine 17 gp=0xc000084380 m=nil [chan receive]: runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?) /usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000044718 sp=0xc0000446f8 pc=0x45da2e runtime.chanrecv(0xc000098070, 0x0, 0x1) /usr/local/go-faketime/src/runtime/chan.go:639 +0x41c fp=0xc000044790 sp=0xc000044718 pc=0x40587c runtime.chanrecv1(0x0?, 0x0?) /usr/local/go-faketime/src/runtime/chan.go:489 +0x12 fp=0xc0000447b8 sp=0xc000044790 pc=0x405452 runtime.unique_runtime_registerUniqueMapCleanup.func1(...) /usr/local/go-faketime/src/runtime/mgc.go:1732 runtime.unique_runtime_registerUniqueMapCleanup.gowrap1() /usr/local/go-faketime/src/runtime/mgc.go:1735 +0x2f fp=0xc0000447e0 sp=0xc0000447b8 pc=0x414fef runtime.goexit({}) /usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000447e8 sp=0xc0000447e0 pc=0x463ea1 created by unique.runtime_registerUniqueMapCleanup in goroutine 1 /usr/local/go-faketime/src/runtime/mgc.go:1730 +0x96 Program exited. ```

What did you see happen?

fatal error: getWeakHandle on invalid pointer

What did you expect to see?

no panic

gabyhelp commented 1 month ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

haruyama480 commented 1 month ago

struct{} has no value, so there may be no way to implement unique.Make(sturct{}{}). However, a runtime panic may be somewhat surprising.

ianlancetaylor commented 1 month ago

CC @mknyszek

cuonglm commented 1 month ago

The address of *T where T is zero-size type will be the address of runtime.zerobase, which does not belong to heap address, thus the runtime panic.

It seems to me that we need a special treatment for zerobase.

haruyama480 commented 1 month ago

It seems to me that we need a special treatment for zerobase. yes.

I think a traditional approach is treat zerobase as a null pointer in Handle[T], though I don't know Go can treat a null pointer. To apply this handling for all types, we can treat zero value too, to omit insertion of default value.

mknyszek commented 1 month ago

I think we do need to handle zero-sized types specially, but I don't think the unique package needs to be aware of runtime.zerobase.

In theory we can have all zero-sized types use the same pointer under the hood, because you can't compare two Handle[T] where T differs anyway, and all zero-sized values of the same type compare equal with each other (really, there's only one value for each type).

package unique

var zero uintptr

func Make[T comparable](value T) Handle[T] {
    if unsafe.Sizeof(*new(T)) == 0 {
        return Handle[T]{(*T)(unsafe.Pointer(&zero))}
    }
    // Rest of Make.
    ...
}

func (h Handle[T]) Value() T {
    if unsafe.Sizeof(*new(T)) == 0 {
        return *new(T)
    }
    return *h.value
}

I don't really see the use-case for this, so it doesn't seem urgent, but I can send a patch for this.

mknyszek commented 1 month ago

Actually @haruyama480 is right, we don't need var zero uintptr and &zero, we can just say nil. That works too.

gopherbot commented 1 month ago

Change https://go.dev/cl/613397 mentions this issue: unique: handle zero-size types

golang / go