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.

Sign up for GitHub

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

Jump to bottom

cmd/cgo: go tool cgo -godefs wrongly strips prefix from struct field name #48396

Closed
tklauser opened this issue Sep 15, 2021 · 7 comments
Closed

cmd/cgo: go tool cgo -godefs wrongly strips prefix from struct field name #48396

tklauser opened this issue Sep 15, 2021 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@tklauser
Copy link
Member

tklauser commented Sep 15, 2021 •
edited
Loading

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

$ go version
go version go1.17.1 linux/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/tklauser/.cache/go-build"
GOENV="/home/tklauser/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/tklauser/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/tklauser/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build3880404132=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run go tool cgo -godefs kcm.go given the following content in kcm.go:

package unix

/*
struct kcm_attach {
        int fd;
        int bpf_fd;
};
*/
import "C"

type KCMAttach C.struct_kcm_attach

What did you expect to see?

// Code generated by cmd/cgo -godefs; DO NOT EDIT.
// cgo -godefs /home/tklauser/go/src/kcm.go

package unix

type KCMAttach struct {
	Fd	int32
	Bpf_fd	int32
}

What did you see instead?

// Code generated by cmd/cgo -godefs; DO NOT EDIT.
// cgo -godefs /home/tklauser/go/src/kcm.go

package unix

type KCMAttach struct {
	Fd	int32
	Fd	int32
}

This is a minimal reproducer for the behaviors observed in https://golang.org/cl/348449 where struct kcm_attach was wrongly converted the same way, see https://go-review.googlesource.com/c/sys/+/348449/comments/409e53a5_defd6d5f

/cc @ianlancetaylor @mdlayher
@tklauser tklauser added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 15, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/350159 mentions this issue: cmd/cgo: for godefs, don't let field prefix removal cause duplicates

@elagergren-spideroak
Copy link

@gopherbot please consider this for backport to 1.17.2 x

@ianlancetaylor
Copy link
Member

This isn't a regression, so it doesn't follow our usual backport guidelines. What is the reason that we should backport it?

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #48448 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Sep 17, 2021
Closed
@elagergren-spideroak
Copy link

@ianlancetaylor doesn't the minor release criteria include bugs without workarounds? I could be mistaken.

@ianlancetaylor
Copy link
Member

The criteria isn't any bug without a workaround; it's serious bugs without workarounds (https://golang.org/wiki/MinorReleases).

So, you're right, but why is this a serious problem? Very few people use the -godefs option with cgo, which is really just for internal use.

@elagergren-spideroak
Copy link

Gotcha. I suppose it doesn't count as serious, at least compared to other backported fixes. If only minor releases included unfortunate bugs. Alas.

I don't have the necessary permissions to close #48448. Sorry.

which is really just for internal use.

FWIW, -godefs is used a fair bit outside of syscall and x/sys/unix because it's a very useful tool for generating wrappers for C libraries.

@golang golang locked and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tklauser @ianlancetaylor @gopherbot @elagergren-spideroak