x/sys/unix: possible missed change in generated file ztypes_solaris_amd64.go

[nshalman]

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

$ go version
go1.19.1 solaris/amd64

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="amd64"
GOBIN=""
GOCACHE="/home/nshalman/.cache/go-build"
GOENV="/home/nshalman/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="solaris"
GOINSECURE=""
GOMODCACHE="/home/nshalman/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="solaris"
GOPATH="/home/nshalman/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/nshalman/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/nshalman/go/pkg/tool/solaris_amd64"
GOVCS=""
GOVERSION="go1.19.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/nshalman/sys/go.mod"
GOWORK=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2608192519=/tmp/go-build -gno-record-gcc-switches"

What did you do?

As of commit f11e5e49a4ec85883999349b0dc6373df034397b I ran the following command in an unmodified checkout:

go tool cgo -godefs types_solaris.go | go run mkpost.go > ztypes_solaris_amd64.go

What did you expect to see?

No change to working tree.

What did you see instead?

I get the follow output from git diff:

diff --git a/unix/ztypes_solaris_amd64.go b/unix/ztypes_solaris_amd64.go
index c1a9b83..f1cb9ee 100644
--- a/unix/ztypes_solaris_amd64.go
+++ b/unix/ztypes_solaris_amd64.go
@@ -195,7 +195,7 @@ type IPv6Mreq struct {
 type Msghdr struct {
        Name         *byte
        Namelen      uint32
-       Iov          *Iovec
+       Iov          *_Ctype_struct_iovec
        Iovlen       int32
        Accrights    *int8
        Accrightslen int32

Is that a change that should be / have been applied to that file?

I was about to touch types_solaris.go for a different change and found this bit confusing.

I think this is related to go #52885 / https://go-review.googlesource.com/c/sys/+/412496

[nshalman]

Experimentation suggests that the autogenerated change is incorrect. I think this is something we would fix in mkpost.go, perhaps like so? :

diff --git a/unix/mkpost.go b/unix/mkpost.go
index 76075c3..5337eca 100644
--- a/unix/mkpost.go
+++ b/unix/mkpost.go
@@ -96,6 +96,11 @@ func main() {
                })
        }

+       if goos == "solaris" {
+               iovecspec := regexp.MustCompile(`_Ctype_struct_iovec`)
+               b = iovecspec.ReplaceAll(b, []byte("Iovec"))
+       }
+
        // Intentionally export __val fields in Fsid and Sigset_t
        valRegex := regexp.MustCompile(`type (Fsid|Sigset_t) struct {(\s+)X__(bits|val)(\s+\S+\s+)}`)
        b = valRegex.ReplaceAll(b, []byte("type $1 struct {${2}Val$4}"))

[nshalman]

I will open a CR with nshalman/sys@7bb33b5

[gopherbot]

Change https://go.dev/cl/437356 mentions this issue: unix: add solaris Iovec cleanup in mkpost.go

[tklauser]

/cc @ianlancetaylor

[tklauser]

I wonder whether instead of defining type goIovec like https://go.dev/cl/412496 did and then modifying its use, we should instead directly modify the Base field of the original type Iovec (generated from struct iovec) in mkpost.go, akin to how we already change []int8 to []byte for several other types, e.g. here:

https://github.com/golang/sys/blob/f11e5e49a4ec85883999349b0dc6373df034397b/unix/mkpost.go#L135-L142

[nshalman]

I wonder whether instead of defining type goIovec like https://go.dev/cl/412496 did and then modifying its use, we should instead directly modify the Base field of the original type Iovec (generated from struct iovec) in mkpost.go, akin to how we already change []int8 to []byte for several other types, e.g. here:

https://github.com/golang/sys/blob/f11e5e49a4ec85883999349b0dc6373df034397b/unix/mkpost.go#L135-L142

I defer to the expertise of others what the right solution to this is. I'd really love independent confirmation that this issue is real and not just something quirky on my system. If my solution is acceptable, the CR is there to be merged, but if a different approach is preferred, I'd love to let someone else fix it so that I can focus my limited spare time on the porting work that this interrupted.

Thank you for your time and attention!!

[ianlancetaylor]

I think that @tklauser is right and that CL 412496 (which I wrote) was wrong. We should try to copy the existing int8 -> byte code in mkpost.go.

[nshalman]

I think that @tklauser is right and that CL 412496 (which I wrote) was wrong. We should try to copy the existing int8 -> byte code in mkpost.go.

I've abandoned https://go.dev/cl/437356 accordingly.