reflect: ArenaNew panic non-pointer type #60528

578559967 · 2023-05-31T02:02:26Z

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

$ go version
go version go1.20.4 darwin/arm64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/r/Library/Caches/go-build"
GOENV="/Users/r/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOMODCACHE="/Users/r/go/pkg/mod"
GOOS="darwin"
GOPATH="/Users/r/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/r/sdk/go1.20.4"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/r/sdk/go1.20.4/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.4"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fdebug-prefix-map=/var/folders/j_/rq9ph2cd3h50w468sgv34lwh0000gn/T/go-build3924141508=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.20.4 darwin/arm64
GOROOT/bin/go tool compile -V: compile version go1.20.4
uname -v: Darwin Kernel Version 22.4.0: Mon Mar  6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000
ProductName:		macOS
ProductVersion:		13.3.1
ProductVersionExtra:	(a)
BuildVersion:		22E772610a
lldb --version: lldb-1403.0.17.64
Apple Swift version 5.8 (swiftlang-5.8.0.124.2 clang-1403.0.22.11.100)

What did you do?

test.go:

package main

import (
	"arena"
	"fmt"
	"reflect"
)

func main() {
	a := arena.NewArena()
	i := 0
	v := reflect.ArenaNew(a, reflect.TypeOf(i))
	fmt.Println(v.Type(), *v.Interface().(*int))
	a.Free()
}

GOEXPERIMENT=arenas go1.20.4 run -a test.go

What did you expect to see?

print

*int 0

What did you see instead?

panic:

fatal error: arena_New: non-pointer type

goroutine 1 [running]:
runtime.throw({0x1025c532b?, 0x10254070c?})
        /Users/r/sdk/go1.20.4/src/runtime/panic.go:1047 +0x40 fp=0x140000a4e20 sp=0x140000a4df0 pc=0x102564dd0
arena.runtime_arena_arena_New(0x129cc1408?, {0x1025fa760, 0x1025eb8a0})
        /Users/r/sdk/go1.20.4/src/runtime/arena.go:113 +0x78 fp=0x140000a4e60 sp=0x140000a4e20 pc=0x10258d318
reflect.arena_New(0x140000a4ec8?, {0x1025fa760?, 0x1025eb8a0?})
        /Users/r/sdk/go1.20.4/src/arena/arena.go:89 +0x28 fp=0x140000a4e90 sp=0x140000a4e60 pc=0x1025952d8
reflect.ArenaNew(0x1025f06e0?, {0x1025ff038?, 0x1025eb8a0?})
        /Users/r/sdk/go1.20.4/src/reflect/arena.go:15 +0x2c fp=0x140000a4ed0 sp=0x140000a4e90 pc=0x1025a39fc
main.main()
        /Users/r/workspace/testcode/testarena/test.go:12 +0xa0 fp=0x140000a4f70 sp=0x140000a4ed0 pc=0x1025c1050
runtime.main()
        /Users/r/sdk/go1.20.4/src/runtime/proc.go:250 +0x248 fp=0x140000a4fd0 sp=0x140000a4f70 pc=0x1025673f8
runtime.goexit()
        /Users/r/sdk/go1.20.4/src/runtime/asm_arm64.s:1172 +0x4 fp=0x140000a4fd0 sp=0x140000a4fd0 pc=0x102592364

If I change the code to v := reflect.ArenaNew(a, reflect.TypeOf(&i)), it will pass.

But it conflicts with the comment of reflect.ArenaNew:

// ArenaNew returns a Value representing a pointer to a new zero value for the
// specified type, allocating storage for it in the provided arena. That is,
// the returned Value's Type is PointerTo(typ).

The text was updated successfully, but these errors were encountered:

mknyszek · 2023-05-31T15:12:13Z

Thanks. I think the right thing to do in this circumstance is to fix the function itself, since the GOEXPERIMENT is not backed by the compatibility guarantee. The simple thing would be to just call PointerTo on the type before passing it down in src/arena/reflect.go.

ianlancetaylor · 2023-05-31T18:12:25Z

@mknyszek I don't agree. While this code is not covered by the compatibility guarantee, there's no particular reason to change it. I think we should just change the documentation to match the actual behavior.

mknyszek · 2023-05-31T18:33:20Z

@ianlancetaylor Yeah, sure. That's a good point and fine with me. I don't feel strongly either way.

mknyszek · 2023-05-31T19:53:03Z

I also wanted to clarify: the reason I wanted to change the implementation is that this is reflect function is supposed to be a reflection of (ha) arena.New, which takes a type argument T and returns a *T value, and so it's unfortunate that that doesn't really line up here in the implementation (the documentation is correct).

However, I still agree that not breaking people unnecessarily (even in a GOEXPERIMENT) makes more sense.

578559967 · 2023-06-01T02:35:26Z

I agree with @mknyszek . This reflect function should behave like arena.New. It should be fixed as early as possible before moving from experiment to standard api. Or it will last forever and confuses people continuously. Don't let this inconsistency become a permanent scar. It's better to endure short-term pain than prolonged suffering.

ianlancetaylor · 2023-06-01T03:35:40Z

I don't think the argument that arena.New and reflect.ArenaNew should behave the same is very strong. They are very different functions.

That said, I admit that I found only one call to reflect.ArenaNew (https://github.com/huandu/go-clone/blob/fcc492e9d68d20fe88146b17e8e45b1c9d9e6b8e/generic/arena.go#L62) so maybe it's OK to change.

See also #56234.

578559967 · 2023-06-01T04:20:21Z

https://github.com/huandu/go-clone/blob/fcc492e9d68d20fe88146b17e8e45b1c9d9e6b8e/generic/arena.go#L62
This piece of code appears to be fixing the issue precisely.

reflect.New also returns the Value with the Type of PointerTo(typ). It's better to keep the save behavior.

// New returns a Value representing a pointer to a new zero value
// for the specified type. That is, the returned Value's Type is PointerTo(typ).
func New(typ Type) Value

randall77 · 2023-06-01T05:38:35Z

I think that that this is worth fixing even if it breaks existing uses.

gopherbot · 2023-06-08T18:45:36Z

Change https://go.dev/cl/501860 mentions this issue: reflect: fix ArenaNew to match documentation

gopherbot added the compiler/runtime label May 31, 2023

mknyszek added this to the Backlog milestone May 31, 2023

mknyszek added the NeedsFix label May 31, 2023

mknyszek self-assigned this May 31, 2023

mknyszek added the Documentation label May 31, 2023

mknyszek added this to Go Compiler / Runtime May 31, 2023

mknyszek moved this to Todo in Go Compiler / Runtime May 31, 2023

mknyszek removed the Documentation label Jun 2, 2023

gopherbot closed this as completed in 2b0ff4b Jun 16, 2023

github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Jun 16, 2023

dmitshur modified the milestones: Backlog, Go1.21 Jun 17, 2023

mknyszek removed this from Go Compiler / Runtime Oct 25, 2023

golang locked and limited conversation to collaborators Jun 16, 2024

gopherbot added the FrozenDueToAge label Jun 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reflect: ArenaNew panic non-pointer type #60528

reflect: ArenaNew panic non-pointer type #60528

578559967 commented May 31, 2023

mknyszek commented May 31, 2023

ianlancetaylor commented May 31, 2023

mknyszek commented May 31, 2023 •

edited

Loading

mknyszek commented May 31, 2023

578559967 commented Jun 1, 2023

ianlancetaylor commented Jun 1, 2023

578559967 commented Jun 1, 2023

randall77 commented Jun 1, 2023

gopherbot commented Jun 8, 2023

reflect: ArenaNew panic non-pointer type #60528

reflect: ArenaNew panic non-pointer type #60528

Comments

578559967 commented May 31, 2023

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

mknyszek commented May 31, 2023

ianlancetaylor commented May 31, 2023

mknyszek commented May 31, 2023 • edited Loading

mknyszek commented May 31, 2023

578559967 commented Jun 1, 2023

ianlancetaylor commented Jun 1, 2023

578559967 commented Jun 1, 2023

randall77 commented Jun 1, 2023

gopherbot commented Jun 8, 2023

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

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

mknyszek commented May 31, 2023 •

edited

Loading