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

cmd/cgo: inconsistent compiler behaviour when compiling a C.struct #52611

Closed
twystd opened this issue Apr 28, 2022 · 9 comments
Closed

cmd/cgo: inconsistent compiler behaviour when compiling a C.struct #52611

twystd opened this issue Apr 28, 2022 · 9 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@twystd
Copy link

twystd commented Apr 28, 2022

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

$ go version
go version go1.18 darwin/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="on"
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w4/4zn787bj71j0cs7h24l60xsw0000gn/T/go-build2072774135=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

When building a shared-lib/DLL with cgo it fails to compile with the following error:

> go build -trimpath -buildmode=c-shared  -o ../lib/libuhppoted.so ./...

profile.ID undefined (type *_Ctype_struct_TimeProfile has no field or method ID)

if the struct C.struct_TimeProfile is defined in main.go and is passed as an argument to a function in another file in the the same (main) package. However, it compiles successfully if:

  • the Go func using C.struct_TimeProfile is in main.go
  • or the Go func using C.struct_TimeProfile is in in an entirely different Go file
  • or the C.struct_TimeProfile is unpacked to a Go struct in main.go

Other:

  • C.struct_TimeProfile is identical (other than names) to another struct (C.struct_Card) that does not show the same behaviour
  • Renaming structs, fields and files (on the possibility that TimeProfile might just be reserved) does not change the behaviour

I've attached tar.gz files with minimal examples that fail and succeed.

What did you expect to see?

Compiles without error irrespective of where the function is located

What did you see instead?

Compiles with the following error unless the func is a specific file:

dll/time_profiles.go:22:10: profile.profile_id undefined (type *_Ctype_struct_TimeProfile has no field or method profile_id)

fails.tar.gz
succeeds.tar.gz

@ianlancetaylor ianlancetaylor changed the title cmd/cgo: Inconsistent compiler behaviour when compiling a C.struct cmd/cgo: inconsistent compiler behaviour when compiling a C.struct Apr 28, 2022
@ianlancetaylor ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 28, 2022
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Apr 28, 2022
@twystd
Copy link
Author

twystd commented Apr 29, 2022

Additional information - can confirm that I've been able to reproduce the problem on:

  1. A newly created VPS with:

    • Ubuntu 22.04 x64
    • go version go1.18.1 linux/amd64
  2. A newly created VPS with:

    • Windows 2022 Standard
    • go version go1.18.1 windows/amd64
    • gcc (tdm64-1) 10.3.0

@kkHAIKE
Copy link
Contributor

kkHAIKE commented May 4, 2022

root cause is the files order, last file time_profiles.go need the type override main.go define the type(cgo tool use global var typedef), so i guess can't ref type cross file in cgo(satisfy the current go file)

@twystd
Copy link
Author

twystd commented May 4, 2022

Oh, right .. if I build with the source files specified individually and main.go as the last file it compiles ok.

go build -trimpath -buildmode=c-shared -o ./lib/libuhppoted.so ./... seemingly compiles the files in (reverse ?) alphabetical order (or presumably directory order) which is why cards.go compiles just fine but time_profiles.go does not. Not having come across this problem before I had always assumed the compiler/linker was automatically resolving ordering issues.

Thanks - I'll dig a bit into the options to see if there's an alternative to manually specifying the files, but this issue can be closed.

@twystd
Copy link
Author

twystd commented May 4, 2022

Ref. go/issues/38186: cmd/compile: builds not reproducible if input file order is different

To ensure reproducible initialization behavior, build systems are encouraged to present multiple files belonging to the same package in lexical file name order to a compiler.
(https://tip.golang.org/ref/spec#Package_initialization)

In short, the order in which files are presented to the compiler matters. It is the responsibility of the build tool to provide the files in some reproducible order.

I am going to close this since the spec is pretty clear. I believe the go tool sorts the filenames for reproducible behavior

@twystd twystd closed this as completed May 4, 2022
twystd pushed a commit to uhppoted/uhppoted-dll that referenced this issue May 4, 2022
@ianlancetaylor
Copy link
Contributor

Reopening because I think there is still a bug here. Renaming the source files in a package should not have a significant effect on the result. It certainly shouldn't change between a successful build and a failing build.

@ianlancetaylor ianlancetaylor reopened this May 5, 2022
@twystd
Copy link
Author

twystd commented May 5, 2022

When phrased like that it does sound like there may be more to it - it was certainly unexpected behaviour and the workaround is inconvenient even in such a small project.

Some additional info - building with the -work option the generated _cgo_gotypes.go file shows the difference in the generated code:

succeeded:

type _Ctype_struct_Card struct {
	card_number _Ctype_uint32_t
}

type _Ctype_struct_TimeProfile struct {
	profile_id _Ctype_uint32_t
}

failed:

type _Ctype_struct_Card struct {
	card_number _Ctype_uint32_t
}

//go:notinheap
type _Ctype_struct_TimeProfile struct{}

@gopherbot
Copy link

Change https://go.dev/cl/404497 mentions this issue: cmd/cgo: dont override declared struct type

@twystd
Copy link
Author

twystd commented May 6, 2022

Ok, I can confirm that 404497: cmd/cgo: dont override declared struct type seems to fix the immediate problem:

go version
go version devel go1.19-a4af356075 Fri May 6 20:17:52 2022 +0000 linux/amd64

go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so ./...
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/cards.go         dll/main.go          dll/time_profiles.go
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/cards.go         dll/time_profiles.go dll/main.go
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/main.go          dll/cards.go         dll/time_profiles.go 
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/main.go          dll/time_profiles.go dll/cards.go
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/time_profiles.go dll/cards.go         dll/main.go
go build -trimpath -buildmode=c-shared  -o ./lib/libuhppoted.so dll/time_profiles.go dll/main.go          dll/cards.go

I'm still a bit puzzled about why cgo was generating incorrect an incorrect struct definition in the first place i.e. why did it create:

//go:notinheap
type _Ctype_struct_TimeProfile struct{}

when the only mention in time_profiles.go is as a func parameter:

func getTimeProfile(profile *C.struct_TimeProfile, deviceID uint32, profileID uint32) error {
...
}

@kkHAIKE
Copy link
Contributor

kkHAIKE commented May 8, 2022

go/src/cmd/cgo/gcc.go

Lines 2523 to 2527 in a131fd1

// anyone allocate one on the Go side. As a side effect of this annotation,
// pointers to this type will not be considered pointers in Go. They won't
// get writebarrier-ed or adjusted during a stack copy. This should handle
// all the cases badPointerTypedef used to handle, but hopefully will
// continue to work going forward without any more need for cgo changes.

cgo dont know player how to use the struct declaration, in pure c, empty declaration is use for previously-declared struct, most of time use for handle.

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Fixes golang#52611

Change-Id: I835df8d6a98a37482446ec00af768c96fd8ee4fe
GitHub-Last-Rev: ea54dd6
GitHub-Pull-Request: golang#52733
Reviewed-on: https://go-review.googlesource.com/c/go/+/404497
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dexter Ouyang <kkhaike@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Rakoczy <alex@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants